btrfs: send: fix duplicated rmdir operations when using extrefs

Commit 29d6d30f5c8a ("Btrfs: send, don't send rmdir for same target
multiple times") has fixed an issue that a send stream contained a rmdir
operation for the same directory multiple times. After that fix we keep
track of the last directory for which we sent a rmdir operation and
compare with it before sending a rmdir for the parent inode of a deleted
hardlink we are processing. But there is still a corner case that in
between rmdir dir operations for the same inode we find deleted hardlinks
for other parent inodes, so tracking just the last inode for which we sent
a rmdir operation is not enough.

Hardlinks of a file in the same directory are stored in the same INODE_REF
item, but if the number of hardlinks is too large and can not fit in a
leaf, we use INODE_EXTREF items to store them. The key of an INODE_EXTREF
item is (inode_id, INODE_EXTREF, hash[name, parent ino]), so between two
hardlinks for the same parent directory, we can find others for other
parent directories. For example for the reproducer below we get the
following (from a btrfs inspect-internal dump-tree output):

    item 0 key (259 INODE_EXTREF 2309449) itemoff 16257 itemsize 26
            index 6925 parent 257 namelen 8 name: foo.6923
    item 1 key (259 INODE_EXTREF 2311350) itemoff 16231 itemsize 26
            index 6588 parent 258 namelen 8 name: foo.6587
    item 2 key (259 INODE_EXTREF 2457395) itemoff 16205 itemsize 26
            index 6611 parent 257 namelen 8 name: foo.6609
    (...)

So tracking the last directory's inode number does not work in this case
since we process a link for parent inode 257, then for 258 and then back
again for 257, and that second time we process a deleted link for 257 we
think we have not yet sent a rmdir operation.

Fix this by using a rbtree to keep track of all the directories for which
we have already sent rmdir operations, and add those directories to the
'check_dirs' ref list in process_recorded_refs() only if the directory is
not yet in the rbtree, otherwise skip it since it means we have already
sent a rmdir operation for that directory.

The following test script reproduces the problem:

  $ cat test.sh
  #!/bin/bash

  DEV=/dev/sdi
  MNT=/mnt/sdi

  mkfs.btrfs -f $DEV
  mount $DEV $MNT

  mkdir $MNT/a $MNT/b

  echo 123 > $MNT/a/foo
  for ((i = 1; i <= 1000; i++)); do
     ln $MNT/a/foo $MNT/a/foo.$i
     ln $MNT/a/foo $MNT/b/foo.$i
  done

  btrfs subvolume snapshot -r $MNT $MNT/snap1
  btrfs send $MNT/snap1 -f /tmp/base.send

  rm -r $MNT/a $MNT/b

  btrfs subvolume snapshot -r $MNT $MNT/snap2
  btrfs send -p $MNT/snap1 $MNT/snap2 -f /tmp/incremental.send

  umount $MNT
  mkfs.btrfs -f $DEV
  mount $DEV $MNT

  btrfs receive $MNT -f /tmp/base.send
  btrfs receive $MNT -f /tmp/incremental.send

  rm -f /tmp/base.send /tmp/incremental.send

  umount $MNT

When running it, it fails like this:

  $ ./test.sh
  (...)
  At subvol snap1
  At snapshot snap2
  ERROR: rmdir o257-9-0 failed: No such file or directory

CC: <stable@vger.kernel.org>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Ting-Chang Hou <tchou@synology.com>
[ Updated changelog ]
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 6144e66661f583abefc105ae74823485855cd2ed..96a030d28e0910cd66a43f2ddc81a783eae4c972 100644 (file)
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -4102,6 +4102,48 @@ out:
        return ret;
 }
 
+static int rbtree_check_dir_ref_comp(const void *k, const struct rb_node *node)
+{
+       const struct recorded_ref *data = k;
+       const struct recorded_ref *ref = rb_entry(node, struct recorded_ref, node);
+
+       if (data->dir > ref->dir)
+               return 1;
+       if (data->dir < ref->dir)
+               return -1;
+       if (data->dir_gen > ref->dir_gen)
+               return 1;
+       if (data->dir_gen < ref->dir_gen)
+               return -1;
+       return 0;
+}
+
+static bool rbtree_check_dir_ref_less(struct rb_node *node, const struct rb_node *parent)
+{
+       const struct recorded_ref *entry = rb_entry(node, struct recorded_ref, node);
+
+       return rbtree_check_dir_ref_comp(entry, parent) < 0;
+}
+
+static int record_check_dir_ref_in_tree(struct rb_root *root,
+                       struct recorded_ref *ref, struct list_head *list)
+{
+       struct recorded_ref *tmp_ref;
+       int ret;
+
+       if (rb_find(ref, root, rbtree_check_dir_ref_comp))
+               return 0;
+
+       ret = dup_ref(ref, list);
+       if (ret < 0)
+               return ret;
+
+       tmp_ref = list_last_entry(list, struct recorded_ref, list);
+       rb_add(&tmp_ref->node, root, rbtree_check_dir_ref_less);
+       tmp_ref->root = root;
+       return 0;
+}
+
 static int rename_current_inode(struct send_ctx *sctx,
                                struct fs_path *current_path,
                                struct fs_path *new_path)
@@ -4129,11 +4171,11 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
        struct recorded_ref *cur;
        struct recorded_ref *cur2;
        LIST_HEAD(check_dirs);
+       struct rb_root rbtree_check_dirs = RB_ROOT;
        struct fs_path *valid_path = NULL;
        u64 ow_inode = 0;
        u64 ow_gen;
        u64 ow_mode;
-       u64 last_dir_ino_rm = 0;
        bool did_overwrite = false;
        bool is_orphan = false;
        bool can_rename = true;
@@ -4437,7 +4479,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
                                        goto out;
                        }
                }
-               ret = dup_ref(cur, &check_dirs);
+               ret = record_check_dir_ref_in_tree(&rbtree_check_dirs, cur, &check_dirs);
                if (ret < 0)
                        goto out;
        }
@@ -4465,7 +4507,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
                }
 
                list_for_each_entry(cur, &sctx->deleted_refs, list) {
-                       ret = dup_ref(cur, &check_dirs);
+                       ret = record_check_dir_ref_in_tree(&rbtree_check_dirs, cur, &check_dirs);
                        if (ret < 0)
                                goto out;
                }
@@ -4475,7 +4517,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
                 * We have a moved dir. Add the old parent to check_dirs
                 */
                cur = list_first_entry(&sctx->deleted_refs, struct recorded_ref, list);
-               ret = dup_ref(cur, &check_dirs);
+               ret = record_check_dir_ref_in_tree(&rbtree_check_dirs, cur, &check_dirs);
                if (ret < 0)
                        goto out;
        } else if (!S_ISDIR(sctx->cur_inode_mode)) {
@@ -4509,7 +4551,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
                                if (is_current_inode_path(sctx, cur->full_path))
                                        fs_path_reset(&sctx->cur_inode_path);
                        }
-                       ret = dup_ref(cur, &check_dirs);
+                       ret = record_check_dir_ref_in_tree(&rbtree_check_dirs, cur, &check_dirs);
                        if (ret < 0)
                                goto out;
                }
@@ -4552,8 +4594,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
                        ret = cache_dir_utimes(sctx, cur->dir, cur->dir_gen);
                        if (ret < 0)
                                goto out;
-               } else if (ret == inode_state_did_delete &&
-                          cur->dir != last_dir_ino_rm) {
+               } else if (ret == inode_state_did_delete) {
                        ret = can_rmdir(sctx, cur->dir, cur->dir_gen);
                        if (ret < 0)
                                goto out;
@@ -4565,7 +4606,6 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
                                ret = send_rmdir(sctx, valid_path);
                                if (ret < 0)
                                        goto out;
-                               last_dir_ino_rm = cur->dir;
                        }
                }
        }