Re: [PATCH 2/6] mm/mglru: batch update lrugen->protected in inc_min_seq()
From: Xueyuan Chen
Date: Thu Aug 27 2026 - 08:14:56 EST
On Thu, Aug 27, 2026 at 1:09 PM Barry Song <baohua@xxxxxxxxxx> wrote:
>
> On Wed, Aug 26, 2026 at 5:10 PM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
> >
> > On 08/21/26 at 06:25pm, Barry Song (Xiaomi) wrote:
> > > Avoid updating lrugen->protected with WRITE_ONCE() for each folio,
> > > which may prevent potential compiler optimizations. Accumulate the
> > > updates locally and apply them in a batch instead.
> >
> > Wondering how much efficiency this can bring, is there a number for this
> > standalone patch?
>
> This is a good question. I reverted the protection batching to check its
> impact:
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 996b48344ed0..c9a2fd9ad844 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3957,7 +3957,7 @@ static bool inc_min_seq(struct lruvec *lruvec,
> int type, int swappiness)
> for (zone = 0; zone < MAX_NR_ZONES; zone++) {
> struct list_head *target_list =
> &lrugen->folios[target_gen][type][zone];
> struct list_head *head = &lrugen->folios[old_gen][type][zone];
> - unsigned long protected[MAX_NR_TIERS] = {}, delta = 0;
> + unsigned long delta = 0;
> struct list_head *pos = head->next;
> struct list_head *batch_end = NULL;
>
> @@ -3984,7 +3984,8 @@ static bool inc_min_seq(struct lruvec *lruvec,
> int type, int swappiness)
> if (refs + workingset !=
> BIT(LRU_REFS_WIDTH) + 1) {
> int tier =
> lru_tier_from_refs(refs, workingset);
>
> - protected[tier] += nr_pages;
> +
> WRITE_ONCE(lrugen->protected[hist][type][tier],
> +
> lrugen->protected[hist][type][tier] + nr_pages);
> }
> } else {
> flush_lru_batch(head, &batch_end, target_list);
> @@ -3999,9 +4000,6 @@ static bool inc_min_seq(struct lruvec *lruvec,
> int type, int swappiness)
> lrugen->nr_pages[old_gen][type][zone] - delta);
> WRITE_ONCE(lrugen->nr_pages[target_gen][type][zone],
> lrugen->nr_pages[target_gen][type][zone] + delta);
> - for (int tier = 0; tier < MAX_NR_TIERS; tier++)
> - WRITE_ONCE(lrugen->protected[hist][type][tier],
> - lrugen->protected[hist][type][tier]
> + protected[tier]);
> if (!remaining)
> return false;
> }
>
>
> And I see:
>
> With protection batching:
>
> Running scope as unit: agetest.scope
> mmap: 0x7c4b1d200000, size: 512 MB
> memcg: 12893 (/system.slice/agetest.scope)
> aging generation 3 -> 103
> gen 3: 7.538 ms
> gen 4: 0.937 ms
> gen 5: 2.348 ms
> gen 6: 2.300 ms
> gen 7: 2.302 ms
> gen 8: 2.294 ms
> gen 9: 2.296 ms
> ...
> gen 100: 2.292 ms
> gen 101: 2.307 ms
> gen 102: 2.293 ms
> gen 103: 2.293 ms
>
> Total: 235.718 ms
> Average: 2.334 ms
>
> Without protection batching:
>
> Running scope as unit: agetest.scope
> mmap: 0x775682e00000, size: 512 MB
> memcg: 12823 (/system.slice/agetest.scope)
> aging generation 3 -> 103
> gen 3: 7.542 ms
> gen 4: 0.948 ms
> gen 5: 2.338 ms
> ...
> gen 101: 2.340 ms
> gen 102: 2.342 ms
> gen 103: 2.347 ms
>
> Total: 240.505 ms
> Average: 2.381 ms
>
> It’s 2.334 vs. 2.381, which is really minor.
>
> I’m curious to see what the data looks like on ARM. If there’s no
> significant difference, we may drop this patch in v2 to reduce the amount
> of code change.
>
> Xueyuan, since you’ve been testing this patchset on ARM[1], would you mind
> checking this on ARM as well?
>
Hi Barry,
I reverted patch 2 and ran the test again on arm64, with THP=never
like yours:
full series: 2.964 ms
w/o patch 2: 2.996 ms
So it's 2.964 vs. 2.996, about 1.1% here too. Consistent with your
x86 data.
Thanks,
Xueyuan
> [1] https://lore.kernel.org/linux-mm/20260827035416.3012015-1-xueyuan.chen21@xxxxxxxxx/
>
> >
> > >
> > > Signed-off-by: Barry Song (Xiaomi) <baohua@xxxxxxxxxx>
> > > ---
> > > mm/vmscan.c | 8 +++++---
> > > 1 file changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index 0d74fc00abd3..99ee3c833d54 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > > @@ -3931,7 +3931,7 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
> > > /* prevent cold/hot inversion if the type is evictable */
> > > for (zone = 0; zone < MAX_NR_ZONES; zone++) {
> > > struct list_head *head = &lrugen->folios[old_gen][type][zone];
> > > - unsigned long delta = 0;
> > > + unsigned long protected[MAX_NR_TIERS] = {}, delta = 0;
> > >
> > > while (!list_empty(head)) {
> > > struct folio *folio = lru_to_folio(head);
> > > @@ -3953,8 +3953,7 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
> > > if (refs + workingset != BIT(LRU_REFS_WIDTH) + 1) {
> > > int tier = lru_tier_from_refs(refs, workingset);
> > >
> > > - WRITE_ONCE(lrugen->protected[hist][type][tier],
> > > - lrugen->protected[hist][type][tier] + nr_pages);
> > > + protected[tier] += nr_pages;
> > > }
> > >
> > > if (!--remaining)
> > > @@ -3964,6 +3963,9 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
> > > lrugen->nr_pages[old_gen][type][zone] - delta);
> > > WRITE_ONCE(lrugen->nr_pages[target_gen][type][zone],
> > > lrugen->nr_pages[target_gen][type][zone] + delta);
> > > + for (int tier = 0; tier < MAX_NR_TIERS; tier++)
> > > + WRITE_ONCE(lrugen->protected[hist][type][tier],
> > > + lrugen->protected[hist][type][tier] + protected[tier]);
> > > if (!remaining)
> > > return false;
> > > }
> > > --
> > > 2.34.1
> > >
>
> Thanks
> Barry