Re: [PATCH bpf-next v5 4/7] memcg: Use trylock to access memcg stock_lock.

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

On Wed, Jan 15, 2025 at 4:12 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> On Tue, Jan 14, 2025 at 06:17:43PM -0800, Alexei Starovoitov wrote:
> > From: Alexei Starovoitov <ast@kernel.org>
> >
> > Teach memcg to operate under trylock conditions when spinning locks
> > cannot be used.
> >
> > local_trylock might fail and this would lead to charge cache bypass if
> > the calling context doesn't allow spinning (gfpflags_allow_spinning).
> > In those cases charge the memcg counter directly and fail early if
> > that is not possible. This might cause a pre-mature charge failing
> > but it will allow an opportunistic charging that is safe from
> > try_alloc_pages path.
> >
> > Acked-by: Michal Hocko <mhocko@suse.com>
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
>
> > @@ -1851,7 +1856,14 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
> >  {
> >       unsigned long flags;
> >
> > -     local_lock_irqsave(&memcg_stock.stock_lock, flags);
> > +     if (!local_trylock_irqsave(&memcg_stock.stock_lock, flags)) {
> > +             /*
> > +              * In case of unlikely failure to lock percpu stock_lock
> > +              * uncharge memcg directly.
> > +              */
> > +             mem_cgroup_cancel_charge(memcg, nr_pages);
>
> mem_cgroup_cancel_charge() has been removed by a patch in mm-tree. Maybe
> we can either revive mem_cgroup_cancel_charge() or simply inline it
> here.

Ouch.

this one?
https://lore.kernel.org/all/20241211203951.764733-4-joshua.hahnjy@gmail.com/

Joshua,

could you hold on to that clean up?
Or leave mem_cgroup_cancel_charge() in place ?