Re: [PATCH v4 02/17] mm/mglru: make generation page counters atomic

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: mglru: lost LRU page counter updates in __lru_gen_reparent_memcg()
--

commit 82d0af39bfdf4a479c7e716d6991b3cf9cc6e2b4
Author: Kairui Song <kasong@tencent.com>

mm/mglru: make generation page counters atomic

This commit converts the multi-gen LRU generation page counters to atomic
longs. This allows the counters to be updated without holding the LRU lock,
relying on eventual consistency.

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index daa7e37c2236a..cab3ba14d20c2 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c

[ ... ]

> @@ -4764,7 +4761,7 @@ static void __lru_gen_reparent_memcg(struct lruvec *child_lruvec, struct lruvec
>  
>  	for (i = 0; i < get_nr_gens(child_lruvec, type); i++) {
>  		int gen = lru_gen_from_seq(child_lrugen->max_seq - i);
> -		long nr_pages = child_lrugen->nr_pages[gen][type][zone];
> +		long nr_pages = atomic_long_read(&child_lrugen->nr_pages[gen][type][zone]);
>  		int child_lru_active = lru_gen_is_active(child_lruvec, gen) ? LRU_ACTIVE : 0;
>  		int parent_lru_active = lru_gen_is_active(parent_lruvec, gen) ? LRU_ACTIVE : 0;
>  
> @@ -4772,9 +4769,8 @@ static void __lru_gen_reparent_memcg(struct lruvec *child_lruvec, struct lruvec
>  		list_splice_tail_init(&child_lrugen->folios[gen][type][zone],
>  				      &parent_lrugen->folios[gen][type][zone]);
>  
> -		WRITE_ONCE(child_lrugen->nr_pages[gen][type][zone], 0);
> -		WRITE_ONCE(parent_lrugen->nr_pages[gen][type][zone],
> -			   parent_lrugen->nr_pages[gen][type][zone] + nr_pages);
> +		atomic_long_set(&child_lrugen->nr_pages[gen][type][zone], 0);
> +		atomic_long_add(nr_pages, &parent_lrugen->nr_pages[gen][type][zone]);
[Severity: High]
Does the non-atomic extraction and clearing of the child's counter create
a read-modify-write window that wipes out lockless updates from
folio_inc_lru_refs()?

Because __lru_gen_reparent_memcg() reads the child's counter, executes a
list_splice, and then unconditionally overwrites the counter via
atomic_long_set(0), any concurrent lockless counter additions or
subtractions that land during this window appear to be completely wiped
out and never propagated to the parent memcg.

Could we replace the read and zeroing with atomic_long_xchg(&..., 0) after
the list splice? This would atomically extract and clear the counter,
ensuring updates during the window are passed to the parent.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-mglru-fg-v4-0-2106750c0bfa@tencent.com?part=2