[PATCH v4 2/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE
Matthias Goergens <[email protected]>
| Newsgroups | org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <94348cc10b4f6f7f8e2a58ee385d5349560c0c9f.1789653814.git.matthias.goergens@gmail.com> |
Deduplication tools such as duperemove, bees and rmlint find matching
ranges in two files, call FIDEDUPERANGE on each match and advance their
file offsets by the bytes_deduped the kernel returns. They rely on that
value to know where to continue.
The kernel does not give them a value they can act on.
vfs_dedupe_file_range() passes REMAP_FILE_CAN_SHORTEN, so
generic_remap_checks() rounds a request whose length is not block
aligned down to a block multiple unless it ends at both files' EOF, but
the ioctl then reports the length it asked for (the request, capped at 1
GiB per call) in bytes_deduped, not the shortened one. The caller cannot
tell that the tail of its request was left alone. Measured with rmlint
2.10.3 on btrfs with 4 KiB blocks: rmlint --dedupe on a 100000-byte file
against a 250000-byte file with the same prefix issues one call and is
told bytes_deduped=100000 with status SAME, while FIEMAP shows 24 shared
blocks, 98304 bytes. rmlint's loop ends because bytes_deduped equals the
file size, so it reports the pair fully deduplicated with 1696 bytes not
shared. duperemove (process_dedupes()) and bees advance the same way,
and jdupes advances by its own requested length without reading the
field, so all of them skip such a tail without noticing.
Add a flag that a caller sets to get a value it can act on. With
FILE_DEDUPE_RANGE_REPORT_PROGRESS set in file_dedupe_range.flags,
bytes_deduped in each destination's info is the length the filesystem
reports as deduplicated when status is FILE_DEDUPE_RANGE_SAME, and 0
when status is FILE_DEDUPE_RANGE_DIFFERS or an error. A caller advances
by it as it advances today, and must treat 0 as "stop or subdivide"
rather than retry unchanged. One cause of a SAME result of 0 is a
request shorter than a block that does not end at both files' EOF,
which the generic range preparation shortens to nothing before any
remapping. On DIFFERS the kernel has no usable progress or mismatch
offset to report, so it reports 0 and leaves subdividing the range to
the caller, as rmlint already does.
The default cannot change. Reporting the shortened length by default
was done once, in commit 4a57a8400075 ("vf/remap: return the amount of
bytes actually deduplicated"), and reverted the next day because
generic/517 expected the old value and the effect on deployed callers
was unknown. That effect is now known: duperemove re-queues a request
while its status is 0 and has no check for bytes_deduped == 0, so a 0
with status SAME on a sub-block request would make it re-issue the
same request forever.
Without the flag nothing changes. Unknown flag bits are rejected. The
flags field is an anonymous union with the old reserved2 name, so
existing source that spells .reserved2 keeps compiling and the layout
is unchanged. Kernels since 4.5, when the VFS took over the ioctl,
reject a non-zero field with -EINVAL, so a new caller cannot get the
old semantics by accident and can fall back to a call without the
flag.
Suggested-by: Darrick J. Wong <[email protected]>
Link: https://lore.kernel.org/linux-fsdevel/[email protected]/
Signed-off-by: Matthias Goergens <[email protected]>
---
See the cover letter for the changes since v3. Note for C++ callers: a
positional initialiser of struct file_dedupe_range now needs braces
around the union member under -Wmissing-braces; designated initialisers
with either .reserved2 or .flags are unaffected.
fs/remap_range.c | 4 +++-
include/uapi/linux/fs.h | 8 +++++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/remap_range.c b/fs/remap_range.c
index 26afbbbfb10c2..63f1b6f90c161 100644
--- a/fs/remap_range.c
+++ b/fs/remap_range.c
@@ -503,7 +503,7 @@ int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
if (!(file->f_mode & FMODE_READ))
return -EINVAL;
- if (same->reserved1 || same->reserved2)
+ if (same->reserved1 || (same->flags & ~FILE_DEDUPE_RANGE_REPORT_PROGRESS))
return -EINVAL;
off = same->src_offset;
@@ -555,6 +555,8 @@ int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
info->status = FILE_DEDUPE_RANGE_DIFFERS;
else if (deduped < 0)
info->status = deduped;
+ else if (same->flags & FILE_DEDUPE_RANGE_REPORT_PROGRESS)
+ info->bytes_deduped = deduped;
else
info->bytes_deduped = len;
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 34c6f219462a5..4e40855e1ef8e 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -178,13 +178,19 @@ struct file_dedupe_range_info {
__u32 reserved; /* must be zero */
};
+/* flags for struct file_dedupe_range */
+#define FILE_DEDUPE_RANGE_REPORT_PROGRESS (1U << 0)
+
/* from struct btrfs_ioctl_file_extent_same_args */
struct file_dedupe_range {
__u64 src_offset; /* in - start of extent in source */
__u64 src_length; /* in - length of extent */
__u16 dest_count; /* in - total elements in info array */
__u16 reserved1; /* must be zero */
- __u32 reserved2; /* must be zero */
+ union {
+ __u32 reserved2; /* must be zero (older callers) */
+ __u32 flags; /* in - FILE_DEDUPE_RANGE_* flags */
+ };
struct file_dedupe_range_info info[];
};
--
2.55.0