* [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely
@ 2026-08-05 7:14 Matthias Goergens
2026-08-05 7:14 ` [PATCH 1/2] vfs: fail dedupe requests that cannot make progress Matthias Goergens
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Matthias Goergens @ 2026-08-05 7:14 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara
Cc: Matthias Goergens, linux-fsdevel, linux-kernel,
Ansgar Lößer, Darrick J . Wong, Dave Chinner,
Amir Goldstein
FIDEDUPERANGE currently reports the requested length even when the
filesystem shortens a destination range and deduplicates fewer bytes. A
previous one-line correction was reverted after generic/517 exposed the old
expectation and reviewers raised the risk that existing consumers could loop
on a successful zero-progress result.
Patch 1 makes a non-zero request shortened to zero fail per destination with
-EINVAL, while preserving explicit zero-length success. Patch 2 then reports
the filesystem's actual positive progress. This ordering keeps every
intermediate kernel safe for callers that advance by bytes_deduped.
The paired fstests update corrects generic/517 and adds raw ioctl coverage for
zero-length and mixed multi-destination results. Both tests pass on Btrfs and
XFS. Installed duperemove exits successfully on the measured corpus. Installed
rmlint does not hang or silently over-report; it exits 1 after the final
unaligned tail receives -EINVAL, which is recorded explicitly for review.
A current-source consumer audit supports that ABI choice: duperemove completes
the request on a non-zero status; rmlint, bees and jdupes surface -EINVAL as
failure without retrying; dduper and xfs_io stop but can still report command
success. None retries, hangs or risks data corruption. Thus -EINVAL is the
only truthful result that also avoids exposing successful zero progress to
deployed duperemove binaries.
Matthias Goergens (2):
vfs: fail dedupe requests that cannot make progress
vfs: report the amount of bytes actually deduplicated
fs/remap_range.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
--
2.55.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] vfs: fail dedupe requests that cannot make progress 2026-08-05 7:14 [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely Matthias Goergens @ 2026-08-05 7:14 ` Matthias Goergens 2026-08-05 7:14 ` [PATCH 2/2] vfs: report the amount of bytes actually deduplicated Matthias Goergens ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Matthias Goergens @ 2026-08-05 7:14 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Jan Kara Cc: Matthias Goergens, linux-fsdevel, linux-kernel, Ansgar Lößer, Darrick J . Wong, Dave Chinner, Amir Goldstein FIDEDUPERANGE allows the VFS to shorten each destination range. An unaligned, non-EOF request shorter than the filesystem block size can therefore be shortened to zero. vfs_dedupe_file_range_one() then returns zero, but vfs_dedupe_file_range() reports the original length and success even though it made no progress. Reporting the actual return value would expose this as a successful zero-byte operation. While diagnosing the over-reporting, Darrick Wong pointed out that a caller such as duperemove, which advances only by bytes_deduped and has no zero-progress guard, can retry that range forever. Return per-destination -EINVAL when a nonzero request is shortened to zero. Keep the historical result for an explicit zero-length request: bytes_deduped remains zero with FILE_DEDUPE_RANGE_SAME. Other destinations in the same ioctl continue to be processed. Apply this guard before correcting bytes_deduped so that no intermediate kernel exposes a successful zero-progress result. With this guard and the following reporting correction, installed duperemove rounded its match to 98304 bytes and exited with status 0. Installed rmlint exercised the guard: it received 98304 bytes of progress, then 0/-EINVAL for the 1696-byte remainder, and exited with status 1 rather than retrying indefinitely. Link: https://lore.kernel.org/linux-fsdevel/Y93BkIA4Nd3IJAk+@magnolia/ Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com> --- fs/remap_range.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/remap_range.c b/fs/remap_range.c index 26afbbbfb10c2..53330aa26b86f 100644 --- a/fs/remap_range.c +++ b/fs/remap_range.c @@ -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 (!deduped && len) + info->status = -EINVAL; else info->bytes_deduped = len; -- 2.55.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] vfs: report the amount of bytes actually deduplicated 2026-08-05 7:14 [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely Matthias Goergens 2026-08-05 7:14 ` [PATCH 1/2] vfs: fail dedupe requests that cannot make progress Matthias Goergens @ 2026-08-05 7:14 ` Matthias Goergens 2026-08-12 7:46 ` [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely Christian Brauner 2026-08-14 8:23 ` [PATCH v2 0/1] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens 3 siblings, 0 replies; 12+ messages in thread From: Matthias Goergens @ 2026-08-05 7:14 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Jan Kara Cc: Matthias Goergens, linux-fsdevel, linux-kernel, Ansgar Lößer, Darrick J . Wong, Dave Chinner, Amir Goldstein FIDEDUPERANGE promises to return the number of bytes successfully deduplicated in bytes_deduped. vfs_dedupe_file_range_one() can shorten a request and returns the resulting byte count. However, vfs_dedupe_file_range() discards that value and reports the original request length. The VFS ioctl originally accumulated the returned byte count in commit 54dbc1517237 ("vfs: hoist the btrfs deduplication ioctl to the vfs"). Commit 5740c99e9d30 ("vfs: dedupe: return int") changed the filesystem callback to return status and substituted the requested length. The helper once again returns a loff_t byte count, but the stale assignment remained. Ansgar Lößer corrected the assignment in commit 4a57a8400075 ("vf/remap: return the amount of bytes actually deduplicated"), after reports from Max Schlecht and Björn Scheuermann. The change was reverted the next day after generic/517 exposed its expectation of the over-reported value and the userspace impact still needed investigation. Restore that correction. A nonzero request shortened to zero now fails with -EINVAL due to the preceding change. Callers that advance by bytes_deduped therefore cannot retry such a range forever. Updated generic/517 and raw multi-destination coverage in generic/806 pass on both btrfs and XFS. Installed duperemove rounded its match to 98304 bytes and exited with status 0. Installed rmlint received 98304 bytes of progress followed by 0/-EINVAL for the 1696-byte remainder and exited with status 1 instead of silently accepting the over-reported request. Link: https://lore.kernel.org/linux-fsdevel/5548ef63-62f9-4f46-5793-03165ceccacc@tu-darmstadt.de/ Link: https://lore.kernel.org/all/20220714223238.GH3600936@dread.disaster.area/ Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com> --- fs/remap_range.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/remap_range.c b/fs/remap_range.c index 53330aa26b86f..9fedc22b52761 100644 --- a/fs/remap_range.c +++ b/fs/remap_range.c @@ -558,7 +558,7 @@ int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same) else if (!deduped && len) info->status = -EINVAL; else - info->bytes_deduped = len; + info->bytes_deduped = deduped; next_loop: if (fatal_signal_pending(current)) -- 2.55.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely 2026-08-05 7:14 [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely Matthias Goergens 2026-08-05 7:14 ` [PATCH 1/2] vfs: fail dedupe requests that cannot make progress Matthias Goergens 2026-08-05 7:14 ` [PATCH 2/2] vfs: report the amount of bytes actually deduplicated Matthias Goergens @ 2026-08-12 7:46 ` Christian Brauner 2026-08-13 15:33 ` Amir Goldstein 2026-08-14 8:23 ` [PATCH v2 0/1] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens 3 siblings, 1 reply; 12+ messages in thread From: Christian Brauner @ 2026-08-12 7:46 UTC (permalink / raw) To: Matthias Goergens Cc: Alexander Viro, Christian Brauner, Jan Kara, linux-fsdevel, linux-kernel, Ansgar Lößer, Darrick J . Wong, Dave Chinner, Amir Goldstein On 2026-08-05 15:14 +0800, Matthias Goergens wrote: > FIDEDUPERANGE currently reports the requested length even when the > filesystem shortens a destination range and deduplicates fewer bytes. A > previous one-line correction was reverted after generic/517 exposed the old > expectation and reviewers raised the risk that existing consumers could loop > on a successful zero-progress result. > > Patch 1 makes a non-zero request shortened to zero fail per destination with > -EINVAL, while preserving explicit zero-length success. Patch 2 then reports > the filesystem's actual positive progress. This ordering keeps every > intermediate kernel safe for callers that advance by bytes_deduped. > > The paired fstests update corrects generic/517 and adds raw ioctl coverage for > zero-length and mixed multi-destination results. Both tests pass on Btrfs and > XFS. Installed duperemove exits successfully on the measured corpus. Installed > rmlint does not hang or silently over-report; it exits 1 after the final > unaligned tail receives -EINVAL, which is recorded explicitly for review. > > A current-source consumer audit supports that ABI choice: duperemove completes > the request on a non-zero status; rmlint, bees and jdupes surface -EINVAL as > failure without retrying; dduper and xfs_io stop but can still report command > success. None retries, hangs or risks data corruption. Thus -EINVAL is the > only truthful result that also avoids exposing successful zero progress to > deployed duperemove binaries. > > Matthias Goergens (2): > vfs: fail dedupe requests that cannot make progress > vfs: report the amount of bytes actually deduplicated Needs input from Amir. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely 2026-08-12 7:46 ` [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely Christian Brauner @ 2026-08-13 15:33 ` Amir Goldstein 2026-08-13 20:09 ` Darrick J. Wong 0 siblings, 1 reply; 12+ messages in thread From: Amir Goldstein @ 2026-08-13 15:33 UTC (permalink / raw) To: Christian Brauner Cc: Matthias Goergens, Alexander Viro, Jan Kara, linux-fsdevel, linux-kernel, Ansgar Lößer, Darrick J . Wong, Dave Chinner On Wed, Aug 12, 2026 at 9:46 AM Christian Brauner <brauner@kernel.org> wrote: > > On 2026-08-05 15:14 +0800, Matthias Goergens wrote: > > FIDEDUPERANGE currently reports the requested length even when the > > filesystem shortens a destination range and deduplicates fewer bytes. A > > previous one-line correction was reverted after generic/517 exposed the old > > expectation and reviewers raised the risk that existing consumers could loop > > on a successful zero-progress result. > > > > Patch 1 makes a non-zero request shortened to zero fail per destination with > > -EINVAL, while preserving explicit zero-length success. Patch 2 then reports > > the filesystem's actual positive progress. This ordering keeps every > > intermediate kernel safe for callers that advance by bytes_deduped. > > > > The paired fstests update corrects generic/517 and adds raw ioctl coverage for > > zero-length and mixed multi-destination results. Both tests pass on Btrfs and > > XFS. Installed duperemove exits successfully on the measured corpus. Installed > > rmlint does not hang or silently over-report; it exits 1 after the final > > unaligned tail receives -EINVAL, which is recorded explicitly for review. > > > > A current-source consumer audit supports that ABI choice: duperemove completes > > the request on a non-zero status; rmlint, bees and jdupes surface -EINVAL as > > failure without retrying; dduper and xfs_io stop but can still report command > > success. None retries, hangs or risks data corruption. Thus -EINVAL is the > > only truthful result that also avoids exposing successful zero progress to > > deployed duperemove binaries. > > > > Matthias Goergens (2): > > vfs: fail dedupe requests that cannot make progress > > vfs: report the amount of bytes actually deduplicated > > Needs input from Amir. > The logic seems sound to me. Main well tested and accounted for, the suggested fixed already aligned with the man page documentation even: EINVAL The filesystem does not support deduplicating the ranges of the given files. could be interpreted to apply to this unaligned dedupe case Feel free to add Reviewed-by: Amir Goldstein <amir73il@gmail.com> to both patches, Thanks, Amir. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely 2026-08-13 15:33 ` Amir Goldstein @ 2026-08-13 20:09 ` Darrick J. Wong 0 siblings, 0 replies; 12+ messages in thread From: Darrick J. Wong @ 2026-08-13 20:09 UTC (permalink / raw) To: Amir Goldstein Cc: Christian Brauner, Matthias Goergens, Alexander Viro, Jan Kara, linux-fsdevel, linux-kernel, Ansgar Lößer, Dave Chinner On Thu, Aug 13, 2026 at 05:33:36PM +0200, Amir Goldstein wrote: > On Wed, Aug 12, 2026 at 9:46 AM Christian Brauner <brauner@kernel.org> wrote: > > > > On 2026-08-05 15:14 +0800, Matthias Goergens wrote: > > > FIDEDUPERANGE currently reports the requested length even when the > > > filesystem shortens a destination range and deduplicates fewer bytes. A > > > previous one-line correction was reverted after generic/517 exposed the old > > > expectation and reviewers raised the risk that existing consumers could loop > > > on a successful zero-progress result. > > > > > > Patch 1 makes a non-zero request shortened to zero fail per destination with > > > -EINVAL, while preserving explicit zero-length success. Patch 2 then reports > > > the filesystem's actual positive progress. This ordering keeps every > > > intermediate kernel safe for callers that advance by bytes_deduped. > > > > > > The paired fstests update corrects generic/517 and adds raw ioctl coverage for > > > zero-length and mixed multi-destination results. Both tests pass on Btrfs and > > > XFS. Installed duperemove exits successfully on the measured corpus. Installed > > > rmlint does not hang or silently over-report; it exits 1 after the final > > > unaligned tail receives -EINVAL, which is recorded explicitly for review. > > > > > > A current-source consumer audit supports that ABI choice: duperemove completes > > > the request on a non-zero status; rmlint, bees and jdupes surface -EINVAL as > > > failure without retrying; dduper and xfs_io stop but can still report command > > > success. None retries, hangs or risks data corruption. Thus -EINVAL is the > > > only truthful result that also avoids exposing successful zero progress to > > > deployed duperemove binaries. > > > > > > Matthias Goergens (2): > > > vfs: fail dedupe requests that cannot make progress > > > vfs: report the amount of bytes actually deduplicated > > > > Needs input from Amir. > > > > The logic seems sound to me. > Main well tested and accounted for, > the suggested fixed already aligned with the man page documentation > even: > EINVAL The filesystem does not support deduplicating the ranges of > the given files. > could be interpreted to apply to this unaligned dedupe case The problem is that the weird bytes_deduped = len behavior has been around for years, even before any of it got hoisted to the VFS: https://elixir.bootlin.com/linux/v4.0.9/source/fs/btrfs/ioctl.c#L3025 IOWs, the manpage is wrong. I suppose you could just merge this fix and the changes for xfsprogs/fstests and take your chances that nobody complains, but afaict duperemove isn't going to be happy: $ git grep bytes_deduped btrfs-extent-same.c:40: uint64_t bytes_deduped; /* out - total # of bytes we btrfs-extent-same.c:138: printf("i: %d, status: %d, bytes_deduped: %llu\n", i, btrfs-extent-same.c:139: info->status, (unsigned long long)info->bytes_deduped); btrfs-extent-same.c:141: bytes += info->bytes_deduped; dedupe.c:110: "%llu, bytes_deduped: %llu, status: %d\n", dedupe.c:113: (unsigned long long)info->bytes_deduped, info->status); dedupe.c:233: info->bytes_deduped = 0; dedupe.c:279: if (info->bytes_deduped > max_deduped) dedupe.c:280: max_deduped = info->bytes_deduped; dedupe.c:282: req->req_loff += info->bytes_deduped; dedupe.c:283: req->req_total += info->bytes_deduped; dedupe.c:348: uint64_t *off, uint64_t *bytes_deduped, dedupe.c:364: *bytes_deduped = req->req_total; dedupe.h:77: uint64_t *off, uint64_t *bytes_deduped, ioctl.h:16: __u64 bytes_deduped; /* out - total # of bytes we were able Notice how it increments a file offset based on bytes_deduped? A safer option would be to define a flags field and add a flag to enable the behavior that is documented and obviously makes more sense. I don't know when you'd get bytes_deduped==0 since you'd think that would result in info->status being set to FILE_DEDUPE_RANGE_DIFFERS? --D > Feel free to add > Reviewed-by: Amir Goldstein <amir73il@gmail.com> > > to both patches, > > Thanks, > Amir. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 0/1] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE 2026-08-05 7:14 [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely Matthias Goergens ` (2 preceding siblings ...) 2026-08-12 7:46 ` [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely Christian Brauner @ 2026-08-14 8:23 ` Matthias Goergens 2026-08-14 8:23 ` [PATCH v2] " Matthias Goergens 2026-08-17 11:56 ` [PATCH v3 0/1] " Matthias Goergens 3 siblings, 2 replies; 12+ messages in thread From: Matthias Goergens @ 2026-08-14 8:23 UTC (permalink / raw) To: linux-fsdevel Cc: viro, brauner, jack, linux-kernel, ansgar.loesser, djwong, david, amir73il Changes since v1 (<20260805071414.3414870-1-matthias.goergens@gmail.com>): Darrick is right that bytes_deduped = len predates the VFS hoisting - the manpage describes the behaviour the patch would introduce, not the historical default, and deployed consumers (duperemove advances file offsets by bytes_deduped) depend on the old semantics. v2 therefore keeps the default unchanged and gates the truthful behaviour behind a new flag: - FILE_DEDUPE_RANGE_REPORT_PROGRESS (the former reserved2 field is now flags): bytes_deduped reports the bytes actually deduplicated, and a non-zero request shortened to zero fails per destination with -EINVAL instead of reporting success with no progress. - Unknown flag bits are rejected with -EINVAL. The fstests series will follow in the same shape: the generic/517 golden output stays as it is (unflagged behaviour is unchanged), and the new test exercises the flag, including one unflagged case pinning the legacy reporting. Darrick also notes the manpage is wrong - with the flag in place, the right fix is to document the flag in ioctl_fideduperange(2) rather than change the default; a man-pages patch can follow once the flag name and semantics are settled. Amir: you reviewed the v1 logic favourably - could you have a look at the flag shape instead? In particular: the name (FILE_DEDUPE_RANGE_REPORT_PROGRESS), the semantics (actual bytes plus -EINVAL on zero progress), and whether the manpage fix should ride along with the kernel patch or follow separately. If this shape works for you, the Reviewed-by on the flagged behaviour would be very welcome. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE 2026-08-14 8:23 ` [PATCH v2 0/1] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens @ 2026-08-14 8:23 ` Matthias Goergens 2026-08-14 16:16 ` Darrick J. Wong 2026-08-17 11:56 ` [PATCH v3 0/1] " Matthias Goergens 1 sibling, 1 reply; 12+ messages in thread From: Matthias Goergens @ 2026-08-14 8:23 UTC (permalink / raw) To: linux-fsdevel Cc: viro, brauner, jack, linux-kernel, ansgar.loesser, djwong, david, amir73il FIDEDUPERANGE reports the requested length in bytes_deduped even when the filesystem shortens a destination range and deduplicates fewer bytes. This predates the VFS hoisting of the ioctl (the btrfs ioctl behaved the same way), and changing the default would change an ABI that deployed consumers such as duperemove depend on: they advance their offsets by bytes_deduped and expect the historical semantics. Add a flag to opt into the truthful behaviour: with FILE_DEDUPE_RANGE_REPORT_PROGRESS set, bytes_deduped reports the bytes actually deduplicated, and a non-zero request shortened to zero fails per destination with -EINVAL instead of reporting success with no progress. Unknown flag bits are rejected. The flag leaves every existing caller's behaviour unchanged and lets new callers request accurate progress reporting. Suggested-by: Darrick J. Wong <djwong@kernel.org> Link: https://lore.kernel.org/linux-fsdevel/20260805071414.3414870-1-matthias.goergens@gmail.com/ Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com> --- fs/remap_range.c | 7 ++++++- include/uapi/linux/fs.h | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/fs/remap_range.c b/fs/remap_range.c index 26afbbbfb10c2..16192ec19bb6d 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,11 @@ 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) && + !deduped && len) + info->status = -EINVAL; + 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 bd87262f2e349..abee40359bfd5 100644 --- a/include/uapi/linux/fs.h +++ b/include/uapi/linux/fs.h @@ -179,12 +179,14 @@ struct file_dedupe_range_info { }; /* from struct btrfs_ioctl_file_extent_same_args */ +#define FILE_DEDUPE_RANGE_REPORT_PROGRESS 0x1 + 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 */ + __u32 flags; /* FILE_DEDUPE_RANGE_* flags; was reserved2 */ struct file_dedupe_range_info info[]; }; -- 2.55.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE 2026-08-14 8:23 ` [PATCH v2] " Matthias Goergens @ 2026-08-14 16:16 ` Darrick J. Wong 0 siblings, 0 replies; 12+ messages in thread From: Darrick J. Wong @ 2026-08-14 16:16 UTC (permalink / raw) To: Matthias Goergens Cc: linux-fsdevel, viro, brauner, jack, linux-kernel, ansgar.loesser, david, amir73il On Fri, Aug 14, 2026 at 04:23:26PM +0800, Matthias Goergens wrote: > FIDEDUPERANGE reports the requested length in bytes_deduped even when > the filesystem shortens a destination range and deduplicates fewer > bytes. This predates the VFS hoisting of the ioctl (the btrfs ioctl > behaved the same way), and changing the default would change an ABI > that deployed consumers such as duperemove depend on: they advance > their offsets by bytes_deduped and expect the historical semantics. > > Add a flag to opt into the truthful behaviour: with > FILE_DEDUPE_RANGE_REPORT_PROGRESS set, bytes_deduped reports the bytes > actually deduplicated, and a non-zero request shortened to zero fails > per destination with -EINVAL instead of reporting success with no > progress. Unknown flag bits are rejected. > > The flag leaves every existing caller's behaviour unchanged and lets > new callers request accurate progress reporting. > > Suggested-by: Darrick J. Wong <djwong@kernel.org> > Link: https://lore.kernel.org/linux-fsdevel/20260805071414.3414870-1-matthias.goergens@gmail.com/ > Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com> > --- > fs/remap_range.c | 7 ++++++- > include/uapi/linux/fs.h | 4 +++- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/fs/remap_range.c b/fs/remap_range.c > index 26afbbbfb10c2..16192ec19bb6d 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,11 @@ 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) && > + !deduped && len) > + info->status = -EINVAL; Thinking about this (deduped == 0 && len > 0) case some more -- the filesystem didn't return EBADE (aka FILE_DEDUPE_RANGE_DIFFERS), which means that len wasn't long enough for the filesystem to do any interesting work. For example, if you try to dedupe two 65 byte files when the blocksize is 4k, the generic_remap_check* functions can change 65 to 0, and that's what we get here. Seeing as we've now a new flag to play with, do we really need to return EINVAL here? Only new code will use the new flag, so we don't have to worry about old code; and the flag has already established that info->bytes_deduped is the quantity of deduplication achieved, *not* the amount by which to bump the source/dest offset for the next call. So passing out zero here should be ok. Hmm. Waitaminute, we have *two* output fields -- status and bytes_deduped. There are three different cases that I can see: 1) If an operational error occurs, we set info->status to the negative error number and bytes_deduped is 0. 2) If the file ranges starting at src_offset/dest_offset can be deduplicated, we set info->status to FILE_DEDUPE_RANGE_SAME and set info->bytes_deduped to the number of bytes that were deduplicated. 3) If the file ranges cannot be deduplicated, we set info->status to FILE_DEDUPE_RANGE_DIFFERS and ... do nothing with info->bytes_deduped, so it remains 0. In cases 2 and 3, userspace is likely to want to try again with any remaining file range. For case 2 this is trivial: add bytes_deduped to src_offset and dest_offset; subtract it from src_length; and call the kernel again. In other words, the kernel replies "ok I dedupe'd 72k of data" and the program advances the file offsets by 72k and asks the kernel to try again. For case 3 there isn't currently any obvious way for the ioctl code to communicate how far userspace should advance the file offsets before calling again. Most likely (since only blocks can be remapped) that quantity is i_blocksize, but then userspace has to figure out what that is -- is it statvfs' f_blocksize? Or statx' stx_blksize? What if we just did that work for userspace? if (deduped == -EBADE) { info->status = FILE_DEDUPE_RANGE_DIFFERS; if (same->flags & FILE_DEDUPE_RANGE_REPORT_PROGRESS) info->bytes_deduped = i_blocksize(src); } 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; } And then the manpage can say: FILE_DEDUPE_RANGE_REPORT_PROGRESS If this flag is set, the output value of status and bytes_deduped are redefined as follows: If an operational error occurred, status contains the negative error number. bytes_deduped field is set to zero. If status is FILE_DEDUPE_RANGE_DIFFERS, this is the number of bytes that were examined but could not be deduplicated. If status is FILE_DEDUPE_RANGE_SAME, this is the number of bytes that were successfully deduplicated at the start of the provided file ranges. The intent here is that a userspace program could use bytes_deduplicated to advance the input file ranges in preparation for another kernel call. In both cases, a zero value for bytes_deduped means that no further work is possible. What do you think of that? > + 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 bd87262f2e349..abee40359bfd5 100644 > --- a/include/uapi/linux/fs.h > +++ b/include/uapi/linux/fs.h > @@ -179,12 +179,14 @@ struct file_dedupe_range_info { > }; > > /* from struct btrfs_ioctl_file_extent_same_args */ > +#define FILE_DEDUPE_RANGE_REPORT_PROGRESS 0x1 Nitpicking: This should be (1U << 0), not 0x1, because 0x1 is treated as a signed int and the flags field is unsigned. --D > + > 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 */ > + __u32 flags; /* FILE_DEDUPE_RANGE_* flags; was reserved2 */ > struct file_dedupe_range_info info[]; > }; > > -- > 2.55.0 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 0/1] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE 2026-08-14 8:23 ` [PATCH v2 0/1] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens 2026-08-14 8:23 ` [PATCH v2] " Matthias Goergens @ 2026-08-17 11:56 ` Matthias Goergens 2026-08-17 11:56 ` [PATCH v3] " Matthias Goergens 1 sibling, 1 reply; 12+ messages in thread From: Matthias Goergens @ 2026-08-17 11:56 UTC (permalink / raw) To: linux-fsdevel Cc: viro, brauner, jack, linux-kernel, ansgar.loesser, djwong, david, amir73il Changes since v2 (<20260814082326.3756669-1-matthias.goergens@gmail.com>): Adopt Darrick's three-case semantics for bytes_deduped under the flag: drop the -EINVAL on zero progress (a zero-progress success now simply reports 0), report a safe advance step on FILE_DEDUPE_RANGE_DIFFERS, and keep the actual byte count on FILE_DEDUPE_RANGE_SAME. The flag constant is (1U << 0). One deviation from the sketch: the DIFFERS advance step is capped to the requested length, min(i_blocksize(src), len). Without the cap, a sub-block request on files whose ranges end at EOF (permitted by generic_remap_checks()) would report an advance larger than the whole request - e.g. two differing 512-byte files report an advance of 4096 - and a caller following the hint would step past EOF instead of stopping. With the cap, "zero means no further work" holds and the hint can never overshoot. Measured on a patched kernel (btrfs): the 512-byte pair reports bytes_deduped=512, and full-size differing files still report one block. One point I would like opinions on: where the flags field lives. Repurposing reserved2 needs a name, and there are two precedents. A plain rename (as fscrypt and statx did with reserved fields) is tidier, but it breaks source that spells out .reserved2 - which the "must be zero" documentation invited; I verified with installed headers that such code stops compiling. v3 instead puts flags in an anonymous union with the old reserved2 name (the io_uring_sqe pattern): both spellings compile and the binary layout is untouched. If the plain rename is preferred as a matter of taste, the code change is trivial. Two review notes worth surfacing rather than hiding. The DIFFERS advance hint's safety argument assumes -EBADE comes from the generic remap prep's compare, which holds for every in-tree dedupe implementation (btrfs, XFS, ocfs2) and for bcachefs out of tree. And two independent review passes attacked the one-block hint itself: on stacked filesystems the top-level inode's block size can be degenerate (overlayfs inodes report i_blkbits == 0, so the hint would be one byte - v3 falls back to the requested length there), and a caller that only ever advances by the hint walks past identical prefix blocks that a subdividing caller could still deduplicate. If reporting the examined request length on DIFFERS in all cases would be preferable to the one-block step - it is simpler and needs no block-size knowledge - I am happy to re-roll that way. The paired fstests v2 (generic/806, on the fstests list) exercises all four flagged cases plus an unflagged legacy-pinning case; every expected line there was produced by a kernel with this patch applied. A man-pages patch for ioctl_fideduperange(2) documenting the flag will follow once the semantics settle. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE 2026-08-17 11:56 ` [PATCH v3 0/1] " Matthias Goergens @ 2026-08-17 11:56 ` Matthias Goergens 2026-08-18 8:50 ` Christoph Hellwig 0 siblings, 1 reply; 12+ messages in thread From: Matthias Goergens @ 2026-08-17 11:56 UTC (permalink / raw) To: linux-fsdevel Cc: viro, brauner, jack, linux-kernel, ansgar.loesser, djwong, david, amir73il On success FIDEDUPERANGE reports the requested length in bytes_deduped even when the filesystem shortens a destination range and deduplicates fewer bytes. This predates the VFS hoisting of the ioctl (the btrfs ioctl behaved the same way), and changing the default would change an ABI that deployed consumers such as duperemove depend on: they advance their offsets by bytes_deduped and expect the historical semantics. Add a flag to opt into the truthful behaviour. With FILE_DEDUPE_RANGE_REPORT_PROGRESS set, bytes_deduped in each destination's info is an advance hint for the next call on that destination: - if status is an error, bytes_deduped is 0; - if status is FILE_DEDUPE_RANGE_DIFFERS, bytes_deduped is a safe advance step: one filesystem block, capped to the requested length so a sub-block request ending at EOF cannot be advanced past its end, and falling back to the requested length when the top-level inode reports a degenerate block size (stacked filesystems); - if status is FILE_DEDUPE_RANGE_SAME, bytes_deduped is the number of bytes actually deduplicated; - in both success cases a zero value means no further work is possible. Unknown flag bits are rejected. The flags field shares an anonymous union with the old reserved2 name, so existing source keeps compiling and the binary layout is unchanged; old kernels require the field to be zero, so new callers setting the flag on old kernels get -EINVAL rather than silently the old semantics. Suggested-by: Darrick J. Wong <djwong@kernel.org> Link: https://lore.kernel.org/linux-fsdevel/20260805071414.3414870-1-matthias.goergens@gmail.com/ Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com> --- fs/remap_range.c | 25 +++++++++++++++++++++---- include/uapi/linux/fs.h | 5 ++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/fs/remap_range.c b/fs/remap_range.c index 26afbbbfb10c2..ac88a81c12739 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; @@ -551,12 +551,29 @@ int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same) deduped = vfs_dedupe_file_range_one(file, off, fd_file(dst_fd), info->dest_offset, len, REMAP_FILE_CAN_SHORTEN); - if (deduped == -EBADE) + if (deduped == -EBADE) { info->status = FILE_DEDUPE_RANGE_DIFFERS; - else if (deduped < 0) + if (same->flags & FILE_DEDUPE_RANGE_REPORT_PROGRESS) { + u64 step = i_blocksize(src); + + /* + * Stacked filesystems (e.g. overlayfs) can + * report a degenerate block size here; a + * one-byte step would only misalign the + * next call, so advance past the whole + * request instead. + */ + if (step <= 1) + step = len; + info->bytes_deduped = min_t(u64, step, len); + } + } else if (deduped < 0) { info->status = deduped; - else + } else if (same->flags & FILE_DEDUPE_RANGE_REPORT_PROGRESS) { + info->bytes_deduped = deduped; + } else { info->bytes_deduped = len; + } next_loop: if (fatal_signal_pending(current)) diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h index bd87262f2e349..471f698beaa93 100644 --- a/include/uapi/linux/fs.h +++ b/include/uapi/linux/fs.h @@ -178,13 +178,16 @@ 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 */ + __u32 flags; /* in - FILE_DEDUPE_RANGE_* flags */ struct file_dedupe_range_info info[]; }; -- 2.55.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE 2026-08-17 11:56 ` [PATCH v3] " Matthias Goergens @ 2026-08-18 8:50 ` Christoph Hellwig 0 siblings, 0 replies; 12+ messages in thread From: Christoph Hellwig @ 2026-08-18 8:50 UTC (permalink / raw) To: Matthias Goergens Cc: linux-fsdevel, viro, brauner, jack, linux-kernel, ansgar.loesser, djwong, david, amir73il On Mon, Aug 17, 2026 at 07:56:09PM +0800, Matthias Goergens wrote: > On success FIDEDUPERANGE reports the requested length in bytes_deduped > even when the filesystem shortens a destination range and deduplicates > fewer bytes. This predates the VFS hoisting of the ioctl (the btrfs > ioctl behaved the same way), and changing the default would change an > ABI that deployed consumers such as duperemove depend on: they advance > their offsets by bytes_deduped and expect the historical semantics. > > Add a flag to opt into the truthful behaviour. With What is "truthful"? It just is different. And you completely fail to explain why it is useful here, instead spewing a weird AI-like monologue just duplicating the patch content. Start with why you care, i.e. what application or type of application cares how much actually was deduplicated, and how you define the user visible behavior of that having happened. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-08-18 8:50 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-05 7:14 [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely Matthias Goergens 2026-08-05 7:14 ` [PATCH 1/2] vfs: fail dedupe requests that cannot make progress Matthias Goergens 2026-08-05 7:14 ` [PATCH 2/2] vfs: report the amount of bytes actually deduplicated Matthias Goergens 2026-08-12 7:46 ` [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely Christian Brauner 2026-08-13 15:33 ` Amir Goldstein 2026-08-13 20:09 ` Darrick J. Wong 2026-08-14 8:23 ` [PATCH v2 0/1] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens 2026-08-14 8:23 ` [PATCH v2] " Matthias Goergens 2026-08-14 16:16 ` Darrick J. Wong 2026-08-17 11:56 ` [PATCH v3 0/1] " Matthias Goergens 2026-08-17 11:56 ` [PATCH v3] " Matthias Goergens 2026-08-18 8:50 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®