Re: [PATCH v5] jbd2: fix shrinker scan budget accounting in jbd2_journal_shrink_scan()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] jbd2: shrinker busy-loops rescanning the same busy checkpoint items
--

commit 22efbdb58b6c52d305510bf987737313bb851b41
Author: Qiliang Yuan <odys.yuan@gmail.com>

jbd2: fix shrinker scan budget accounting in jbd2_journal_shrink_scan()

This patch updates jbd2_journal_shrink_scan() to report the actual number of
scanned buffers back to the shrinker subsystem. It sets sc->nr_scanned based
on how many buffers jbd2_journal_shrink_checkpoint_list() examined, and uses
this value to determine whether to return SHRINK_STOP.

> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 00f5a98f3d4fe..eac5f84a1afc5 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1263,10 +1263,22 @@ static unsigned long jbd2_journal_shrink_scan(struct shrinker *shrink,
>  	trace_jbd2_shrink_scan_enter(journal, sc->nr_to_scan, count);
>  
>  	nr_shrunk = jbd2_journal_shrink_checkpoint_list(journal, &nr_to_scan);
> +	sc->nr_scanned = sc->nr_to_scan - nr_to_scan;
>  
>  	count = percpu_counter_read_positive(&journal->j_checkpoint_jh_count);
>  	trace_jbd2_shrink_scan_exit(journal, nr_to_scan, nr_shrunk, count);
>  
> +	/*
> +	 * Key SHRINK_STOP off sc->nr_scanned (buffers actually examined),
> +	 * not nr_shrunk (buffers actually freed): a batch can find every
> +	 * buffer in its transactions busy and free none, while later
> +	 * transactions may still hold buffers whose writeback has
> +	 * completed. sc->nr_scanned only comes back 0 when the checkpoint
> +	 * list had nothing left to walk.
> +	 */
> +	if (sc->nr_scanned == 0)

[Severity: High]
Does this condition cause do_shrink_slab() to busy-loop when the checkpoint
list is fully exhausted but contains a few busy items?

If the shrinker budget is large (for example, due to a stale percpu counter)
but the checkpoint list has only a small number of busy items (e.g., 5),
jbd2_journal_shrink_checkpoint_list() will scan them and set sc->nr_scanned
to 5. Upon reaching the end of the list, it resets
journal->j_shrink_transaction to NULL.

Because sc->nr_scanned is 5, this fails to return SHRINK_STOP.

Will do_shrink_slab() then subtract 5 from its budget and loop immediately,
restarting from the beginning of the list and rescanning those exact same 5
busy items? If this repeats until the large budget is completely drained in
increments of 5, could it cause CPU busy-looping and j_list_lock contention
in the memory reclaim path?

> +		return SHRINK_STOP;
> +
>  	return nr_shrunk;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001-fix-jbd2-shrink-scan-nr-scanned-v5-1-5348b51efce2@gmail.com?part=1