[PATCH v3 0/1] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE

Matthias Goergens posted 1 patch 1 month, 1 week ago
There is a newer version of this series
[PATCH v3 0/1] vfs: add FILE_DEDUPE_RANGE_REPORT_PROGRESS flag to FIDEDUPERANGE
Posted by Matthias Goergens 1 month, 1 week ago
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.