Re: [PATCH v6 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Joshua Hahn <joshua.hahnjy@gmail.com>]

On Fri, 18 Sep 2026 09:58:17 +0200 Michal Koutný <mkoutny@suse.com> wrote:

> Hi.
> 
> On Thu, Sep 17, 2026 at 10:57:00AM -0700, Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> > 
> > This is true, but this is already the behavior for vanilla memcg.
> > In this series I'm hoping to preserve all existing semantics without
> > changing behaviors, so I can fix this problem in a separate issue.
> > 
> > Specifically, in vanilla try_charge_memcg:
> > 
> > done_restock:
> > 	if (batch > nr_pages)
> > 		refill_stock(memcg, batch - nr_pages);
> > 
> > ...
> > 			current->memcg_nr_pages_over_high += batch;
> > 
> > So I've just preserved the exact semantics that we used to have before.
> 
> Kudos to you for the conservative approach.

Hi Michal, thanks for your kind words : -)

> > 
> > The problem isn't that big anyways though, it's a transient inflation
> > in memcg_over_high and will be wiped on the next high handling run,
> > and there is no effect on accounting or permanent inflations.
> 
> It reminds me [1] where stockage imprecision could even trigger OOM but
> it was reportedly only visible in LTP.

So I was thinking about this part quite a bit in the older versions of
this series where I changed the draining from being async to sync.

If I understand the issue you saw in LTP correctly, it's not one of
actual memory OOMs but that there was stock that the memcg could use,
just not in the current CPU that was trying to fulfill the charge,
and also the draining for the remote CPUs did not happen in the time
that it took to run through 16 retry attempts.

IOW seems to me we are synchronously waiting for asynchronous drains.

To me this did sound a little bit unfortunate since there really is
memory that the memcg could have used, just cached in the "wrong"
place (and for some reason that CPU is too busy to do the drain).
One of the versions that I worked on previously (v4, [2]) used an atomic
for the stock so that we just always do a synchronous drain via
cmpxchg and we wouldn't have the problem above.

Obviously there are pros and cons. Synchronous means the reclaim path
has the ability to reclaim more memory NOW and prevent futile reclaim
retries, but also it makes each flushing operation more expensive as
the cmpxchg operation is more expensive than acquiring a local trylock,
even if it succeeds on the first try. 

In my opinion just doing the synchronous reclamation makes a bit more
sense to me since it gives a stronger guarantee that we are going to
get the charges we need to fulfill the charge attempt NOW rather than
spinning until some remote CPU gets scheduled to perform a local flush.

But this is just personal taste, I haven't really seen too much 
evidence that we are wasting too much time on the reclaims. So maybe
asynchronous drains are OK here. I can do some more digging in our
production data to see if we're ever just waiting for remote drains
to finish.

Anyways, I still intend on separating out the two efforts so that this
one is just a simple code move (and make stock more scalable) for
cgroup v2 users (and I hope not-so-big impact for cgroup v1 users)
and I can work on removing the 7-slot limit and improving the draining
as a follow-up work.

> HTH,
> Michal

Thanks for your thoughts Michal. Have a great day!
Joshua

> [1] https://lore.kernel.org/all/20250530151858.672391-1-mkoutny@suse.com/
[2] https://lore.kernel.org/all/20260623180124.868655-2-joshua.hahnjy@gmail.com/