[PATCH v2 0/2] mm/mglru: clean up isolate_folios for readability and clarity

[PATCH v2 0/2] mm/mglru: clean up isolate_folios for readability and clarity

[Barry Song (Xiaomi)]

Right now, `isolate_folios()` is quite difficult to follow:

1. It uses `for_each_evictable_type(i, swappiness)` to iterate over
   the types, but `i` is not actually used as the type within the
   loop body.

2. It retries the same type when folios were scanned but none could
   be isolated, but the retry is implemented in a rather subtle way
   that is difficult to understand.

This patchset makes both behaviors explicit and much easier to follow.

There are no functional changes for swappiness values from 1 to 200.
There is a slight functional change for 0 and 201: with the existing
code, there is no chance to retry for these values because
`for_each_evictable_type()` only iterates once. After this patch, 0 and
201 have behavior that is more consistent with the 1-200 range.

-v2:
* Rename patch 1, to address Baoquan's comments;
* Drop patches 2/3. Patch 2 seems to improve the zRAM case
  but negatively affect the SSD/NVMe case, according to Baolin
  and Kairui. Drop it to keep the patchset focused on readability.
* We received many tags from Baolin, Kairui, Ridong, and Lian.
  Since patch 1/3 and 3/3 were renamed and patches 2/3 were dropped,
  I did not carry the tags forward. Many thanks for the reviews,
  and hopefully you can re-review this version.
-v1:
  https://lore.kernel.org/linux-mm/20260820045603.68809-1-baohua@kernel.org/

Barry Song (Xiaomi) (1):
  mm/mglru: make retry logic explicit in isolate_folios()

Ridong Chen (1):
  mm/mglru: make type fallback logic explicit in isolate_folios()

 mm/vmscan.c | 56 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 20 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] mm/mglru: make type fallback logic explicit in isolate_folios()

[Barry Song (Xiaomi)]

From: Ridong Chen <chenridong@xiaomi.com>

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@xiaomi.com>
Co-developed-by: Barry Song (Xiaomi) <baohua@kernel.org>
Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 mm/vmscan.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index fdd13299a04a..35a233623368 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4838,35 +4838,41 @@ static int get_type_to_scan(struct lruvec *lruvec, int swappiness)
 	return positive_ctrl_err(&sp, &pv);
 }
 
+static inline bool is_single_type_reclaim(int swappiness)
+{
+	return swappiness == MIN_SWAPPINESS ||
+	       swappiness == SWAPPINESS_ANON_ONLY;
+}
+
 static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 			  struct scan_control *sc, int swappiness,
 			  struct list_head *list, int *isolated,
 			  int *isolate_type, int *isolate_scanned)
 {
-	int i;
-	int total_scanned = 0;
+	bool type_fallback_allowed = !is_single_type_reclaim(swappiness);
 	int type = get_type_to_scan(lruvec, swappiness);
+	int total_scanned = 0, scanned, tier;
 
-	for_each_evictable_type(i, swappiness) {
-		int scanned;
-		int tier = get_tier_idx(lruvec, type);
+retry:
+	tier = get_tier_idx(lruvec, type);
+	scanned = scan_folios(nr_to_scan, lruvec, sc,
+			      type, tier, list, isolated);
 
-		scanned = scan_folios(nr_to_scan, lruvec, sc,
-				      type, tier, list, isolated);
+	total_scanned += scanned;
+	if (*isolated) {
+		*isolate_type = type;
+		*isolate_scanned = scanned;
+		return total_scanned;
+	}
 
-		total_scanned += scanned;
-		if (*isolated) {
-			*isolate_type = type;
-			*isolate_scanned = scanned;
-			break;
-		}
-		/*
-		 * If scanned > 0 and isolated == 0, avoid falling back to the
-		 * other type, as this type remains sufficient. Falling back
-		 * too readily can disrupt the positive_ctrl_err() bias.
-		 */
-		if (!scanned)
-			type = !type;
+	/*
+	 * We are running out of the current reclaim type. Fall back to
+	 * the other type if allowed.
+	 */
+	if (!scanned && type_fallback_allowed) {
+		type = !type;
+		type_fallback_allowed = false;
+		goto retry;
 	}
 
 	return total_scanned;
-- 
2.34.1

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

[Baolin Wang]

On 8/29/26 3:42 PM, Barry Song (Xiaomi) wrote:
> From: Ridong Chen <chenridong@xiaomi.com>
> 
> 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@xiaomi.com>
> Co-developed-by: Barry Song (Xiaomi) <baohua@kernel.org>
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---

LGTM.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

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

[Lian Wang]

> Make the fallback behavior explicit in the code and remove the
> opaque for_each_evictable_type(i, swappiness).

Looks good to me.

Reviewed-by: Lian Wang <lianux.mm@gmail.com>

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

[Baoquan He]

On 08/29/26 at 03:42pm, Barry Song (Xiaomi) wrote:
> From: Ridong Chen <chenridong@xiaomi.com>
> 
> 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@xiaomi.com>
> Co-developed-by: Barry Song (Xiaomi) <baohua@kernel.org>
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---
>  mm/vmscan.c | 46 ++++++++++++++++++++++++++--------------------
>  1 file changed, 26 insertions(+), 20 deletions(-)

Reviewed-by: Baoquan He <baoquan.he@linux.dev>

> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index fdd13299a04a..35a233623368 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4838,35 +4838,41 @@ static int get_type_to_scan(struct lruvec *lruvec, int swappiness)
>  	return positive_ctrl_err(&sp, &pv);
>  }
>  
> +static inline bool is_single_type_reclaim(int swappiness)
> +{
> +	return swappiness == MIN_SWAPPINESS ||
> +	       swappiness == SWAPPINESS_ANON_ONLY;
> +}
> +
>  static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>  			  struct scan_control *sc, int swappiness,
>  			  struct list_head *list, int *isolated,
>  			  int *isolate_type, int *isolate_scanned)
>  {
> -	int i;
> -	int total_scanned = 0;
> +	bool type_fallback_allowed = !is_single_type_reclaim(swappiness);
>  	int type = get_type_to_scan(lruvec, swappiness);
> +	int total_scanned = 0, scanned, tier;
>  
> -	for_each_evictable_type(i, swappiness) {
> -		int scanned;
> -		int tier = get_tier_idx(lruvec, type);
> +retry:
> +	tier = get_tier_idx(lruvec, type);
> +	scanned = scan_folios(nr_to_scan, lruvec, sc,
> +			      type, tier, list, isolated);
>  
> -		scanned = scan_folios(nr_to_scan, lruvec, sc,
> -				      type, tier, list, isolated);
> +	total_scanned += scanned;
> +	if (*isolated) {
> +		*isolate_type = type;
> +		*isolate_scanned = scanned;
> +		return total_scanned;
> +	}
>  
> -		total_scanned += scanned;
> -		if (*isolated) {
> -			*isolate_type = type;
> -			*isolate_scanned = scanned;
> -			break;
> -		}
> -		/*
> -		 * If scanned > 0 and isolated == 0, avoid falling back to the
> -		 * other type, as this type remains sufficient. Falling back
> -		 * too readily can disrupt the positive_ctrl_err() bias.
> -		 */
> -		if (!scanned)
> -			type = !type;
> +	/*
> +	 * We are running out of the current reclaim type. Fall back to
> +	 * the other type if allowed.
> +	 */
> +	if (!scanned && type_fallback_allowed) {
> +		type = !type;
> +		type_fallback_allowed = false;
> +		goto retry;
>  	}
>  
>  	return total_scanned;
> -- 
> 2.34.1
> 

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

[Barry Song (Xiaomi)]

The existing mainline code retries the same type once in a rather
subtle way. `for_each_evictable_type()` may provide one more iteration,
allowing the same type to be retried if we scanned some folios but
failed to isolate any due to protections, promotions, or races. This
patch makes the retry behavior explicit.

Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 mm/vmscan.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 35a233623368..718f59ffc688 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4852,6 +4852,7 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	bool type_fallback_allowed = !is_single_type_reclaim(swappiness);
 	int type = get_type_to_scan(lruvec, swappiness);
 	int total_scanned = 0, scanned, tier;
+	bool tried = false;
 
 retry:
 	tier = get_tier_idx(lruvec, type);
@@ -4871,9 +4872,18 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	 */
 	if (!scanned && type_fallback_allowed) {
 		type = !type;
+		tried = true;
 		type_fallback_allowed = false;
 		goto retry;
 	}
+	/*
+	 * We scanned some folios but failed to isolate any due to promotions,
+	 * protections, or races. Retry once to avoid a larger loop.
+	 */
+	if (scanned && !tried) {
+		tried = true;
+		goto retry;
+	}
 
 	return total_scanned;
 }
-- 
2.34.1

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

[Baolin Wang]

On 8/29/26 3:42 PM, Barry Song (Xiaomi) wrote:
> The existing mainline code retries the same type once in a rather
> subtle way. `for_each_evictable_type()` may provide one more iteration,
> allowing the same type to be retried if we scanned some folios but
> failed to isolate any due to protections, promotions, or races. This
> patch makes the retry behavior explicit.
> 
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---

LGTM.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

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

[Baoquan He]

Hi Barry,

On 08/29/26 at 03:42pm, Barry Song (Xiaomi) wrote:
> The existing mainline code retries the same type once in a rather
> subtle way. `for_each_evictable_type()` may provide one more iteration,
> allowing the same type to be retried if we scanned some folios but
> failed to isolate any due to protections, promotions, or races. This
> patch makes the retry behavior explicit.
> 
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---
>  mm/vmscan.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 35a233623368..718f59ffc688 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4852,6 +4852,7 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>  	bool type_fallback_allowed = !is_single_type_reclaim(swappiness);
>  	int type = get_type_to_scan(lruvec, swappiness);
>  	int total_scanned = 0, scanned, tier;
> +	bool tried = false;
>  
>  retry:
>  	tier = get_tier_idx(lruvec, type);
> @@ -4871,9 +4872,18 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>  	 */
>  	if (!scanned && type_fallback_allowed) {
>  		type = !type;
> +		tried = true;
>  		type_fallback_allowed = false;
>  		goto retry;
>  	}
> +	/*
> +	 * We scanned some folios but failed to isolate any due to promotions,
> +	 * protections, or races. Retry once to avoid a larger loop.
> +	 */
> +	if (scanned && !tried) {
> +		tried = true;
> +		goto retry;

Seems patch 1 and 2 makes not minor difference than mainline kernel on
behaviour.

1, if swappiness is 0 because no swap, it will run two times if
(scanned != 0). This is not corner case, but usually seen on some
systems w/o swap device. The 2nd no gain run could decrease efficiency.

static int get_swappiness(struct lruvec *lruvec, struct scan_control *sc)
{       
	...                

        if (!sc->may_swap)
                return 0;
	...
}

2, for swappiness (0, 200), the behavious is minor changed.

Mark one scan_folios() result as one of:
      iso     *isolated > 0
      empty   scanned == 0 && !*isolated
      busy    scanned > 0  && !*isolated

mainline:  T(busy) -> T(empty)         -> return   (2 scans, no fallback)
v2:        T(busy) -> T(empty) -> !T(...)        (a 3rd scan_folios())

Maybe we can go like below:

static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
                          struct scan_control *sc, int swappiness,
                          struct list_head *list, int *isolated,
                          int *isolate_type, int *isolate_scanned)
	...

      for (attempt = 0; attempt < 2; attempt++) {
              int scanned = scan_folios(nr_to_scan, lruvec, sc, type,
                                        get_tier_idx(lruvec, type), list, isolated);

              total_scanned += scanned;
              if (*isolated) {
                      *isolate_type = type;
                      *isolate_scanned = scanned;
                      return total_scanned;
              }
              if (attempt)                /* already retried / fell back once */
                      break;
              if (scanned)
                      continue;           /* retry the same type once */
              if (single_type)
                      break;              /* no fallback for 0 / anon-only */
              type = !type;               /* empty: fall back to the other type */
      }

      return total_scanned;
}

This preserves mainline for 1..200 exactly, keeps the intended 0/201
same-type retry, and cannot produce a third scan. Just personal opinion.

> +	}
>  
>  	return total_scanned;
>  }
> -- 
> 2.34.1
> 

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

[Barry Song]

On Wed, Sep 2, 2026 at 4:07 PM Baoquan He <baoquan.he@linux.dev> wrote:
>
> Hi Barry,
>
> On 08/29/26 at 03:42pm, Barry Song (Xiaomi) wrote:
> > The existing mainline code retries the same type once in a rather
> > subtle way. `for_each_evictable_type()` may provide one more iteration,
> > allowing the same type to be retried if we scanned some folios but
> > failed to isolate any due to protections, promotions, or races. This
> > patch makes the retry behavior explicit.
> >
> > Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> > ---
> >  mm/vmscan.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 35a233623368..718f59ffc688 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -4852,6 +4852,7 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> >       bool type_fallback_allowed = !is_single_type_reclaim(swappiness);
> >       int type = get_type_to_scan(lruvec, swappiness);
> >       int total_scanned = 0, scanned, tier;
> > +     bool tried = false;
> >
> >  retry:
> >       tier = get_tier_idx(lruvec, type);
> > @@ -4871,9 +4872,18 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> >        */
> >       if (!scanned && type_fallback_allowed) {
> >               type = !type;
> > +             tried = true;
> >               type_fallback_allowed = false;
> >               goto retry;
> >       }
> > +     /*
> > +      * We scanned some folios but failed to isolate any due to promotions,
> > +      * protections, or races. Retry once to avoid a larger loop.
> > +      */
> > +     if (scanned && !tried) {
> > +             tried = true;
> > +             goto retry;
>
> Seems patch 1 and 2 makes not minor difference than mainline kernel on
> behaviour.
>
> 1, if swappiness is 0 because no swap, it will run two times if
> (scanned != 0). This is not corner case, but usually seen on some
> systems w/o swap device. The 2nd no gain run could decrease efficiency.
>
> static int get_swappiness(struct lruvec *lruvec, struct scan_control *sc)
> {
>         ...
>
>         if (!sc->may_swap)
>                 return 0;
>         ...
> }

Yep. For swappiness 0 and 201, this patch slightly changes the
behavior, as I mentioned in the cover letter:
"
 There is a slight functional change for 0 and 201: with the existing
 code, there is no chance to retry for these values because
 `for_each_evictable_type()` only iterates once. After this patch, 0 and
 201 have behavior that is more consistent with the 1-200 range.
"
I did this intentionally, as it makes the behavior more consistent with
the 1-200 range, where we retry the same type once to avoid having a
larger outer loop.

>
> 2, for swappiness (0, 200), the behavious is minor changed.

I guess you actually mean swappiness (1, 200)?

>
> Mark one scan_folios() result as one of:
>       iso     *isolated > 0
>       empty   scanned == 0 && !*isolated
>       busy    scanned > 0  && !*isolated
>
> mainline:  T(busy) -> T(empty)         -> return   (2 scans, no fallback)
> v2:        T(busy) -> T(empty) -> !T(...)        (a 3rd scan_folios())
>
> Maybe we can go like below:

Yes, you're right. For swappiness (1, 200), I didn't realize there was
this slight change.

>
> static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>                           struct scan_control *sc, int swappiness,
>                           struct list_head *list, int *isolated,
>                           int *isolate_type, int *isolate_scanned)
>         ...
>
>       for (attempt = 0; attempt < 2; attempt++) {
>               int scanned = scan_folios(nr_to_scan, lruvec, sc, type,
>                                         get_tier_idx(lruvec, type), list, isolated);
>
>               total_scanned += scanned;
>               if (*isolated) {
>                       *isolate_type = type;
>                       *isolate_scanned = scanned;
>                       return total_scanned;
>               }
>               if (attempt)                /* already retried / fell back once */
>                       break;

This `if (attempt) break` makes the loop look rather strange,
especially for a loop with a maximum of 2 iterations, where we break
when `attempt` reaches 1 :-)

What about just changing one line?

diff --git a/mm/vmscan.c b/mm/vmscan.c
index bf2786c7247d..ba7adf36e69f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4939,7 +4939,7 @@ static int isolate_folios(unsigned long
nr_to_scan, struct lruvec *lruvec,
         * We are running out of the current reclaim type. Fall back to
         * the other type if allowed.
         */
-       if (!scanned && type_fallback_allowed) {
+       if (!scanned && !tried && type_fallback_allowed) {
                type = !type;
                tried = true;
                type_fallback_allowed = false;

>               if (scanned)
>                       continue;           /* retry the same type once */
>               if (single_type)
>                       break;              /* no fallback for 0 / anon-only */
>               type = !type;               /* empty: fall back to the other type */
>       }
>
>       return total_scanned;
> }
>
> This preserves mainline for 1..200 exactly, keeps the intended 0/201

I guess you mean removing the same-type retry for swappiness 0/201,
rather than keeping it?

Thanks
Barry

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

[Baoquan He]

On 09/02/26 at 05:20pm, Barry Song wrote:
> On Wed, Sep 2, 2026 at 4:07 PM Baoquan He <baoquan.he@linux.dev> wrote:
> >
> > Hi Barry,
> >
> > On 08/29/26 at 03:42pm, Barry Song (Xiaomi) wrote:
> > > The existing mainline code retries the same type once in a rather
> > > subtle way. `for_each_evictable_type()` may provide one more iteration,
> > > allowing the same type to be retried if we scanned some folios but
> > > failed to isolate any due to protections, promotions, or races. This
> > > patch makes the retry behavior explicit.
> > >
> > > Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> > > ---
> > >  mm/vmscan.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index 35a233623368..718f59ffc688 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > > @@ -4852,6 +4852,7 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> > >       bool type_fallback_allowed = !is_single_type_reclaim(swappiness);
> > >       int type = get_type_to_scan(lruvec, swappiness);
> > >       int total_scanned = 0, scanned, tier;
> > > +     bool tried = false;
> > >
> > >  retry:
> > >       tier = get_tier_idx(lruvec, type);
> > > @@ -4871,9 +4872,18 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> > >        */
> > >       if (!scanned && type_fallback_allowed) {
> > >               type = !type;
> > > +             tried = true;
> > >               type_fallback_allowed = false;
> > >               goto retry;
> > >       }
> > > +     /*
> > > +      * We scanned some folios but failed to isolate any due to promotions,
> > > +      * protections, or races. Retry once to avoid a larger loop.
> > > +      */
> > > +     if (scanned && !tried) {
> > > +             tried = true;
> > > +             goto retry;
> >
> > Seems patch 1 and 2 makes not minor difference than mainline kernel on
> > behaviour.
> >
> > 1, if swappiness is 0 because no swap, it will run two times if
> > (scanned != 0). This is not corner case, but usually seen on some
> > systems w/o swap device. The 2nd no gain run could decrease efficiency.
> >
> > static int get_swappiness(struct lruvec *lruvec, struct scan_control *sc)
> > {
> >         ...
> >
> >         if (!sc->may_swap)
> >                 return 0;
> >         ...
> > }
> 
> Yep. For swappiness 0 and 201, this patch slightly changes the
> behavior, as I mentioned in the cover letter:
> "
>  There is a slight functional change for 0 and 201: with the existing
>  code, there is no chance to retry for these values because
>  `for_each_evictable_type()` only iterates once. After this patch, 0 and
>  201 have behavior that is more consistent with the 1-200 range.
> "
> I did this intentionally, as it makes the behavior more consistent with
> the 1-200 range, where we retry the same type once to avoid having a
> larger outer loop.
> 
> >
> > 2, for swappiness (0, 200), the behavious is minor changed.
> 
> I guess you actually mean swappiness (1, 200)?
> 
> >
> > Mark one scan_folios() result as one of:
> >       iso     *isolated > 0
> >       empty   scanned == 0 && !*isolated
> >       busy    scanned > 0  && !*isolated
> >
> > mainline:  T(busy) -> T(empty)         -> return   (2 scans, no fallback)
> > v2:        T(busy) -> T(empty) -> !T(...)        (a 3rd scan_folios())
> >
> > Maybe we can go like below:
> 
> Yes, you're right. For swappiness (1, 200), I didn't realize there was
> this slight change.
> 
> >
> > static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> >                           struct scan_control *sc, int swappiness,
> >                           struct list_head *list, int *isolated,
> >                           int *isolate_type, int *isolate_scanned)
> >         ...
> >
> >       for (attempt = 0; attempt < 2; attempt++) {
> >               int scanned = scan_folios(nr_to_scan, lruvec, sc, type,
> >                                         get_tier_idx(lruvec, type), list, isolated);
> >
> >               total_scanned += scanned;
> >               if (*isolated) {
> >                       *isolate_type = type;
> >                       *isolate_scanned = scanned;
> >                       return total_scanned;
> >               }
> >               if (attempt)                /* already retried / fell back once */
> >                       break;
> 
> This `if (attempt) break` makes the loop look rather strange,
> especially for a loop with a maximum of 2 iterations, where we break
> when `attempt` reaches 1 :-)
> 
> What about just changing one line?

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?

> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bf2786c7247d..ba7adf36e69f 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4939,7 +4939,7 @@ static int isolate_folios(unsigned long
> nr_to_scan, struct lruvec *lruvec,
>          * We are running out of the current reclaim type. Fall back to
>          * the other type if allowed.
>          */
> -       if (!scanned && type_fallback_allowed) {
> +       if (!scanned && !tried && type_fallback_allowed) {
>                 type = !type;
>                 tried = true;
>                 type_fallback_allowed = false;
> 
> >               if (scanned)
> >                       continue;           /* retry the same type once */
> >               if (single_type)
> >                       break;              /* no fallback for 0 / anon-only */
> >               type = !type;               /* empty: fall back to the other type */
> >       }
> >
> >       return total_scanned;
> > }
> >
> > This preserves mainline for 1..200 exactly, keeps the intended 0/201
> 
> I guess you mean removing the same-type retry for swappiness 0/201,
> rather than keeping it?

You are right, it was a slip of the tongue.

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

[Barry Song]

On Wed, Sep 2, 2026 at 6:17 PM Baoquan He <baoquan.he@linux.dev> wrote:
>
> On 09/02/26 at 05:20pm, Barry Song wrote:
> > On Wed, Sep 2, 2026 at 4:07 PM Baoquan He <baoquan.he@linux.dev> 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

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@linux.dev> wrote:
> >
> > On 09/02/26 at 05:20pm, Barry Song wrote:
> > > On Wed, Sep 2, 2026 at 4:07 PM Baoquan He <baoquan.he@linux.dev> 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.

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

[Barry Song (Xiaomi)]

On Thu, Sep 3, 2026 at 9:27 AM Baoquan He <baoquan.he@linux.dev> wrote:
>
[...]
>
> 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.
>

Hi Baoquan,

Thanks very much for your suggestions and review.
We have two patches:

1. `mm/mglru: make type fallback logic explicit in isolate_folios()`
2. `mm/mglru: make retry logic explicit in isolate_folios()`

One handles fallback, while the other handles retry. I think this
separation makes the logic clearer. Could we keep the current
separation?

But we may really need Andrew's kind help to squash the change
you found below:


From 1fb8046662a8b23be1b87a21cc9964e9ff9e1eb0 Mon Sep 17 00:00:00 2001
From: "Barry Song (Xiaomi)" <baohua@kernel.org>
Date: Thu, 3 Sep 2026 14:48:52 +0800
Subject: [PATCH] mm: revert slight behavior change for swappiness 1-200

Baoquan's review found that we unexpectedly introduced a slight behavior
change for swappiness 1-200. We could now have a case like:

1. First scan -> `scanned != 0`
2. Second scan -> `scanned = 0`
3. Type fallback

Step 3 was impossible before. Let's remove this possibility.

Reported-by: Baoquan He <baoquan.he@linux.dev>
Closes: https://lore.kernel.org/linux-mm/apfZQE1X6zGAsBb_@fedora/
Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 mm/vmscan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index bf2786c7247d..ba7adf36e69f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4939,7 +4939,7 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	 * We are running out of the current reclaim type. Fall back to
 	 * the other type if allowed.
 	 */
-	if (!scanned && type_fallback_allowed) {
+	if (!scanned && !tried && type_fallback_allowed) {
 		type = !type;
 		tried = true;
 		type_fallback_allowed = false;
-- 
2.39.3 (Apple Git-146)

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

[Baoquan He]

On 09/03/26 at 03:05pm, Barry Song (Xiaomi) wrote:
> On Thu, Sep 3, 2026 at 9:27 AM Baoquan He <baoquan.he@linux.dev> wrote:
> >
> [...]
> >
> > 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.
> >
> 
> Hi Baoquan,
> 
> Thanks very much for your suggestions and review.
> We have two patches:
> 
> 1. `mm/mglru: make type fallback logic explicit in isolate_folios()`
> 2. `mm/mglru: make retry logic explicit in isolate_folios()`
> 
> One handles fallback, while the other handles retry. I think this
> separation makes the logic clearer. Could we keep the current
> separation?

Yeah, it's also fine to me. Thanks for the effort.

> 
> But we may really need Andrew's kind help to squash the change
> you found below:
> 
> 
> From 1fb8046662a8b23be1b87a21cc9964e9ff9e1eb0 Mon Sep 17 00:00:00 2001
> From: "Barry Song (Xiaomi)" <baohua@kernel.org>
> Date: Thu, 3 Sep 2026 14:48:52 +0800
> Subject: [PATCH] mm: revert slight behavior change for swappiness 1-200
> 
> Baoquan's review found that we unexpectedly introduced a slight behavior
> change for swappiness 1-200. We could now have a case like:
> 
> 1. First scan -> `scanned != 0`
> 2. Second scan -> `scanned = 0`
> 3. Type fallback
> 
> Step 3 was impossible before. Let's remove this possibility.
> 
> Reported-by: Baoquan He <baoquan.he@linux.dev>
> Closes: https://lore.kernel.org/linux-mm/apfZQE1X6zGAsBb_@fedora/
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---
>  mm/vmscan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bf2786c7247d..ba7adf36e69f 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4939,7 +4939,7 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>  	 * We are running out of the current reclaim type. Fall back to
>  	 * the other type if allowed.
>  	 */
> -	if (!scanned && type_fallback_allowed) {
> +	if (!scanned && !tried && type_fallback_allowed) {
>  		type = !type;
>  		tried = true;
>  		type_fallback_allowed = false;
> -- 
> 2.39.3 (Apple Git-146)
>