Re: [PATCH v2 2/2] mm/mglru: make retry logic explicit in isolate_folios()

[Baoquan He]

On 09/03/26 at 06:08am, Barry Song wrote:
> On Wed, Sep 2, 2026 at 6:17 PM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
> >
> > On 09/02/26 at 05:20pm, Barry Song wrote:
> > > On Wed, Sep 2, 2026 at 4:07 PM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
> > > >
> [...]
> >
> > Hi Barry,
> >
> > Agreed on the one-line change for the (1, 200) case - I traced it and it
> > now matches mainline exactly (no extra third scan). I personally prefer
> > the for (attempt = 0... ) style because I feel that makes logic clearer,
> > while everybody truly has different code taste, LOL, just a weak opinion.
> >
> > For 0/201: my concern is that on no-swap systems (swappiness 0 is
> > file-only), the same-type retry when the first scan is busy may be a
> > no-gain run if the file generation is dominated by protected/ineligible
> > folios - the retry re-scans the same sort results. But if you see a case
> > where the retry does isolate folios on the second pass for single-type
> > reclaim, keeping it for consistency is defensible. Do you have such a
> > case, or should we drop the retry for 0/201?
> >
> 
> 201 only applies to proactive reclamation. I believe the retry helps
> avoid having an outer loop. For 0, I ran a kernel build test on x86
> with swap disabled:
> 
> # free
>                total        used        free      shared  buff/cache   available
> Mem:        23991248     1315020    20554564      331312     2121664    22099256
> Swap:              0           0           0
> 
> #  time systemd-run --scope --unit=kernel-build -p MemoryMax=1500M
> make ARCH=arm64 \
>         CROSS_COMPILE=aarch64-linux-gnu- vmlinux -j20 1>/dev/null 2>/dev/null
> 
> With the following patch for counting:
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bf2786c7247d..e8d5603cd56e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4913,6 +4913,28 @@ static inline bool is_single_type_reclaim(int swappiness)
>                swappiness == SWAPPINESS_ANON_ONLY;
>  }
> 
> +#include <linux/proc_fs.h>
> +
> +static atomic64_t tried_isolated;
> +static atomic64_t tried_not_isolated;
> +static int reclaim_stats_show(struct seq_file *m, void *v)
> +{
> +       seq_printf(m, "tried_isolated: %lld\n",
> +                  atomic64_read(&tried_isolated));
> +       seq_printf(m, "tried_not_isolated: %lld\n",
> +                  atomic64_read(&tried_not_isolated));
> +
> +       return 0;
> +}
> +       return 0;
> +}
> +static int __init reclaim_stats_init(void)
> +{
> +       proc_create_single("reclaim_stats", 0444, NULL,
> +                          reclaim_stats_show);
> +
> +       return 0;
> +}
> +fs_initcall(reclaim_stats_init);
> +
>  static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>                           struct scan_control *sc, int swappiness,
>                           struct list_head *list, int *isolated,
> @@ -4928,6 +4950,13 @@ static int isolate_folios(unsigned long
> nr_to_scan, struct lruvec *lruvec,
>         scanned = scan_folios(nr_to_scan, lruvec, sc,
>                               type, tier, list, isolated);
> 
> +       if (tried) {
> +               if (*isolated)
> +                       atomic64_inc(&tried_isolated);
> +               else
> +                       atomic64_inc(&tried_not_isolated);
> +       }
> +
>         total_scanned += scanned;
>         if (*isolated) {
>                 *isolate_type = type;
> 
> I got:
> 
> # cat /proc/reclaim_stats
> tried_isolated: 12096
> tried_not_isolated: 23061
> 
> So we see some cases where the retry gets isolated folios, while in
> others we still encounter promoted or protected folios. But my gut
> feeling is that even if we don't retry and instead go back to the outer
> loop for another iteration, we'll still encounter those folios, since
> they are still on the LRU. We would just reach those folios in a more
> costly way.

Thanks, Barry. These number is very convincing. The retry for swappiness
0 is worthy. Then the patchset feels like doing two things: refactoring
the for() loop; improving the eviction for swappiness 0/201 by adding a
retry and this also makes them be consistent with (1, 200). While the
cover letter subject, patch 1 and patch 2 feels like it's not easy to
match them to the corresponding part. Maybe merging them to one patch,
or rearranging them? Just personal opinion.