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

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

On Mon, 2 Nov 2026 13:25:28 +0100 Michal Hocko <mhocko@suse.com> wrote:

> On Mon 26-10-26 08:59:37, Joshua Hahn wrote:
> > > > To make future page_counters scalable and performant, move stock from
> > > > mem_cgroup to page_counter so that each page_counter can opt into its
> > > > own per-CPU cache of pre-charged pages.
> > > 
> > > What does it mean that each counter can opt into its own per-cpu
> > > caching?
> > 
> > So right now for cgroupv2, there's kind of a 1-1 mapping between
> > page_counter and memcg. I say "kind of" because there still is a
> > page_counter for swap, but it takes a completely different charging
> > path (i.e. not try_charge_memcg).
> > 
> > This means that currently stocking charge for memcg is equivalent to
> > stocking charge for the memcg->memory page_counter.
> 
> correct
> 
> > However, one of the series that I'm currently working on [1] adds new
> > page_counters to struct memcg to track the usage of memory
> > per-memcg, per-tier. The goal is to (1) track how much memory of a
> > cgroup lives on which tier, and (2) to limit the usage so that a single
> > cgroup doesn't use up all the fast tier memory alone, and shares it
> > fairly with all the other cgroups on the host.
> 
> Right. I guess you have layed down your motivation quite well in your
> original cover letter.

Thanks, Michal!

> > When adding a new per-tier page_counter to struct mem_cgroup that gets
> > charged for every memcg_charge, I cannot use the stock fastpath since
> > the stock does not distinguish where the memory resides. It would
> > essentialy boil down to this pattern:
> > 
> > try_charge_memcg
> >    \__ page_couner_try_charge(memcg->tier[0])
> >    \__ consume_stock()
> > 
> > Where every path still has to do a hierarchy charge to the tier it's
> > charging to, whereas this patch gets us:
> > 
> > try_charge_memcg
> >    \__ page_counter_try_charge(memcg->tier[0])
> >           \__ consume_stock(memcg->tier[0])
> >    \__ page_countertry_charge(memcg->memory)
> >           \__ consume_stock(memcg->memory)
> > 
> > So that we can avoid the hierarchy walk for most charges.
> > 
> > I'd be happy to submit a new cover letter which better explains this
> > idea!
> 
> I guess I still do not understand what you mean by opting-in to the
> caching.

Reading my original cover letter again, yeah. It does feel a bit strange
to call it "opt-in"... I guess I just meant that not all page_coutners
will be getting it, it would only really be the memory page_counter
(and memsw page_counter for cgroup v2). I'll change the wording in the
next version.

> I believe you should focus less about what you plan to do and
> rather explain what _this_ patchset achieves and how it works.
> Also focus on describing why do we need to keep caches on all layers of
> the hierarchy.

Yes, I'll do that. Actually, this patchset is really quite limited in
what it does. I actually explicitly tried to do no optimizations or
behavioral changes at all, this is mostly just a clean-cut code
move from memcg code to page_counter code. 

You may recall earlier versions of this series did a bunch more stuff
(making the stock scale past 7 memcgs, dynamically allocating the stock,
at some point I even had a version that only cached leaf cgroups, etc.)

I got some feedback that these optimizations were orthogonal to the
movement of stock from memcg to page_counter, which is why this version
explicitly does none of the fancy optimizations on top. It mostly is
just moving code from memcg to page_counter.

> > > > 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.

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.

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
brings the counters to their real values (like what happens right now).

Finally, this is a change that is only limited to cgroup v1 users.
This entire series actually has no effect at all on cgroup v2 users,
which I think is a nice perk. Obviously I'm not out here trying to
make cgroup v1 unusable, but I wonder if anyone is still using the
legacy feature.

I hope this wasn't too much of a brain dump on you. Thanks for your
feedback Michal. Please let me know if there is anything I can do to
clarify my intent.

Would you prefer if we continud the discussion here, or if I sent out
a new series with just the cover letter changed? (I don't anticipate
any code changes, based on sashiko review or anything else that I see).

Thanks again. I hope you have a great day!
Joshua