* [PATCH v5 0/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE @ 2026-09-26 4:38 Matthias Goergens 2026-09-26 4:38 ` [PATCH v5 1/2] dax: return the comparison error from dax_dedupe_file_range_compare() Matthias Goergens 2026-09-26 4:38 ` [PATCH v5 2/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens 0 siblings, 2 replies; 6+ messages in thread From: Matthias Goergens @ 2026-09-26 4:38 UTC (permalink / raw) To: linux-fsdevel, viro, brauner, jack Cc: linux-kernel, hch, djwong, david, amir73il, ansgar.loesser, Matthias Goergens Dedupe tools advance their file offsets by bytes_deduped, but when the kernel shortens a request to a block boundary it still reports the requested length. Patch 2 adds a flag under which bytes_deduped is what the filesystem actually deduplicated. Patch 1 fixes the DAX comparator, whose return value would otherwise surface through the flag. Amir, thanks for the review; both points are taken in v5. Changes since v4 (https://lore.kernel.org/linux-fsdevel/cover.1789653814.git.matthias.goergens@gmail.com/): - flags is a plain rename of reserved2, without the union (Amir). - Dropped the paragraph about old callers and old kernels (Amir). - Documented the flag's effect on bytes_deduped in the UAPI header. - Patch 1 unchanged. The fstests test follows as generic/806 v3 in reply to its v2 on the fstests list. Matthias Goergens (2): dax: return the comparison error from dax_dedupe_file_range_compare() vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE fs/dax.c | 2 +- fs/remap_range.c | 4 +++- include/uapi/linux/fs.h | 11 ++++++++++- 3 files changed, 14 insertions(+), 3 deletions(-) -- 2.55.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 1/2] dax: return the comparison error from dax_dedupe_file_range_compare() 2026-09-26 4:38 [PATCH v5 0/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens @ 2026-09-26 4:38 ` Matthias Goergens 2026-09-27 15:01 ` Darrick J. Wong 2026-09-26 4:38 ` [PATCH v5 2/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens 1 sibling, 1 reply; 6+ messages in thread From: Matthias Goergens @ 2026-09-26 4:38 UTC (permalink / raw) To: linux-fsdevel, viro, brauner, jack Cc: linux-kernel, hch, djwong, david, amir73il, ansgar.loesser, Matthias Goergens dax_dedupe_file_range_compare() returns ret, the positive result of the last iomap_iter() call, when dax_range_compare_iter() fails. The caller treats any non-zero return as the result of the range preparation, and xfs_file_remap_range() returns it as the remap result, so a failed comparison on a DAX file reports success with a small positive length instead of the error. Return the error itself. Fixes: 0e79e3736d54 ("fsdax: dedupe: iter two files at the same time") Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com> --- fs/dax.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/dax.c b/fs/dax.c index 1fbba0d21c13..de11bbbb6a38 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -2264,7 +2264,7 @@ int dax_dedupe_file_range_compare(struct inode *src, loff_t srcoff, status = dax_range_compare_iter(&src_iter, &dst_iter, min(src_iter.len, dst_iter.len), same); if (status < 0) - return ret; + return status; src_iter.status = dst_iter.status = status; } return ret; -- 2.55.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/2] dax: return the comparison error from dax_dedupe_file_range_compare() 2026-09-26 4:38 ` [PATCH v5 1/2] dax: return the comparison error from dax_dedupe_file_range_compare() Matthias Goergens @ 2026-09-27 15:01 ` Darrick J. Wong 0 siblings, 0 replies; 6+ messages in thread From: Darrick J. Wong @ 2026-09-27 15:01 UTC (permalink / raw) To: Matthias Goergens Cc: linux-fsdevel, viro, brauner, jack, linux-kernel, hch, david, amir73il, ansgar.loesser On Sat, Sep 26, 2026 at 12:38:35PM +0800, Matthias Goergens wrote: > dax_dedupe_file_range_compare() returns ret, the positive result of the > last iomap_iter() call, when dax_range_compare_iter() fails. The caller > treats any non-zero return as the result of the range preparation, and > xfs_file_remap_range() returns it as the remap result, so a failed > comparison on a DAX file reports success with a small positive length > instead of the error. > > Return the error itself. > > Fixes: 0e79e3736d54 ("fsdax: dedupe: iter two files at the same time") > Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com> That seems like a bug, I'm sorta surprised nobody noticed... Cc: <stable@vger.kernel.org> # v6.2 Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > --- > fs/dax.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/dax.c b/fs/dax.c > index 1fbba0d21c13..de11bbbb6a38 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -2264,7 +2264,7 @@ int dax_dedupe_file_range_compare(struct inode *src, loff_t srcoff, > status = dax_range_compare_iter(&src_iter, &dst_iter, > min(src_iter.len, dst_iter.len), same); > if (status < 0) > - return ret; > + return status; > src_iter.status = dst_iter.status = status; > } > return ret; > -- > 2.55.0 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 2/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE 2026-09-26 4:38 [PATCH v5 0/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens 2026-09-26 4:38 ` [PATCH v5 1/2] dax: return the comparison error from dax_dedupe_file_range_compare() Matthias Goergens @ 2026-09-26 4:38 ` Matthias Goergens 2026-09-27 7:27 ` Amir Goldstein 2026-09-27 15:02 ` Darrick J. Wong 1 sibling, 2 replies; 6+ messages in thread From: Matthias Goergens @ 2026-09-26 4:38 UTC (permalink / raw) To: linux-fsdevel, viro, brauner, jack Cc: linux-kernel, hch, djwong, david, amir73il, ansgar.loesser, Matthias Goergens Deduplication tools such as duperemove, bees and rmlint advance their file offsets by the bytes_deduped the kernel returns for each FIDEDUPERANGE call. vfs_dedupe_file_range() passes REMAP_FILE_CAN_SHORTEN, so generic_remap_checks() may round the length down to a block multiple, but the ioctl still reports the requested length in bytes_deduped. The caller cannot tell that the tail of its request was left alone: rmlint 2.10.3 on btrfs (4 KiB blocks) deduping a 100000-byte file against a 250000-byte file with the same prefix is told bytes_deduped=100000 with status SAME while only 98304 bytes were actually shared; its loop ends and it reports the pair fully deduplicated. duperemove and bees advance the same way, and jdupes advances by its own requested length without reading the field; all of them skip such a tail. Add FILE_DEDUPE_RANGE_REPORT_PROGRESS for file_dedupe_range.flags: with it, bytes_deduped is the length the filesystem actually deduplicated when status is FILE_DEDUPE_RANGE_SAME, and 0 on FILE_DEDUPE_RANGE_DIFFERS or error. Callers advance by it as today, but must treat 0 as "stop or subdivide", not retry unchanged. One cause of SAME with 0 is a sub-block request that does not end at both files' EOF, which the generic range preparation shortens to nothing. On DIFFERS there is no sound progress or mismatch offset to report, so 0 leaves subdividing to the caller, as rmlint already does. The default cannot change: commit 4a57a8400075 ("vf/remap: return the amount of bytes actually deduplicated") did exactly that and was reverted the next day; among deployed callers, duperemove re-queues a request while its status is 0 and would re-issue the same sub-block request forever. Without the flag nothing changes. 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 | 4 +++- include/uapi/linux/fs.h | 11 ++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/fs/remap_range.c b/fs/remap_range.c index 26afbbbfb10c..63f1b6f90c16 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 34c6f219462a..c61bc98909cc 100644 --- a/include/uapi/linux/fs.h +++ b/include/uapi/linux/fs.h @@ -178,13 +178,22 @@ struct file_dedupe_range_info { __u32 reserved; /* must be zero */ }; +/* flags for struct file_dedupe_range */ +/* + * Without this flag, bytes_deduped is the requested length on success, + * even if the filesystem deduplicated fewer bytes (e.g. after shortening + * the request to a block boundary). With this flag, bytes_deduped is + * the number of bytes actually deduplicated. + */ +#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] 6+ messages in thread
* Re: [PATCH v5 2/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE 2026-09-26 4:38 ` [PATCH v5 2/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens @ 2026-09-27 7:27 ` Amir Goldstein 2026-09-27 15:02 ` Darrick J. Wong 1 sibling, 0 replies; 6+ messages in thread From: Amir Goldstein @ 2026-09-27 7:27 UTC (permalink / raw) To: Matthias Goergens Cc: linux-fsdevel, viro, brauner, jack, linux-kernel, hch, djwong, david, ansgar.loesser On Sat, Sep 26, 2026 at 6:38 AM Matthias Goergens <matthias.goergens@gmail.com> wrote: > > Deduplication tools such as duperemove, bees and rmlint advance their > file offsets by the bytes_deduped the kernel returns for each > FIDEDUPERANGE call. > > vfs_dedupe_file_range() passes REMAP_FILE_CAN_SHORTEN, so > generic_remap_checks() may round the length down to a block multiple, > but the ioctl still reports the requested length in bytes_deduped. The > caller cannot tell that the tail of its request was left alone: rmlint > 2.10.3 on btrfs (4 KiB blocks) deduping a 100000-byte file against a > 250000-byte file with the same prefix is told bytes_deduped=100000 > with status SAME while only 98304 bytes were actually shared; its loop > ends and it reports the pair fully deduplicated. duperemove and bees > advance the same way, and jdupes advances by its own requested length > without reading the field; all of them skip such a tail. > > Add FILE_DEDUPE_RANGE_REPORT_PROGRESS for file_dedupe_range.flags: > with it, bytes_deduped is the length the filesystem actually > deduplicated when status is FILE_DEDUPE_RANGE_SAME, and 0 on > FILE_DEDUPE_RANGE_DIFFERS or error. Callers advance by it as today, > but must treat 0 as "stop or subdivide", not retry unchanged. One > cause of SAME with 0 is a sub-block request that does not end at both > files' EOF, which the generic range preparation shortens to nothing. > On DIFFERS there is no sound progress or mismatch offset to report, so > 0 leaves subdividing to the caller, as rmlint already does. > > The default cannot change: commit 4a57a8400075 ("vf/remap: return the > amount of bytes actually deduplicated") did exactly that and was > reverted the next day; among deployed callers, duperemove re-queues a > request while its status is 0 and would re-issue the same sub-block > request forever. > > Without the flag nothing changes. > > 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> Reviewed-by: Amir Goldstein <amir73il@gmail.com> > --- > fs/remap_range.c | 4 +++- > include/uapi/linux/fs.h | 11 ++++++++++- > 2 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/fs/remap_range.c b/fs/remap_range.c > index 26afbbbfb10c..63f1b6f90c16 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 34c6f219462a..c61bc98909cc 100644 > --- a/include/uapi/linux/fs.h > +++ b/include/uapi/linux/fs.h > @@ -178,13 +178,22 @@ struct file_dedupe_range_info { > __u32 reserved; /* must be zero */ > }; > > +/* flags for struct file_dedupe_range */ > +/* > + * Without this flag, bytes_deduped is the requested length on success, > + * even if the filesystem deduplicated fewer bytes (e.g. after shortening > + * the request to a block boundary). With this flag, bytes_deduped is > + * the number of bytes actually deduplicated. > + */ > +#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] 6+ messages in thread
* Re: [PATCH v5 2/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE 2026-09-26 4:38 ` [PATCH v5 2/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens 2026-09-27 7:27 ` Amir Goldstein @ 2026-09-27 15:02 ` Darrick J. Wong 1 sibling, 0 replies; 6+ messages in thread From: Darrick J. Wong @ 2026-09-27 15:02 UTC (permalink / raw) To: Matthias Goergens Cc: linux-fsdevel, viro, brauner, jack, linux-kernel, hch, david, amir73il, ansgar.loesser On Sat, Sep 26, 2026 at 12:38:36PM +0800, Matthias Goergens wrote: > Deduplication tools such as duperemove, bees and rmlint advance their > file offsets by the bytes_deduped the kernel returns for each > FIDEDUPERANGE call. > > vfs_dedupe_file_range() passes REMAP_FILE_CAN_SHORTEN, so > generic_remap_checks() may round the length down to a block multiple, > but the ioctl still reports the requested length in bytes_deduped. The > caller cannot tell that the tail of its request was left alone: rmlint > 2.10.3 on btrfs (4 KiB blocks) deduping a 100000-byte file against a > 250000-byte file with the same prefix is told bytes_deduped=100000 > with status SAME while only 98304 bytes were actually shared; its loop > ends and it reports the pair fully deduplicated. duperemove and bees > advance the same way, and jdupes advances by its own requested length > without reading the field; all of them skip such a tail. > > Add FILE_DEDUPE_RANGE_REPORT_PROGRESS for file_dedupe_range.flags: > with it, bytes_deduped is the length the filesystem actually > deduplicated when status is FILE_DEDUPE_RANGE_SAME, and 0 on > FILE_DEDUPE_RANGE_DIFFERS or error. Callers advance by it as today, > but must treat 0 as "stop or subdivide", not retry unchanged. One > cause of SAME with 0 is a sub-block request that does not end at both > files' EOF, which the generic range preparation shortens to nothing. > On DIFFERS there is no sound progress or mismatch offset to report, so > 0 leaves subdividing to the caller, as rmlint already does. > > The default cannot change: commit 4a57a8400075 ("vf/remap: return the > amount of bytes actually deduplicated") did exactly that and was > reverted the next day; among deployed callers, duperemove re-queues a > request while its status is 0 and would re-issue the same sub-block > request forever. > > Without the flag nothing changes. > > 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 | 4 +++- > include/uapi/linux/fs.h | 11 ++++++++++- > 2 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/fs/remap_range.c b/fs/remap_range.c > index 26afbbbfb10c..63f1b6f90c16 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; Very good! I'm glad this is resolved now. Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > else > info->bytes_deduped = len; > > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h > index 34c6f219462a..c61bc98909cc 100644 > --- a/include/uapi/linux/fs.h > +++ b/include/uapi/linux/fs.h > @@ -178,13 +178,22 @@ struct file_dedupe_range_info { > __u32 reserved; /* must be zero */ > }; > > +/* flags for struct file_dedupe_range */ > +/* > + * Without this flag, bytes_deduped is the requested length on success, > + * even if the filesystem deduplicated fewer bytes (e.g. after shortening > + * the request to a block boundary). With this flag, bytes_deduped is > + * the number of bytes actually deduplicated. > + */ > +#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] 6+ messages in thread
end of thread, other threads:[~2026-09-27 15:02 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-26 4:38 [PATCH v5 0/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens 2026-09-26 4:38 ` [PATCH v5 1/2] dax: return the comparison error from dax_dedupe_file_range_compare() Matthias Goergens 2026-09-27 15:01 ` Darrick J. Wong 2026-09-26 4:38 ` [PATCH v5 2/2] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE Matthias Goergens 2026-09-27 7:27 ` Amir Goldstein 2026-09-27 15:02 ` 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
all inboxes | Powered by JetHome®