[PATCH v2] mm/mglru: yield the lru_lock earlier in fill/drain_evictable()

[PATCH v2] mm/mglru: yield the lru_lock earlier 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, or when the lock has waiters and the current hold has moved
at least MIN_LRU_BATCH folios; the caller restarts where it left off.
TIF_NEED_RESCHED can still be set with interrupts off: a remote
resched_curr() sets it directly in the target's thread_info.  The
minimum batch avoids lock bouncing under sustained contention, while
a waiter now waits ~15us instead of ~1ms.

This follows lru_gen_look_around() in this file, security/keys/gc.c
and jbd2_log_do_checkpoint(), and commit 6e9be217a3ce ("mm/mglru:
use a smaller batch for reclaim").

Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
Changlog:
v2:
Also yield on lock contention after a minimum batch (Kairui Song).

 mm/vmscan.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 6a1af9b92349..c1231925159f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -5533,7 +5533,10 @@ 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() ||
+			    (MAX_LRU_BATCH - remaining >= MIN_LRU_BATCH &&
+			     spin_is_contended(&lruvec->lru_lock)))
 				return false;
 		}
 	}
@@ -5562,7 +5565,10 @@ 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() ||
+			    (MAX_LRU_BATCH - remaining >= MIN_LRU_BATCH &&
+			     spin_is_contended(&lruvec->lru_lock)))
 				return false;
 		}
 	}
-- 
2.53.0

Re: [PATCH v2] mm/mglru: yield the lru_lock earlier in fill/drain_evictable()

[Baolin Wang]

On 11/13/26 10:01 AM, 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, or when the lock has waiters and the current hold has moved
> at least MIN_LRU_BATCH folios; the caller restarts where it left off.
> TIF_NEED_RESCHED can still be set with interrupts off: a remote
> resched_curr() sets it directly in the target's thread_info.  The
> minimum batch avoids lock bouncing under sustained contention, while
> a waiter now waits ~15us instead of ~1ms.
> 
> This follows lru_gen_look_around() in this file, security/keys/gc.c
> and jbd2_log_do_checkpoint(), and commit 6e9be217a3ce ("mm/mglru:
> use a smaller batch for reclaim").
> 
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> ---
> Changlog:
> v2:
> Also yield on lock contention after a minimum batch (Kairui Song).
> 
>   mm/vmscan.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 6a1af9b92349..c1231925159f 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -5533,7 +5533,10 @@ 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() ||
> +			    (MAX_LRU_BATCH - remaining >= MIN_LRU_BATCH &&
> +			     spin_is_contended(&lruvec->lru_lock)))
>   				return false;

IIUC, Kairui's point[1] is to just drop the 'MAX_LRU_BATCH - remaining 
 >= MIN_LRU_BATCH' condition, no?

if (!--remaining || need_resched() || spin_is_contended(&lruvec->lru_lock))

[1] 
https://lore.kernel.org/all/CAMgjq7CUMuNSdCXZ7_kDdo9+0BFvPNHuO=mq=ct_Q5YUyyckvQ@mail.gmail.com/

Re: [PATCH v2] mm/mglru: yield the lru_lock earlier in fill/drain_evictable()

[Hui Zhu]

2026/11/13 10:50, Baolin Wang :
>
>
> On 11/13/26 10:01 AM, 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, or when the lock has waiters and the current hold has moved
>> at least MIN_LRU_BATCH folios; the caller restarts where it left off.
>> TIF_NEED_RESCHED can still be set with interrupts off: a remote
>> resched_curr() sets it directly in the target's thread_info. The
>> minimum batch avoids lock bouncing under sustained contention, while
>> a waiter now waits ~15us instead of ~1ms.
>>
>> This follows lru_gen_look_around() in this file, security/keys/gc.c
>> and jbd2_log_do_checkpoint(), and commit 6e9be217a3ce ("mm/mglru:
>> use a smaller batch for reclaim").
>>
>> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
>> ---
>> Changlog:
>> v2:
>> Also yield on lock contention after a minimum batch (Kairui Song).
>>
>>   mm/vmscan.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 6a1af9b92349..c1231925159f 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -5533,7 +5533,10 @@ 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() ||
>> +                (MAX_LRU_BATCH - remaining >= MIN_LRU_BATCH &&
>> +                 spin_is_contended(&lruvec->lru_lock)))
>>                   return false;
>
> IIUC, Kairui's point[1] is to just drop the 'MAX_LRU_BATCH - remaining 
> >= MIN_LRU_BATCH' condition, no?
>
> if (!--remaining || need_resched() || 
> spin_is_contended(&lruvec->lru_lock))
>
> [1] 
> https://lore.kernel.org/all/CAMgjq7CUMuNSdCXZ7_kDdo9+0BFvPNHuO=mq=ct_Q5YUyyckvQ@mail.gmail.com/ 
>

Not quite.  [1] was about the extra `moved` counter in the version
discussed on the v1 thread; Kairui was fine with the rest.  Since
`remaining` counts down from MAX_LRU_BATCH and one call is exactly one
lock hold, `MAX_LRU_BATCH - remaining` is the number of folios moved
in the current hold, so the condition is just `moved >= MIN_LRU_BATCH`
without the extra variable.

The minimum-batch test itself is a response to Michal's lock bouncing
concern [2].  Without it, under sustained contention the loop could
yield after every single folio: 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 throughout.  With it, each hold
moves at least MIN_LRU_BATCH (64) folios, i.e. does ~15us of work
before it may hand the lock over, while a waiter still waits only
~15us instead of the ~1ms of a full MAX_LRU_BATCH batch.

[2] https://lore.kernel.org/all/avV0IbvSx4iuv0tF@tiehlicka/

Best,
Hui

Re: [PATCH v2] mm/mglru: yield the lru_lock earlier in fill/drain_evictable()

[Baolin Wang]

On 11/13/26 11:08 AM, Hui Zhu wrote:
> 
> 2026/11/13 10:50, Baolin Wang :
>>
>>
>> On 11/13/26 10:01 AM, 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, or when the lock has waiters and the current hold has moved
>>> at least MIN_LRU_BATCH folios; the caller restarts where it left off.
>>> TIF_NEED_RESCHED can still be set with interrupts off: a remote
>>> resched_curr() sets it directly in the target's thread_info. The
>>> minimum batch avoids lock bouncing under sustained contention, while
>>> a waiter now waits ~15us instead of ~1ms.
>>>
>>> This follows lru_gen_look_around() in this file, security/keys/gc.c
>>> and jbd2_log_do_checkpoint(), and commit 6e9be217a3ce ("mm/mglru:
>>> use a smaller batch for reclaim").
>>>
>>> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
>>> ---
>>> Changlog:
>>> v2:
>>> Also yield on lock contention after a minimum batch (Kairui Song).
>>>
>>>   mm/vmscan.c | 10 ++++++++--
>>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>> index 6a1af9b92349..c1231925159f 100644
>>> --- a/mm/vmscan.c
>>> +++ b/mm/vmscan.c
>>> @@ -5533,7 +5533,10 @@ 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() ||
>>> +                (MAX_LRU_BATCH - remaining >= MIN_LRU_BATCH &&
>>> +                 spin_is_contended(&lruvec->lru_lock)))
>>>                   return false;
>>
>> IIUC, Kairui's point[1] is to just drop the 'MAX_LRU_BATCH - remaining 
>> >= MIN_LRU_BATCH' condition, no?
>>
>> if (!--remaining || need_resched() || spin_is_contended(&lruvec- 
>> >lru_lock))
>>
>> [1] https://lore.kernel.org/all/ 
>> CAMgjq7CUMuNSdCXZ7_kDdo9+0BFvPNHuO=mq=ct_Q5YUyyckvQ@mail.gmail.com/
> 
> Not quite.  [1] was about the extra `moved` counter in the version
> discussed on the v1 thread; Kairui was fine with the rest.  Since
> `remaining` counts down from MAX_LRU_BATCH and one call is exactly one
> lock hold, `MAX_LRU_BATCH - remaining` is the number of folios moved
> in the current hold, so the condition is just `moved >= MIN_LRU_BATCH`
> without the extra variable.

If so, why not just set:

int remaining = MIN_LRU_BATCH;
?

> The minimum-batch test itself is a response to Michal's lock bouncing
> concern [2].  Without it, under sustained contention the loop could
> yield after every single folio: 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 throughout.  With it, each hold
> moves at least MIN_LRU_BATCH (64) folios, i.e. does ~15us of work
> before it may hand the lock over, while a waiter still waits only
> ~15us instead of the ~1ms of a full MAX_LRU_BATCH batch.

fill_evictable()/drain_evictable() are really cold paths, I really don't 
see any practical impact from this. At least we (Alibaba) won't be 
dynamically switching MGLRU state in practice.