btrfs: reserve correct number of items for rename
authorOmar Sandoval <osandov@fb.com>
Thu, 10 Mar 2022 01:31:32 +0000 (17:31 -0800)
committerDavid Sterba <dsterba@suse.com>
Mon, 16 May 2022 15:03:06 +0000 (17:03 +0200)
btrfs_rename() and btrfs_rename_exchange() don't account for enough
items. Replace the incorrect explanations with a specific breakdown of
the number of items and account them accurately.

Note that this glosses over RENAME_WHITEOUT because the next commit is
going to rework that, too.

Reviewed-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Signed-off-by: Omar Sandoval <osandov@fb.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/inode.c

index 388259204aae91aa37975ffaf0374d9eee37df5d..c320995c62dc3a22a8bd6302f9ed0d76d304c1f9 100644 (file)
@@ -9051,6 +9051,7 @@ static int btrfs_rename_exchange(struct inode *old_dir,
 {
        struct btrfs_fs_info *fs_info = btrfs_sb(old_dir->i_sb);
        struct btrfs_trans_handle *trans;
+       unsigned int trans_num_items;
        struct btrfs_root *root = BTRFS_I(old_dir)->root;
        struct btrfs_root *dest = BTRFS_I(new_dir)->root;
        struct inode *new_inode = new_dentry->d_inode;
@@ -9082,14 +9083,37 @@ static int btrfs_rename_exchange(struct inode *old_dir,
                down_read(&fs_info->subvol_sem);
 
        /*
-        * We want to reserve the absolute worst case amount of items.  So if
-        * both inodes are subvols and we need to unlink them then that would
-        * require 4 item modifications, but if they are both normal inodes it
-        * would require 5 item modifications, so we'll assume their normal
-        * inodes.  So 5 * 2 is 10, plus 2 for the new links, so 12 total items
-        * should cover the worst case number of items we'll modify.
+        * For each inode:
+        * 1 to remove old dir item
+        * 1 to remove old dir index
+        * 1 to add new dir item
+        * 1 to add new dir index
+        * 1 to update parent inode
+        *
+        * If the parents are the same, we only need to account for one
         */
-       trans = btrfs_start_transaction(root, 12);
+       trans_num_items = (old_dir == new_dir ? 9 : 10);
+       if (old_ino == BTRFS_FIRST_FREE_OBJECTID) {
+               /*
+                * 1 to remove old root ref
+                * 1 to remove old root backref
+                * 1 to add new root ref
+                * 1 to add new root backref
+                */
+               trans_num_items += 4;
+       } else {
+               /*
+                * 1 to update inode item
+                * 1 to remove old inode ref
+                * 1 to add new inode ref
+                */
+               trans_num_items += 3;
+       }
+       if (new_ino == BTRFS_FIRST_FREE_OBJECTID)
+               trans_num_items += 4;
+       else
+               trans_num_items += 3;
+       trans = btrfs_start_transaction(root, trans_num_items);
        if (IS_ERR(trans)) {
                ret = PTR_ERR(trans);
                goto out_notrans;
@@ -9368,21 +9392,45 @@ static int btrfs_rename(struct user_namespace *mnt_userns,
        if (new_inode && S_ISREG(old_inode->i_mode) && new_inode->i_size)
                filemap_flush(old_inode->i_mapping);
 
-       /* close the racy window with snapshot create/destroy ioctl */
-       if (old_ino == BTRFS_FIRST_FREE_OBJECTID)
+       if (old_ino == BTRFS_FIRST_FREE_OBJECTID) {
+               /* Close the race window with snapshot create/destroy ioctl */
                down_read(&fs_info->subvol_sem);
+               /*
+                * 1 to remove old root ref
+                * 1 to remove old root backref
+                * 1 to add new root ref
+                * 1 to add new root backref
+                */
+               trans_num_items = 4;
+       } else {
+               /*
+                * 1 to update inode
+                * 1 to remove old inode ref
+                * 1 to add new inode ref
+                */
+               trans_num_items = 3;
+       }
        /*
-        * We want to reserve the absolute worst case amount of items.  So if
-        * both inodes are subvols and we need to unlink them then that would
-        * require 4 item modifications, but if they are both normal inodes it
-        * would require 5 item modifications, so we'll assume they are normal
-        * inodes.  So 5 * 2 is 10, plus 1 for the new link, so 11 total items
-        * should cover the worst case number of items we'll modify.
-        * If our rename has the whiteout flag, we need more 5 units for the
-        * new inode (1 inode item, 1 inode ref, 2 dir items and 1 xattr item
-        * when selinux is enabled).
-        */
-       trans_num_items = 11;
+        * 1 to remove old dir item
+        * 1 to remove old dir index
+        * 1 to update old parent inode
+        * 1 to add new dir item
+        * 1 to add new dir index
+        * 1 to update new parent inode (if it's not the same as the old parent)
+        */
+       trans_num_items += 6;
+       if (new_dir != old_dir)
+               trans_num_items++;
+       if (new_inode) {
+               /*
+                * 1 to update inode
+                * 1 to remove inode ref
+                * 1 to remove dir item
+                * 1 to remove dir index
+                * 1 to possibly add orphan item
+                */
+               trans_num_items += 5;
+       }
        if (flags & RENAME_WHITEOUT)
                trans_num_items += 5;
        trans = btrfs_start_transaction(root, trans_num_items);