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

[Michal Hocko <mhocko@suse.com>]

On Mon 02-11-26 08:30:14, Joshua Hahn wrote:
> On Mon, 2 Nov 2026 17:16:12 +0100 Michal Hocko <mhocko@suse.com> wrote:
> 
> > > > > > > We get an added benefit of simplifying try_charge_memcg(), which now
> > > > > > > has all the stock management handled transparently within the
> > > > > > > page_counter layer.
> > > > > > 
> > > > > > This is quite a large change and I believe it will be really benefitial
> > > > > > to have some high level design overview in the cover rather than trying to
> > > > > > chase it in respective patches.
> > > > > 
> > > > > Sounds good with me. I can add the call traces above which should
> > > > > explain my thought process a bit more. Hopefully the problem I am
> > > > > trying to solve is evident there.
> > > > 
> > > > I believe I follow where you are heading. I am not sure this is the
> > > > right path but that is a separate discussion. I would focus on whether
> > > > moving cache to the counter makes sense on its own (without that future
> > > > work) at this stage.
> > > 
> > > If I'm being honest, most of the generic benefits that exist beyond that
> > > work were in the other part of this series that I got rid of to scale
> > > down the scope of this version. But to explicitly name the generic
> > > benefits, it would be unbounded memcg stock caching per-cpu, eliminating
> > > random evictions, (like you mentioned above as well) limiting stocks
> > > to leaf memcgs only, etc.
> > 
> > limiting cache to leaf memcgs makes a lot of sense and addresses a big
> > concern I had. How exactly does this work?
> 
> It's not a big change, actually. This was the implementation from the
> origianl RFC of the series:
> 
> https://lore.kernel.org/all/20260410210742.550489-8-joshua.hahnjy@gmail.com/
> 
> The idea is that you treat a cgroup as a leaf until it becomes a parent,
> at which point you drain the stock. Stock can land on the parent due
> to hierarchy walks, but I can also write another part to this that
> prevents stock from getting cached when a cgroup has children.

OK, makes sense.

> > > I think there was a proposal to make the number of memcg stocks per cpu
> > > even lower than it already is, because the author wanted to include
> > > additional fields into the stock that would make 7 no longer fit into
> > > a single cacheline. I feel like we are reaching some limits with that
> > > side as well, so the "unbounded" portion seems to be quite opportune.
> > > 
> > > But I believe those benefits that we would like to see are made better
> > > by this series, which makes the stocking transparent such that the new
> > > optimizations don't make the existing memcg charging code any more
> > > complex than it is. By abstracting away this mechanism, we can make sure
> > > that memcg just handles the logical charging semantics, and page_counter
> > > handles the implementation-level details of stock management.
> > 
> > Yes, these are all good arguments. Especially when clarified that this
> > leaf only feature. I have to say I was struggling to see how caching on
> > all levels of the hierarchy would work in practice.
> 
> Sounds good. If it makes sense to you then, I can fold in the
> leaf-only caching into this series to maintain more parity with the
> existing code. How does that sound? 

good

> > > Personally I am a big fan of how try_charge_memcg looks after this
> > > series, what do you think? 
> > > 
> > > > Quite honestly I am still not convinced this move
> > > > is the right thing but I would like to fully understand the mechanics
> > > > first. Having discrepancies among different counters just sounds like a
> > > > potential source of future problems.
> > > 
> > > I hope that I can present a good argument for why this is not as
> > > concern as it may seem initially.
> > > 
> > > My first point is that the memory usage counters were always fuzzy, as
> > > specified in the memcg v1 documentation too: 
> > > 
> > > usage_in_bytes ... doesn't show 'exact' value of memory (and swap)
> > > usage, it's a fuzz value for efficient access.
> > > 
> > > Secondly, the raw value of the usage is actually always accurate. It is
> > > just the stock portion that overcharge the memory at times, making it
> > > seem like there is a discrepancy. Flushing the memcg stock immediately
> > > bring the counters to their real values (like what happens right now).
> > > 
> > > Finally, this is a change that is only limited to cgroup v1 users.
> > 
> > Right but is there any actual point in adding caching for memsw charges
> > now? If you have a way to opt-in to caching then you can simply disabled
> > that for memsw counter altogether. Fewer problems to deal with, explain
> > etc...
> 
> I think the problem actually still exists, unfortunately. In fact I
> think the issue becomes bigger if you remove stocking for memsw.

Yes, you are right. We do actually cache memsw through the charging path
which charges both mem and memsw with the batch. 
-- 
Michal Hocko
SUSE Labs