Re: [PATCH v3 1/2] ext4: fix shrinker scan budget accounting in ext4_es_scan()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ext4: premature SHRINK_STOP in ext4_es_scan() breaks LRU reclaim
--

commit 38269e532034036f670a12b2f9dedde4248280c0
Author: Qiliang Yuan <odys.yuan@gmail.com>

ext4: fix shrinker scan budget accounting in ext4_es_scan()

The extents_status shrinker's count_objects() callback reports the number of
shrinkable extent_status objects via a percpu counter. This patch returns
SHRINK_STOP when __es_shrink() frees nothing to prevent burning through the
scan budget against a stale freeable count.

> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 6e4a191e82191..d88af807d36e7 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -1783,6 +1783,16 @@ static unsigned long ext4_es_scan(struct shrinker *shrink,
>  
>  	ret = percpu_counter_read_positive(&sbi->s_es_stats.es_stats_shk_cnt);
>  	trace_ext4_es_shrink_scan_exit(sbi->s_sb, nr_shrunk, ret);
> +
> +	/*
> +	 * es_stats_shk_cnt is a percpu counter, so it can stay positive
> +	 * after sbi->s_es_list has nothing left to give. Stop this reclaim
> +	 * pass once a call frees nothing, instead of burning through
> +	 * do_shrink_slab()'s scan budget against a stale count.
> +	 */
> +	if (nr_shrunk == 0)
> +		return SHRINK_STOP;

[Severity: High]
Does this prematurely abort the shrinker when scanning referenced items?

If __es_shrink() processes a full batch (e.g., 128 items) but all of them are
referenced, it clears their referenced bit (aging them) but frees none,
resulting in nr_shrunk == 0.

For example, in es_do_reclaim_extents():

	(*nr_to_scan)--;
	node = rb_next(&es->rb_node);

	if (ext4_es_must_keep(es))
		goto next;
	if (ext4_es_is_referenced(es)) {
		ext4_es_clear_referenced(es);
		goto next;
	}

Because scanning referenced or unfreeable extents correctly consumes the scan
budget without incrementing nr_shrunk, won't this conflate "scanned a batch of
hot items" with "the cache is empty", breaking the second-chance LRU algorithm
under memory pressure?

>  	return nr_shrunk;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-fix-ext4-es-scan-nr-scanned-v3-0-9aef0be87d21@gmail.com?part=1