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

[Zhang Yi]

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.