BRT: Fix FICLONE/FICLONERANGE shortened copy

On Linux the ioctl_ficlonerange() and ioctl_ficlone() system calls
are expected to either fully clone the specified range or return an
error.  The range may be for an entire file.  While internally ZFS
supports cloning partial ranges there's no way to return the length
cloned to the caller so we need to make this all or nothing.

As part of this change support for the REMAP_FILE_CAN_SHORTEN flag
has been added.  When REMAP_FILE_CAN_SHORTEN is set zfs_clone_range()
will return a shortened range when encountering pending dirty records.
When it's clear zfs_clone_range() will block and wait for the records
to be written out allowing the blocks to be cloned.

Furthermore, the file rangelock is held over the region being cloned
to prevent it from being modified while cloning.  This doesn't quite
provide an atomic semantics since if an error is encountered only a
portion of the range may be cloned.  This will be converted to an
error if REMAP_FILE_CAN_SHORTEN was not provided and returned to the
caller.  However, the destination file range is left in an undefined
state.

A test case has been added which exercises this functionality by
verifying that `cp --reflink=never|auto|always` works correctly.

Signed-off-by: Brian D Behlendorf <[email protected]>
Issue #15728

Author: behlendorf

include/os/linux/zfs/sys/zfs_vfsops_os.h

@@ -46,6 +46,7 @@ typedef struct zfsvfs zfsvfs_t;
 struct znode;
 
 extern int zfs_bclone_enabled;
+extern int zfs_bclone_wait_dirty;
 
 /*
  * This structure emulates the vfs_t from other platforms.  It's purpose

include/sys/zfs_vnops.h

@@ -32,7 +32,7 @@ extern int zfs_write(znode_t *, zfs_uio_t *, int, cred_t *);
 extern int zfs_holey(znode_t *, ulong_t, loff_t *);
 extern int zfs_access(znode_t *, int, int, cred_t *);
 extern int zfs_clone_range(znode_t *, uint64_t *, znode_t *, uint64_t *,
-    uint64_t *, cred_t *);
+    uint64_t *, cred_t *, boolean_t);
 extern int zfs_clone_range_replay(znode_t *, uint64_t, uint64_t, uint64_t,
     const blkptr_t *, size_t);
 

man/man4/zfs.4

@@ -1159,6 +1159,14 @@ Enable the experimental block cloning feature.
 If this setting is 0, then even if feature@block_cloning is enabled,
 attempts to clone blocks will act as though the feature is disabled.
 .
+.It Sy zfs_bclone_wait_dirty Ns = Ns Sy 0 Ns | Ns 1 Pq int
+When set to 1 the FICLONE and FICLONERANGE ioctls will wait for
+dirty blocks in the file to be written to disk.
+This allows the clone operation to reliably succeed when it is
+posible but may take a significant amount of time.
+By default this setting is 0 which results in the clone operation
+immediately failing when a dirty block is encountered.
+.
 .It Sy zfs_blake3_impl Ns = Ns Sy fastest Pq string
 Select a BLAKE3 implementation.
 .Pp

module/os/freebsd/zfs/zfs_vfsops.c

@@ -93,6 +93,10 @@ int zfs_bclone_enabled = 1;
 SYSCTL_INT(_vfs_zfs, OID_AUTO, bclone_enabled, CTLFLAG_RWTUN,
 	&zfs_bclone_enabled, 0, "Enable block cloning");
 
+int zfs_bclone_wait_dirty = 0;
+SYSCTL_INT(_vfs_zfs, OID_AUTO, bclone_wait_dirty, CTLFLAG_RWTUN,
+	&zfs_bclone_wait_dirty, 0, "Wait for dirty blocks when cloning");
+
 struct zfs_jailparam {
 	int mount_snapshot;
 };

module/os/freebsd/zfs/zfs_vnops_os.c

@@ -6304,7 +6304,7 @@ zfs_freebsd_copy_file_range(struct vop_copy_file_range_args *ap)
 		goto out_locked;
 
 	error = zfs_clone_range(VTOZ(invp), ap->a_inoffp, VTOZ(outvp),
-	    ap->a_outoffp, &len, ap->a_outcred);
+	    ap->a_outoffp, &len, ap->a_outcred, zfs_bclone_wait_dirty);
 	if (error == EXDEV || error == EAGAIN || error == EINVAL ||
 	    error == EOPNOTSUPP)
 		goto bad_locked_fallback;

module/os/linux/zfs/zfs_vnops_os.c

@@ -4257,7 +4257,11 @@ module_param(zfs_delete_blocks, ulong, 0644);
 MODULE_PARM_DESC(zfs_delete_blocks, "Delete files larger than N blocks async");
 
 /* CSTYLED */
-module_param(zfs_bclone_enabled, uint, 0644);
+module_param(zfs_bclone_enabled, int, 0644);
 MODULE_PARM_DESC(zfs_bclone_enabled, "Enable block cloning");
 
+/* CSTYLED */
+module_param(zfs_bclone_wait_dirty, int, 0644);
+MODULE_PARM_DESC(zfs_bclone_wait_dirty, "Wait for dirty blocks when cloning");
+
 #endif

module/os/linux/zfs/zpl_file_range.c

@@ -32,16 +32,21 @@
 #include <sys/zfeature.h>
 
 int zfs_bclone_enabled = 1;
+int zfs_bclone_wait_dirty = 0;
 
 /*
  * Clone part of a file via block cloning.
  *
  * Note that we are not required to update file offsets; the kernel will take
  * care of that depending on how it was called.
+ *
+ * When the caller cannot handle a shortened range wait_dirty may be set
+ * to request that zfs_clone_range() wait on transaction groups as needed
+ * to allow the clone to succeed if possible.
  */
 static ssize_t
-__zpl_clone_file_range(struct file *src_file, loff_t src_off,
-    struct file *dst_file, loff_t dst_off, size_t len)
+zpl_clone_file_range_impl(struct file *src_file, loff_t src_off,
+    struct file *dst_file, loff_t dst_off, size_t len, boolean_t wait_dirty)
 {
 	struct inode *src_i = file_inode(src_file);
 	struct inode *dst_i = file_inode(dst_file);
@@ -67,7 +72,7 @@ __zpl_clone_file_range(struct file *src_file, loff_t src_off,
 	cookie = spl_fstrans_mark();
 
 	err = -zfs_clone_range(ITOZ(src_i), &src_off_o, ITOZ(dst_i),
-	    &dst_off_o, &len_o, cr);
+	    &dst_off_o, &len_o, cr, wait_dirty);
 
 	spl_fstrans_unmark(cookie);
 	crfree(cr);
@@ -96,12 +101,13 @@ zpl_copy_file_range(struct file *src_file, loff_t src_off,
 {
 	ssize_t ret;
 
+	/* Flags is reserved for future extensions and must be zero. */
 	if (flags != 0)
 		return (-EINVAL);
 
-	/* Try to do it via zfs_clone_range() */
-	ret = __zpl_clone_file_range(src_file, src_off,
-	    dst_file, dst_off, len);
+	/* Try to do it via zfs_clone_range() and allow shortening. */
+	ret = zpl_clone_file_range_impl(src_file, src_off,
+	    dst_file, dst_off, len, zfs_bclone_wait_dirty);
 
 #ifdef HAVE_VFS_GENERIC_COPY_FILE_RANGE
 	/*
@@ -137,6 +143,11 @@ zpl_copy_file_range(struct file *src_file, loff_t src_off,
  * FIDEDUPERANGE is for turning a non-clone into a clone, that is, compare the
  * range in both files and if they're the same, arrange for them to be backed
  * by the same storage.
+ *
+ * REMAP_FILE_CAN_SHORTEN lets us know we can clone less than the given range
+ * if we want. It's designed for filesystems that may need to shorten the
+ * length for alignment, EOF, or any other requirement. ZFS may shorten the
+ * request when there is outstanding dirty data which hasn't been written.
  */
 loff_t
 zpl_remap_file_range(struct file *src_file, loff_t src_off,
@@ -145,24 +156,21 @@ zpl_remap_file_range(struct file *src_file, loff_t src_off,
 	if (flags & ~(REMAP_FILE_DEDUP | REMAP_FILE_CAN_SHORTEN))
 		return (-EINVAL);
 
-	/*
-	 * REMAP_FILE_CAN_SHORTEN lets us know we can clone less than the given
-	 * range if we want. Its designed for filesystems that make data past
-	 * EOF available, and don't want it to be visible in both files. ZFS
-	 * doesn't do that, so we just turn the flag off.
-	 */
-	flags &= ~REMAP_FILE_CAN_SHORTEN;
-
+	/* No support for dedup yet */
 	if (flags & REMAP_FILE_DEDUP)
-		/* No support for dedup yet */
 		return (-EOPNOTSUPP);
 
 	/* Zero length means to clone everything to the end of the file */
 	if (len == 0)
 		len = i_size_read(file_inode(src_file)) - src_off;
 
-	return (__zpl_clone_file_range(src_file, src_off,
-	    dst_file, dst_off, len));
+	ssize_t ret = zpl_clone_file_range_impl(src_file, src_off,
+	    dst_file, dst_off, len, zfs_bclone_wait_dirty);
+
+	if (!(flags & REMAP_FILE_CAN_SHORTEN) && (ret != len))
+		ret = -EINVAL;
+
+	return (ret);
 }
 #endif /* HAVE_VFS_REMAP_FILE_RANGE */
 
@@ -179,8 +187,14 @@ zpl_clone_file_range(struct file *src_file, loff_t src_off,
 	if (len == 0)
 		len = i_size_read(file_inode(src_file)) - src_off;
 
-	return (__zpl_clone_file_range(src_file, src_off,
-	    dst_file, dst_off, len));
+	/* The entire length must be cloned or this is an error. */
+	ssize_t ret = zpl_clone_file_range_impl(src_file, src_off,
+	    dst_file, dst_off, len, zfs_bclone_wait_dirty);
+
+	if (len != ret)
+		ret = -EINVAL;
+
+	return (ret);
 }
 #endif /* HAVE_VFS_CLONE_FILE_RANGE || HAVE_VFS_FILE_OPERATIONS_EXTEND */
 
@@ -214,8 +228,8 @@ zpl_ioctl_ficlone(struct file *dst_file, void *arg)
 
 	size_t len = i_size_read(file_inode(src_file));
 
-	ssize_t ret =
-	    __zpl_clone_file_range(src_file, 0, dst_file, 0, len);
+	ssize_t ret = zpl_clone_file_range_impl(src_file, 0, dst_file, 0, len,
+	    zfs_bclone_wait_dirty);
 
 	fput(src_file);
 
@@ -253,8 +267,8 @@ zpl_ioctl_ficlonerange(struct file *dst_file, void __user *arg)
 	if (len == 0)
 		len = i_size_read(file_inode(src_file)) - fcr.fcr_src_offset;
 
-	ssize_t ret = __zpl_clone_file_range(src_file, fcr.fcr_src_offset,
-	    dst_file, fcr.fcr_dest_offset, len);
+	ssize_t ret = zpl_clone_file_range_impl(src_file, fcr.fcr_src_offset,
+	    dst_file, fcr.fcr_dest_offset, len, zfs_bclone_wait_dirty);
 
 	fput(src_file);
 

module/zfs/zfs_vnops.c

@@ -1030,7 +1030,7 @@ zfs_exit_two(zfsvfs_t *zfsvfs1, zfsvfs_t *zfsvfs2, const char *tag)
  */
 int
 zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
-    uint64_t *outoffp, uint64_t *lenp, cred_t *cr)
+    uint64_t *outoffp, uint64_t *lenp, cred_t *cr, boolean_t wait_dirty)
 {
 	zfsvfs_t	*inzfsvfs, *outzfsvfs;
 	objset_t	*inos, *outos;
@@ -1049,6 +1049,7 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
 	size_t		maxblocks, nbps;
 	uint_t		inblksz;
 	uint64_t	clear_setid_bits_txg = 0;
+	uint64_t	last_synced_txg = 0;
 
 	inoff = *inoffp;
 	outoff = *outoffp;
@@ -1287,15 +1288,23 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
 		}
 
 		nbps = maxblocks;
+		last_synced_txg = spa_last_synced_txg(dmu_objset_spa(inos));
 		error = dmu_read_l0_bps(inos, inzp->z_id, inoff, size, bps,
 		    &nbps);
 		if (error != 0) {
 			/*
 			 * If we are trying to clone a block that was created
-			 * in the current transaction group, error will be
-			 * EAGAIN here, which we can just return to the caller
-			 * so it can fallback if it likes.
+			 * in the current transaction group, the error will be
+			 * EAGAIN here.  Based on sync_wait either return a
+			 * shortened range to the caller so it can fallback,
+			 * or wait for the next sync and check again.
 			 */
+			if (error == EAGAIN && wait_dirty) {
+				txg_wait_synced(dmu_objset_pool(inos),
+				    last_synced_txg + 1);
+				continue;
+			}
+
 			break;
 		}
 

tests/runfiles/common.run

@@ -631,7 +631,7 @@ tests = ['compress_001_pos', 'compress_002_pos', 'compress_003_pos',
 tags = ['functional', 'compression']
 
 [tests/functional/cp_files]
-tests = ['cp_files_001_pos', 'cp_stress']
+tests = ['cp_files_001_pos', 'cp_files_002_pos', 'cp_stress']
 tags = ['functional', 'cp_files']
 
 [tests/functional/crtime]

tests/test-runner/bin/zts-report.py.in

@@ -176,6 +176,7 @@ if sys.platform.startswith('freebsd'):
         'cli_root/zpool_wait/zpool_wait_trim_cancel': ['SKIP', trim_reason],
         'cli_root/zpool_wait/zpool_wait_trim_flag': ['SKIP', trim_reason],
         'cli_root/zfs_unshare/zfs_unshare_008_pos': ['SKIP', na_reason],
+        'cp_files/cp_files_002_pos': ['SKIP', na_reason],
         'link_count/link_count_001': ['SKIP', na_reason],
         'casenorm/mixed_create_failure': ['FAIL', 13215],
         'mmap/mmap_sync_001_pos': ['SKIP', na_reason],

tests/zfs-tests/include/tunables.cfg

@@ -94,6 +94,7 @@ VOL_MODE			vol.mode			zvol_volmode
 VOL_RECURSIVE			vol.recursive			UNSUPPORTED
 VOL_USE_BLK_MQ			UNSUPPORTED			zvol_use_blk_mq
 BCLONE_ENABLED			zfs_bclone_enabled		zfs_bclone_enabled
+BCLONE_WAIT_DIRTY		zfs_bclone_wait_dirty		zfs_bclone_wait_dirty
 XATTR_COMPAT			xattr_compat			zfs_xattr_compat
 ZEVENT_LEN_MAX			zevent.len_max			zfs_zevent_len_max
 ZEVENT_RETAIN_MAX		zevent.retain_max		zfs_zevent_retain_max

tests/zfs-tests/tests/Makefile.am

@@ -1394,6 +1394,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	functional/compression/setup.ksh \
 	functional/cp_files/cleanup.ksh \
 	functional/cp_files/cp_files_001_pos.ksh \
+	functional/cp_files/cp_files_002_pos.ksh \
 	functional/cp_files/cp_stress.ksh \
 	functional/cp_files/setup.ksh \
 	functional/crtime/cleanup.ksh \