Re: [EXT] Re: [PATCH v5 2/2] mem: telemetry support for system memory information

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

2022-10-11 07:10 (UTC+0000), Amit Prakash Shukla:
> Thanks David for the feedback. Please find the proposed fixed inline.
> 
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Saturday, October 8, 2022 1:17 AM
> > To: Amit Prakash Shukla <amitprakashs@marvell.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Anatoly Burakov <anatoly.burakov@intel.com>;
> > dmitry.kozliuk@gmail.com
> > Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> > bruce.richardson@intel.com; ciara.power@intel.com
> > Subject: [EXT] Re: [PATCH v5 2/2] mem: telemetry support for system
> > memory information
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > On Thu, Sep 29, 2022 at 1:44 PM Amit Prakash Shukla
> > <amitprakashs@marvell.com> wrote:  
> > >
> > > Changes adds telemetry support to display system memory information,
> > > allocated using calls malloc, calloc, mmap, etc. This patch is based
> > > on malloc_info. This patch adds following endpoints:  
> > 
> > malloc_info is a GNU extension.
> > It is not available in musl and it breaks compilation in Alpine Linux.
> > So I can't take this patch.  
> 
> Shall we limit this for Glibc at compile time ?
> 
> +#if defined __GLIBC__ && defined __GLIBC_PREREQ
> +#if __GLIBC_PREREQ(2, 10)
>         /* Gets system memory stat's XML format. */
>         ret = malloc_info(0, fp);
> +#endif
> +#else
> +       RTE_LOG(DEBUG, EAL, "Error: malloc_info not supported by libc\n");
> +       fclose(fp);
> +       return -1;
> +#endif
> 
> I will send out a v6 if the above solution is fine.
> 
> > 
> >   
> > >
> > > 1. /sysmem/sys_heap_list
> > > The commands displays the arenas currently in use.
> > > Example:  
> > > --> /sysmem/sys_heap_list  
> > > {"/sysmem/sys_heap_list": [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]}
> > >  
> > 
> > I am unsure about the command names.
> > 
> > Those commands are really low level and tied to glibc malloc.
> > It is unlikely we will have an unified naming with other libc/OS.
> > 
> > I would prefer another pair of eyes, especially on this patch.
> > Dmitry, Anatoly?

I agree with David.

Microsoft CRT provides completely different statistics.
I can't say for FreeBSD.
Even if GNU libc is used, the process can be run with jemalloc, for example,
and this statistics become meaningless then.
What is "system" memory, anyway? It can be acquired bypassing malloc,
it can be hugepages mmap'd without DPDK, etc.

What is the task this API must solve?
My guess is that monitoring needs to know how much physical memory
1) the process uses and 2) how much it can use, so on Linux this API
could be implemented using getrusage() and getrlimit().
I don't think that a fully portable implementation is possible,
but at least API will be unified.