mm/page_counter: use WRITE_ONCE() for emin/elow when parent is the root

PATCH Applied Shakeel Butt 5 participants 5 messages Oct 21 – Oct 22

Subsystems (3)

Memory management cgroup memcg

lore

Share & tools

lore

Shakeel Butt

author maintainer

2026-10-21 20:48 UTC #0 lore raw

PATCH 1/1 mm/page_counter: use WRITE_ONCE() for emin/elow when parent is the root

page_counter_calculate_protection() runs without locks. Several reclaimers can update the same counter's emin and elow at the same time, and readers such as mem_cgroup_below_min(), mem_cgroup_protection() and the dmem eviction code load them with READ_ONCE().

Commit b3a7822e5e75 ("mm, memcg: prevent mem_cgroup_protected store tearing") made the main path store emin and elow with WRITE_ONCE(), but it missed the early return for a counter whose parent is the reclaim root. Those two stores are the only plain accesses to emin and elow left.

With CONFIG_KCSAN_STRICT=y, a few tasks writing to the same cgroup's memory.reclaim trigger reports like this within seconds:

  BUG: KCSAN: data-race in page_counter_calculate_protection / shrink_node

read (marked) to 0xff1f91b146d40e40 of 8 bytes by task 163 on cpu 4:

   shrink_node+0x57a/0x1210
   do_try_to_free_pages+0x194/0x930
   try_to_free_mem_cgroup_pages+0x204/0x3f0
   user_proactive_reclaim+0x28a/0x4d0
   memory_reclaim+0x35/0x50
   ...

write to 0xff1f91b146d40e40 of 8 bytes by task 167 on cpu 6:

   page_counter_calculate_protection+0x1fa/0x360
   mem_cgroup_calculate_protection+0x4e/0x70
   shrink_node+0x53e/0x1210
   ...

The default KCSAN config does not report it, because it treats aligned word-sized plain writes as atomic.

Use WRITE_ONCE() for these two stores as well.

There is no functional change. Readers already accept stale values, and compilers emit a single word-sized store either way: with GCC 14 the x86-64 object code is the same before and after this change.

Fixes b3a7822e5e75 ("mm, memcg: prevent mem_cgroup_protected store tearing")

Reported-by Sashiko <[email protected]>

Signed-off-by Shakeel Butt <[email protected]>

---
 mm/page_counter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

mm/page_counter.c +2 -2

index 9167ffd1380c..5293460831f8 100644
@@ -472,8 +472,8 @@ void page_counter_calculate_protection(struct page_counter *root,
 		return;
 
 	if (parent == root) {
-		counter->emin = READ_ONCE(counter->min);
-		counter->elow = READ_ONCE(counter->low);
+		WRITE_ONCE(counter->emin, READ_ONCE(counter->min));
+		WRITE_ONCE(counter->elow, READ_ONCE(counter->low));
 		return;
 	}

Signature · 1 line

2.53.0-Meta

Gregory Price

Reviewed-by on v1 1/1

2026-10-21 21:24 UTC #1 lore raw

Quoted 5 lines · Shakeel Butt

page_counter_calculate_protection() runs without locks. Several reclaimers can update the same counter's emin and elow at the same time, and readers such as mem_cgroup_below_min(), mem_cgroup_protection() and the dmem eviction code load them with READ_ONCE().

...

Quoted 10 lines

Use WRITE_ONCE() for these two stores as well.
There is no functional change. Readers already accept stale values, and compilers emit a single word-sized store either way: with GCC 14 the x86-64 object code is the same before and after this change.
Fixes b3a7822e5e75 ("mm, memcg: prevent mem_cgroup_protected store tearing")
Reported-by Sashiko <[email protected]>
Signed-off-by Shakeel Butt <[email protected]>

Reviewed-by Gregory Price (Meta) <[email protected]>

SJ Park

Reviewed-by on v1 1/1

2026-10-22 00:53 UTC #2 lore raw

In reply to Shakeel Butt

Quoted 30 lines · On Wed, 21 Oct 2026 13:48:18 -0700 Shakeel Butt <[email protected]> wrote:

page_counter_calculate_protection() runs without locks. Several reclaimers can update the same counter's emin and elow at the same time, and readers such as mem_cgroup_below_min(), mem_cgroup_protection() and the dmem eviction code load them with READ_ONCE().
Commit b3a7822e5e75 ("mm, memcg: prevent mem_cgroup_protected store tearing") made the main path store emin and elow with WRITE_ONCE(), but it missed the early return for a counter whose parent is the reclaim root. Those two stores are the only plain accesses to emin and elow left.
With CONFIG_KCSAN_STRICT=y, a few tasks writing to the same cgroup's memory.reclaim trigger reports like this within seconds:
  BUG: KCSAN: data-race in page_counter_calculate_protection / shrink_node
read (marked) to 0xff1f91b146d40e40 of 8 bytes by task 163 on cpu 4:
   shrink_node+0x57a/0x1210
   do_try_to_free_pages+0x194/0x930
   try_to_free_mem_cgroup_pages+0x204/0x3f0
   user_proactive_reclaim+0x28a/0x4d0
   memory_reclaim+0x35/0x50
   ...
write to 0xff1f91b146d40e40 of 8 bytes by task 167 on cpu 6:
   page_counter_calculate_protection+0x1fa/0x360
   mem_cgroup_calculate_protection+0x4e/0x70
   shrink_node+0x53e/0x1210
   ...

Quoted 8 lines

The default KCSAN config does not report it, because it treats aligned word-sized plain writes as atomic.
Use WRITE_ONCE() for these two stores as well.
There is no functional change. Readers already accept stale values, and compilers emit a single word-sized store either way: with GCC 14 the x86-64 object code is the same before and after this change.

Makes sense to me.

Quoted 4 lines

Fixes b3a7822e5e75 ("mm, memcg: prevent mem_cgroup_protected store tearing")
Reported-by Sashiko <[email protected]>
Signed-off-by Shakeel Butt <[email protected]>

Reviewed-by SJ Park <[email protected]>

Thanks,
SJ

[...]

Muchun Song

reviewer Acked-by on v1 1/1

2026-10-22 02:18 UTC #3 lore raw

In reply to Shakeel Butt

Quoted 39 lines · Shakeel Butt

page_counter_calculate_protection() runs without locks. Several reclaimers can update the same counter's emin and elow at the same time, and readers such as mem_cgroup_below_min(), mem_cgroup_protection() and the dmem eviction code load them with READ_ONCE().
Commit b3a7822e5e75 ("mm, memcg: prevent mem_cgroup_protected store tearing") made the main path store emin and elow with WRITE_ONCE(), but it missed the early return for a counter whose parent is the reclaim root. Those two stores are the only plain accesses to emin and elow left.
With CONFIG_KCSAN_STRICT=y, a few tasks writing to the same cgroup's memory.reclaim trigger reports like this within seconds:
 BUG: KCSAN: data-race in page_counter_calculate_protection / shrink_node
read (marked) to 0xff1f91b146d40e40 of 8 bytes by task 163 on cpu 4:
  shrink_node+0x57a/0x1210
  do_try_to_free_pages+0x194/0x930
  try_to_free_mem_cgroup_pages+0x204/0x3f0
  user_proactive_reclaim+0x28a/0x4d0
  memory_reclaim+0x35/0x50
  ...
write to 0xff1f91b146d40e40 of 8 bytes by task 167 on cpu 6:
  page_counter_calculate_protection+0x1fa/0x360
  mem_cgroup_calculate_protection+0x4e/0x70
  shrink_node+0x53e/0x1210
  ...
The default KCSAN config does not report it, because it treats aligned word-sized plain writes as atomic.
Use WRITE_ONCE() for these two stores as well.

Quoted 7 lines

There is no functional change. Readers already accept stale values, and compilers emit a single word-sized store either way: with GCC 14 the x86-64 object code is the same before and after this change.
Fixes b3a7822e5e75 ("mm, memcg: prevent mem_cgroup_protected store tearing")
Reported-by Sashiko <[email protected]>
Signed-off-by Shakeel Butt <[email protected]>

Acked-by Muchun Song <[email protected]>

Thanks.

Anshuman Khandual

Reviewed-by on v1 1/1

2026-10-22 04:52 UTC #4 lore raw

In reply to Shakeel Butt

Quoted 35 lines · Shakeel Butt

page_counter_calculate_protection() runs without locks. Several reclaimers can update the same counter's emin and elow at the same time, and readers such as mem_cgroup_below_min(), mem_cgroup_protection() and the dmem eviction code load them with READ_ONCE().
Commit b3a7822e5e75 ("mm, memcg: prevent mem_cgroup_protected store tearing") made the main path store emin and elow with WRITE_ONCE(), but it missed the early return for a counter whose parent is the reclaim root. Those two stores are the only plain accesses to emin and elow left.
With CONFIG_KCSAN_STRICT=y, a few tasks writing to the same cgroup's memory.reclaim trigger reports like this within seconds:
  BUG: KCSAN: data-race in page_counter_calculate_protection / shrink_node
read (marked) to 0xff1f91b146d40e40 of 8 bytes by task 163 on cpu 4:
   shrink_node+0x57a/0x1210
   do_try_to_free_pages+0x194/0x930
   try_to_free_mem_cgroup_pages+0x204/0x3f0
   user_proactive_reclaim+0x28a/0x4d0
   memory_reclaim+0x35/0x50
   ...
write to 0xff1f91b146d40e40 of 8 bytes by task 167 on cpu 6:
   page_counter_calculate_protection+0x1fa/0x360
   mem_cgroup_calculate_protection+0x4e/0x70
   shrink_node+0x53e/0x1210
   ...
The default KCSAN config does not report it, because it treats aligned word-sized plain writes as atomic.
Use WRITE_ONCE() for these two stores as well.

Quoted 7 lines

There is no functional change. Readers already accept stale values, and compilers emit a single word-sized store either way: with GCC 14 the x86-64 object code is the same before and after this change.
Fixes b3a7822e5e75 ("mm, memcg: prevent mem_cgroup_protected store tearing")
Reported-by Sashiko <[email protected]>
Signed-off-by Shakeel Butt <[email protected]>

Reviewed-by Anshuman Khandual <[email protected]>

Quoted 22 lines

---
 mm/page_counter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

mm/page_counter.c +2 -2

index 9167ffd1380c..5293460831f8 100644
@@ -472,8 +472,8 @@ void page_counter_calculate_protection(struct page_counter *root,
 		return;
 
 	if (parent == root) {
-		counter->emin = READ_ONCE(counter->min);
-		counter->elow = READ_ONCE(counter->low);
+		WRITE_ONCE(counter->emin, READ_ONCE(counter->min));
+		WRITE_ONCE(counter->elow, READ_ONCE(counter->low));
 		return;
 	}

Signature · 1 line

2.53.0-Meta