Re: [PATCH v2 4/5] mm/mglru: avoid redundant rotations of young memcgs

[Barry Song]

On Tue, Nov 3, 2026 at 7:37 PM Max Kellermann <max.kellermann@xxxxxxxxx> wrote:
>
> Concurrent reclaimers can request promotion after another reclaimer
> has already moved a memcg into the young generation.  Requeueing it
> into a random bin again adds list mutations without advancing the
> old-generation scan.
>
> Therefore, skip `MEMCG_LRU_YOUNG` when the memcg is already in the
> default segment of the current young generation.
>
> In my micro benchmark, this increases the request rate by 36% and
> reduces the average duration of each write request by 27%.

Could you share your microbenchmark somewhere (or post it here
if it's not too lengthy), perhaps on GitHub, so others can
reproduce it? You could then refer to it in your changelog.

>
> Signed-off-by: Max Kellermann <max.kellermann@xxxxxxxxx>
> ---
>  mm/vmscan.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 3075949bf641..899f756bd15b 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4620,6 +4620,14 @@ static void lru_gen_rotate_memcg(struct lruvec *lruvec, int op)
>         else
>                 VM_WARN_ON_ONCE(true);
>
> +       if (op == MEMCG_LRU_YOUNG && old == new && lruvec->lrugen.seg == seg)
> +               /*
> +                * This memcg is already in the young generation and
> +                * reshuffling it again cannot help the scan of the
> +                * old generation.
> +                */
> +               goto unlock;

I think this is a correct change. I wonder if we can
reproduce the issue even without concurrent reclaimers.
In other words, can a single thread running in a
memory-limited memcg trigger it as well?

One thing I noticed is that you may not need to call
get_random_u32_below(MEMCG_NR_BINS) upfront. Could you
defer generating the random number until you actually need
to rotate the memcg? I wonder if this could further improve
your microbenchmark results.

Anyway, the compiler might be able to defer it for you.
I'm not sure, though. Could you take a look?

> +
>         WRITE_ONCE(lruvec->lrugen.seg, seg);
>         WRITE_ONCE(lruvec->lrugen.gen, new);
>
> @@ -4633,6 +4641,7 @@ static void lru_gen_rotate_memcg(struct lruvec *lruvec, int op)
>         if (!pgdat->memcg_lru.nr_memcgs[old] && old == get_memcg_gen(pgdat->memcg_lru.seq))
>                 WRITE_ONCE(pgdat->memcg_lru.seq, pgdat->memcg_lru.seq + 1);
>
> +unlock:
>         spin_unlock_irqrestore(&pgdat->memcg_lru.lock, flags);
>  }

Best,
Barry