Re: [PATCH v3 1/3] mm, lru_gen: try to prefetch next page when scanning LRU

[Chris Li]

On Fri, Jan 26, 2024 at 2:31 AM Kairui Song <ryncsn@xxxxxxxxx> wrote:
>

> > > >
> > > > This makes the code flow much harder to follow. Also for architecture
> > > > that does not support prefetch, this will be a net loss.
> > > >
> > > > Can you use refetchw_prev_lru_folio() instead? It will make the code
> > > > much easier to follow. It also turns into no-op when prefetch is not
> > > > supported.
> > > >
> > > > Chris
> > > >
> > >
> > > Hi Chris,
> > >
> > > Thanks for the suggestion.
> > >
> > > Yes, that's doable, I made it this way because in previous series (V1
> > > & V2) I applied the bulk move patch first which needed and introduced
> > > the `prev` variable here, so the prefetch logic just used it.
> > > For V3 I did a rebase and moved the prefetch commit to be the first
> > > one, since it seems to be the most effective one, and just kept the
> >
> > Maybe something like this? Totally not tested. Feel free to use it any way you want.
> >
> > Chris
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 4f9c854ce6cc..2100e786ccc6 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -3684,6 +3684,7 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, bool can_swap)
> >
> >                 while (!list_empty(head)) {
> >                         struct folio *folio = lru_to_folio(head);
> > +                       prefetchw_prev_lru_folio(folio, head, flags);
> >
> >                         VM_WARN_ON_ONCE_FOLIO(folio_test_unevictable(folio), folio);
> >                         VM_WARN_ON_ONCE_FOLIO(folio_test_active(folio), folio);
> > @@ -4346,7 +4347,10 @@ static int scan_folios(struct lruvec *lruvec, struct scan_control *sc,
> >
> >                 while (!list_empty(head)) {
> >                         struct folio *folio = lru_to_folio(head);
> > -                       int delta = folio_nr_pages(folio);
> > +                       int delta;
> > +
> > +                       prefetchw_prev_lru_folio(folio, head, flags);
> > +                       delta = folio_nr_pages(folio);
> >
> >                         VM_WARN_ON_ONCE_FOLIO(folio_test_unevictable(folio), folio);
> >                         VM_WARN_ON_ONCE_FOLIO(folio_test_active(folio), folio);
> >
>
> Thanks!
>
> Actually if benefits from 2/3 and 3/3 is trivial compared to the complexity and not appealing, then let's only keep the prefetch one, which will be just a one liner change with good result.

That is great. I did take a look at 2/3 and 3/3 and come to the same
conclusion regarding the complexity part.

If you resend the one liner for 1/3, you can consider it having my Ack.

Chris