Re: [PATCH v3] tools/cgroup/slabinfo: Fix use of slab.memcg_data

[Hongfu Li]

Hi Hao,

Thanks for your review.

> > After the introduce slabobj_ext to support slab object extensions, the
> > memcg_slabinfo tool broke. An attempt to run it produces a trace like
> > this:
> > Traceback (most recent call last):
> >    File "/usr/local/bin/drgn", line 8, in <module>
> >      sys.exit(_main())
> >               ^^^^^^^
> >    File "/usr/local/lib64/python3.11/site-packages/drgn/cli.py", line 688, in _main
> >      runpy.run_path(
> >    File "<frozen runpy>", line 291, in run_path
> >    File "<frozen runpy>", line 98, in _run_module_code
> >    File "<frozen runpy>", line 88, in _run_code
> >    File "/root/memcg_slabinfo.py", line 225, in <module>
> >      main()
> >    File "/root/memcg_slabinfo.py", line 195, in main
> >      objcg_vec_raw = slab.memcg_data.value_()
> > AttributeError: 'struct slab' has no member 'memcg_data'
> >
> > Fixes: 21c690a349ba ("mm: introduce slabobj_ext to support slab object extensions")
> > Fixes: 5ba6bc27b1f9 ("slab: decouple pointer to barn from kmem_cache_node")
> > Signed-off-by: Hongfu Li <lihongfu@xxxxxxxxxx>
> > ---
> > v3:
> > - Add a compatibility accessor for per-node cache state (per_node[nid].node vs node[nid])
> > - Link to v2: https://lore.kernel.org/all/20260421055829.3930289-1-lihongfu@xxxxxxxxxx/
> > v2:
> > - Skip slabs after masking when the base is zero (OBJEXTS_ALLOC_FAIL).
> > - Walk slab->obj_exts using base + stride * i (same indexing as slab_obj_ext()).
> > - Link to v1: https://lore.kernel.org/all/20260417020729.952897-1-lihongfu@xxxxxxxxxx/
> > ---
> >   tools/cgroup/memcg_slabinfo.py | 45 ++++++++++++++++++++++++++++------
> >   1 file changed, 38 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/cgroup/memcg_slabinfo.py b/tools/cgroup/memcg_slabinfo.py
> > index 6bf4bde77903..f49512831d6a 100644
> > --- a/tools/cgroup/memcg_slabinfo.py
> > +++ b/tools/cgroup/memcg_slabinfo.py
> > @@ -33,6 +33,21 @@ def err(s):
> >       sys.exit(1)
> >   
> >   
> > +def objexts_flags_mask():
> > +    try:
> > +        return int(prog.constant('__NR_OBJEXTS_FLAGS')) - 1
> > +    except:
> > +        return 0x7
> > +
> > +
> > +def slab_obj_ext_stride(slab):
> > +    # Match slab_obj_ext() for both layouts: contiguous ext[] or ext-in-object
> > +    try:
> > +        return slab.stride.value_()
> > +    except AttributeError:
> > +        return prog.type('struct slabobj_ext').size
> > +
> > +
> >   def find_memcg_ids(css=prog['root_mem_cgroup'].css, prefix=''):
> >       if not list_empty(css.children.address_of_()):
> >           for css in list_for_each_entry('struct cgroup_subsys_state',
> > @@ -68,6 +83,13 @@ def oo_objects(s):
> >       return s.oo.x & OO_MASK
> >   
> >   
> > +def kmem_cache_get_node(s, nid):
> > +    try:
> > +        return s.per_node[nid].node
> > +    except AttributeError:
> > +        return s.node[nid]
> > +
> 
> This gives me another idea.
> 
> So actually, we don't need to forcibly convert memcg_data to obj_exts, 
> right?
> 
> like this:
> 
> try:
>        raw = slab.memcg_data.value_()
>        new_layout = False
>    except AttributeError:
>        raw = slab.obj_exts.value_()
>        new_layout = True
> 
> and then take the new path based on new_layout.
> 
> But I'm not sure if we need to do this, as it would indeed make the 
> helper handle more and more cases.

Thanks for sharing this idea!

It provides a clean way to support both layouts without forced conversion
of memcg_data to obj_exts. I'm happy to implement this approach in the next
version of the patch.

Thanks again for your review.

Best Regards,
Hongfu