From fa896a1d807248b8bd3343b77f8668d80de1ec98 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 8 Aug 2018 16:33:13 +0200 Subject: [PATCH 1/4] build: rework mallinfo test & find malloc_size Signed-off-by: David Lamparter --- configure.ac | 61 +++++++++++++++++++++++++++++++++++++++++------- lib/memory_vty.c | 7 ++++-- 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index f65b1640d2..370b3bab58 100755 --- a/configure.ac +++ b/configure.ac @@ -1811,15 +1811,58 @@ dnl order to check no alternative allocator dnl has been specified, which might not provide dnl mallinfo, e.g. such as Umem on Solaris. dnl ----------------------------------------- -AC_CHECK_HEADER([malloc.h], - [AC_MSG_CHECKING(whether mallinfo is available) - AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include ]], - [[struct mallinfo ac_x; ac_x = mallinfo ();]])], - [AC_MSG_RESULT(yes) - AC_DEFINE(HAVE_MALLINFO,,mallinfo)], - AC_MSG_RESULT(no) - ) - ], [], FRR_INCLUDES) +AC_CHECK_HEADERS([malloc.h malloc/malloc.h],,, [FRR_INCLUDES]) + +AC_MSG_CHECKING(whether mallinfo is available) +AC_LINK_IFELSE([AC_LANG_PROGRAM([FRR_INCLUDES [ +#ifdef HAVE_MALLOC_H +#include +#endif +#ifdef HAVE_MALLOC_MALLOC_H +#include +#endif +]], [[ +struct mallinfo ac_x; ac_x = mallinfo (); +]])], [ + AC_MSG_RESULT(yes) + AC_DEFINE(HAVE_MALLINFO,,mallinfo) +], [ + AC_MSG_RESULT(no) +]) + +AC_MSG_CHECKING(whether malloc_usable_size is available) +AC_LINK_IFELSE([AC_LANG_PROGRAM([FRR_INCLUDES [ +#ifdef HAVE_MALLOC_H +#include +#endif +#ifdef HAVE_MALLOC_MALLOC_H +#include +#endif +]], [[ +size_t ac_x; ac_x = malloc_usable_size(NULL); +]])], [ + AC_MSG_RESULT(yes) + AC_DEFINE(HAVE_MALLOC_USABLE_SIZE,,malloc_usable_size) +], [ + AC_MSG_RESULT(no) + + AC_MSG_CHECKING(whether malloc_size is available) + AC_LINK_IFELSE([AC_LANG_PROGRAM([[ +#ifdef HAVE_MALLOC_H +#include +#endif +#ifdef HAVE_MALLOC_MALLOC_H +#include +#endif +]], [[ +size_t ac_x; ac_x = malloc_size(NULL); +]])], [ + AC_MSG_RESULT(yes) + AC_DEFINE(HAVE_MALLOC_SIZE,,malloc_size) + ], [ + AC_MSG_RESULT(no) + ]) +]) dnl ------ dnl ZeroMQ diff --git a/lib/memory_vty.c b/lib/memory_vty.c index 972914bf24..388273128e 100644 --- a/lib/memory_vty.c +++ b/lib/memory_vty.c @@ -21,9 +21,12 @@ #include /* malloc.h is generally obsolete, however GNU Libc mallinfo wants it. */ -#if (defined(GNU_LINUX) && defined(HAVE_MALLINFO)) +#ifdef HAVE_MALLOC_H #include -#endif /* HAVE_MALLINFO */ +#endif +#ifdef HAVE_MALLOC_MALLOC_H +#include +#endif #include #include From 602a6584ee4f7c9f881204cc413fab73f18791f0 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 8 Aug 2018 16:44:43 +0200 Subject: [PATCH 2/4] lib: count total memory allocation per MTYPE If malloc_usable_size() or malloc_size() are available, we can count total usage of a particular MTYPE. (Without the functions, we don't know how much to subtract on free.) Signed-off-by: David Lamparter --- lib/memory.c | 28 +++++++++++++++++++++++----- lib/memory.h | 8 ++++++++ lib/memory_vty.c | 13 +++++++++++-- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/lib/memory.c b/lib/memory.c index e279b17d12..87cba69fc5 100644 --- a/lib/memory.c +++ b/lib/memory.c @@ -17,6 +17,12 @@ #include #include +#ifdef HAVE_MALLOC_H +#include +#endif +#ifdef HAVE_MALLOC_MALLOC_H +#include +#endif #include "memory.h" #include "log.h" @@ -28,7 +34,7 @@ DEFINE_MGROUP(LIB, "libfrr") DEFINE_MTYPE(LIB, TMP, "Temporary memory") DEFINE_MTYPE(LIB, PREFIX_FLOWSPEC, "Prefix Flowspec") -static inline void mt_count_alloc(struct memtype *mt, size_t size) +static inline void mt_count_alloc(struct memtype *mt, size_t size, void *ptr) { size_t oldsize; @@ -41,12 +47,24 @@ static inline void mt_count_alloc(struct memtype *mt, size_t size) if (oldsize != 0 && oldsize != size && oldsize != SIZE_VAR) atomic_store_explicit(&mt->size, SIZE_VAR, memory_order_relaxed); + +#ifdef HAVE_MALLOC_USABLE_SIZE + size_t mallocsz = malloc_usable_size(ptr); + + atomic_fetch_add_explicit(&mt->total, mallocsz, memory_order_relaxed); +#endif } -static inline void mt_count_free(struct memtype *mt) +static inline void mt_count_free(struct memtype *mt, void *ptr) { assert(mt->n_alloc); atomic_fetch_sub_explicit(&mt->n_alloc, 1, memory_order_relaxed); + +#ifdef HAVE_MALLOC_USABLE_SIZE + size_t mallocsz = malloc_usable_size(ptr); + + atomic_fetch_sub_explicit(&mt->total, mallocsz, memory_order_relaxed); +#endif } static inline void *mt_checkalloc(struct memtype *mt, void *ptr, size_t size) @@ -58,7 +76,7 @@ static inline void *mt_checkalloc(struct memtype *mt, void *ptr, size_t size) } return NULL; } - mt_count_alloc(mt, size); + mt_count_alloc(mt, size, ptr); return ptr; } @@ -75,7 +93,7 @@ void *qcalloc(struct memtype *mt, size_t size) void *qrealloc(struct memtype *mt, void *ptr, size_t size) { if (ptr) - mt_count_free(mt); + mt_count_free(mt, ptr); return mt_checkalloc(mt, ptr ? realloc(ptr, size) : malloc(size), size); } @@ -87,7 +105,7 @@ void *qstrdup(struct memtype *mt, const char *str) void qfree(struct memtype *mt, void *ptr) { if (ptr) - mt_count_free(mt); + mt_count_free(mt, ptr); free(ptr); } diff --git a/lib/memory.h b/lib/memory.h index 1fbbbe4231..c39f34e3a7 100644 --- a/lib/memory.h +++ b/lib/memory.h @@ -24,12 +24,20 @@ #define array_size(ar) (sizeof(ar) / sizeof(ar[0])) +#if defined(HAVE_MALLOC_SIZE) && !defined(HAVE_MALLOC_USABLE_SIZE) +#define malloc_usable_size(x) malloc_size(x) +#define HAVE_MALLOC_USABLE_SIZE +#endif + #define SIZE_VAR ~0UL struct memtype { struct memtype *next, **ref; const char *name; _Atomic size_t n_alloc; _Atomic size_t size; +#ifdef HAVE_MALLOC_USABLE_SIZE + _Atomic size_t total; +#endif }; struct memgroup { diff --git a/lib/memory_vty.c b/lib/memory_vty.c index 388273128e..9ee2e52ec7 100644 --- a/lib/memory_vty.c +++ b/lib/memory_vty.c @@ -79,12 +79,21 @@ static int qmem_walker(void *arg, struct memgroup *mg, struct memtype *mt) if (mt->n_alloc != 0) { char size[32]; snprintf(size, sizeof(size), "%6zu", mt->size); - vty_out(vty, "%-30s: %10zu %s\n", mt->name, + +#ifdef HAVE_MALLOC_USABLE_SIZE +#define TSTR " %9zu" +#define TARG , mt->total +#else +#define TSTR "" +#define TARG +#endif + vty_out(vty, "%-30s: %10zu %-16s"TSTR"\n", mt->name, mt->n_alloc, mt->size == 0 ? "" : mt->size == SIZE_VAR ? "(variably sized)" - : size); + : size + TARG); } } return 0; From 0a62b873922e437d0644e5a298d1076ebbde388b Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 8 Aug 2018 17:06:45 +0200 Subject: [PATCH 3/4] doc: add "show memory" user documentation Signed-off-by: David Lamparter --- doc/user/basic.rst | 61 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/doc/user/basic.rst b/doc/user/basic.rst index cb46080055..da22bb2f86 100644 --- a/doc/user/basic.rst +++ b/doc/user/basic.rst @@ -320,6 +320,67 @@ Terminal Mode Commands Shows the current configuration of the logging system. This includes the status of all logging destinations. +.. index:: show memory +.. clicmd:: show memory + + Show information on how much memory is used for which specific things in + |PACKAGE_NAME|. Output may vary depending on system capabilities but will + generally look something like this: + + :: + + frr# show memory + System allocator statistics: + Total heap allocated: 1584 KiB + Holding block headers: 0 bytes + Used small blocks: 0 bytes + Used ordinary blocks: 1484 KiB + Free small blocks: 2096 bytes + Free ordinary blocks: 100 KiB + Ordinary blocks: 2 + Small blocks: 60 + Holding blocks: 0 + (see system documentation for 'mallinfo' for meaning) + --- qmem libfrr --- + Buffer : 3 24 72 + Buffer data : 1 4120 4120 + Host config : 3 (variably sized) 72 + Command Tokens : 3427 72 247160 + Command Token Text : 2555 (variably sized) 83720 + Command Token Help : 2555 (variably sized) 61720 + Command Argument : 2 (variably sized) 48 + Command Argument Name : 641 (variably sized) 15672 + [...] + --- qmem Label Manager --- + --- qmem zebra --- + ZEBRA VRF : 1 912 920 + Route Entry : 11 80 968 + Static route : 1 192 200 + RIB destination : 8 48 448 + RIB table info : 4 16 96 + Nexthop tracking object : 1 200 200 + Zebra Name Space : 1 312 312 + --- qmem Table Manager --- + + To understand system allocator statistics, refer to your system's + :manpage:`mallinfo(3)` man page. + + Below these statistics, statistics on individual memory allocation types + in |PACKAGE_NAME| (so-called `MTYPEs`) is printed: + + * the first column of numbers is the current count of allocations made for + the type (the number decreases when items are freed.) + * the second column is the size of each item. This is only available if + allocations on a type are always made with the same size. + * the third column is the total amount of memory allocated for the + particular type, including padding applied by malloc. This means that + the number may be larger than the first column multiplied by the second. + Overhead incurred by malloc's bookkeeping is not included in this, and + the column may be missing if system support is not available. + + When executing this command from ``vtysh``, each of the daemons' memory + usage is printed sequentially. + .. index:: logmsg LEVEL MESSAGE .. clicmd:: logmsg LEVEL MESSAGE From 7aea4b816790cae57547f3e3d8e0185de69973ce Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 10 Aug 2018 20:45:41 +0200 Subject: [PATCH 4/4] tools/checkpatch: fix some bogus macro warnings checkpatch was throwing an error for "#define FOO , bar" Signed-off-by: David Lamparter --- tools/checkpatch.pl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/checkpatch.pl b/tools/checkpatch.pl index c1fab1029a..55b3e1e564 100755 --- a/tools/checkpatch.pl +++ b/tools/checkpatch.pl @@ -4181,7 +4181,9 @@ sub process { } elsif ($op eq ',') { my $rtrim_before = 0; my $space_after = 0; - if ($ctx =~ /Wx./) { + if ($line=~/\#\s*define/) { + # ignore , spacing in macros + } elsif ($ctx =~ /Wx./) { if (ERROR("SPACING", "space prohibited before that '$op' $at\n" . $hereptr)) { $line_fixed = 1; @@ -4847,6 +4849,7 @@ sub process { my $ctx = ''; my $has_flow_statement = 0; my $has_arg_concat = 0; + my $complex = 0; ($dstat, $dcond, $ln, $cnt, $off) = ctx_statement_block($linenr, $realcnt, 0); $ctx = $dstat; @@ -4865,6 +4868,7 @@ sub process { $define_args = substr($define_args, 1, length($define_args) - 2); $define_args =~ s/\s*//g; @def_args = split(",", $define_args); + $complex = 1; } $dstat =~ s/$;//g; @@ -4932,7 +4936,7 @@ sub process { } elsif ($dstat =~ /;/) { ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE", "Macros with multiple statements should be enclosed in a do - while loop\n" . "$herectx"); - } else { + } elsif ($complex) { ERROR("COMPLEX_MACRO", "Macros with complex values should be enclosed in parentheses\n" . "$herectx"); }