Received-SPF: pass (google.com: domain of linux-kernel+bounces-40663-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240123184552.59758-1-ryncsn@gmail.com> <20240123184552.59758-2-ryncsn@gmail.com>
 <CAF8kJuNZP-uvsSshVrEY0bPsYLB+5Oi-bQKsEQ3RV6yOW+RgNA@mail.gmail.com>
 <CAMgjq7CbxZQ2CmWNjsNjJajBEVkZ839_X5twwLfiiv0-ZgN32A@mail.gmail.com>
 <ZbMDO5mkAFmN2LHz@google.com> <CAMgjq7AMZv7dmhoL_XgROnfHFWhsDDvFhZ--b-jzC47j=z-8ug@mail.gmail.com>
In-Reply-To: <CAMgjq7AMZv7dmhoL_XgROnfHFWhsDDvFhZ--b-jzC47j=z-8ug@mail.gmail.com>
From: Chris Li <chrisl@kernel.org>
Date: Fri, 26 Jan 2024 13:19:39 -0800
Message-ID: <CAF8kJuOz4x_-yG=RHqMU2NP-TxSYNZ_kuDRK8avA3Xfjc3ZoVQ@mail.gmail.com>
Subject: Re: [PATCH v3 1/3] mm, lru_gen: try to prefetch next page when
 scanning LRU
To: Kairui Song <ryncsn@gmail.com>
Cc: linux-mm <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>, 
	Yu Zhao <yuzhao@google.com>, Wei Xu <weixugc@google.com>, 
	Matthew Wilcox <willy@infradead.org>, LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Jan 26, 2024 at 2:31=E2=80=AFAM Kairui Song <ryncsn@gmail.com> wrot=
e:
>

> > > >
> > > > This makes the code flow much harder to follow. Also for architectu=
re
> > > > that does not support prefetch, this will be a net loss.
> > > >
> > > > Can you use refetchw_prev_lru_folio() instead? It will make the cod=
e
> > > > much easier to follow. It also turns into no-op when prefetch is no=
t
> > > > 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, in=
t type, bool can_swap)
> >
> >                 while (!list_empty(head)) {
> >                         struct folio *folio =3D lru_to_folio(head);
> > +                       prefetchw_prev_lru_folio(folio, head, flags);
> >
> >                         VM_WARN_ON_ONCE_FOLIO(folio_test_unevictable(fo=
lio), folio);
> >                         VM_WARN_ON_ONCE_FOLIO(folio_test_active(folio),=
 folio);
> > @@ -4346,7 +4347,10 @@ static int scan_folios(struct lruvec *lruvec, st=
ruct scan_control *sc,
> >
> >                 while (!list_empty(head)) {
> >                         struct folio *folio =3D lru_to_folio(head);
> > -                       int delta =3D folio_nr_pages(folio);
> > +                       int delta;
> > +
> > +                       prefetchw_prev_lru_folio(folio, head, flags);
> > +                       delta =3D folio_nr_pages(folio);
> >
> >                         VM_WARN_ON_ONCE_FOLIO(folio_test_unevictable(fo=
lio), 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 complexi=
ty 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