|
| 1 | +From ed0fc9a36698667cbf2065059b31e06fc43ae309 Mon Sep 17 00:00:00 2001 |
| 2 | +From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= <joerg@thalheim.io> |
| 3 | +Date: Fri, 15 May 2026 10:51:24 +0200 |
| 4 | +Subject: [PATCH 1/2] Linux: extract a locked clone helper from |
| 5 | + zpl_clone_file_range_impl |
| 6 | +MIME-Version: 1.0 |
| 7 | +Content-Type: text/plain; charset=UTF-8 |
| 8 | +Content-Transfer-Encoding: 8bit |
| 9 | + |
| 10 | +Pull the block-cloning capability check and the zfs_clone_range() call |
| 11 | +into separate helpers, leaving zpl_clone_file_range_impl() as a thin |
| 12 | +wrapper that only takes the inode locks. No behavior change. |
| 13 | + |
| 14 | +A FIDEDUPERANGE implementation will need to take its own locks across |
| 15 | +both a compare and a clone, so it cannot reuse a helper that locks |
| 16 | +internally. |
| 17 | + |
| 18 | +Signed-off-by: Joerg Thalheim <joerg@thalheim.io> |
| 19 | +Signed-off-by: Jörg Thalheim <joerg@thalheim.io> |
| 20 | +--- |
| 21 | + module/os/linux/zfs/zpl_file_range.c | 57 ++++++++++++++++++---------- |
| 22 | + 1 file changed, 38 insertions(+), 19 deletions(-) |
| 23 | + |
| 24 | +diff --git a/module/os/linux/zfs/zpl_file_range.c b/module/os/linux/zfs/zpl_file_range.c |
| 25 | +index c40dde046..61a2214e5 100644 |
| 26 | +--- a/module/os/linux/zfs/zpl_file_range.c |
| 27 | ++++ b/module/os/linux/zfs/zpl_file_range.c |
| 28 | +@@ -36,17 +36,16 @@ |
| 29 | + #include <sys/zfeature.h> |
| 30 | + |
| 31 | + /* |
| 32 | +- * Clone part of a file via block cloning. |
| 33 | ++ * Clone part of a file via block cloning. Both inodes must already be |
| 34 | ++ * locked by the caller. |
| 35 | + * |
| 36 | + * Note that we are not required to update file offsets; the kernel will take |
| 37 | + * care of that depending on how it was called. |
| 38 | + */ |
| 39 | + static ssize_t |
| 40 | +-zpl_clone_file_range_impl(struct file *src_file, loff_t src_off, |
| 41 | +- struct file *dst_file, loff_t dst_off, size_t len) |
| 42 | ++zpl_clone_file_range_locked(struct inode *src_i, loff_t src_off, |
| 43 | ++ struct inode *dst_i, loff_t dst_off, size_t len) |
| 44 | + { |
| 45 | +- struct inode *src_i = file_inode(src_file); |
| 46 | +- struct inode *dst_i = file_inode(dst_file); |
| 47 | + uint64_t src_off_o = (uint64_t)src_off; |
| 48 | + uint64_t dst_off_o = (uint64_t)dst_off; |
| 49 | + uint64_t len_o = (uint64_t)len; |
| 50 | +@@ -54,34 +53,54 @@ zpl_clone_file_range_impl(struct file *src_file, loff_t src_off, |
| 51 | + fstrans_cookie_t cookie; |
| 52 | + int err; |
| 53 | + |
| 54 | +- if (!zfs_bclone_enabled) |
| 55 | +- return (-EOPNOTSUPP); |
| 56 | ++ crhold(cr); |
| 57 | ++ cookie = spl_fstrans_mark(); |
| 58 | ++ |
| 59 | ++ err = -zfs_clone_range(ITOZ(src_i), &src_off_o, ITOZ(dst_i), |
| 60 | ++ &dst_off_o, &len_o, cr); |
| 61 | + |
| 62 | ++ spl_fstrans_unmark(cookie); |
| 63 | ++ crfree(cr); |
| 64 | ++ |
| 65 | ++ if (err < 0) |
| 66 | ++ return (err); |
| 67 | ++ |
| 68 | ++ return ((ssize_t)len_o); |
| 69 | ++} |
| 70 | ++ |
| 71 | ++static boolean_t |
| 72 | ++zpl_clone_supported(struct inode *dst_i) |
| 73 | ++{ |
| 74 | ++ if (!zfs_bclone_enabled) |
| 75 | ++ return (B_FALSE); |
| 76 | + if (!spa_feature_is_enabled( |
| 77 | + dmu_objset_spa(ITOZSB(dst_i)->z_os), SPA_FEATURE_BLOCK_CLONING)) |
| 78 | ++ return (B_FALSE); |
| 79 | ++ return (B_TRUE); |
| 80 | ++} |
| 81 | ++ |
| 82 | ++static ssize_t |
| 83 | ++zpl_clone_file_range_impl(struct file *src_file, loff_t src_off, |
| 84 | ++ struct file *dst_file, loff_t dst_off, size_t len) |
| 85 | ++{ |
| 86 | ++ struct inode *src_i = file_inode(src_file); |
| 87 | ++ struct inode *dst_i = file_inode(dst_file); |
| 88 | ++ ssize_t ret; |
| 89 | ++ |
| 90 | ++ if (!zpl_clone_supported(dst_i)) |
| 91 | + return (-EOPNOTSUPP); |
| 92 | + |
| 93 | + if (src_i != dst_i) |
| 94 | + spl_inode_lock_shared(src_i); |
| 95 | + spl_inode_lock(dst_i); |
| 96 | + |
| 97 | +- crhold(cr); |
| 98 | +- cookie = spl_fstrans_mark(); |
| 99 | +- |
| 100 | +- err = -zfs_clone_range(ITOZ(src_i), &src_off_o, ITOZ(dst_i), |
| 101 | +- &dst_off_o, &len_o, cr); |
| 102 | +- |
| 103 | +- spl_fstrans_unmark(cookie); |
| 104 | +- crfree(cr); |
| 105 | ++ ret = zpl_clone_file_range_locked(src_i, src_off, dst_i, dst_off, len); |
| 106 | + |
| 107 | + spl_inode_unlock(dst_i); |
| 108 | + if (src_i != dst_i) |
| 109 | + spl_inode_unlock_shared(src_i); |
| 110 | + |
| 111 | +- if (err < 0) |
| 112 | +- return (err); |
| 113 | +- |
| 114 | +- return ((ssize_t)len_o); |
| 115 | ++ return (ret); |
| 116 | + } |
| 117 | + |
| 118 | + /* |
| 119 | +-- |
| 120 | +2.53.0 |
| 121 | + |
| 122 | + |
| 123 | +From 078d462cc49907e00dad1da85cc72a41043a1499 Mon Sep 17 00:00:00 2001 |
| 124 | +From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= <joerg@thalheim.io> |
| 125 | +Date: Fri, 15 May 2026 10:51:29 +0200 |
| 126 | +Subject: [PATCH 2/2] Linux: implement REMAP_FILE_DEDUP (FIDEDUPERANGE) |
| 127 | +MIME-Version: 1.0 |
| 128 | +Content-Type: text/plain; charset=UTF-8 |
| 129 | +Content-Transfer-Encoding: 8bit |
| 130 | + |
| 131 | +ZFS only handled FICLONE/FICLONERANGE; REMAP_FILE_DEDUP returned |
| 132 | +EOPNOTSUPP. Userspace dedup tools therefore had to compare ranges |
| 133 | +themselves and call FICLONERANGE, leaving a window between compare |
| 134 | +and clone where a concurrent write is silently lost. |
| 135 | + |
| 136 | +Hold both inodes locked, let generic_remap_file_range_prep() do the |
| 137 | +page-cache compare via zpl_read_folio(), and call zfs_clone_range() |
| 138 | +without dropping the locks. zfs_clone_range() still rejects ranges |
| 139 | +that are not blocksize-aligned, as it does for FICLONERANGE. |
| 140 | + |
| 141 | +lock_two_nondirectories() orders the inode locks by address so |
| 142 | +concurrent A<->B dedup operations cannot deadlock. |
| 143 | + |
| 144 | +Signed-off-by: Joerg Thalheim <joerg@thalheim.io> |
| 145 | +Signed-off-by: Jörg Thalheim <joerg@thalheim.io> |
| 146 | +--- |
| 147 | + module/os/linux/zfs/zpl_file_range.c | 44 ++++++++++++++++++++++++++-- |
| 148 | + 1 file changed, 42 insertions(+), 2 deletions(-) |
| 149 | + |
| 150 | +diff --git a/module/os/linux/zfs/zpl_file_range.c b/module/os/linux/zfs/zpl_file_range.c |
| 151 | +index 61a2214e5..142f91458 100644 |
| 152 | +--- a/module/os/linux/zfs/zpl_file_range.c |
| 153 | ++++ b/module/os/linux/zfs/zpl_file_range.c |
| 154 | +@@ -103,6 +103,46 @@ zpl_clone_file_range_impl(struct file *src_file, loff_t src_off, |
| 155 | + return (ret); |
| 156 | + } |
| 157 | + |
| 158 | ++#ifdef HAVE_VFS_REMAP_FILE_RANGE |
| 159 | ++/* |
| 160 | ++ * FIDEDUPERANGE: clone src over dst only if the ranges are byte-identical. |
| 161 | ++ * generic_remap_file_range_prep() does the compare via the page cache; |
| 162 | ++ * both inodes stay locked across compare and clone to exclude writers. |
| 163 | ++ */ |
| 164 | ++static loff_t |
| 165 | ++zpl_dedupe_file_range_impl(struct file *src_file, loff_t src_off, |
| 166 | ++ struct file *dst_file, loff_t dst_off, loff_t len, unsigned int flags) |
| 167 | ++{ |
| 168 | ++ struct inode *src_i = file_inode(src_file); |
| 169 | ++ struct inode *dst_i = file_inode(dst_file); |
| 170 | ++ loff_t ret; |
| 171 | ++ |
| 172 | ++ if (!zpl_clone_supported(dst_i)) |
| 173 | ++ return (-EOPNOTSUPP); |
| 174 | ++ |
| 175 | ++ /* |
| 176 | ++ * vfs_dedupe_file_range_one() ensures src and dst share a |
| 177 | ++ * superblock (same dataset) but does not lock the inodes. |
| 178 | ++ * lock_two_nondirectories() orders by address so concurrent |
| 179 | ++ * A<->B dedup ops cannot ABBA-deadlock. |
| 180 | ++ */ |
| 181 | ++ lock_two_nondirectories(src_i, dst_i); |
| 182 | ++ |
| 183 | ++ ret = generic_remap_file_range_prep(src_file, src_off, dst_file, |
| 184 | ++ dst_off, &len, flags); |
| 185 | ++ if (ret < 0 || len == 0) |
| 186 | ++ goto out; |
| 187 | ++ |
| 188 | ++ ret = zpl_clone_file_range_locked(src_i, src_off, dst_i, dst_off, |
| 189 | ++ len); |
| 190 | ++ |
| 191 | ++out: |
| 192 | ++ unlock_two_nondirectories(src_i, dst_i); |
| 193 | ++ |
| 194 | ++ return (ret); |
| 195 | ++} |
| 196 | ++#endif /* HAVE_VFS_REMAP_FILE_RANGE */ |
| 197 | ++ |
| 198 | + /* |
| 199 | + * Entry point for copy_file_range(). Copy len bytes from src_off in src_file |
| 200 | + * to dst_off in dst_file. We are permitted to do this however we like, so we |
| 201 | +@@ -178,9 +218,9 @@ zpl_remap_file_range(struct file *src_file, loff_t src_off, |
| 202 | + if (flags & ~(REMAP_FILE_DEDUP | REMAP_FILE_CAN_SHORTEN)) |
| 203 | + return (-EINVAL); |
| 204 | + |
| 205 | +- /* No support for dedup yet */ |
| 206 | + if (flags & REMAP_FILE_DEDUP) |
| 207 | +- return (-EOPNOTSUPP); |
| 208 | ++ return (zpl_dedupe_file_range_impl(src_file, src_off, |
| 209 | ++ dst_file, dst_off, len, flags)); |
| 210 | + |
| 211 | + /* Zero length means to clone everything to the end of the file */ |
| 212 | + if (len == 0) |
| 213 | +-- |
| 214 | +2.53.0 |
| 215 | + |
0 commit comments