mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
btrfs: downgrade the reflink source inode lock for tree walking
When doing a reflink, we are not writing to the src inode or modifying it in any way, so it is counter-intuitive to have to lock it exclusively. This has the cost of blocking other shared lock users of the src file for the duration of any new writeback since the optimistic pre-flush and all the actual reflinking. So we would like to downgrade the lock to shared as much as possible. There are two important gotchas to do with direct io that must be addressed to make this idea work. The first gotcha is that dio writes that come in under EOF try to take inode->i_rwsem shared, which would result in messed up views of the extents in the reflinked file. Therefore, add "being used as a reflink src" as another inode flag exception for the dio write shared tests that already exist to check for other dangerous conditions. In order to ensure that this flag is visible to the dio writer, reflink must first take i_rwsem exclusive, then set the flag, then downgrade to shared. The second is that dio reads increment inode->i_dio_count which blocks inode_dio_wait() in __generic_remap_file_range_prep(). Therefore, we must remain exclusive to dio reads through finishing the prep or risk starving the clone on a stream of dio readers. There is still O(extents) work in btrfs_clone() which can be done without holding the lock exclusive, so we should downgrade to shared and admit dio readers (and other shared lock users like lseek) after we are finished with inode_dio_wait(). By running concurrent clones and lseeks or dio reads on a src file with 32k extents on a vm, I can demonstrate 100ms+ stalls on the readers without this patch and they go away entirely with the patch. Reviewed-by: Filipe Manana <fdmanana@suse.com> Signed-off-by: Boris Burkov <boris@bur.io> Signed-off-by: David Sterba <dsterba@suse.com>
This commit is contained in:
parent
e0c9d01770
commit
cf02c310d0
|
|
@ -99,6 +99,12 @@ enum {
|
|||
* range).
|
||||
*/
|
||||
BTRFS_INODE_COW_WRITE_ERROR,
|
||||
/*
|
||||
* Set by reflink while it holds the source's VFS inode lock shared.
|
||||
* Direct IO writes also take that lock shared; one that finds this bit
|
||||
* set after acquiring it must retry with the exclusive lock.
|
||||
*/
|
||||
BTRFS_INODE_REFLINK_SRC,
|
||||
/*
|
||||
* Indicate this is a directory that points to a subvolume for which
|
||||
* there is no root reference item. That's a case like the following:
|
||||
|
|
|
|||
|
|
@ -913,6 +913,13 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
|
|||
goto relock;
|
||||
}
|
||||
|
||||
if ((ilock_flags & BTRFS_ILOCK_SHARED) &&
|
||||
test_bit(BTRFS_INODE_REFLINK_SRC, &BTRFS_I(inode)->runtime_flags)) {
|
||||
btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
|
||||
ilock_flags &= ~BTRFS_ILOCK_SHARED;
|
||||
goto relock;
|
||||
}
|
||||
|
||||
ret = generic_write_checks(iocb, from);
|
||||
if (ret <= 0) {
|
||||
btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
|
||||
|
|
|
|||
|
|
@ -939,6 +939,7 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
|
|||
struct btrfs_inode *dst_inode = BTRFS_I(file_inode(dst_file));
|
||||
bool same_inode = dst_inode == src_inode;
|
||||
u64 wb_start, wb_len;
|
||||
bool src_downgraded = false;
|
||||
int ret;
|
||||
|
||||
if (btrfs_is_shutdown(src_inode->root->fs_info))
|
||||
|
|
@ -976,6 +977,12 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
|
|||
if (ret < 0 || len == 0)
|
||||
goto out_unlock;
|
||||
|
||||
if (!same_inode) {
|
||||
set_bit(BTRFS_INODE_REFLINK_SRC, &src_inode->runtime_flags);
|
||||
downgrade_write(&src_inode->vfs_inode.i_rwsem);
|
||||
src_downgraded = true;
|
||||
}
|
||||
|
||||
if (remap_flags & REMAP_FILE_DEDUP)
|
||||
ret = btrfs_extent_same(src_inode, off, len, dst_inode, destoff);
|
||||
else
|
||||
|
|
@ -986,8 +993,14 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
|
|||
btrfs_inode_unlock(src_inode, BTRFS_ILOCK_MMAP);
|
||||
} else {
|
||||
btrfs_double_mmap_unlock(src_inode, dst_inode);
|
||||
unlock_two_nondirectories(&src_inode->vfs_inode,
|
||||
&dst_inode->vfs_inode);
|
||||
if (src_downgraded) {
|
||||
clear_bit(BTRFS_INODE_REFLINK_SRC, &src_inode->runtime_flags);
|
||||
inode_unlock_shared(&src_inode->vfs_inode);
|
||||
inode_unlock(&dst_inode->vfs_inode);
|
||||
} else {
|
||||
unlock_two_nondirectories(&src_inode->vfs_inode,
|
||||
&dst_inode->vfs_inode);
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user