From 900dfd33819332a7e0e2b31ff9b30f6cf574b9a8 Mon Sep 17 00:00:00 2001 From: Dave Barach Date: Mon, 17 Jun 2019 08:28:16 -0400 Subject: vlib: cherry-pick "memory-trace stats-segment" and "leak-check { }" Ticket: VPP-1703 Type: feature Change-Id: Ie020fd7e2618284a63efbeb9895068f27c0fb9ab Signed-off-by: Dave Barach --- src/vlib/cli.c | 213 +++++++++++++++++++++++++++++++++++--------- src/vppinfra/dlmalloc.c | 29 +++--- src/vppinfra/dlmalloc.h | 5 +- src/vppinfra/mem.h | 2 + src/vppinfra/mem_dlmalloc.c | 31 +++++-- src/vppinfra/mem_mheap.c | 6 ++ 6 files changed, 225 insertions(+), 61 deletions(-) diff --git a/src/vlib/cli.c b/src/vlib/cli.c index 4e8f3ae5ae5..60d01354c93 100644 --- a/src/vlib/cli.c +++ b/src/vlib/cli.c @@ -44,6 +44,10 @@ #include #include +static void *current_traced_heap; + +uword clib_mem_trace_enable_disable (uword enable); + /* Root of all show commands. */ /* *INDENT-OFF* */ VLIB_CLI_COMMAND (vlib_cli_show_command, static) = { @@ -552,6 +556,39 @@ vlib_cli_dispatch_sub_commands (vlib_main_t * vm, parent_command_index); unformat_free (&sub_input); } + else if (unformat (input, "leak-check %U", + unformat_vlib_cli_sub_input, &sub_input)) + { + u8 *leak_report; + if (current_traced_heap) + { + void *oldheap; + oldheap = clib_mem_set_heap (current_traced_heap); + clib_mem_trace (0); + clib_mem_set_heap (oldheap); + current_traced_heap = 0; + } + clib_mem_trace (1); + error = + vlib_cli_dispatch_sub_commands (vm, cm, &sub_input, + parent_command_index); + unformat_free (&sub_input); + + /* Otherwise, the clib_error_t shows up as a leak... */ + if (error) + { + vlib_cli_output (vm, "%v", error->what); + clib_error_free (error); + error = 0; + } + + (void) clib_mem_trace_enable_disable (0); + leak_report = format (0, "%U", format_mheap, clib_mem_get_heap (), + 1 /* verbose, i.e. print leaks */ ); + clib_mem_trace (0); + vlib_cli_output (vm, "%v", leak_report); + vec_free (leak_report); + } else if (unformat_user (input, unformat_vlib_cli_sub_command, vm, parent, &c)) @@ -745,15 +782,35 @@ vlib_cli_output (vlib_main_t * vm, char *fmt, ...) } void *vl_msg_push_heap (void) __attribute__ ((weak)); +void * +vl_msg_push_heap (void) +{ + return 0; +} + void vl_msg_pop_heap (void *oldheap) __attribute__ ((weak)); +void +vl_msg_pop_heap (void *oldheap) +{ +} + +void *vlib_stats_push_heap (void *) __attribute__ ((weak)); +void * +vlib_stats_push_heap (void *notused) +{ + return 0; +} static clib_error_t * show_memory_usage (vlib_main_t * vm, unformat_input_t * input, vlib_cli_command_t * cmd) { - int verbose __attribute__ ((unused)) = 0, api_segment = 0; + int verbose __attribute__ ((unused)) = 0; + int api_segment = 0, stats_segment = 0, main_heap = 0; clib_error_t *error; u32 index = 0; + uword was_enabled; + while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT) { @@ -761,6 +818,10 @@ show_memory_usage (vlib_main_t * vm, verbose = 1; else if (unformat (input, "api-segment")) api_segment = 1; + else if (unformat (input, "stats-segment")) + stats_segment = 1; + else if (unformat (input, "main-heap")) + main_heap = 1; else { error = clib_error_return (0, "unknown input `%U'", @@ -769,9 +830,14 @@ show_memory_usage (vlib_main_t * vm, } } + if ((api_segment + stats_segment + main_heap) == 0) + return clib_error_return + (0, "Please supply one of api-segment, stats-segment or main-heap"); + if (api_segment) { void *oldheap = vl_msg_push_heap (); + was_enabled = clib_mem_trace_enable_disable (0); u8 *s_in_svm = format (0, "%U\n", format_mheap, clib_mem_get_heap (), 1); vl_msg_pop_heap (oldheap); @@ -779,10 +845,31 @@ show_memory_usage (vlib_main_t * vm, oldheap = vl_msg_push_heap (); vec_free (s_in_svm); + clib_mem_trace_enable_disable (was_enabled); vl_msg_pop_heap (oldheap); - vlib_cli_output (vm, "API segment start:"); + vlib_cli_output (vm, "API segment"); + vlib_cli_output (vm, "%v", s); + vec_free (s); + } + if (stats_segment) + { + void *oldheap = vlib_stats_push_heap (0); + was_enabled = clib_mem_trace_enable_disable (0); + u8 *s_in_svm = + format (0, "%U\n", format_mheap, clib_mem_get_heap (), 1); + if (oldheap) + clib_mem_set_heap (oldheap); + u8 *s = vec_dup (s_in_svm); + + oldheap = vlib_stats_push_heap (0); + vec_free (s_in_svm); + if (oldheap) + { + clib_mem_trace_enable_disable (was_enabled); + clib_mem_set_heap (oldheap); + } + vlib_cli_output (vm, "Stats segment"); vlib_cli_output (vm, "%v", s); - vlib_cli_output (vm, "API segment end:"); vec_free (s); } @@ -803,36 +890,37 @@ show_memory_usage (vlib_main_t * vm, /* *INDENT-ON* */ #else { - uword clib_mem_trace_enable_disable (uword enable); - uword was_enabled; - - /* - * Note: the foreach_vlib_main cause allocator traffic, - * so shut off tracing before we go there... - */ - was_enabled = clib_mem_trace_enable_disable (0); - - /* *INDENT-OFF* */ - foreach_vlib_main ( - ({ - struct dlmallinfo mi; - void *mspace; - mspace = clib_per_cpu_mheaps[index]; + if (main_heap) + { + /* + * Note: the foreach_vlib_main causes allocator traffic, + * so shut off tracing before we go there... + */ + was_enabled = clib_mem_trace_enable_disable (0); - mi = mspace_mallinfo (mspace); - vlib_cli_output (vm, "%sThread %d %s\n", index ? "\n":"", index, - vlib_worker_threads[index].name); - vlib_cli_output (vm, " %U\n", format_page_map, - pointer_to_uword (mspace_least_addr(mspace)), - mi.arena); - vlib_cli_output (vm, " %U\n", format_mheap, clib_per_cpu_mheaps[index], - verbose); - index++; - })); - /* *INDENT-ON* */ + /* *INDENT-OFF* */ + foreach_vlib_main ( + ({ + struct dlmallinfo mi; + void *mspace; + mspace = clib_per_cpu_mheaps[index]; + + mi = mspace_mallinfo (mspace); + vlib_cli_output (vm, "%sThread %d %s\n", index ? "\n":"", index, + vlib_worker_threads[index].name); + vlib_cli_output (vm, " %U\n", format_page_map, + pointer_to_uword (mspace_least_addr(mspace)), + mi.arena); + vlib_cli_output (vm, " %U\n", format_mheap, + clib_per_cpu_mheaps[index], + verbose); + index++; + })); + /* *INDENT-ON* */ - /* Restore the trace flag */ - clib_mem_trace_enable_disable (was_enabled); + /* Restore the trace flag */ + clib_mem_trace_enable_disable (was_enabled); + } } #endif /* USE_DLMALLOC */ return 0; @@ -841,7 +929,7 @@ show_memory_usage (vlib_main_t * vm, /* *INDENT-OFF* */ VLIB_CLI_COMMAND (show_memory_usage_command, static) = { .path = "show memory", - .short_help = "[verbose | api-segment] Show current memory usage", + .short_help = "show memory [api-segment][stats-segment][verbose]", .function = show_memory_usage, }; /* *INDENT-ON* */ @@ -877,7 +965,6 @@ VLIB_CLI_COMMAND (show_cpu_command, static) = { .short_help = "Show cpu information", .function = show_cpu, }; - /* *INDENT-ON* */ static clib_error_t * @@ -886,11 +973,12 @@ enable_disable_memory_trace (vlib_main_t * vm, vlib_cli_command_t * cmd) { unformat_input_t _line_input, *line_input = &_line_input; - int enable; + int enable = 1; int api_segment = 0; + int stats_segment = 0; + int main_heap = 0; void *oldheap; - if (!unformat_user (input, unformat_line_input, line_input)) return 0; @@ -900,6 +988,10 @@ enable_disable_memory_trace (vlib_main_t * vm, ; else if (unformat (line_input, "api-segment")) api_segment = 1; + else if (unformat (line_input, "stats-segment")) + stats_segment = 1; + else if (unformat (line_input, "main-heap")) + main_heap = 1; else { unformat_free (line_input); @@ -908,11 +1000,52 @@ enable_disable_memory_trace (vlib_main_t * vm, } unformat_free (line_input); + if ((api_segment + stats_segment + main_heap + (enable == 0)) == 0) + { + return clib_error_return + (0, "Need one of main-heap, stats-segment or api-segment"); + } + + /* Turn off current trace, if any */ + if (current_traced_heap) + { + void *oldheap; + oldheap = clib_mem_set_heap (current_traced_heap); + clib_mem_trace (0); + clib_mem_set_heap (oldheap); + current_traced_heap = 0; + } + + if (enable == 0) + return 0; + + /* API segment */ if (api_segment) - oldheap = vl_msg_push_heap (); - clib_mem_trace (enable); - if (api_segment) - vl_msg_pop_heap (oldheap); + { + oldheap = vl_msg_push_heap (); + current_traced_heap = clib_mem_get_heap (); + clib_mem_trace (1); + vl_msg_pop_heap (oldheap); + + } + + /* Stats segment */ + if (stats_segment) + { + oldheap = vlib_stats_push_heap (0); + current_traced_heap = clib_mem_get_heap (); + clib_mem_trace (stats_segment); + /* We don't want to call vlib_stats_pop_heap... */ + if (oldheap) + clib_mem_set_heap (oldheap); + } + + /* main_heap */ + if (main_heap) + { + current_traced_heap = clib_mem_get_heap (); + clib_mem_trace (main_heap); + } return 0; } @@ -920,7 +1053,7 @@ enable_disable_memory_trace (vlib_main_t * vm, /* *INDENT-OFF* */ VLIB_CLI_COMMAND (enable_disable_memory_trace_command, static) = { .path = "memory-trace", - .short_help = "on|off [api-segment] Enable/disable memory allocation trace", + .short_help = "memory-trace on|off [api-segment][stats-segment][main-heap]\n", .function = enable_disable_memory_trace, }; /* *INDENT-ON* */ diff --git a/src/vppinfra/dlmalloc.c b/src/vppinfra/dlmalloc.c index 9ed1e04f776..37721bb15fd 100644 --- a/src/vppinfra/dlmalloc.c +++ b/src/vppinfra/dlmalloc.c @@ -4093,7 +4093,7 @@ void mspace_get_address_and_size (mspace msp, char **addrp, size_t *sizep) { mstate ms; msegment *this_seg; - + ms = (mstate)msp; this_seg = &ms->seg; @@ -4155,9 +4155,18 @@ int mspace_enable_disable_trace (mspace msp, int enable) return (was_enabled); } -void* mspace_get_aligned (mspace msp, +int mspace_is_traced (mspace msp) +{ + mstate ms = (mstate)msp; + + if (use_trace(ms)) + return 1; + return 0; +} + +void* mspace_get_aligned (mspace msp, unsigned long n_user_data_bytes, - unsigned long align, + unsigned long align, unsigned long align_offset) { char *rv; unsigned long searchp; @@ -4165,13 +4174,13 @@ void* mspace_get_aligned (mspace msp, mstate ms = (mstate)msp; /* - * Allocate space for the "Where's Waldo?" pointer + * Allocate space for the "Where's Waldo?" pointer * the base of the dlmalloc object */ n_user_data_bytes += sizeof(unsigned); - /* - * Alignment requests less than the size of an mmx vector are ignored + /* + * Alignment requests less than the size of an mmx vector are ignored */ if (align < 16) { rv = mspace_malloc (msp, n_user_data_bytes); @@ -4181,7 +4190,7 @@ void* mspace_get_aligned (mspace msp, if (use_trace(ms)) { mchunkptr p = mem2chunk(rv); size_t psize = chunksize(p); - + mheap_get_trace ((unsigned long)rv + sizeof (unsigned), psize); } @@ -4196,15 +4205,15 @@ void* mspace_get_aligned (mspace msp, * Alignment requests greater than 4K must be at offset zero, * and must be freed using mspace_free_no_offset - or never freed - * since the "Where's Waldo?" pointer would waste too much space. - * - * Waldo is the address of the chunk of memory returned by mspace_malloc, + * + * Waldo is the address of the chunk of memory returned by mspace_malloc, * which we need later to call mspace_free... */ if (align > 4<<10 || align_offset == ~0UL) { n_user_data_bytes -= sizeof(unsigned); assert(align_offset == 0); rv = internal_memalign(ms, (size_t)align, n_user_data_bytes); - + /* Trace the allocation */ if (rv && use_trace(ms)) { mchunkptr p = mem2chunk(rv); diff --git a/src/vppinfra/dlmalloc.h b/src/vppinfra/dlmalloc.h index 216df4737ca..b8adf74831d 100644 --- a/src/vppinfra/dlmalloc.h +++ b/src/vppinfra/dlmalloc.h @@ -1447,9 +1447,9 @@ DLMALLOC_EXPORT int mspace_trim(mspace msp, size_t pad); */ DLMALLOC_EXPORT int mspace_mallopt(int, int); -DLMALLOC_EXPORT void* mspace_get_aligned (mspace msp, +DLMALLOC_EXPORT void* mspace_get_aligned (mspace msp, unsigned long n_user_data_bytes, - unsigned long align, + unsigned long align, unsigned long align_offset); DLMALLOC_EXPORT int mspace_is_heap_object (mspace msp, void *p); @@ -1463,6 +1463,7 @@ DLMALLOC_EXPORT void *mspace_least_addr (mspace msp); DLMALLOC_EXPORT void mheap_get_trace (uword offset, uword size); DLMALLOC_EXPORT void mheap_put_trace (uword offset, uword size); DLMALLOC_EXPORT int mspace_enable_disable_trace (mspace msp, int enable); +DLMALLOC_EXPORT int mspace_is_traced (mspace msp); #endif /* MSPACES */ diff --git a/src/vppinfra/mem.h b/src/vppinfra/mem.h index 65b25f004a7..a357422d9f9 100644 --- a/src/vppinfra/mem.h +++ b/src/vppinfra/mem.h @@ -274,6 +274,8 @@ void clib_mem_validate (void); void clib_mem_trace (int enable); +int clib_mem_is_traced (void); + typedef struct { /* Total number of objects allocated. */ diff --git a/src/vppinfra/mem_dlmalloc.c b/src/vppinfra/mem_dlmalloc.c index e83e0e89d21..99f1c04f3eb 100644 --- a/src/vppinfra/mem_dlmalloc.c +++ b/src/vppinfra/mem_dlmalloc.c @@ -56,6 +56,10 @@ typedef struct /* Hash table mapping mheap offset to trace index. */ uword *trace_index_by_offset; + + /* So we can easily shut off current segment trace, if any */ + void *current_traced_mheap; + } mheap_trace_main_t; mheap_trace_main_t mheap_trace_main; @@ -69,7 +73,7 @@ mheap_get_trace (uword offset, uword size) mheap_trace_t trace; uword save_enabled; - if (tm->enabled == 0) + if (tm->enabled == 0 || (clib_mem_get_heap () != tm->current_traced_mheap)) return; /* Spurious Coverity warnings be gone. */ @@ -80,11 +84,6 @@ mheap_get_trace (uword offset, uword size) if (n_callers == 0) return; - /* $$$ This looks like dreck to remove... */ - if (0) - for (i = n_callers; i < ARRAY_LEN (trace.callers); i++) - trace.callers[i] = 0; - clib_spinlock_lock (&tm->lock); /* Turn off tracing to avoid embarrassment... */ @@ -291,7 +290,8 @@ format_mheap_trace (u8 * s, va_list * va) int i; clib_spinlock_lock (&tm->lock); - if (vec_len (tm->traces) > 0) + if (vec_len (tm->traces) > 0 && + clib_mem_get_heap () == tm->current_traced_mheap) { have_traces = 1; @@ -372,7 +372,8 @@ format_mheap (u8 * s, va_list * va) format (s, "\n max total allocated %U", format_msize, mi.usmblks); } - s = format (s, "\n%U", format_mheap_trace, tm, verbose); + if (mspace_is_traced (heap)) + s = format (s, "\n%U", format_mheap_trace, tm, verbose); return s; } @@ -404,9 +405,21 @@ void clib_mem_trace (int enable) { mheap_trace_main_t *tm = &mheap_trace_main; + void *current_heap = clib_mem_get_heap (); tm->enabled = enable; - mheap_trace (clib_mem_get_heap (), enable); + mheap_trace (current_heap, enable); + + if (enable) + tm->current_traced_mheap = current_heap; + else + tm->current_traced_mheap = 0; +} + +int +clib_mem_is_traced (void) +{ + return mspace_is_traced (clib_mem_get_heap ()); } uword diff --git a/src/vppinfra/mem_mheap.c b/src/vppinfra/mem_mheap.c index 4077bf279fd..3046bd22d0a 100644 --- a/src/vppinfra/mem_mheap.c +++ b/src/vppinfra/mem_mheap.c @@ -148,6 +148,12 @@ clib_mem_trace (int enable) mheap_trace (clib_mem_get_heap (), enable); } +int +clib_mem_is_traced (void) +{ + return mheap_is_traced (clib_mem_get_heap ()); +} + /* * fd.io coding-style-patch-verification: ON * -- cgit 1.2.3-korg