]> git.hungrycats.org Git - linux/commitdiff
btrfs: resume the delayed-ref merge walk instead of restarting it 6.18/topics/delayed-ref-fixes
authorZygo Blaxell <ce3g8jdj@umail.furryterror.org>
Fri, 18 Sep 2026 18:40:54 +0000 (14:40 -0400)
committerZygo Blaxell <ce3g8jdj@umail.furryterror.org>
Sat, 19 Sep 2026 00:18:17 +0000 (20:18 -0400)
btrfs_merge_delayed_refs() restarts its walk of a head's ref tree from
rb_first_cached() every time merge_ref() reports a merge:

again:
for (node = rb_first_cached(&head->ref_tree); node;
     node = rb_next(node)) {
ref = rb_entry(node, ...);
if (seq && ref->seq >= seq)
continue;
if (merge_ref(fs_info, delayed_refs, head, ref, seq))
goto again;
}

The restart is there because merge_ref() returns true only when it has
freed the caller's cursor node, so the caller cannot call rb_next() on it.
That makes the walk quadratic in the number of refs on the head, and it
runs with head->lock held -- a spinlock -- so it cannot be broken up.

On a filesystem doing continuous backref resolution this is not
theoretical. Each backref walk advances fs_info->tree_mod_seq, and
init_delayed_ref_common() stamps that seq into every new fs-tree ref, so
refs to the same block and root stop comparing equal and stop being merged
at insert time by insert_delayed_ref().  They accumulate as separate nodes
in one contiguous group instead, and the merge walk restarts across the
whole group on every cancelling pair.

Measured with the ftrace function profiler on a filesystem running a
dedupe agent and a verifier that both walk backrefs continuously, during a
stall, over a 60 second window:

  btrfs_merge_delayed_refs()         115,166 calls
  comp_refs()                  1,171,273,171 calls

comp_refs() is called once per node the merge walk compares, so that is
10,170 comparisons per walk.  Solving the restart structure's N^2/2 puts
roughly 143 refs on a head.  comp_refs() is a leaf, so its own total is
not distorted by nesting: 38.9 CPU-seconds of that 60 second window went
to comparing delayed refs and nothing else, with the transaction thread
pinned at 100% of a core and zero tasks anywhere in the machine waiting on
IO.

The cost is not subtle.  Every other task queued behind the commit;
relocation of a single block group made no progress for three and a half
hours; snapshot deletion, which had been clearing about 49 subvolumes a
minute, stopped entirely.  An earlier instance on the same machine
produced a 26 second soft lockup with the stack inside the merge walk.

merge_ref() does not actually need the caller to restart.  It advances its
own cursor past a node before freeing that node:

next = rb_entry(node, ...);
node = rb_next(node);
...
drop_delayed_ref(fs_info, delayed_refs, head, next);

so it always holds a position that survives the frees, and there are only
three ways it can end having freed the caller's node:

 - it swapped, and the survivor keeps a non-zero ref_mod.  The caller's node is
   freed; the survivor is still in the tree and may still merge with what
   follows, so the walk resumes at the survivor.

 - the caller's ref_mod reached zero, with or without a swap.  Both nodes are
   gone, and the local cursor -- already advanced past the freed one -- is the
   resume point, or NULL to end the walk.

Return whether @ref was freed, and hand the position back through an out
parameter.  The two cannot be folded into one pointer: the resume position
is legitimately NULL when the freed node was last in the tree, and a NULL
return would then tell the caller its own node is still live. The walk becomes strictly forward and linear, and nothing is left
unmerged.

Skipping the re-examination that the restart performed does not lose
merges. comp_refs() keys on type and root-or-parent, neither of which any
merge changes, so merging cannot make two previously incomparable refs
comparable. Mergeable refs are contiguous, because the tree is sorted by
comp_refs(..., check_seq = true) and the merge key is the same comparison
without the seq -- a prefix of the sort key.  And a ref skipped by the seq
test stays skipped, since seq is read once per btrfs_merge_delayed_refs()
call.

The restart has had this shape since commit 0e0adbcfdc90 ("btrfs: track
refs in a rb_tree instead of a list") in v4.15.

Assisted-by: Claude:claude-opus-5[1m]
fs/btrfs/delayed-ref.c

index f8fc26272f76c5b098e2c53f9587bb32b8f2a87d..bc5bdb688f86de7e7394b433d4a165a3d9b85de9 100644 (file)
@@ -381,16 +381,32 @@ static inline void drop_delayed_ref(struct btrfs_fs_info *fs_info,
        btrfs_delayed_refs_rsv_release(fs_info, 1, 0);
 }
 
+/*
+ * Merge @ref with the refs that follow it in the head's tree.
+ *
+ * Returns false if @ref itself is still in the tree, so the caller can simply
+ * step to the next node.  Returns true if @ref had to be freed, and then
+ * @resume holds a position still valid to continue from, or NULL if the walk
+ * is finished -- which is what lets the caller avoid restarting the walk from
+ * rb_first_cached() every time a merge happens.
+ *
+ * The freed/not-freed answer cannot be folded into the resume pointer.  The
+ * resume position is legitimately NULL when the freed node was last in the
+ * tree, and a NULL return would then tell the caller its own node is still
+ * live.  That is a use-after-free; KASAN found it in rb_next().
+ */
 static bool merge_ref(struct btrfs_fs_info *fs_info,
                      struct btrfs_delayed_ref_root *delayed_refs,
                      struct btrfs_delayed_ref_head *head,
                      struct btrfs_delayed_ref_node *ref,
-                     u64 seq)
+                     u64 seq, struct rb_node **resume)
 {
        struct btrfs_delayed_ref_node *next;
        struct rb_node *node = rb_next(&ref->ref_node);
        bool done = false;
 
+       *resume = NULL;
+
        while (!done && node) {
                int mod;
 
@@ -406,6 +422,13 @@ static bool merge_ref(struct btrfs_fs_info *fs_info,
                } else {
                        if (ref->ref_mod < next->ref_mod) {
                                swap(ref, next);
+                               /*
+                                * The caller's node is @next now, and it is
+                                * about to be freed.  @ref survives and may
+                                * still merge with what follows it, so that is
+                                * where the walk has to resume.
+                                */
+                               *resume = &ref->ref_node;
                                done = true;
                        }
                        mod = -next->ref_mod;
@@ -415,6 +438,13 @@ static bool merge_ref(struct btrfs_fs_info *fs_info,
                ref->ref_mod += mod;
                if (ref->ref_mod == 0) {
                        drop_delayed_ref(fs_info, delayed_refs, head, ref);
+                       /*
+                        * Both @ref and @next are gone now.  @node was advanced
+                        * past @next before @next was freed, so it is still a
+                        * valid position to resume from (or NULL, which simply
+                        * ends the walk).
+                        */
+                       *resume = node;
                        done = true;
                } else {
                        /*
@@ -446,14 +476,19 @@ void btrfs_merge_delayed_refs(struct btrfs_fs_info *fs_info,
                return;
 
        seq = btrfs_tree_mod_log_lowest_seq(fs_info);
-again:
-       for (node = rb_first_cached(&head->ref_tree); node;
-            node = rb_next(node)) {
+       node = rb_first_cached(&head->ref_tree);
+       while (node) {
+               struct rb_node *resume;
+
                ref = rb_entry(node, struct btrfs_delayed_ref_node, ref_node);
-               if (seq && ref->seq >= seq)
+               if (seq && ref->seq >= seq) {
+                       node = rb_next(node);
                        continue;
-               if (merge_ref(fs_info, delayed_refs, head, ref, seq))
-                       goto again;
+               }
+               if (merge_ref(fs_info, delayed_refs, head, ref, seq, &resume))
+                       node = resume;
+               else
+                       node = rb_next(node);
        }
 }