Re: [PATCH 6/6] slab: Introduce kmalloc_nolock() and kfree_nolock().

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Mon, May 5, 2025 at 6:25 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> > > >               obj_exts[off].objcg = NULL;
> > > > -             refill_obj_stock(objcg, obj_size, true, -obj_size,
> > > > -                              slab_pgdat(slab), cache_vmstat_idx(s));
> > > > +             if (unlikely(lock_held)) {
> > > > +                     atomic_add(obj_size, &objcg->nr_charged_bytes);
> > >
> > > objcg->nr_charged_bytes is stats ignorant and the relevant stats need to
> > > be updated before putting stuff into it.
> >
> > I'm not following.
> > It's functionally equivalent to refill_obj_stock() without
> > __account_obj_stock().
> > And the stats are not ignored.
> > The next __memcg_slab_free_hook() from good context will update
> > them. It's only a tiny delay in update.
> > I don't see why it's an issue.
>
> For the slab object of size obj_size which is being freed here, we need
> to update NR_SLAB_RECLAIMABLE_B or NR_SLAB_UNRECLAIMABLE_B stat for the
> corresponding objcg by the amount of obj_size. If we don't call
> __account_obj_stock() here we will loose the context and information to
> update these stats later.

Lose context?
pgdat has to match objcg, so I don't think we lose anything.
Later refill_obj_stock() will take objcg->nr_charged_bytes and
apply to appropriate NR_SLAB_[UN]RECLAIMABLE_B.
The patch introduces a delay in update to stats.
I still think it's correct, but I agree it's harder to reason about it,
since stats may not reflect the real numbers at that very instance.
But seeing how rstat flush is periodic and racy too I don't see
how it's a problem in practice.

>
> The css_rstat_updated() is the new name of cgroup_rstat_updated() and it
> is only a piece of the puzzle. My plan is to memcg stats reentrant which
> would allow to call __account_obj_stock (or whatever new name would be)
> in nmi context and then comsume_obj_stock() and refill_obj_stock() would
> work very similar to consume_stock() and refill_stock().
>
> Please give me couple of days and I can share the full RFC of the memcg
> side.

Sure thing. I don't insist on this particular way of charge/uncharge.
Sounds like I should drop mm/memcontrol.c bits, since your approach
will address the same re-entrance issue but in a more synchronous
(easier to reason about) way. Thanks!