Re: [PATCH 0/1] alloc_tag: add per-numa node stats

[Suren Baghdasaryan <surenb@google.com>]

On Tue, Jun 3, 2025 at 5:55 PM Casey Chen <cachen@purestorage.com> wrote:
>
> On Tue, Jun 3, 2025 at 8:01 AM Suren Baghdasaryan <surenb@google.com> wrote:
> >
> > On Mon, Jun 2, 2025 at 2:32 PM Suren Baghdasaryan <surenb@google.com> wrote:
> > >
> > > On Mon, Jun 2, 2025 at 1:48 PM Casey Chen <cachen@purestorage.com> wrote:
> > > >
> > > > On Fri, May 30, 2025 at 5:05 PM Kent Overstreet
> > > > <kent.overstreet@linux.dev> wrote:
> > > > >
> > > > > On Fri, May 30, 2025 at 02:45:57PM -0700, Casey Chen wrote:
> > > > > > On Thu, May 29, 2025 at 6:11 PM Kent Overstreet
> > > > > > <kent.overstreet@linux.dev> wrote:
> > > > > > >
> > > > > > > On Thu, May 29, 2025 at 06:39:43PM -0600, Casey Chen wrote:
> > > > > > > > The patch is based 4aab42ee1e4e ("mm/zblock: make active_list rcu_list")
> > > > > > > > from branch mm-new of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > > > >
> > > > > > > > The patch adds per-NUMA alloc_tag stats. Bytes/calls in total and per-NUMA
> > > > > > > > nodes are displayed in a single row for each alloc_tag in /proc/allocinfo.
> > > > > > > > Also percpu allocation is marked and its stats is stored on NUMA node 0.
> > > > > > > > For example, the resulting file looks like below.
> > > > > > > >
> > > > > > > > percpu y total         8588     2147 numa0         8588     2147 numa1            0        0 kernel/irq/irqdesc.c:425 func:alloc_desc
> > > > > > > > percpu n total       447232     1747 numa0       269568     1053 numa1       177664      694 lib/maple_tree.c:165 func:mt_alloc_bulk
> > > > > > > > percpu n total        83200      325 numa0        30976      121 numa1        52224      204 lib/maple_tree.c:160 func:mt_alloc_one
> > > > > > > > ...
> > > > > > > > percpu n total       364800     5700 numa0       109440     1710 numa1       255360     3990 drivers/net/ethernet/mellanox/mlx5/core/cmd.c:1410 [mlx5_core] func:mlx5_alloc_cmd_msg
> > > > > > > > percpu n total      1249280    39040 numa0       374784    11712 numa1       874496    27328 drivers/net/ethernet/mellanox/mlx5/core/cmd.c:1376 [mlx5_core] func:alloc_cmd_box
> > > > > > >
> > > > > > > Err, what is 'percpu y/n'?
> > > > > > >
> > > > > >
> > > > > > Mark percpu allocation with 'percpu y/n' because for percpu allocation
> > > > > > stats, 'bytes' is per-cpu, we have to multiply it by the number of
> > > > > > CPUs to get the total bytes. Mark it so we know the exact amount of
> > > > > > memory used. Any /proc/allocinfo parser can understand it and make
> > > > > > correct calculations.
> > > > >
> > > > > Ok, just wanted to be sure it wasn't something else. Let's shorten that
> > > > > though, a single character should suffice (we already have a header that
> > > > > can explain what it is) - if you're growing the width we don't want to
> > > > > overflow.
> > > > >
> > > >
> > > > Does it have a header ?
> > >
> > > Yes. See print_allocinfo_header().
> >
> > I was thinking if instead of changing /proc/allocinfo format to
> > contain both total and per-node information we can keep it as is
> > (containing only totals) while exposing per-node information inside
> > new /sys/devices/system/node/node<node_no>/allocinfo files. That seems
> > cleaner to me.
> >
>
> The output of /sys/devices/system/node/node<node_no>/allocinfo is
> strictly limited to a single PAGE_SIZE and it cannot display stats for
> all tags.

Ugh, that's a pity. Another option would be to add "nid" column like
this when this config is specified:

nid     bytes    calls
0         8588     2147             kernel/irq/irqdesc.c:425 func:alloc_desc
1         0           0                   kernel/irq/irqdesc.c:425
func:alloc_desc
...

It bloats the file size but looks more structured to me.

>
> > I'm also not a fan of "percpu y" tags as that requires the reader to
> > know how many CPUs were in the system to make the calculation (you
> > might get the allocinfo content from a system you have no access to
> > and no additional information). Maybe we can have "per-cpu bytes" and
> > "total bytes" columns instead? For per-cpu allocations these will be
> > different, for all other allocations these two columns will contain
> > the same number.
>
> I plan to remove 'percpu y/n' from this patch and implement it later.
>
> >
> > >
> > > >
> > > > > >
> > > > > > > >
> > > > > > > > To save memory, we dynamically allocate per-NUMA node stats counter once the
> > > > > > > > system boots up and knows how many NUMA nodes available. percpu allocators
> > > > > > > > are used for memory allocation hence increase PERCPU_DYNAMIC_RESERVE.
> > > > > > > >
> > > > > > > > For in-kernel alloc_tags, pcpu_alloc_noprof() is called so the memory for
> > > > > > > > these counters are not accounted in profiling stats.
> > > > > > > >
> > > > > > > > For loadable modules, __alloc_percpu_gfp() is called and memory is accounted.
> > > > > > >
> > > > > > > Intruiging, but I'd make it a kconfig option, AFAIK this would mainly be
> > > > > > > of interest to people looking at optimizing allocations to make sure
> > > > > > > they're on the right numa node?
> > > > > >
> > > > > > Yes, to help us know if there is an NUMA imbalance issue and make some
> > > > > > optimizations. I can make it a kconfig. Does anybody else have any
> > > > > > opinion about this feature ? Thanks!
> > > > >
> > > > > I would like to see some other opinions from potential users, have you
> > > > > been circulating it?
> > > >
> > > > We have been using it internally for a while. I don't know who the
> > > > potential users are and how to reach them so I am sharing it here to
> > > > collect opinions from others.
> > >
> > > Should definitely have a separate Kconfig option. Have you measured
> > > the memory and performance overhead of this change?