[PATCH v2 0/3] mm: page_counter: move hierarchical protection out of struct page_counter

linuszeng via B4 Relay posted 3 patches 2 weeks, 2 days ago
include/linux/memcontrol.h   | 15 ++++++---
include/linux/page_counter.h | 73 +++++++++++++++++++++++++++++++-------------
kernel/cgroup/dmem.c         | 21 +++++++------
mm/hugetlb_cgroup.c          |  4 +--
mm/memcontrol.c              | 29 ++++++++++--------
mm/page_counter.c            | 61 +++++++++++++++++++++++-------------
6 files changed, 133 insertions(+), 70 deletions(-)
[PATCH v2 0/3] mm: page_counter: move hierarchical protection out of struct page_counter
Posted by linuszeng via B4 Relay 2 weeks, 2 days ago
Hierarchical memory protection (memory.min / memory.low) is built on
struct page_counter today: every counter carries the full protection
state - emin/elow, the protected-usage trackers (min_usage,
children_min_usage, low_usage, children_low_usage), the configured
min/low values and a protection_support flag - although only the memory
page counter (and dmem pools) ever participates in protection.
swap/memsw, kmem, tcpmem and hugetlb counters ship this state around
unused.

This series moves that state into a dedicated struct
page_counter_protection, instantiated only for the counters that
actually support protection, which shrinks struct page_counter by one
cache line.

Patch 1 adds struct page_counter_protection and links it to struct
page_counter through a ->prot pointer (NULL when protection is not
supported).  page_counter_init() loses its protection_support argument,
and the new page_counter_init_protection() attaches the context.
Protection stays enabled only on the cgroup v2 hierarchy, matching the
previous page_counter_init(..., memcg_on_dfl) behaviour, and the root
memcg keeps it unconditionally.

Patch 2 migrates the read/write side of protection onto the new
structure: propagate_protected_usage(), page_counter_set_min()/low()
and page_counter_calculate_protection() now operate on the protection
context, and the memcg and dmem accessors (mem_cgroup_protection,
mem_cgroup_below_min/low, the dmem below_min/low helpers and the dmem
eviction check) read emin/elow/children_*_usage from it.

Patch 3 deletes the now-unused fields from struct page_counter.  On
64-bit the structure drops from three cache lines to two, one cache
line saved per counter.  For reference, pahole shows the layout before
and after (x86_64, 64-byte cache lines):

    before:                                after:
     0  usage                               0  usage
     8  failcnt                             8  failcnt
    64  emin                               64  watermark
    72  min_usage                          72  local_watermark
    80  children_min_usage                 80  track_failcnt
    88  elow                               88  high
    96  low_usage                          96  max
   104  children_low_usage                104  parent
   112  watermark                         112  prot
   120  local_watermark
   128  protection_support                 size 128, 2 cachelines,
   129  track_failcnt                         11 members
   136  min
   144  low                                (the protection fields moved
   152  high                               into struct
   160  max                                page_counter_protection,
   168  parent                              72 bytes, allocated only
                                           where protection is used)
   size 192, 3 cachelines, 19 members

The four embedded page counters of struct mem_cgroup all shrink by 64
bytes, which translates to 128 bytes saved per cgroup once the one
embedded page_counter_protection is accounted for (2176 -> 2048 bytes
with CONFIG_MEMCG_V1=y, verified with pahole).

No functional change is intended: protection semantics and the cgroup
v1/v2 behaviour are preserved.

Signed-off-by: linuszeng <linuszeng@tencent.com>
---
Changes in v2:
- dmem: fix up prot.parent in get_cg_pool_locked() too, so bottom-up
  created pools keep hierarchical protection.
- Drop the orphaned _pad2_ padding and its stale comment from struct
  page_counter.
- Link to v1: https://lore.kernel.org/r/20260909-descriptive-name-v1-0-1828961cb01a@tencent.com

---
linuszeng (3):
      mm: page_counter: add page_counter_protection struct and init API
      mm: page_counter: track protection state in page_counter_protection
      mm: page_counter: drop protection fields from struct page_counter

 include/linux/memcontrol.h   | 15 ++++++---
 include/linux/page_counter.h | 73 +++++++++++++++++++++++++++++++-------------
 kernel/cgroup/dmem.c         | 21 +++++++------
 mm/hugetlb_cgroup.c          |  4 +--
 mm/memcontrol.c              | 29 ++++++++++--------
 mm/page_counter.c            | 61 +++++++++++++++++++++++-------------
 6 files changed, 133 insertions(+), 70 deletions(-)
---
base-commit: d118502628f8b673be9023db8bdf878f64a7ed45
change-id: 20260909-descriptive-name-e382a3f978dd

Best regards,
-- 
linuszeng <linuszeng@tencent.com>
Re: [PATCH v2 0/3] mm: page_counter: move hierarchical protection out of struct page_counter
Posted by Michal Koutný 2 weeks ago
Hi.

On Wed, Sep 09, 2026 at 05:44:18PM +0800, linuszeng via B4 Relay <devnull+linuszeng.tencent.com@kernel.org> wrote:
> No functional change is intended: protection semantics and the cgroup
> v1/v2 behaviour are preserved.

It is not clear from the description what is the intention then :-)

Do you have any measurements that the reduced cache footprint
changes performance for setups without protection?
And what is the positive impact on protected scenarios where the
counters are in (possibly) different cacheline and one indirection
further?

Thanks,
Michal
Re: [PATCH v2 0/3] mm: page_counter: move hierarchical protection out of struct page_counter
Posted by jingxiang zeng 1 week, 1 day ago
On Fri, 11 Sept 2026 at 23:17, Michal Koutný <mkoutny@suse.com> wrote:
>
> Hi.
>
> On Wed, Sep 09, 2026 at 05:44:18PM +0800, linuszeng via B4 Relay <devnull+linuszeng.tencent.com@kernel.org> wrote:
> > No functional change is intended: protection semantics and the cgroup
> > v1/v2 behaviour are preserved.
>
> It is not clear from the description what is the intention then :-)

You are right, and that is my fault for posting these three patches
without the context they came from.

They implement what Johannes asked for when I last tried to make a
combined memory+swap limit available on the default hierarchy [1]:

  My suggestion is to factor out from struct page_counter all the stuff
  that is not necessary for all users, and then have separate counters
  for swap and memsw.

  The protection stuff is long overdue for this. It makes up nearly half
  of the struct's members, but is only used by the memory counter. Even
  before your patches this is unnecessary bloat in the swap/memsw, kmem
  and tcpmem counters.

  Fix that and having separate counters is a non-issue.

and, in the same thread, about the cost of a second counter [2]:

  It seems like a good opportunity to refactor struct page_counter.

So the intention is not the cache footprint on its own.

The combined memory+swap counter is the one v1 exposes as
memsw.limit_in_bytes.  On the default hierarchy it does not exist:
struct mem_cgroup keeps swap and memsw in a union, because v1 only ever
charges memsw and v2 only ever charges swap, so the two never needed to
be live at the same time.  Making the combined limit available on v2
means charging both on both hierarchies, which means giving them separate
page counters.

That is where struct page_counter comes in.  Adding a counter costs a
cgroup one more of them, and at 192 bytes each that is 192 bytes per
cgroup for a feature most of them will not use.  Trimming the counter to
128 bytes first frees 128 bytes per cgroup, which is very nearly what the
new counter then costs, so the combined limit becomes close to free in
struct mem_cgroup rather than something every cgroup pays for.  This is
the "good opportunity to refactor struct page_counter" from [2], and it
is why the preparation is a prerequisite rather than a cleanup I happened
to do on the side.

The follow-up is written and tested; I should have posted it together
with these patches instead of sending the preparation on its own, and I
will do that now (details at the end).

>
> Do you have any measurements that the reduced cache footprint
> changes performance for setups without protection?

No.  So far I have only used pahole to look at the cache line footprint
of struct mem_cgroup and struct page_counter (pahole, x86_64,
64-byte cache lines, CONFIG_MEMCG_V1=y)::

  struct page_counter             192 -> 128 bytes  (3 -> 2 cache lines)
  struct page_counter_protection    -  ->  72 bytes
  struct mem_cgroup              2176 -> 2048 bytes

The four embedded counters lose 64 bytes each and the one protection
context takes 72 back, which nets out to 128 bytes per cgroup.  The
counters that never participate in protection - swap/memsw, kmem, tcpmem
and hugetlb - are also down from three cache lines to two.

The reason I need these patches is the prerequisite
above, and those 128 bytes are exactly what the follow-up needs to afford
splitting the swap and memsw counters.

> And what is the positive impact on protected scenarios where the
> counters are in (possibly) different cacheline and one indirection
> further?

There is none, and in the form I posted it was worse than before.  Your
reading of the layout was correct.

propagate_protected_usage() runs once per level on every charge and
uncharge, and touches min, low, min_usage, low_usage and the parent's
children_{min,low}_usage.  Counting the cache lines each level touches:

  before this series   page_counter 3 + parent 1 = 4, no indirection
  v2 as posted         page_counter 2 + prot 2 + parent prot 1 = 5
  with the fix below   page_counter 2 + prot 1 + parent prot 1 = 4

In v2 the new structure kept the field order of the old one, which put
min at offset 56 and low at 64.  The two values the propagation path
reads together ended up on either side of a cache line boundary, while
emin and elow - which are only recomputed by
page_counter_calculate_protection() during reclaim - occupied the first
line.  That is how the count got to 5.

The seven fields the charge path touches are 56 bytes and do fit in one
line, so I have reordered the structure to parent, min, low, min_usage,
low_usage, children_min_usage, children_low_usage, then emin and elow
last.  Both embedders already place the context on a cache line boundary
- offset 384 in struct mem_cgroup, 192 in the dmem pool state - so no
alignment attribute is needed and the structure stays 72 bytes.  That
brings the per-level line count back to what it was before the series.

The dependent load of ->prot stays; it cannot be removed while the state
lives outside the counter.  The pointer shares a line with ->parent and
->local_watermark, which the same loop reads anyway, so it costs an
address dependency rather than an extra miss.

To summarise honestly: this series is size-neutral for a cgroup and,
after the reorder, layout-neutral for protected charging.  It earns its
place only as groundwork for the combined limit.

So rather than reposting these three patches on their own, I am going to
send them as the first half of

  [PATCH 0/6] mm/memcontrol: implement the memsw limit on cgroup v2

which is not posted yet; it will follow shortly after this reply, with
the reordered protection context folded into patch 1.  The second half
builds directly on them: it splits the swap and memsw page counters out
of their union - which is what needs struct page_counter to have stopped
carrying the protection state - maintains the combined counter on both
hierarchies, and adds memory.memsw.current and memory.memsw.max to the
default hierarchy.

[1] https://lore.kernel.org/all/20250320144722.GH1876369@cmpxchg.org/
[2] https://lore.kernel.org/all/20250320142846.GG1876369@cmpxchg.org/

Thanks for looking at this.

>
> Thanks,
> Michal