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

[Johannes Weiner <hannes@cmpxchg.org>]

On Thu, Jan 16, 2025 at 12:07:28PM -0800, Joshua Hahn wrote:
> On Wed, 15 Jan 2025 18:22:28 -0800 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > 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:
> > > > @@ -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 ?
> 
> Hi Andrew,
> 
> I think that the patch was moved into mm-stable earlier this week.
> I was wondering if it would be possible to revert the patch and
> replace it with this one below. The only difference is that I leave
> mem_cgroup_cancel_charge untouched in this version.

Let's not revert.

This is a bit of a weird function to keep around without the rest of
the transaction API. It doesn't need to be external linkage, either.

Alexei, can you please just open-code the two page_counter calls?