Re: [PATCH 0/2] vfs: report truthful FIDEDUPERANGE progress safely

On Thu, Aug 13, 2026 at 05:33:36PM +0200, Amir Goldstein wrote:
> On Wed, Aug 12, 2026 at 9:46 AM Christian Brauner <[email protected]> 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 <[email protected]>
> 
> to both patches,
> 
> Thanks,
> Amir.