All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Qiliang Yuan" <odys.yuan@gmail.com>
Cc: linux-ext4@vger.kernel.org, tytso@mit.edu
Subject: Re: [PATCH v4] jbd2: fix shrinker scan budget accounting in jbd2_journal_shrink_scan()
Date: Wed, 30 Sep 2026 07:27:19 +0000	[thread overview]
Message-ID: <20260930072719.7BD131F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260930-fix-jbd2-shrink-scan-nr-scanned-v4-1-eb9a3ccc2524@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] fs/jbd2: premature reclaim abort in jbd2_journal_shrink_scan()
--

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

jbd2: fix shrinker scan budget accounting in jbd2_journal_shrink_scan()

This commit modifies jbd2_journal_shrink_scan() to return SHRINK_STOP
when jbd2_journal_shrink_checkpoint_list() frees zero items. The goal
is to prevent the shrinker from continually scanning a stale freeable count
when the checkpoint list only contains busy buffers.

> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 00f5a98f3d4fe..c921afce7d061 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1267,6 +1267,16 @@ static unsigned long jbd2_journal_shrink_scan(struct shrinker *shrink,
>  	count = percpu_counter_read_positive(&journal->j_checkpoint_jh_count);
>  	trace_jbd2_shrink_scan_exit(journal, nr_to_scan, nr_shrunk, count);
>  
> +	/*
> +	 * A checkpoint list full of busy buffers can keep reporting a
> +	 * stale, positive freeable count after nothing more can be
> +	 * reclaimed. Stop this reclaim pass once a call frees nothing,
> +	 * instead of burning through do_shrink_slab()'s scan budget
> +	 * retrying against buffers whose writeback won't finish any sooner.
> +	 */
> +	if (nr_shrunk == 0)
> +		return SHRINK_STOP;

[Severity: High]
Could this unintentionally abort the entire reclaim pass when the code hits
a chunk of busy buffers or gets interrupted by the scheduler?

If do_shrink_slab() provides a scan budget and the first batch of buffers
happen to be busy writeback buffers, jbd2_journal_shrink_checkpoint_list()
will return 0. This change then translates that 0 into SHRINK_STOP, which
aborts the rest of the scanning budget for this reclaim pass.

Additionally, jbd2_journal_shrink_checkpoint_list() can return 0 early due
to scheduling or lock contention:

fs/jbd2/checkpoint.c:jbd2_journal_shrink_checkpoint_list()
    ...
		if (need_resched() || spin_needbreak(&journal->j_list_lock))
			break;
    ...

Will returning SHRINK_STOP in these scenarios prematurely skip remaining
transactions that might contain clean, freeable buffers?

> +
>  	return nr_shrunk;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-fix-jbd2-shrink-scan-nr-scanned-v4-1-eb9a3ccc2524@gmail.com?part=1

  reply	other threads:[~2026-09-30  7:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30  7:11 [PATCH v4] jbd2: fix shrinker scan budget accounting in jbd2_journal_shrink_scan() Qiliang Yuan
2026-09-30  7:27 ` sashiko-bot [this message]
2026-09-30 17:46 ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260930072719.7BD131F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=odys.yuan@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.