[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.