[PATCH v2 03/12] mm/mglru: relocate the LRU scan batch limit to callers

Kairui Song via B4 Relay posted 12 patches 4 days, 16 hours ago
[PATCH v2 03/12] mm/mglru: relocate the LRU scan batch limit to callers
Posted by Kairui Song via B4 Relay 4 days, 16 hours ago
From: Kairui Song <kasong@tencent.com>

Same as active / inactive LRU, MGLRU isolates and scans folios in
batches.  The batch split is done hidden deep in the helper, which
makes the code harder to follow.  The helper's arguments are also
confusing since callers usually request more folios than the batch
size, so the helper almost never processes the full requested amount.

Move the batch splitting into the top loop to make it cleaner, there
should be no behavior change.

Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>
Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/vmscan.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index f336f89a2de6..963362523782 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4695,10 +4695,10 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	int scanned = 0;
 	int isolated = 0;
 	int skipped = 0;
-	int scan_batch = min(nr_to_scan, MAX_LRU_BATCH);
-	int remaining = scan_batch;
+	unsigned long remaining = nr_to_scan;
 	struct lru_gen_folio *lrugen = &lruvec->lrugen;
 
+	VM_WARN_ON_ONCE(nr_to_scan > MAX_LRU_BATCH);
 	VM_WARN_ON_ONCE(!list_empty(list));
 
 	if (get_nr_gens(lruvec, type) == MIN_NR_GENS)
@@ -4751,7 +4751,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	mod_lruvec_state(lruvec, item, isolated);
 	mod_lruvec_state(lruvec, PGREFILL, sorted);
 	mod_lruvec_state(lruvec, PGSCAN_ANON + type, isolated);
-	trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, scan_batch,
+	trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan,
 				scanned, skipped, isolated,
 				type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
 	if (type == LRU_GEN_FILE)
@@ -4987,7 +4987,7 @@ static bool should_abort_scan(struct lruvec *lruvec, struct scan_control *sc)
 
 static bool try_to_shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
 {
-	long nr_to_scan;
+	long nr_batch, nr_to_scan;
 	unsigned long scanned = 0;
 	int swappiness = get_swappiness(lruvec, sc);
 
@@ -4998,7 +4998,8 @@ static bool try_to_shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
 		if (nr_to_scan <= 0)
 			break;
 
-		delta = evict_folios(nr_to_scan, lruvec, sc, swappiness);
+		nr_batch = min(nr_to_scan, MAX_LRU_BATCH);
+		delta = evict_folios(nr_batch, lruvec, sc, swappiness);
 		if (!delta)
 			break;
 
@@ -5623,6 +5624,7 @@ static int run_aging(struct lruvec *lruvec, unsigned long seq,
 static int run_eviction(struct lruvec *lruvec, unsigned long seq, struct scan_control *sc,
 			int swappiness, unsigned long nr_to_reclaim)
 {
+	int nr_batch;
 	DEFINE_MAX_SEQ(lruvec);
 
 	if (seq + MIN_NR_GENS > max_seq)
@@ -5639,8 +5641,8 @@ static int run_eviction(struct lruvec *lruvec, unsigned long seq, struct scan_co
 		if (sc->nr_reclaimed >= nr_to_reclaim)
 			return 0;
 
-		if (!evict_folios(nr_to_reclaim - sc->nr_reclaimed, lruvec, sc,
-				  swappiness))
+		nr_batch = min(nr_to_reclaim - sc->nr_reclaimed, MAX_LRU_BATCH);
+		if (!evict_folios(nr_batch, lruvec, sc, swappiness))
 			return 0;
 
 		cond_resched();

-- 
2.53.0
Re: [PATCH v2 03/12] mm/mglru: relocate the LRU scan batch limit to callers
Posted by Baolin Wang 3 days, 4 hours ago

On 3/29/26 3:52 AM, Kairui Song via B4 Relay wrote:
> From: Kairui Song <kasong@tencent.com>
> 
> Same as active / inactive LRU, MGLRU isolates and scans folios in
> batches.  The batch split is done hidden deep in the helper, which
> makes the code harder to follow.  The helper's arguments are also
> confusing since callers usually request more folios than the batch
> size, so the helper almost never processes the full requested amount.
> 
> Move the batch splitting into the top loop to make it cleaner, there
> should be no behavior change.
> 
> Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---

Some nits as follows, otherwise LGTM.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

>   mm/vmscan.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f336f89a2de6..963362523782 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4695,10 +4695,10 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>   	int scanned = 0;
>   	int isolated = 0;
>   	int skipped = 0;
> -	int scan_batch = min(nr_to_scan, MAX_LRU_BATCH);
> -	int remaining = scan_batch;
> +	unsigned long remaining = nr_to_scan;
>   	struct lru_gen_folio *lrugen = &lruvec->lrugen;
>   
> +	VM_WARN_ON_ONCE(nr_to_scan > MAX_LRU_BATCH);
>   	VM_WARN_ON_ONCE(!list_empty(list));
>   
>   	if (get_nr_gens(lruvec, type) == MIN_NR_GENS)
> @@ -4751,7 +4751,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>   	mod_lruvec_state(lruvec, item, isolated);
>   	mod_lruvec_state(lruvec, PGREFILL, sorted);
>   	mod_lruvec_state(lruvec, PGSCAN_ANON + type, isolated);
> -	trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, scan_batch,
> +	trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan,
>   				scanned, skipped, isolated,
>   				type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
>   	if (type == LRU_GEN_FILE)
> @@ -4987,7 +4987,7 @@ static bool should_abort_scan(struct lruvec *lruvec, struct scan_control *sc)
>   
>   static bool try_to_shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
>   {
> -	long nr_to_scan;
> +	long nr_batch, nr_to_scan;

Nit: Since evict_folios() expects an unsgined long, why not define 
'unsigned long nr_batch'?

>   	unsigned long scanned = 0;
>   	int swappiness = get_swappiness(lruvec, sc);
>   
> @@ -4998,7 +4998,8 @@ static bool try_to_shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
>   		if (nr_to_scan <= 0)
>   			break;
>   
> -		delta = evict_folios(nr_to_scan, lruvec, sc, swappiness);
> +		nr_batch = min(nr_to_scan, MAX_LRU_BATCH);
> +		delta = evict_folios(nr_batch, lruvec, sc, swappiness);
>   		if (!delta)
>   			break;
>   
> @@ -5623,6 +5624,7 @@ static int run_aging(struct lruvec *lruvec, unsigned long seq,
>   static int run_eviction(struct lruvec *lruvec, unsigned long seq, struct scan_control *sc,
>   			int swappiness, unsigned long nr_to_reclaim)
>   {
> +	int nr_batch;

Nit: since 'nr_to_reclaim' is unsigned long, better to use unsigned long 
for 'nr_batch'.

>   	DEFINE_MAX_SEQ(lruvec);
>   
>   	if (seq + MIN_NR_GENS > max_seq)
> @@ -5639,8 +5641,8 @@ static int run_eviction(struct lruvec *lruvec, unsigned long seq, struct scan_co
>   		if (sc->nr_reclaimed >= nr_to_reclaim)
>   			return 0;
>   
> -		if (!evict_folios(nr_to_reclaim - sc->nr_reclaimed, lruvec, sc,
> -				  swappiness))
> +		nr_batch = min(nr_to_reclaim - sc->nr_reclaimed, MAX_LRU_BATCH);
> +		if (!evict_folios(nr_batch, lruvec, sc, swappiness))
>   			return 0;
>   
>   		cond_resched();
>
Re: [PATCH v2 03/12] mm/mglru: relocate the LRU scan batch limit to callers
Posted by Barry Song 1 day, 12 hours ago
On Mon, Mar 30, 2026 at 4:15 PM Baolin Wang
<baolin.wang@linux.alibaba.com> wrote:
>
>
>
> On 3/29/26 3:52 AM, Kairui Song via B4 Relay wrote:
> > From: Kairui Song <kasong@tencent.com>
> >
> > Same as active / inactive LRU, MGLRU isolates and scans folios in
> > batches.  The batch split is done hidden deep in the helper, which
> > makes the code harder to follow.  The helper's arguments are also
> > confusing since callers usually request more folios than the batch
> > size, so the helper almost never processes the full requested amount.
> >
> > Move the batch splitting into the top loop to make it cleaner, there
> > should be no behavior change.
> >
> > Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>
> > Signed-off-by: Kairui Song <kasong@tencent.com>
> > ---
>
> Some nits as follows, otherwise LGTM.
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

With the same nits addressed,

Reviewed-by: Barry Song <baohua@kernel.org>

>
> >   mm/vmscan.c | 16 +++++++++-------
> >   1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index f336f89a2de6..963362523782 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -4695,10 +4695,10 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> >       int scanned = 0;
> >       int isolated = 0;
> >       int skipped = 0;
> > -     int scan_batch = min(nr_to_scan, MAX_LRU_BATCH);
> > -     int remaining = scan_batch;
> > +     unsigned long remaining = nr_to_scan;
> >       struct lru_gen_folio *lrugen = &lruvec->lrugen;
> >
> > +     VM_WARN_ON_ONCE(nr_to_scan > MAX_LRU_BATCH);
> >       VM_WARN_ON_ONCE(!list_empty(list));
> >
> >       if (get_nr_gens(lruvec, type) == MIN_NR_GENS)
> > @@ -4751,7 +4751,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> >       mod_lruvec_state(lruvec, item, isolated);
> >       mod_lruvec_state(lruvec, PGREFILL, sorted);
> >       mod_lruvec_state(lruvec, PGSCAN_ANON + type, isolated);
> > -     trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, scan_batch,
> > +     trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan,
> >                               scanned, skipped, isolated,
> >                               type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
> >       if (type == LRU_GEN_FILE)
> > @@ -4987,7 +4987,7 @@ static bool should_abort_scan(struct lruvec *lruvec, struct scan_control *sc)
> >
> >   static bool try_to_shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
> >   {
> > -     long nr_to_scan;
> > +     long nr_batch, nr_to_scan;
>
> Nit: Since evict_folios() expects an unsgined long, why not define
> 'unsigned long nr_batch'?

I guess the confusion comes from nr_to_scan being a long
rather than an unsigned long. This is the only place where
nr_to_scan is defined as a long.

I think we should clean up get_nr_to_scan(). Right now, it
clearly returns more than it should, uses -1 to indicate
something else, and also calls try_to_inc_max_seq(), which
is not part of nr_to_scan.

That might be better addressed in a separate patch.

Thanks
Barry