From: David Lamparter Date: Fri, 26 Jul 2024 23:44:59 +0000 (-0700) Subject: lib: refactor memstats logging, fix ACTIVEATEXIT X-Git-Tag: base_10.3~310^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=refs%2Fpull%2F17155%2Fhead;p=mirror%2Ffrr.git lib: refactor memstats logging, fix ACTIVEATEXIT Move the various destinations handling into lib/memory.c, include "normal" logging as target, and make `ACTIVEATEXIT` properly non-error as it was intended to be. Signed-off-by: David Lamparter --- diff --git a/lib/libfrr.c b/lib/libfrr.c index 00ee57d8aa..b9b70f60f7 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -1245,10 +1245,6 @@ void frr_early_fini(void) void frr_fini(void) { - FILE *fp; - char filename[128]; - int have_leftovers = 0; - hook_call(frr_fini); vty_terminate(); @@ -1289,26 +1285,7 @@ void frr_fini(void) frrmod_terminate(); - /* also log memstats to stderr when stderr goes to a file*/ - if (debug_memstats_at_exit || !isatty(STDERR_FILENO)) - have_leftovers = log_memstats(stderr, di->name); - - /* in case we decide at runtime that we want exit-memstats for - * a daemon - * (only do this if we actually have something to print though) - */ - if (!debug_memstats_at_exit || !have_leftovers) - return; - - snprintf(filename, sizeof(filename), "/tmp/frr-memstats-%s-%llu-%llu", - di->name, (unsigned long long)getpid(), - (unsigned long long)time(NULL)); - - fp = fopen(filename, "w"); - if (fp) { - log_memstats(fp, di->name); - fclose(fp); - } + log_memstats(di->name, debug_memstats_at_exit); } struct json_object *frr_daemon_state_load(void) diff --git a/lib/log.c b/lib/log.c index 04b789b5da..2b049cebe4 100644 --- a/lib/log.c +++ b/lib/log.c @@ -306,7 +306,7 @@ void memory_oom(size_t size, const char *name) "out of memory: failed to allocate %zu bytes for %s object", size, name); zlog_backtrace(LOG_CRIT); - log_memstats(stderr, "log"); + log_memstats(zlog_progname, true); abort(); } diff --git a/lib/memory.c b/lib/memory.c index ac39516edd..9da46bdb89 100644 --- a/lib/memory.c +++ b/lib/memory.c @@ -148,35 +148,108 @@ int qmem_walk(qmem_walk_fn *func, void *arg) } struct exit_dump_args { - FILE *fp; - const char *prefix; + const char *daemon_name; + bool do_log; + bool do_file; + bool do_stderr; int error; + FILE *fp; + struct memgroup *last_mg; }; +static void qmem_exit_fopen(struct exit_dump_args *eda) +{ + char filename[128]; + + if (eda->fp || !eda->do_file || !eda->daemon_name) + return; + + snprintf(filename, sizeof(filename), "/tmp/frr-memstats-%s-%llu-%llu", eda->daemon_name, + (unsigned long long)getpid(), (unsigned long long)time(NULL)); + eda->fp = fopen(filename, "w"); + + if (!eda->fp) { + zlog_err("failed to open memstats dump file %pSQq: %m", filename); + /* don't try opening file over and over again */ + eda->do_file = false; + } +} + static int qmem_exit_walker(void *arg, struct memgroup *mg, struct memtype *mt) { struct exit_dump_args *eda = arg; + const char *prefix = eda->daemon_name ?: "NONE"; + char size[32]; + + if (!mt) + /* iterator calls mg=X, mt=NULL first */ + return 0; - if (!mt) { - fprintf(eda->fp, - "%s: showing active allocations in memory group %s\n", - eda->prefix, mg->name); + if (!mt->n_alloc) + return 0; - } else if (mt->n_alloc) { - char size[32]; - if (!mg->active_at_exit) - eda->error++; + if (mt->size != SIZE_VAR) snprintf(size, sizeof(size), "%10zu", mt->size); - fprintf(eda->fp, "%s: memstats: %-30s: %6zu * %s\n", - eda->prefix, mt->name, mt->n_alloc, - mt->size == SIZE_VAR ? "(variably sized)" : size); + else + snprintf(size, sizeof(size), "(variably sized)"); + + if (mg->active_at_exit) { + /* not an error - this memgroup has allocations remain active + * at exit. Only printed to zlog_debug. + */ + if (!eda->do_log) + return 0; + + if (eda->last_mg != mg) { + zlog_debug("showing active allocations in memory group %s (not an error)", + mg->name); + eda->last_mg = mg; + } + zlog_debug("memstats: %-30s: %6zu * %s", mt->name, mt->n_alloc, size); + return 0; } + + eda->error++; + if (eda->do_file) + qmem_exit_fopen(eda); + + if (eda->last_mg != mg) { + if (eda->do_log) + zlog_warn("showing active allocations in memory group %s", mg->name); + if (eda->do_stderr) + fprintf(stderr, "%s: showing active allocations in memory group %s\n", + prefix, mg->name); + if (eda->fp) + fprintf(eda->fp, "%s: showing active allocations in memory group %s\n", + prefix, mg->name); + eda->last_mg = mg; + } + + if (eda->do_log) + zlog_warn("memstats: %-30s: %6zu * %s", mt->name, mt->n_alloc, size); + if (eda->do_stderr) + fprintf(stderr, "%s: memstats: %-30s: %6zu * %s\n", prefix, mt->name, mt->n_alloc, + size); + if (eda->fp) + fprintf(eda->fp, "%s: memstats: %-30s: %6zu * %s\n", prefix, mt->name, mt->n_alloc, + size); return 0; } -int log_memstats(FILE *fp, const char *prefix) +int log_memstats(const char *daemon_name, bool enabled) { - struct exit_dump_args eda = {.fp = fp, .prefix = prefix, .error = 0}; + struct exit_dump_args eda = { + .daemon_name = daemon_name, + .do_log = enabled, + .do_file = enabled, + .do_stderr = enabled || !isatty(STDERR_FILENO), + .error = 0, + }; + qmem_walk(qmem_exit_walker, &eda); + if (eda.fp) + fclose(eda.fp); + if (eda.error && eda.do_log) + zlog_warn("exiting with %d leaked MTYPEs", eda.error); return eda.error; } diff --git a/lib/memory.h b/lib/memory.h index 957a9611b5..8658018832 100644 --- a/lib/memory.h +++ b/lib/memory.h @@ -181,8 +181,7 @@ static inline size_t mtype_stats_alloc(struct memtype *mt) * last value from qmem_walk_fn. */ typedef int qmem_walk_fn(void *arg, struct memgroup *mg, struct memtype *mt); extern int qmem_walk(qmem_walk_fn *func, void *arg); -extern int log_memstats(FILE *fp, const char *); -#define log_memstats_stderr(prefix) log_memstats(stderr, prefix) +extern int log_memstats(const char *daemon_name, bool enabled); extern __attribute__((__noreturn__)) void memory_oom(size_t size, const char *name); diff --git a/tests/isisd/test_fuzz_isis_tlv.c b/tests/isisd/test_fuzz_isis_tlv.c index 627ccfee6f..a3acd0786f 100644 --- a/tests/isisd/test_fuzz_isis_tlv.c +++ b/tests/isisd/test_fuzz_isis_tlv.c @@ -22,7 +22,7 @@ static bool atexit_registered; static void show_meminfo_at_exit(void) { - log_memstats(stderr, "isis fuzztest"); + log_memstats(NULL, true); } static int comp_line(const void *p1, const void *p2) diff --git a/tests/isisd/test_isis_spf.c b/tests/isisd/test_isis_spf.c index e5a8f7a513..9c1ce3d193 100644 --- a/tests/isisd/test_isis_spf.c +++ b/tests/isisd/test_isis_spf.c @@ -475,7 +475,7 @@ static void vty_do_exit(int isexit) yang_terminate(); event_master_free(master); - log_memstats(stderr, "test-isis-spf"); + log_memstats(NULL, true); if (!isexit) exit(0); } diff --git a/tests/lib/cli/common_cli.c b/tests/lib/cli/common_cli.c index 342a91cc79..5c23a71258 100644 --- a/tests/lib/cli/common_cli.c +++ b/tests/lib/cli/common_cli.c @@ -43,7 +43,7 @@ static void vty_do_exit(int isexit) yang_terminate(); event_master_free(master); - log_memstats(stderr, "testcli"); + log_memstats(NULL, true); if (!isexit) exit(0); } diff --git a/tests/lib/northbound/test_oper_data.c b/tests/lib/northbound/test_oper_data.c index fdc9e53ca3..3d700d8a19 100644 --- a/tests/lib/northbound/test_oper_data.c +++ b/tests/lib/northbound/test_oper_data.c @@ -427,7 +427,7 @@ static void vty_do_exit(int isexit) yang_terminate(); event_master_free(master); - log_memstats(stderr, "test-nb-oper-data"); + log_memstats(NULL, true); if (!isexit) exit(0); } diff --git a/tests/lib/test_typelist.c b/tests/lib/test_typelist.c index 070a304335..3ce6683ae5 100644 --- a/tests/lib/test_typelist.c +++ b/tests/lib/test_typelist.c @@ -156,6 +156,6 @@ int main(int argc, char **argv) test_ATOMSORT_UNIQ(); test_ATOMSORT_NONUNIQ(); - log_memstats_stderr("test: "); + log_memstats(NULL, true); return 0; } diff --git a/tests/lib/test_zmq.c b/tests/lib/test_zmq.c index 2cd9d47cb4..5cb518d81a 100644 --- a/tests/lib/test_zmq.c +++ b/tests/lib/test_zmq.c @@ -285,7 +285,7 @@ static void run_server(int syncfd) zmq_close(zmqsock); frrzmq_finish(); event_master_free(master); - log_memstats_stderr("test"); + log_memstats(NULL, true); } int main(void)