[RFC PATCH v4 3/3] ext4: fast commit: bound the tracked range count

[Daejun Park]

The rbtree grows dynamically, so a pathologically scattered inode could
accumulate an unbounded number of disjoint ranges between commits -- the
fixed-size array this replaced was inherently bounded, so this is a
regression to close.

Cap the tracked ranges at EXT4_FC_SNAPSHOT_MAX_RANGES (2048) -- the limit
the snapshot already enforces on emitted ranges -- and at the cap fall
back to a full commit (mark the transaction ineligible) instead of
growing the tree further.  A GFP_ATOMIC node allocation failure is handled
the same way, so no modification is silently dropped.

Reusing the snapshot's own limit is deliberate, not an arbitrary bound.
The commit snapshots each disjoint tracked range as at least one
ADD_RANGE, so the emitted-tag count is >= the tracked-range count.  A
tracked set larger than EXT4_FC_SNAPSHOT_MAX_RANGES can never fit under
the snapshot cap and would fall back regardless, so that count is exactly
the point past which tracking cannot produce a fast commit.  Falling back
there is the same outcome the snapshot would reach, without growing memory
past the point where a fast commit is already impossible; it also answers
"how many ranges before tracking loses to the fallback" precisely.  This
is why the cap falls back rather than collapsing the tree to a single
[min,max] span and continuing: that span snapshots to at least as many
tags and falls back too, so a collapse only adds a wasted snapshot before
the same result.  The bound is reported as NOMEM: it is a deliberate limit
on how much range state we keep per inode.

ext4_fc_mark_ineligible() takes the s_fc_lock mutex, so both fallback
sites drop and retake the i_fc_lock spinlock around it (the lock order is
s_fc_lock before i_fc_lock), as __track_dentry_update() already does.

Signed-off-by: Daejun Park <daejun7.park@xxxxxxxxxxx>
---
 fs/ext4/fast_commit.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 1a72d7f31135..752e579066b5 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -681,11 +681,8 @@ static void ext4_fc_rt_link(struct rb_root *root, struct ext4_fc_rnode *new,
  * [start,end] into one merged node, so the set stays sorted and disjoint with
  * no fixed bound.  Adjacency bounds are evaluated in 64 bits: the largest valid
  * lblk is EXT_MAX_BLOCKS - 1, so "end + 1" and a node's "lblk + len" can reach
- * EXT_MAX_BLOCKS and must not wrap ext4_lblk_t (u32).  Caller holds i_fc_lock.
- *
- * On -ENOMEM (GFP_ATOMIC under i_fc_lock) the new range cannot be recorded, so
- * the transaction falls back to a full commit, which logs everything -- no
- * modification is lost.  Rare under memory pressure.
+ * EXT_MAX_BLOCKS and must not wrap ext4_lblk_t (u32).  Caller holds i_fc_lock
+ * and the tree is live.
  */
 static void ext4_fc_range_add(handle_t *handle, struct inode *inode,
 			      ext4_lblk_t start, ext4_lblk_t end)
@@ -696,6 +693,34 @@ static void ext4_fc_range_add(handle_t *handle, struct inode *inode,
 	struct rb_node *node;
 	unsigned int merged = 0;
 
+	/*
+	 * Cap the tracked ranges at EXT4_FC_SNAPSHOT_MAX_RANGES -- the same limit
+	 * the snapshot enforces on emitted ranges.  The rbtree is dynamic, so
+	 * without a cap a pathologically scattered inode could grow it without
+	 * bound (the fixed array this replaced could not).
+	 *
+	 * That is the right limit to reuse: the commit snapshots each disjoint
+	 * tracked range as at least one ADD_RANGE, so a tracked set larger than
+	 * EXT4_FC_SNAPSHOT_MAX_RANGES can never fit under the snapshot cap and would
+	 * fall back regardless.  So at the cap we fall back to a full commit now,
+	 * the same outcome the snapshot would reach, instead of growing memory past
+	 * the point where a fast commit is already impossible.  (This is also why
+	 * we do not collapse the tree to a single [min,max] span and keep going:
+	 * that span snapshots to at least as many tags and falls back too.)
+	 * Reported as NOMEM: a deliberate limit on per-inode range state.
+	 *
+	 * ext4_fc_mark_ineligible() takes the s_fc_lock mutex, which must not be
+	 * acquired under the i_fc_lock spinlock the caller holds (and the lock
+	 * order is s_fc_lock before i_fc_lock); drop and retake i_fc_lock around
+	 * it, as __track_dentry_update() does.
+	 */
+	if (ei->i_fc_nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES) {
+		spin_unlock(&ei->i_fc_lock);
+		ext4_fc_mark_ineligible(inode->i_sb, EXT4_FC_REASON_NOMEM, handle);
+		spin_lock(&ei->i_fc_lock);
+		return;
+	}
+
 	/* Allocate first so the merge/erase below cannot fail partway. */
 	new = ext4_fc_rnode_alloc();
 	if (!new) {
@@ -703,12 +728,8 @@ static void ext4_fc_range_add(handle_t *handle, struct inode *inode,
 		 * Could not record the range under memory pressure.  A fast
 		 * commit must log every modified range or replay would restore
 		 * an incomplete inode, so fall back to a full commit rather than
-		 * silently drop it.
-		 *
-		 * ext4_fc_mark_ineligible() takes the s_fc_lock mutex, which must
-		 * not be acquired under the i_fc_lock spinlock the caller holds
-		 * (lock order is s_fc_lock before i_fc_lock); drop and retake
-		 * i_fc_lock around it, as __track_dentry_update() does.
+		 * silently drop it.  Drop/retake i_fc_lock around mark_ineligible
+		 * as above.
 		 */
 		spin_unlock(&ei->i_fc_lock);
 		ext4_fc_mark_ineligible(inode->i_sb, EXT4_FC_REASON_NOMEM, handle);
-- 
2.43.0