Re: [PATCH 5/6] mm/mglru: move folios from oldest gen to second-oldest gen from head to tail

[Barry Song]

On Sat, Aug 22, 2026 at 1:45 PM Kairui Song <ryncsn@xxxxxxxxx> wrote:
>
> On Fri, Aug 21, 2026 at 6:38 PM Barry Song (Xiaomi) <baohua@xxxxxxxxxx> wrote:
> >
> > For reclamation, it makes sense to reclaim folios from tail to
> > head, as folios near the head are relatively hot. However, when
> > moving folios from the oldest generation to the second-oldest
> > generation, using the tail-to-head order would effectively cause
> > a cold/hot inversion.
> >
> > Signed-off-by: Barry Song (Xiaomi) <baohua@xxxxxxxxxx>
>
> Hi Barry
>
> This looks a really good idea, thanks!
>
> > ---
> >  mm/vmscan.c | 19 +++++++++++++++++--
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 7bd01875fade..2fd82b2ca4d1 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -191,8 +191,20 @@ struct scan_control {
> >                         prefetchw(&prev->_field);                       \
> >                 }                                                       \
> >         } while (0)
> > +#define prefetchw_next_lru_folio(_folio, _base, _field)                        \
> > +       do {                                                            \
> > +               if ((_folio)->lru.next != _base) {                      \
> > +                       struct folio *next;                             \
> > +                                                                       \
> > +                       next = list_entry((_folio)->lru.next,           \
> > +                                       struct folio, lru);             \
> > +                       prefetchw(&next->_field);                       \
> > +               }                                                       \
> > +       } while (0)
> > +
>
> I got following warning from checkpatch:
>
>     ● checkpatch.pl: 92: WARNING: Argument '_folio' is not used in
> function-like macro
>     ● checkpatch.pl: 92: WARNING: Argument '_base' is not used in
> function-like macro
>     ● checkpatch.pl: 92: WARNING: Argument '_field' is not used in
> function-like macro
>
> Maybe you could try b4; it helps run these checks automatically. Feel
> free to ignore if you think these warning at pointless.

Yep. I vividly remember adding this rule to the coding style, and Xining
added the corresponding `checkpatch.pl` change:

commit 6813216bbdba1 ("Documentation: coding-style: ask function-like
macros to evaluate parameters")
commit  b1be5844c1a01 ("scripts: checkpatch: check unused parameters
for function-like macro")

But once we started writing the code, I was influenced by its context and
ended up following that context instead.

>
> >  #else
> >  #define prefetchw_prev_lru_folio(_folio, _base, _field) do { } while (0)
> > +#define prefetchw_next_lru_folio(_folio, _base, _field) do { } while (0)
> >  #endif
> >
> >  /*
> > @@ -3932,9 +3944,10 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
> >         for (zone = 0; zone < MAX_NR_ZONES; zone++) {
> >                 struct list_head *head = &lrugen->folios[old_gen][type][zone];
> >                 unsigned long protected[MAX_NR_TIERS] = {}, delta = 0;
> > +               struct list_head *pos = head->next;
> >
> > -               while (!list_empty(head)) {
> > -                       struct folio *folio = lru_to_folio(head);
> > +               while (pos != head) {
>
> Do we need to change the while condition now? Since this commit still
> moves folios one by one, will it stop when the list is empty?

For this patch, we don't need to change the `while` condition. That change
is more relevant to the next patch:
https://lore.kernel.org/linux-mm/20260821102538.22642-7-baohua@xxxxxxxxxx/

So I guess I could move this while (pos != head) change to the next patch
if that makes the review easier.

>
> > +                       prefetchw_next_lru_folio(folio, head, flags);
> > +                       pos = pos->next;
>
> I tried prefetching in MGLRU previously, and it didn't look very good
> but there is no regression either; perhaps it's very arch-dependent.
> Anyway, I think we can keep it here, maybe further optimize the
> prefetch later.

Yep. I guess we may want to keep it to help architectures that are
sensitive to prefetching.

>
> >                         new_gen = __folio_inc_gen(folio, old_gen, &gen_increased);
> >                         if (gen_increased) {
> >                                 delta += nr_pages;
>
> The rest looks good to me, thanks!

Thanks for the review!

Best Regards
Barry