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

[Anshuman Khandual]

On Wed, Oct 21, 2026 at 01:48:18PM -0700, Shakeel Butt 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
>    ...
> 
> 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 <sashiko-bot@xxxxxxxxxx>
> Signed-off-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>

Reviewed-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>

> ---
>  mm/page_counter.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_counter.c b/mm/page_counter.c
> index 9167ffd1380c..5293460831f8 100644
> --- a/mm/page_counter.c
> +++ b/mm/page_counter.c
> @@ -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;
>  	}
>  
> -- 
> 2.53.0-Meta
>