[PATCH] mm/mglru: check need_resched() in fill/drain_evictable()

[PATCH] mm/mglru: check need_resched() in fill/drain_evictable()

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

lru_gen_change_state() migrates folios in batches of up to
MAX_LRU_BATCH (4096) folios under lruvec_lock_irq().  A full batch
keeps local interrupts off for up to about a millisecond, and CPUs
contending for the same lru_lock spin with interrupts disabled for
that long.

The caller already reschedules between batches, but a batch itself
only ends after 4096 folios.  Stop it early when a resched is
pending; the caller restarts the migration where it left off.  A
remote resched_curr() sets TIF_NEED_RESCHED directly in the target's
thread_info, so the flag can still be set with interrupts off.

This follows walk_pud_range(), which already pairs need_resched()
with a MAX_LRU_BATCH cap, and commit 6e9be217a3ce ("mm/mglru: use a
smaller batch for reclaim").

Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 mm/vmscan.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 6a1af9b92349..47f2cb25eb46 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -5533,7 +5533,8 @@ static bool fill_evictable(struct lruvec *lruvec)
 			success = lru_gen_add_folio(lruvec, folio, !active);
 			VM_WARN_ON_ONCE(!success);
 
-			if (!--remaining)
+			/* stop early to shorten the irq-off section */
+			if (!--remaining || need_resched())
 				return false;
 		}
 	}
@@ -5562,7 +5563,8 @@ static bool drain_evictable(struct lruvec *lruvec)
 			VM_WARN_ON_ONCE(!success);
 			lruvec_add_folio(lruvec, folio);
 
-			if (!--remaining)
+			/* stop early to shorten the irq-off section */
+			if (!--remaining || need_resched())
 				return false;
 		}
 	}
-- 
2.53.0

Re: [PATCH] mm/mglru: check need_resched() in fill/drain_evictable()

[Kairui Song]

On Thu, Nov 12, 2026 at 03:58:27PM +0800, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> lru_gen_change_state() migrates folios in batches of up to
> MAX_LRU_BATCH (4096) folios under lruvec_lock_irq().  A full batch
> keeps local interrupts off for up to about a millisecond, and CPUs
> contending for the same lru_lock spin with interrupts disabled for
> that long.
> 
> The caller already reschedules between batches, but a batch itself
> only ends after 4096 folios.  Stop it early when a resched is
> pending; the caller restarts the migration where it left off.  A
> remote resched_curr() sets TIF_NEED_RESCHED directly in the target's
> thread_info, so the flag can still be set with interrupts off.
> 
> This follows walk_pud_range(), which already pairs need_resched()
> with a MAX_LRU_BATCH cap, and commit 6e9be217a3ce ("mm/mglru: use a
> smaller batch for reclaim").
> 
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> ---
>  mm/vmscan.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 6a1af9b92349..47f2cb25eb46 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -5533,7 +5533,8 @@ static bool fill_evictable(struct lruvec *lruvec)
>  			success = lru_gen_add_folio(lruvec, folio, !active);
>  			VM_WARN_ON_ONCE(!success);
>  
> -			if (!--remaining)
> +			/* stop early to shorten the irq-off section */
> +			if (!--remaining || need_resched())

Hi Hui,

I've been thinking about a different design, what if we just
delete the `--remaining` check completely, and check
spin_is_contended on the lruvec and need_resched()?

In the optimal case we don't need to keep lock/unlock the lruvec
at all so this is faster. And it still avoids lock contention or
blocked schedule, and it might be even more responsive as we are
release the lock earlier on contention.

Re: [PATCH] mm/mglru: check need_resched() in fill/drain_evictable()

[Michal Hocko]

On Thu 12-11-26 16:03:54, Kairui Song wrote:
> On Thu, Nov 12, 2026 at 03:58:27PM +0800, Hui Zhu wrote:
> > From: Hui Zhu <zhuhui@kylinos.cn>
> > 
> > lru_gen_change_state() migrates folios in batches of up to
> > MAX_LRU_BATCH (4096) folios under lruvec_lock_irq().  A full batch
> > keeps local interrupts off for up to about a millisecond, and CPUs
> > contending for the same lru_lock spin with interrupts disabled for
> > that long.
> > 
> > The caller already reschedules between batches, but a batch itself
> > only ends after 4096 folios.  Stop it early when a resched is
> > pending; the caller restarts the migration where it left off.  A
> > remote resched_curr() sets TIF_NEED_RESCHED directly in the target's
> > thread_info, so the flag can still be set with interrupts off.
> > 
> > This follows walk_pud_range(), which already pairs need_resched()
> > with a MAX_LRU_BATCH cap, and commit 6e9be217a3ce ("mm/mglru: use a
> > smaller batch for reclaim").
> > 
> > Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> > ---
> >  mm/vmscan.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 6a1af9b92349..47f2cb25eb46 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -5533,7 +5533,8 @@ static bool fill_evictable(struct lruvec *lruvec)
> >  			success = lru_gen_add_folio(lruvec, folio, !active);
> >  			VM_WARN_ON_ONCE(!success);
> >  
> > -			if (!--remaining)
> > +			/* stop early to shorten the irq-off section */
> > +			if (!--remaining || need_resched())
> 
> Hi Hui,
> 
> I've been thinking about a different design, what if we just
> delete the `--remaining` check completely, and check
> spin_is_contended on the lruvec and need_resched()?
> 
> In the optimal case we don't need to keep lock/unlock the lruvec
> at all so this is faster. And it still avoids lock contention or
> blocked schedule, and it might be even more responsive as we are
> release the lock earlier on contention.

Wouldn't this result in too much lock bouncing in the end? Would it make
sense to reduce the batch instead?

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm/mglru: check need_resched() in fill/drain_evictable()

[Kairui Song]

On Thu, Nov 12, 2026 at 4:04 PM Kairui Song <ryncsn@gmail.com> wrote:
>
> On Thu, Nov 12, 2026 at 03:58:27PM +0800, Hui Zhu wrote:
> > From: Hui Zhu <zhuhui@kylinos.cn>
> >
> > lru_gen_change_state() migrates folios in batches of up to
> > MAX_LRU_BATCH (4096) folios under lruvec_lock_irq().  A full batch
> > keeps local interrupts off for up to about a millisecond, and CPUs
> > contending for the same lru_lock spin with interrupts disabled for
> > that long.
> >
> > The caller already reschedules between batches, but a batch itself
> > only ends after 4096 folios.  Stop it early when a resched is
> > pending; the caller restarts the migration where it left off.  A
> > remote resched_curr() sets TIF_NEED_RESCHED directly in the target's
> > thread_info, so the flag can still be set with interrupts off.
> >
> > This follows walk_pud_range(), which already pairs need_resched()
> > with a MAX_LRU_BATCH cap, and commit 6e9be217a3ce ("mm/mglru: use a
> > smaller batch for reclaim").
> >
> > Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> > ---
> >  mm/vmscan.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 6a1af9b92349..47f2cb25eb46 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -5533,7 +5533,8 @@ static bool fill_evictable(struct lruvec *lruvec)
> >                       success = lru_gen_add_folio(lruvec, folio, !active);
> >                       VM_WARN_ON_ONCE(!success);
> >
> > -                     if (!--remaining)
> > +                     /* stop early to shorten the irq-off section */
> > +                     if (!--remaining || need_resched())
>
> Hi Hui,
>
> I've been thinking about a different design, what if we just
> delete the `--remaining` check completely, and check
> spin_is_contended on the lruvec and need_resched()?
>
> In the optimal case we don't need to keep lock/unlock the lruvec
> at all so this is faster. And it still avoids lock contention or
> blocked schedule, and it might be even more responsive as we are
> release the lock earlier on contention.

Hmm, nevermind, I just realized we have irq off here so it's
definitely a bad idea to drop batch limit. But maybe we can check the
lock too besides need_resched()?

Re: [PATCH] mm/mglru: check need_resched() in fill/drain_evictable()

[Kairui Song]

On Thu, Nov 12, 2026 at 4:09 PM Michal Hocko <mhocko@suse.com> wrote:
>
> On Thu 12-11-26 16:03:54, Kairui Song wrote:
> > On Thu, Nov 12, 2026 at 03:58:27PM +0800, Hui Zhu wrote:
> > > From: Hui Zhu <zhuhui@kylinos.cn>
> > >
> > > lru_gen_change_state() migrates folios in batches of up to
> > > MAX_LRU_BATCH (4096) folios under lruvec_lock_irq().  A full batch
> > > keeps local interrupts off for up to about a millisecond, and CPUs
> > > contending for the same lru_lock spin with interrupts disabled for
> > > that long.
> > >
> > > The caller already reschedules between batches, but a batch itself
> > > only ends after 4096 folios.  Stop it early when a resched is
> > > pending; the caller restarts the migration where it left off.  A
> > > remote resched_curr() sets TIF_NEED_RESCHED directly in the target's
> > > thread_info, so the flag can still be set with interrupts off.
> > >
> > > This follows walk_pud_range(), which already pairs need_resched()
> > > with a MAX_LRU_BATCH cap, and commit 6e9be217a3ce ("mm/mglru: use a
> > > smaller batch for reclaim").
> > >
> > > Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> > > ---
> > >  mm/vmscan.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index 6a1af9b92349..47f2cb25eb46 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > > @@ -5533,7 +5533,8 @@ static bool fill_evictable(struct lruvec *lruvec)
> > >                     success = lru_gen_add_folio(lruvec, folio, !active);
> > >                     VM_WARN_ON_ONCE(!success);
> > >
> > > -                   if (!--remaining)
> > > +                   /* stop early to shorten the irq-off section */
> > > +                   if (!--remaining || need_resched())
> >
> > Hi Hui,
> >
> > I've been thinking about a different design, what if we just
> > delete the `--remaining` check completely, and check
> > spin_is_contended on the lruvec and need_resched()?
> >
> > In the optimal case we don't need to keep lock/unlock the lruvec
> > at all so this is faster. And it still avoids lock contention or
> > blocked schedule, and it might be even more responsive as we are
> > release the lock earlier on contention.
>
> Wouldn't this result in too much lock bouncing in the end? Would it make
> sense to reduce the batch instead?

Hi Michal, thanks for the suggestion.

The fill/eviction loop here is a quite cold path so probably fine, I
did just found another reason not to do that though.

Re: [PATCH] mm/mglru: check need_resched() in fill/drain_evictable()

[Hui Zhu]

Hi Michal and Kairui,

2026/11/12 16:09, Michal Hocko:
> On Thu 12-11-26 16:03:54, Kairui Song wrote:
>> On Thu, Nov 12, 2026 at 03:58:27PM +0800, Hui Zhu wrote:
>>> From: Hui Zhu <zhuhui@kylinos.cn>
>>>
>>> lru_gen_change_state() migrates folios in batches of up to
>>> MAX_LRU_BATCH (4096) folios under lruvec_lock_irq().  A full batch
>>> keeps local interrupts off for up to about a millisecond, and CPUs
>>> contending for the same lru_lock spin with interrupts disabled for
>>> that long.
>>>
>>> The caller already reschedules between batches, but a batch itself
>>> only ends after 4096 folios.  Stop it early when a resched is
>>> pending; the caller restarts the migration where it left off.  A
>>> remote resched_curr() sets TIF_NEED_RESCHED directly in the target's
>>> thread_info, so the flag can still be set with interrupts off.
>>>
>>> This follows walk_pud_range(), which already pairs need_resched()
>>> with a MAX_LRU_BATCH cap, and commit 6e9be217a3ce ("mm/mglru: use a
>>> smaller batch for reclaim").
>>>
>>> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
>>> ---
>>>   mm/vmscan.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>> index 6a1af9b92349..47f2cb25eb46 100644
>>> --- a/mm/vmscan.c
>>> +++ b/mm/vmscan.c
>>> @@ -5533,7 +5533,8 @@ static bool fill_evictable(struct lruvec *lruvec)
>>>   			success = lru_gen_add_folio(lruvec, folio, !active);
>>>   			VM_WARN_ON_ONCE(!success);
>>>   
>>> -			if (!--remaining)
>>> +			/* stop early to shorten the irq-off section */
>>> +			if (!--remaining || need_resched())
>> Hi Hui,
>>
>> I've been thinking about a different design, what if we just
>> delete the `--remaining` check completely, and check
>> spin_is_contended on the lruvec and need_resched()?
>>
>> In the optimal case we don't need to keep lock/unlock the lruvec
>> at all so this is faster. And it still avoids lock contention or
>> blocked schedule, and it might be even more responsive as we are
>> release the lock earlier on contention.
I like the spin_is_contended() idea and will add it.  This file
already uses it (lru_gen_look_around() bails out if the PTL is
contended), and security/keys/gc.c uses exactly this combination in a
long list walk under a spinlock.
> Wouldn't this result in too much lock bouncing in the end? Would it make
> sense to reduce the batch instead?
>
This worry is fair.  Under sustained contention each yield costs an
unlock, a requeue and a restart (microseconds), while one folio move
costs a few hundred nanoseconds, so the migration could slow down by
tens of times, with cgroup_lock and cpus_read_lock held for all of it.
Requiring a minimum of work per hold bounds this: only honor
spin_is_contended() after MIN_LRU_BATCH folios in the current hold.
Each hold then moves at least ~64 folios (~15us), and a waiter still
waits only ~15us instead of the ~1ms of a full MAX_LRU_BATCH batch.

I'd still keep the batch cap itself as a backstop, though.  Both
triggers are event driven: with interrupts off the local tick cannot
set TIF_NEED_RESCHED, so it takes a remote wakeup targeting this CPU,
and spin_is_contended() takes somebody grabbing this lru_lock.  On a
quiet system neither may ever fire, and one batch becomes the whole
lruvec: 100GB of 4K folios is ~25M of them, i.e. seconds of irq-off
inside a single rcu_read_lock() section, enough for an RCU stall. And
toggling MGLRU on a quiet machine is a common case.

Reducing the batch alone would also work and is simpler, but it pays
the unlock/relock round trip even when nobody is waiting, and on a
loaded machine the unconditional cond_resched() every small batch
stretches the whole switch more than yielding only when somebody
actually wants the lock or the CPU.

So how about all three combined, like jbd2_log_do_checkpoint() does
(batch cap || need_resched() || spin_needbreak()):

     if (!--remaining || need_resched() ||
         (moved >= MIN_LRU_BATCH && spin_is_contended(&lruvec->lru_lock)))
         return false;

Should I send a v2 with this?

Best,
Hui

Re: [PATCH] mm/mglru: check need_resched() in fill/drain_evictable()

[Kairui Song]

On Thu, Nov 12, 2026 at 5:01 PM Hui Zhu <hui.zhu@linux.dev> wrote:
>
> Hi Michal and Kairui,
>
> 2026/11/12 16:09, Michal Hocko:
> > On Thu 12-11-26 16:03:54, Kairui Song wrote:
> >> On Thu, Nov 12, 2026 at 03:58:27PM +0800, Hui Zhu wrote:
> >>> From: Hui Zhu <zhuhui@kylinos.cn>
> >>>
> >>> lru_gen_change_state() migrates folios in batches of up to
> >>> MAX_LRU_BATCH (4096) folios under lruvec_lock_irq().  A full batch
> >>> keeps local interrupts off for up to about a millisecond, and CPUs
> >>> contending for the same lru_lock spin with interrupts disabled for
> >>> that long.
> >>>
> >>> The caller already reschedules between batches, but a batch itself
> >>> only ends after 4096 folios.  Stop it early when a resched is
> >>> pending; the caller restarts the migration where it left off.  A
> >>> remote resched_curr() sets TIF_NEED_RESCHED directly in the target's
> >>> thread_info, so the flag can still be set with interrupts off.
> >>>
> >>> This follows walk_pud_range(), which already pairs need_resched()
> >>> with a MAX_LRU_BATCH cap, and commit 6e9be217a3ce ("mm/mglru: use a
> >>> smaller batch for reclaim").
> >>>
> >>> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> >>> ---
> >>>   mm/vmscan.c | 6 ++++--
> >>>   1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/mm/vmscan.c b/mm/vmscan.c
> >>> index 6a1af9b92349..47f2cb25eb46 100644
> >>> --- a/mm/vmscan.c
> >>> +++ b/mm/vmscan.c
> >>> @@ -5533,7 +5533,8 @@ static bool fill_evictable(struct lruvec *lruvec)
> >>>                     success = lru_gen_add_folio(lruvec, folio, !active);
> >>>                     VM_WARN_ON_ONCE(!success);
> >>>
> >>> -                   if (!--remaining)
> >>> +                   /* stop early to shorten the irq-off section */
> >>> +                   if (!--remaining || need_resched())
> >> Hi Hui,
> >>
> >> I've been thinking about a different design, what if we just
> >> delete the `--remaining` check completely, and check
> >> spin_is_contended on the lruvec and need_resched()?
> >>
> >> In the optimal case we don't need to keep lock/unlock the lruvec
> >> at all so this is faster. And it still avoids lock contention or
> >> blocked schedule, and it might be even more responsive as we are
> >> release the lock earlier on contention.
> I like the spin_is_contended() idea and will add it.  This file
> already uses it (lru_gen_look_around() bails out if the PTL is
> contended), and security/keys/gc.c uses exactly this combination in a
> long list walk under a spinlock.
> > Wouldn't this result in too much lock bouncing in the end? Would it make
> > sense to reduce the batch instead?
> >
> This worry is fair.  Under sustained contention each yield costs an
> unlock, a requeue and a restart (microseconds), while one folio move
> costs a few hundred nanoseconds, so the migration could slow down by
> tens of times, with cgroup_lock and cpus_read_lock held for all of it.
> Requiring a minimum of work per hold bounds this: only honor
> spin_is_contended() after MIN_LRU_BATCH folios in the current hold.
> Each hold then moves at least ~64 folios (~15us), and a waiter still
> waits only ~15us instead of the ~1ms of a full MAX_LRU_BATCH batch.
>
> I'd still keep the batch cap itself as a backstop, though.  Both
> triggers are event driven: with interrupts off the local tick cannot
> set TIF_NEED_RESCHED, so it takes a remote wakeup targeting this CPU,
> and spin_is_contended() takes somebody grabbing this lru_lock.  On a
> quiet system neither may ever fire, and one batch becomes the whole
> lruvec: 100GB of 4K folios is ~25M of them, i.e. seconds of irq-off
> inside a single rcu_read_lock() section, enough for an RCU stall. And
> toggling MGLRU on a quiet machine is a common case.
>
> Reducing the batch alone would also work and is simpler, but it pays
> the unlock/relock round trip even when nobody is waiting, and on a
> loaded machine the unconditional cond_resched() every small batch
> stretches the whole switch more than yielding only when somebody
> actually wants the lock or the CPU.
>
> So how about all three combined, like jbd2_log_do_checkpoint() does
> (batch cap || need_resched() || spin_needbreak()):
>
>      if (!--remaining || need_resched() ||
>          (moved >= MIN_LRU_BATCH && spin_is_contended(&lruvec->lru_lock)))
>          return false;

Do we really need the `moved` counter? Other than that it looks good
to me. Thanks!