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

[Qiliang Yuan]

jbd2_journal_shrink_scan() never updates sc->nr_scanned, so
do_shrink_slab() always assumes a full batch was examined and keeps
calling scan_objects() until the one-shot budget derived from the
(possibly stale) percpu checkpoint count is drained, even when the
checkpoint list has nothing left to offer.

journal_shrink_one_cp_list() also conflates buffers examined with
buffers freed: it walks every journal_head in a transaction's list,
including ones skipped as busy, but only counts the ones it frees.

Add an nr_scanned output parameter to journal_shrink_one_cp_list()
that counts every journal_head examined, and use it to decrement the
scan budget in jbd2_journal_shrink_checkpoint_list() instead of the
freed count. Derive sc->nr_scanned from that examined count in
jbd2_journal_shrink_scan().

Trigger SHRINK_STOP on nr_shrunk == 0 (nothing freed) rather than
sc->nr_scanned == 0 (nothing examined): a checkpoint list full of
busy buffers can honestly report a full batch scanned while freeing
nothing, and retrying immediately will not speed up in-flight
writeback.

Tested by creating 20000 small files without an explicit sync and
dropping caches, tracing jbd2_shrink_*:

              scan_objects() calls   calls with nr_shrunk == 0
  before             168                    168 (100%)
  after                7                      6 (86%)

Signed-off-by: Qiliang Yuan <odys.yuan@xxxxxxxxx>
---
V1 -> V2:
- Count examined buffers, not just freed ones, in
  journal_shrink_one_cp_list() (Sashiko AI review finding).
- Trigger SHRINK_STOP on nr_shrunk == 0 instead of
  sc->nr_scanned == 0.

v1: https://lore.kernel.org/r/20260928-fix-jbd2-shrink-scan-nr-scanned-v1-1-e6f4016ec699@xxxxxxxxx
---
 fs/jbd2/checkpoint.c | 23 +++++++++++++++++------
 fs/jbd2/journal.c    | 17 +++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 1508e2f544621..3569e32500b7c 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -361,27 +361,35 @@ int jbd2_cleanup_journal_tail(journal_t *journal)
  * Find all the written-back checkpoint buffers in the given list
  * and try to release them. If the whole transaction is released, set
  * the 'released' parameter. Return the number of released checkpointed
- * buffers.
+ * buffers. Store the number of buffers this function actually looked
+ * at, whether or not they were released, in '*nr_scanned' (a busy
+ * buffer skipped via JBD2_SHRINK_BUSY_SKIP still counts here, since
+ * walking it is real work, unlike the freed count).
  *
  * Called with j_list_lock held.
  */
 static unsigned long journal_shrink_one_cp_list(struct journal_head *jh,
 						enum jbd2_shrink_type type,
+						unsigned long *nr_scanned,
 						bool *released)
 {
 	struct journal_head *last_jh;
 	struct journal_head *next_jh = jh;
 	unsigned long nr_freed = 0;
+	unsigned long nr_examined = 0;
 	int ret;
 
 	*released = false;
-	if (!jh)
+	if (!jh) {
+		*nr_scanned = 0;
 		return 0;
+	}
 
 	last_jh = jh->b_cpprev;
 	do {
 		jh = next_jh;
 		next_jh = jh->b_cpnext;
+		nr_examined++;
 
 		if (type == JBD2_SHRINK_DESTROY) {
 			ret = __jbd2_journal_remove_checkpoint(jh);
@@ -404,6 +412,7 @@ static unsigned long journal_shrink_one_cp_list(struct journal_head *jh,
 			break;
 	} while (jh != last_jh);
 
+	*nr_scanned = nr_examined;
 	return nr_freed;
 }
 
@@ -424,7 +433,7 @@ unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal,
 	tid_t first_tid = 0, last_tid = 0, next_tid = 0;
 	tid_t tid = 0;
 	unsigned long nr_freed = 0;
-	unsigned long freed;
+	unsigned long freed, scanned;
 	bool first_set = false;
 
 again:
@@ -458,9 +467,10 @@ unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal,
 		tid = transaction->t_tid;
 
 		freed = journal_shrink_one_cp_list(transaction->t_checkpoint_list,
-						   JBD2_SHRINK_BUSY_SKIP, &released);
+						   JBD2_SHRINK_BUSY_SKIP, &scanned,
+						   &released);
 		nr_freed += freed;
-		(*nr_to_scan) -= min(*nr_to_scan, freed);
+		(*nr_to_scan) -= min(*nr_to_scan, scanned);
 		if (*nr_to_scan == 0)
 			break;
 		if (need_resched() || spin_needbreak(&journal->j_list_lock))
@@ -502,6 +512,7 @@ void __jbd2_journal_clean_checkpoint_list(journal_t *journal,
 					  enum jbd2_shrink_type type)
 {
 	transaction_t *transaction, *last_transaction, *next_transaction;
+	unsigned long __always_unused scanned;
 	bool released;
 
 	WARN_ON_ONCE(type == JBD2_SHRINK_BUSY_SKIP);
@@ -516,7 +527,7 @@ void __jbd2_journal_clean_checkpoint_list(journal_t *journal,
 		transaction = next_transaction;
 		next_transaction = transaction->t_cpnext;
 		journal_shrink_one_cp_list(transaction->t_checkpoint_list,
-					   type, &released);
+					   type, &scanned, &released);
 		/*
 		 * This function only frees up some memory if possible so we
 		 * dont have an obligation to finish processing. Bail out if
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 09efa337649e2..e35d488cbfd23 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1262,10 +1262,27 @@ 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);
 
+	/*
+	 * Give up on this reclaim pass if this call didn't manage to free
+	 * anything. This is deliberately based on nr_shrunk, not on
+	 * sc->nr_scanned: a checkpoint list can be full of buffers that are
+	 * still busy being written back, in which case a call can
+	 * legitimately scan (and correctly report through sc->nr_scanned)
+	 * a full batch of them without freeing a single one. Retrying
+	 * immediately within the same synchronous reclaim pass is not going
+	 * to let any of that in-flight writeback complete any sooner, so
+	 * there is nothing to gain from letting do_shrink_slab() keep
+	 * calling us against the same stale freeable count until its
+	 * one-shot scan budget for this priority level is exhausted.
+	 */
+	if (nr_shrunk == 0)
+		return SHRINK_STOP;
+
 	return nr_shrunk;
 }
 

---
base-commit: 502d801f0ab03e4f32f9a33d203154ce84887921
change-id: 20260928-fix-jbd2-shrink-scan-nr-scanned-c3098cd901e2

Best regards,
-- 
Qiliang Yuan <odys.yuan@xxxxxxxxx>