[PATCH v2] mm/slab_common: fix shrink budget underflow in kfree_rcu_shrink_scan

[PATCH v2] mm/slab_common: fix shrink budget underflow in kfree_rcu_shrink_scan

[Longlong Xia]

From: Longlong Xia <xialonglong@kylinos.cn>

The kfree_rcu shrinker decremented sc->nr_to_scan (unsigned long)
and then tested the result with <= 0. When a single CPU's object
count exceeds the remaining budget, the subtraction wraps to a large
positive value and the <= 0 comparison, which is equivalent to == 0
for an unsigned type, never fires again. The scan loop then iterates
through every possible CPU instead of honouring the reclaim budget.

Accumulate into freed and stop once freed >= nr_to_scan. The shrinker
core treats nr_to_scan as input-only, so dropping the decrement is
safe; freed becomes unsigned long to match the return type.

Suggested-by: Hao Li <hao.li@linux.dev>
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
---
Changes in v2:
- Rework per suggestion from Hao Li: accumulate into freed directly,
  compare freed >= nr_to_scan instead of decrementing nr_to_scan, and
  drop the per-CPU count local; promote freed to unsigned long.

Link: https://lore.kernel.org/all/20260824091838.1692153-1-xialonglong2025@163.com/
---
 mm/slab_common.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 657fd75776ea..e227c2ef2a4e 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -2162,20 +2162,17 @@ kfree_rcu_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
 static unsigned long
 kfree_rcu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 {
-	int cpu, freed = 0;
+	int cpu;
+	unsigned long freed = 0;
 
 	for_each_possible_cpu(cpu) {
-		int count;
 		struct kfree_rcu_cpu *krcp = per_cpu_ptr(&krc, cpu);
 
-		count = krc_count(krcp);
-		count += drain_page_cache(krcp);
+		freed += krc_count(krcp);
+		freed += drain_page_cache(krcp);
 		kfree_rcu_monitor(&krcp->monitor_work.work);
 
-		sc->nr_to_scan -= count;
-		freed += count;
-
-		if (sc->nr_to_scan <= 0)
+		if (freed >= sc->nr_to_scan)
 			break;
 	}
 
-- 
2.43.0

Re: [PATCH v2] mm/slab_common: fix shrink budget underflow in kfree_rcu_shrink_scan

[Hao Li]

On Wed, Aug 26, 2026 at 03:56:53PM +0800, Longlong Xia wrote:
> From: Longlong Xia <xialonglong@kylinos.cn>
> 
> The kfree_rcu shrinker decremented sc->nr_to_scan (unsigned long)
> and then tested the result with <= 0. When a single CPU's object
> count exceeds the remaining budget, the subtraction wraps to a large
> positive value and the <= 0 comparison, which is equivalent to == 0
> for an unsigned type, never fires again. The scan loop then iterates
> through every possible CPU instead of honouring the reclaim budget.
> 
> Accumulate into freed and stop once freed >= nr_to_scan. The shrinker
> core treats nr_to_scan as input-only, so dropping the decrement is
> safe; freed becomes unsigned long to match the return type.
> 
> Suggested-by: Hao Li <hao.li@linux.dev>
> Assisted-by: Codex:gpt-5.6-sol
> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
> ---
> Changes in v2:
> - Rework per suggestion from Hao Li: accumulate into freed directly,
>   compare freed >= nr_to_scan instead of decrementing nr_to_scan, and
>   drop the per-CPU count local; promote freed to unsigned long.
> 
> Link: https://lore.kernel.org/all/20260824091838.1692153-1-xialonglong2025@163.com/
> ---
>  mm/slab_common.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 

Looks good to me. Thanks.
Reviewed-by: Hao Li <hao.li@linux.dev>

> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 657fd75776ea..e227c2ef2a4e 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -2162,20 +2162,17 @@ kfree_rcu_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
>  static unsigned long
>  kfree_rcu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>  {
> -	int cpu, freed = 0;
> +	int cpu;
> +	unsigned long freed = 0;
>  
>  	for_each_possible_cpu(cpu) {
> -		int count;
>  		struct kfree_rcu_cpu *krcp = per_cpu_ptr(&krc, cpu);
>  
> -		count = krc_count(krcp);
> -		count += drain_page_cache(krcp);
> +		freed += krc_count(krcp);
> +		freed += drain_page_cache(krcp);
>  		kfree_rcu_monitor(&krcp->monitor_work.work);
>  
> -		sc->nr_to_scan -= count;
> -		freed += count;
> -
> -		if (sc->nr_to_scan <= 0)
> +		if (freed >= sc->nr_to_scan)
>  			break;
>  	}
>  
> -- 
> 2.43.0
> 

-- 
Thanks,
Hao

Re: [PATCH v2] mm/slab_common: fix shrink budget underflow in kfree_rcu_shrink_scan

[Harry Yoo]

On Wed, Aug 26, 2026 at 03:56:53PM +0800, Longlong Xia wrote:
> From: Longlong Xia <xialonglong@kylinos.cn>

Hi Longlong, I have a few questions.

> The kfree_rcu shrinker decremented sc->nr_to_scan (unsigned long)
> and then tested the result with <= 0. When a single CPU's object
> count exceeds the remaining budget, the subtraction wraps to a large
> positive value and the <= 0 comparison, which is equivalent to == 0
> for an unsigned type, never fires again.

Since when (which commit) has it been broken?
If it has been undiscovered for a very long time, why is that so?

And how did you discover this?

> The scan loop then iterates
> through every possible CPU instead of honouring the reclaim budget.

Did you confirm this actually does happen? If so, how often does the
kernel end up iterating through every possible CPUs, very rarely or
almost always?

Would this affect the kernel's reclamation behavior in some way?

> Accumulate into freed and stop once freed >= nr_to_scan. The shrinker
> core treats nr_to_scan as input-only, so dropping the decrement is
> safe; freed becomes unsigned long to match the return type.
>
> Suggested-by: Hao Li <hao.li@linux.dev>
> Assisted-by: Codex:gpt-5.6-sol
> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
> ---
> Changes in v2:
> - Rework per suggestion from Hao Li: accumulate into freed directly,
>   compare freed >= nr_to_scan instead of decrementing nr_to_scan, and
>   drop the per-CPU count local; promote freed to unsigned long.
> 
> Link: https://lore.kernel.org/all/20260824091838.1692153-1-xialonglong2025@163.com/
> ---
>  mm/slab_common.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 657fd75776ea..e227c2ef2a4e 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -2162,20 +2162,17 @@ kfree_rcu_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
>  static unsigned long
>  kfree_rcu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>  {
> -	int cpu, freed = 0;
> +	int cpu;
> +	unsigned long freed = 0;
>  
>  	for_each_possible_cpu(cpu) {
> -		int count;
>  		struct kfree_rcu_cpu *krcp = per_cpu_ptr(&krc, cpu);
>  
> -		count = krc_count(krcp);
> -		count += drain_page_cache(krcp);
> +		freed += krc_count(krcp);
> +		freed += drain_page_cache(krcp);
>  		kfree_rcu_monitor(&krcp->monitor_work.work);
>  
> -		sc->nr_to_scan -= count;
> -		freed += count;
> -
> -		if (sc->nr_to_scan <= 0)
> +		if (freed >= sc->nr_to_scan)
>  			break;
>  	}
>  
> -- 
> 2.43.0

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH v2] mm/slab_common: fix shrink budget underflow in kfree_rcu_shrink_scan

[Longlong Xia]

Thanks for the reply.

在 2026/8/30 22:22, Harry Yoo 写道:
> On Wed, Aug 26, 2026 at 03:56:53PM +0800, Longlong Xia wrote:
>> From: Longlong Xia <xialonglong@kylinos.cn>
> Hi Longlong, I have a few questions.
>
>> The kfree_rcu shrinker decremented sc->nr_to_scan (unsigned long)
>> and then tested the result with <= 0. When a single CPU's object
>> count exceeds the remaining budget, the subtraction wraps to a large
>> positive value and the <= 0 comparison, which is equivalent to == 0
>> for an unsigned type, never fires again.
> Since when (which commit) has it been broken?
> If it has been undiscovered for a very long time, why is that so?

Since the shrinker was first added: 9154244c1ab6 ("rcu/tree: Add a
shrinker to prevent OOM due to kfree_rcu() batching", v5.8);

>
> And how did you discover this?

While reviewing the kvfree_rcu() batching paths with the help of AI
tooling (hence the Assisted-by: tag in the patch), which flagged the
unsigned subtraction feeding a "<= 0" test.

>> The scan loop then iterates
>> through every possible CPU instead of honouring the reclaim budget.
> Did you confirm this actually does happen? If so, how often does the
> kernel end up iterating through every possible CPUs, very rarely or
> almost always?

Yes, in two ways.


Experiment 1 -- deterministic, instrumented loop.  Per-CPU pr_info()
plus a test entry invoking kfree_rcu_shrink_scan() with a controlled
budget; 4-CPU QEMU guest, 400 objects queued on CPU0, budget 50:
   buggy:   cpu=0 count=400 nr_to_scan_after_sub=18446744073709551266
                 visits all 4 CPUs
   patched: cpu=0 count=400 nr_to_scan=50 BREAK

                stops at CPU0


Experiment 2 -- natural trigger, no kernel modification at all
(kprobes only).  Setup: 1 GiB QEMU guest, 4 possible CPUs, no swap.

   1. Observation: three kprobes.  Scan entry/exit record nr_to_scan
      and the return value; each kfree_rcu_monitor() hit inside a
      scan is one CPU the loop processed.

   2. Memory pressure: ~650 MiB of anonymous memory.

   3. krc backlog: loaded the in-tree test_vmalloc module ,

      which repeatedly vmalloc()s one page and frees it via kvfree_rcu();

     10 million iterations over ~70s kept the per-CPU queues populated.


Result within ~70s: 262 natural invocations, all from kswapd0;
171 of the 261 scans that found a backlog (66%) overshot the budget
(nr_to_scan = 128, freed up to 4479) and visited all 4 possible CPUs.


>
> Would this affect the kernel's reclamation behavior in some way?

The break was meant to cap the scan at the reclaim budget; once
the subtraction wraps, the cap is gone and one scan processes
every possible CPU, draining the whole backlog instead of a
budget-sized slice.


Thanks,

Longlong

>> Accumulate into freed and stop once freed >= nr_to_scan. The shrinker
>> core treats nr_to_scan as input-only, so dropping the decrement is
>> safe; freed becomes unsigned long to match the return type.
>>
>> Suggested-by: Hao Li <hao.li@linux.dev>
>> Assisted-by: Codex:gpt-5.6-sol
>> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
>> ---
>> Changes in v2:
>> - Rework per suggestion from Hao Li: accumulate into freed directly,
>>    compare freed >= nr_to_scan instead of decrementing nr_to_scan, and
>>    drop the per-CPU count local; promote freed to unsigned long.
>>
>> Link: https://lore.kernel.org/all/20260824091838.1692153-1-xialonglong2025@163.com/
>> ---
>>   mm/slab_common.c | 13 +++++--------
>>   1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 657fd75776ea..e227c2ef2a4e 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -2162,20 +2162,17 @@ kfree_rcu_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
>>   static unsigned long
>>   kfree_rcu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>>   {
>> -	int cpu, freed = 0;
>> +	int cpu;
>> +	unsigned long freed = 0;
>>   
>>   	for_each_possible_cpu(cpu) {
>> -		int count;
>>   		struct kfree_rcu_cpu *krcp = per_cpu_ptr(&krc, cpu);
>>   
>> -		count = krc_count(krcp);
>> -		count += drain_page_cache(krcp);
>> +		freed += krc_count(krcp);
>> +		freed += drain_page_cache(krcp);
>>   		kfree_rcu_monitor(&krcp->monitor_work.work);
>>   
>> -		sc->nr_to_scan -= count;
>> -		freed += count;
>> -
>> -		if (sc->nr_to_scan <= 0)
>> +		if (freed >= sc->nr_to_scan)
>>   			break;
>>   	}
>>   
>> -- 
>> 2.43.0

Re: [PATCH v2] mm/slab_common: fix shrink budget underflow in kfree_rcu_shrink_scan

[Harry Yoo]

On Thu, Sep 03, 2026 at 05:13:15PM +0800, Longlong Xia wrote:
> Thanks for the reply.
> 
> 在 2026/8/30 22:22, Harry Yoo 写道:
> > On Wed, Aug 26, 2026 at 03:56:53PM +0800, Longlong Xia wrote:
> > > From: Longlong Xia <xialonglong@kylinos.cn>
> > Hi Longlong, I have a few questions.
> > 
> > > The kfree_rcu shrinker decremented sc->nr_to_scan (unsigned long)
> > > and then tested the result with <= 0. When a single CPU's object
> > > count exceeds the remaining budget, the subtraction wraps to a large
> > > positive value and the <= 0 comparison, which is equivalent to == 0
> > > for an unsigned type, never fires again.
> > Since when (which commit) has it been broken?
> > If it has been undiscovered for a very long time, why is that so?
> 
> Since the shrinker was first added: 9154244c1ab6 ("rcu/tree: Add a
> shrinker to prevent OOM due to kfree_rcu() batching", v5.8);
> 

I see.

> > And how did you discover this?
> 
> While reviewing the kvfree_rcu() batching paths with the help of AI
> tooling (hence the Assisted-by: tag in the patch), which flagged the
> unsigned subtraction feeding a "<= 0" test.

Okay.

> > > The scan loop then iterates
> > > through every possible CPU instead of honouring the reclaim budget.
> > Did you confirm this actually does happen? If so, how often does the
> > kernel end up iterating through every possible CPUs, very rarely or
> > almost always?
> 
> Yes, in two ways.
> 
> Experiment 1 -- deterministic, instrumented loop.  Per-CPU pr_info()
> plus a test entry invoking kfree_rcu_shrink_scan() with a controlled
> budget; 4-CPU QEMU guest, 400 objects queued on CPU0, budget 50:
>   buggy:   cpu=0 count=400 nr_to_scan_after_sub=18446744073709551266
>                 visits all 4 CPUs
>   patched: cpu=0 count=400 nr_to_scan=50 BREAK
> 
>                stops at CPU0
> 
> Experiment 2 -- natural trigger, no kernel modification at all
> (kprobes only).  Setup: 1 GiB QEMU guest, 4 possible CPUs, no swap.
> 
>   1. Observation: three kprobes.  Scan entry/exit record nr_to_scan
>      and the return value; each kfree_rcu_monitor() hit inside a
>      scan is one CPU the loop processed.
> 
>   2. Memory pressure: ~650 MiB of anonymous memory.
> 
>   3. krc backlog: loaded the in-tree test_vmalloc module ,
> 
>      which repeatedly vmalloc()s one page and frees it via kvfree_rcu();
> 
>     10 million iterations over ~70s kept the per-CPU queues populated.
> 
> Result within ~70s: 262 natural invocations, all from kswapd0;
> 171 of the 261 scans that found a backlog (66%) overshot the budget
> (nr_to_scan = 128, freed up to 4479) and visited all 4 possible CPUs.

Thanks for confirming with the experiment.

> > Would this affect the kernel's reclamation behavior in some way?
> 
> The break was meant to cap the scan at the reclaim budget; once
> the subtraction wraps, the cap is gone and one scan processes
> every possible CPU, draining the whole backlog instead of a
> budget-sized slice.

The fix logically makes sense to me. But I'm being cautious to
introduce a functional change even when it was unintentional and
'accidentally' worked. Especially when it has been like this from
the beginning.

IIUC it will almost always end up visiting all CPUs
unless sum of objects exactly matches nr_to_scan at some point.

In most cases it will visit all CPUs because either 1) the counter
overflows or 2) the number of objects was smaller than the budget.

While I agree that the code doesn't work as intended, I don't think
you have enough justification to change the behavior. I'd rather fix
the code to match the current behavior rather than changing the
behavior.

-- 
Cheers,
Harry / Hyeonggon