Re: [PATCH 1/3] mm/mglru: improve readability of isolate_folios()

[Barry Song]

On Fri, Aug 28, 2026 at 11:15 AM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
>
> On 08/20/26 at 12:56pm, Barry Song (Xiaomi) wrote:
> > From: Ridong Chen <chenridong@xxxxxxxxxx>
> >
> > The for_each_evictable_type() loop in isolate_folios()
> > is misleading: it does not actually iterate over each
> > evictable type. Instead, get_type_to_scan() selects the
> > type to scan, while the iterator `i` merely bounds the
> > number of attempts.
> >
> > Make the fallback behavior explicit in the code and remove the
> > opaque for_each_evictable_type(i, swappiness).
> >
> > Signed-off-by: Ridong Chen <chenridong@xxxxxxxxxx>
> > Co-developed-by: Barry Song (Xiaomi) <baohua@xxxxxxxxxx>
> > Signed-off-by: Barry Song (Xiaomi) <baohua@xxxxxxxxxx>
> > ---
> >  mm/vmscan.c | 46 ++++++++++++++++++++++++++--------------------
> >  1 file changed, 26 insertions(+), 20 deletions(-)
>
> The subject doens't reflect the truth. This patch changes the behaviour,
> but not improve readability of isolate_folios() only.
>
> I also noticed the confusion of isolate_folios() implementation, and made
> a patch to only change the local variable and added code comment to
> explain it in my local branch. Surely refactorying is also good.
>
> +        * Scan at most one type per evictable type (anon/file), starting with
> +        * the type get_type_to_scan() picked as statistically colder.
> +        *
> +        * After a scan:
> +        *  - isolated > 0:  got folios, record the type and return.
> +        *  - scanned == 0:  the type is empty; fall back to the other type.
> +        *  - otherwise:     the type has folios but all were hot (or lost an
> +        *                   isolate race); retry the same type rather than
> +        *                   switch, so positive_ctrl_err()'s refault
> +        *                   statistics stay unbiased.
> +        */
>
> While in Ridong's patch, the 3rd case disappeared. It doesn't rescan with
> the original type as the old code is doing, but return directly.
>

right, the 3rd case comes back in patch 3/3:
https://lore.kernel.org/linux-mm/20260820045603.68809-4-baohua@xxxxxxxxxx/

so i guess we can rename patch 1/3?

Best Regards
Barry