* [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
` (2 more replies)
0 siblings, 3 replies; 6+ 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] 6+ 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 2026-08-12 7:46 ` [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely Christian Brauner 2 siblings, 0 replies; 6+ 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 related [flat|nested] 6+ 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 2 siblings, 0 replies; 6+ 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 related [flat|nested] 6+ 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 2 siblings, 1 reply; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread
end of thread, other threads:[~2026-08-13 20:09 UTC | newest] Thread overview: 6+ 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox