The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [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