On Tue, Oct 17, 2023 at 01:12:08PM -0700, Catherine Hoang wrote: > One of our VM cluster management products needs to snapshot KVM image > files so that they can be restored in case of failure. Snapshotting is > done by redirecting VM disk writes to a sidecar file and using reflink > on the disk image, specifically the FICLONE ioctl as used by > "cp --reflink". Reflink locks the source and destination files while it > operates, which means that reads from the main vm disk image are blocked, > causing the vm to stall. When an image file is heavily fragmented, the > copy process could take several minutes. Some of the vm image files have > 50-100 million extent records, and duplicating that much metadata locks > the file for 30 minutes or more. Having activities suspended for such > a long time in a cluster node could result in node eviction. > > Clone operations and read IO do not change any data in the source file, > so they should be able to run concurrently. Demote the exclusive locks > taken by FICLONE to shared locks to allow reads while cloning. While a > clone is in progress, writes will take the IOLOCK_EXCL, so they block > until the clone completes. > > Link: https://lore.kernel.org/linux-xfs/8911B94D-DD29-4D6E-B5BC-32EAF1866245@xxxxxxxxxx/ > Signed-off-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx> > --- > fs/xfs/xfs_file.c | 67 +++++++++++++++++++++++++++++++++++--------- > fs/xfs/xfs_inode.c | 17 +++++++++++ > fs/xfs/xfs_inode.h | 9 ++++++ > fs/xfs/xfs_reflink.c | 4 +++ > 4 files changed, 84 insertions(+), 13 deletions(-) All looks OK - one minor nit below. Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index 203700278ddb..3b9500e18f90 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -214,6 +214,47 @@ xfs_ilock_iocb( > return 0; > } > > +static int > +xfs_ilock_iocb_for_write( > + struct kiocb *iocb, > + unsigned int *lock_mode) > +{ > + ssize_t ret; > + struct xfs_inode *ip = XFS_I(file_inode(iocb->ki_filp)); > + > + ret = xfs_ilock_iocb(iocb, *lock_mode); > + if (ret) > + return ret; > + > + if (*lock_mode == XFS_IOLOCK_EXCL) > + return 0; > + if (!xfs_iflags_test(ip, XFS_IREMAPPING)) > + return 0; > + > + xfs_iunlock(ip, *lock_mode); > + *lock_mode = XFS_IOLOCK_EXCL; > + ret = xfs_ilock_iocb(iocb, *lock_mode); > + if (ret) > + return ret; > + > + return 0; This last bit could simply be: xfs_iunlock(ip, *lock_mode); *lock_mode = XFS_IOLOCK_EXCL; return xfs_ilock_iocb(iocb, *lock_mode); Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
