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

[Hongfu Li]

Hi Harry,

Thanks for your review.

> > > > +def objexts_flags_mask():
> > > > +    try:
> > > > +        return int(prog.constant('__NR_OBJEXTS_FLAGS')) - 1
> > > > +    except:
> > > > +        return 0x7
> > > > +
> > > > +
> > > >   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',
> > > > @@ -192,23 +199,24 @@ def main():
> > > >           # look over all slab folios and look for objects belonging
> > > >           # to the given memory cgroup
> > > >           for slab in for_each_slab(prog):
> > > > -            objcg_vec_raw = slab.memcg_data.value_()
> > > > -            if objcg_vec_raw == 0:
> > > > +            objext_vec_raw = slab.obj_exts.value_()
> > > 
> > > Here, we should also check whether it equals OBJEXTS_ALLOC_FAIL,
> > > 
> > > which indicates that the allocation of obj_exts has failed, Or have I 
> > > missed something here?
> > 
> > An obj_ext with the OBJEXTS_ALLOC_FAIL flag may not belong to any cgroup.
> > Should we add a check and break this round of polling, or add a comment
> > explaining this scenario?
> 
> I think it should just skip that slab and continue.
> 
> But it's very subtle: obj_exts == OBJEXTS_ALLOC_FAIL (bit 0) means
> it failed to allocate the vector, but (obj_exts != OBJEXTS_ALLOC_FAIL)
> && (obj_exts & MEMCG_DATA_OBJEXTS (bit)) means it is a pointer to
> slabobj_ext vector.
> 
> There is one more thing to consider: it may not be an array of
> slabobj_ext as you may expect.
> 
> See how slab_obj_ext() works:
> https://lore.kernel.org/linux-mm/20260113061845.159790-1-harry.yoo@xxxxxxxxxx

I truly appreciate your insightful suggestions; they have been extremely helpful
to me. I'm glad to submit the next patch for review.

Thanks again for your review.

Best Regards,
Hongfu