Re: [PATCH v2] ext4: fix shrinker scan budget accounting in ext4_es_scan()

[Zhang Yi <yizhang089@gmail.com>]

On 9/30/2026 7:48 PM, Jan Kara wrote:
> On Wed 30-09-26 09:45:26, Zhang Yi wrote:
>> On 9/29/2026 10:20 AM, Qiliang Yuan wrote:
>>> @@ -1774,15 +1777,41 @@ static unsigned long ext4_es_scan(struct shrinker *shrink,
>>>   {
>>>   	struct ext4_sb_info *sbi = shrink->private_data;
>>>   	int nr_to_scan = sc->nr_to_scan;
>>> -	int ret, nr_shrunk;
>>> +	int ret, nr_shrunk, nr_scanned;
>>>   
>>>   	ret = percpu_counter_read_positive(&sbi->s_es_stats.es_stats_shk_cnt);
>>>   	trace_ext4_es_shrink_scan_enter(sbi->s_sb, nr_to_scan, ret);
>>>   
>>> -	nr_shrunk = __es_shrink(sbi, nr_to_scan, NULL);
>>> +	nr_shrunk = __es_shrink(sbi, nr_to_scan, NULL, &nr_scanned);
>>> +	sc->nr_scanned = nr_scanned;
>>
>>
>> I'm a bit concerned that modifying sc->nr_scanned here could actually
>> end up increasing the number of scan_objects() calls in real-world
>> usage. If there are other background processes running and continuously
>> accessing certain files, they may keep producing a small number of new
>> reclaimable extent caches. That would mean nr_scanned stays at a
>> relatively small value on each iteration and never drops to 0, so
>> shrinker->scan_objects() may end up going through more loops because it
>> can't return SHRINK_STOP. So I'd tend to leave shrinkctl->nr_scanned
>> alone. What do you think?
> 
> Well, I agree but lying to upper layers about the number of objects we have
> scanned doesn't look like a proper solution? We could return SHRINK_STOP
> when we scanned everything before nr_to_walk dropped to 0. It isn't perfect
> but would somewhat address your concern...
> 
> 								Honza
> 

Yeah, that sounds good to me.

Thanks,
Yi.