From: sashiko-bot@kernel.org
To: "Qiliang Yuan" <odys.yuan@gmail.com>
Cc: linux-ext4@vger.kernel.org, tytso@mit.edu
Subject: Re: [PATCH] jbd2: fix shrinker scan budget accounting in jbd2_journal_shrink_scan()
Date: Mon, 28 Sep 2026 12:40:47 +0000 [thread overview]
Message-ID: <20260928124048.0357D1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260928-fix-jbd2-shrink-scan-nr-scanned-v1-1-e6f4016ec699@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] jbd2: CPU spike due to incorrect shrinker nr_scanned accounting
--
commit 47731fd468777a6abc87ac06fd933280e1966163
Author: Qiliang Yuan <odys.yuan@gmail.com>
jbd2: fix shrinker scan budget accounting in jbd2_journal_shrink_scan()
This commit updates the jbd2 checkpoint shrinker's scan budget accounting
by calculating the number of scanned items, and returning SHRINK_STOP if
no items were scanned.
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 09efa337649e2..2602e5946cc14 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
[ ... ]
> @@ -1262,10 +1262,26 @@ 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;
[Severity: High]
Will this correctly account for the items that were scanned but not freed?
Looking at jbd2_journal_shrink_checkpoint_list() in fs/jbd2/checkpoint.c,
nr_to_scan is only decremented by the number of freed items:
freed = journal_shrink_one_cp_list(transaction->t_checkpoint_list,
JBD2_SHRINK_BUSY_SKIP, &released);
nr_freed += freed;
(*nr_to_scan) -= min(*nr_to_scan, freed);
Furthermore, journal_shrink_one_cp_list() explicitly skips busy buffers
without them counting towards the freed total:
if (type == JBD2_SHRINK_BUSY_SKIP && jh->b_transaction)
continue;
This effectively sets sc->nr_scanned to the number of freed items, not the
actual number of scanned items.
The shrinker API expects sc->nr_scanned to reflect the actual amount of work
done (items scanned). If the shrinker scans many busy buffers but frees very
few of them, it reports a tiny number of scanned items.
Can this cause do_shrink_slab() to repeatedly re-invoke the shrinker without
meaningfully reducing its total_scan budget, leading to high CPU usage or
potential soft lockups under memory pressure?
>
> count = percpu_counter_read_positive(&journal->j_checkpoint_jh_count);
> trace_jbd2_shrink_scan_exit(journal, nr_to_scan, nr_shrunk, count);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260928-fix-jbd2-shrink-scan-nr-scanned-v1-1-e6f4016ec699@gmail.com?part=1
next prev parent reply other threads:[~2026-09-28 12:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-28 12:29 [PATCH] jbd2: fix shrinker scan budget accounting in jbd2_journal_shrink_scan() Qiliang Yuan
2026-09-28 12:40 ` sashiko-bot [this message]
2026-09-29 20:52 ` 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=20260928124048.0357D1F000FF@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.