mirror of https://lore.kernel.org/lkml/
 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
                   ` (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®