From: Omar Sandoval Date: Thu, 10 Mar 2022 01:31:32 +0000 (-0800) Subject: btrfs: reserve correct number of items for rename X-Git-Url: http://git.maquefel.me/?a=commitdiff_plain;h=c16218714307849a949f83cbad00b1b4ec166bb6;p=linux.git btrfs: reserve correct number of items for rename 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 Signed-off-by: Omar Sandoval Reviewed-by: David Sterba Signed-off-by: David Sterba --- diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 388259204aae9..c320995c62dc3 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -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);