| Message ID | 20190226120609.14450-1-fdmanana@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Btrfs: fix deadlock between clone/dedupe and rename | expand |
On Tue, Feb 26, 2019 at 12:06:09PM +0000, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > Reflinking (clone/dedupe) and rename are operations that operate on two > inodes and therefore need to lock them in the same order to avoid ABBA > deadlocks. It happens that Btrfs' reflink implementation always locked > them in a different order from VFS's lock_two_nondirectories() helper, > which is used by the rename code in VFS, resulting in ABBA type deadlocks. > > Btrfs' locking order: > > static void btrfs_double_inode_lock(struct inode *inode1, struct inode *inode2) > { > if (inode1 < inode2) > swap(inode1, inode2); > > inode_lock_nested(inode1, I_MUTEX_PARENT); > inode_lock_nested(inode2, I_MUTEX_CHILD); > } > > VFS's locking order: > > void lock_two_nondirectories(struct inode *inode1, struct inode *inode2) > { > if (inode1 > inode2) > swap(inode1, inode2); > > if (inode1 && !S_ISDIR(inode1->i_mode)) > inode_lock(inode1); > if (inode2 && !S_ISDIR(inode2->i_mode) && inode2 != inode1) > inode_lock_nested(inode2, I_MUTEX_NONDIR2); > } > > Fix this by killing the btrfs helper function that does the double inode > locking and replace it with VFS's helper lock_two_nondirectories(). > > Reported-by: Zygo Blaxell <ce3g8jdj@umail.furryterror.org> > Fixes: 416161db9b63e3 ("btrfs: offline dedupe") > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com>
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 9c8e1734429c..6e1119496721 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3206,21 +3206,6 @@ static long btrfs_ioctl_dev_info(struct btrfs_fs_info *fs_info, return ret; } -static void btrfs_double_inode_unlock(struct inode *inode1, struct inode *inode2) -{ - inode_unlock(inode1); - inode_unlock(inode2); -} - -static void btrfs_double_inode_lock(struct inode *inode1, struct inode *inode2) -{ - if (inode1 < inode2) - swap(inode1, inode2); - - inode_lock_nested(inode1, I_MUTEX_PARENT); - inode_lock_nested(inode2, I_MUTEX_CHILD); -} - static void btrfs_double_extent_unlock(struct inode *inode1, u64 loff1, struct inode *inode2, u64 loff2, u64 len) { @@ -3989,7 +3974,7 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in, if (same_inode) inode_lock(inode_in); else - btrfs_double_inode_lock(inode_in, inode_out); + lock_two_nondirectories(inode_in, inode_out); /* * Now that the inodes are locked, we need to start writeback ourselves @@ -4039,7 +4024,7 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in, if (same_inode) inode_unlock(inode_in); else - btrfs_double_inode_unlock(inode_in, inode_out); + unlock_two_nondirectories(inode_in, inode_out); return ret; } @@ -4069,7 +4054,7 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off, if (same_inode) inode_unlock(src_inode); else - btrfs_double_inode_unlock(src_inode, dst_inode); + unlock_two_nondirectories(src_inode, dst_inode); return ret < 0 ? ret : len; }