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

Matthias Goergens <[email protected]>
Newsgroups org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Changes since v2 (<[email protected]>):

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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.