Re: [PATCH v4 2/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE

[Amir Goldstein]

On Fri, Sep 18, 2026 at 12:37 PM Matthias Goergens
<matthias.goergens@xxxxxxxxx> wrote:
>
> Deduplication tools such as duperemove, bees and rmlint find matching
> ranges in two files, call FIDEDUPERANGE on each match and advance their
> file offsets by the bytes_deduped the kernel returns. They rely on that
> value to know where to continue.
>
> The kernel does not give them a value they can act on.
> vfs_dedupe_file_range() passes REMAP_FILE_CAN_SHORTEN, so
> generic_remap_checks() rounds a request whose length is not block
> aligned down to a block multiple unless it ends at both files' EOF, but
> the ioctl then reports the length it asked for (the request, capped at 1
> GiB per call) in bytes_deduped, not the shortened one. The caller cannot
> tell that the tail of its request was left alone. Measured with rmlint
> 2.10.3 on btrfs with 4 KiB blocks: rmlint --dedupe on a 100000-byte file
> against a 250000-byte file with the same prefix issues one call and is
> told bytes_deduped=100000 with status SAME, while FIEMAP shows 24 shared
> blocks, 98304 bytes. rmlint's loop ends because bytes_deduped equals the
> file size, so it reports the pair fully deduplicated with 1696 bytes not
> shared. duperemove (process_dedupes()) and bees advance the same way,
> and jdupes advances by its own requested length without reading the
> field, so all of them skip such a tail without noticing.
>
> Add a flag that a caller sets to get a value it can act on. With
> FILE_DEDUPE_RANGE_REPORT_PROGRESS set in file_dedupe_range.flags,
> bytes_deduped in each destination's info is the length the filesystem
> reports as deduplicated when status is FILE_DEDUPE_RANGE_SAME, and 0
> when status is FILE_DEDUPE_RANGE_DIFFERS or an error. A caller advances
> by it as it advances today, and must treat 0 as "stop or subdivide"
> rather than retry unchanged. One cause of a SAME result of 0 is a
> request shorter than a block that does not end at both files' EOF,
> which the generic range preparation shortens to nothing before any
> remapping. On DIFFERS the kernel has no usable progress or mismatch
> offset to report, so it reports 0 and leaves subdividing the range to
> the caller, as rmlint already does.
>
> The default cannot change. Reporting the shortened length by default
> was done once, in commit 4a57a8400075 ("vf/remap: return the amount of
> bytes actually deduplicated"), and reverted the next day because
> generic/517 expected the old value and the effect on deployed callers
> was unknown. That effect is now known: duperemove re-queues a request
> while its status is 0 and has no check for bytes_deduped == 0, so a 0
> with status SAME on a sub-block request would make it re-issue the
> same request forever.
>
> Without the flag nothing changes. Unknown flag bits are rejected. The
> flags field is an anonymous union with the old reserved2 name, so
> existing source that spells .reserved2 keeps compiling and the layout

I don't why you think that old programs compile should not break with
new headers. I don't think this is a promise from Linux UAPI.

> is unchanged. Kernels since 4.5, when the VFS took over the ioctl,
> reject a non-zero field with -EINVAL, so a new caller cannot get the
> old semantics by accident and can fall back to a call without the
> flag.
>
> Suggested-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> Link: https://lore.kernel.org/linux-fsdevel/20260805071414.3414870-1-matthias.goergens@xxxxxxxxx/
> Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>
> ---
> See the cover letter for the changes since v3. Note for C++ callers: a
> positional initialiser of struct file_dedupe_range now needs braces
> around the union member under -Wmissing-braces; designated initialisers
> with either .reserved2 or .flags are unaffected.

I think you created a problem yourself and then solved it.
The union is not needed.

>
>  fs/remap_range.c        | 4 +++-
>  include/uapi/linux/fs.h | 8 +++++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/fs/remap_range.c b/fs/remap_range.c
> index 26afbbbfb10c2..63f1b6f90c161 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,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 (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 34c6f219462a5..4e40855e1ef8e 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -178,13 +178,19 @@ struct file_dedupe_range_info {
>         __u32 reserved;         /* must be zero */
>  };
>
> +/* flags for struct file_dedupe_range */
> +#define FILE_DEDUPE_RANGE_REPORT_PROGRESS (1U << 0)
> +

This thought has crossed my mind - perhaps should be dismissed.

These are vfs internal flags to the filesystem dedupe/remap method:

#define REMAP_FILE_DEDUP                (1 << 0)
#define REMAP_FILE_CAN_SHORTEN          (1 << 1)

The above is a dedupe ioctl UAPI flag, which is not needed to be passed
to the filesystem (right?), so it's fine that they live in two
different namespaces.

In the future, it could be that a dedupe UAPI flag will need to be propagated
into the filesystem. Maybe even a flag that would be common to clone and
dedupe.

For now, using bit 1 for both internal and uapi is fine and I don't see
a reason to change that, just a point to consider for the future.


>  /* 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 */
> +       union {
> +               __u32 reserved2;        /* must be zero (older callers) */
> +               __u32 flags;            /* in - FILE_DEDUPE_RANGE_* flags */
> +       };
>         struct file_dedupe_range_info info[];
>  };

Please drop the union.
The text referring to "older callers" in this context is not useful.
It is quite obvious that the old kernel will not accept new flags in a
properly written UAPI.

Thanks,
Amir.