dedupe: complete a request that deduped nothing, rather than re-queueing it - #416
Open
matthiasgoergens wants to merge 1 commit into
Open
dedupe: complete a request that deduped nothing, rather than re-queueing it#416matthiasgoergens wants to merge 1 commit into
matthiasgoergens wants to merge 1 commit into
Conversation
…ing it process_dedupes() advances req_loff and req_total by info->bytes_deduped and re-queues the request whenever the ioctl succeeded and req_total has not yet reached orig_len. If a successful FIDEDUPERANGE ever reports zero bytes deduped, that re-queues a request with identical parameters, and the loop in dedupe_extents() never terminates. The kernel cannot produce that today: vfs_dedupe_file_range() reports the requested length rather than the amount actually deduped, so bytes_deduped is non-zero whenever status is 0. That is itself a bug -- generic_remap_check_len() silently shortens an unaligned length away from EOF, and the loop records "len" where "deduped" holds the real figure. It was fixed in 4a57a8400075 and reverted the next day in b926f2adb044, partly out of concern for how userspace would react to a truthful zero. So this guard is unreachable at present and is deliberately defensive. It costs one condition, and it means duperemove is not the reason the kernel side stays unfixed. req_total already holds what was really deduped, so completing the request reports the correct figure to the caller rather than looping.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #415.
process_dedupes()re-queues a request whenever the ioctl succeeded andreq_totalhas not reachedorig_len. A successfulFIDEDUPERANGEreporting zero bytes therefore resubmits an identical request forever.req_totalalready holds what was really deduplicated, so completing the request reports the correct figure to the caller rather than spinning.This is a workaround for a kernel bug rather than a live failure in duperemove:
vfs_dedupe_file_range()reports the requested length instead of the amount actually deduplicated, sobytes_dedupedis currently never zero whenstatusis 0. Measured on a kernel with that fixed (btrfs, v7.2-rc5 in a VM), a sub-block request away from EOF returnsbytes_deduped=0, status=0, and a plainrmlint --dedupeelicits exactly that in ordinary use. duperemove escapes today only becauseset_aligned_same_length()always rounds down tofs_blocksize— I traced a-drrun and a--dedupe-options=partial -b 4096run and every request was aligned or reached EOF.Worth having regardless of what the kernel does, since duperemove will keep running on kernels without the fix — every LTS since 4.19 over-reports.
No behaviour change on current kernels: builds clean, and a
-drrun over a mixed corpus produces identical output to master.