ZIL: Store aligned full-block sync writes indirectly#18746
Draft
MorganaFuture wants to merge 2 commits into
Draft
ZIL: Store aligned full-block sync writes indirectly#18746MorganaFuture wants to merge 2 commits into
MorganaFuture wants to merge 2 commits into
Conversation
zil_write_state() stores every synchronous write smaller than zfs_immediate_write_sz in the ZIL (WR_COPIED, or WR_NEED_COPY for O_DIRECT), even when the write exactly covers an aligned block. Such a write then reaches the pool twice: once into the log block and again when the TXG commits. On a zvol with a small volblocksize this doubles the write volume of every synchronous full-block write. A write that exactly covers one or more aligned blocks whose size can no longer change is always safe to store indirectly. dmu_sync() performs no read-modify-write, the block is not rewritten at commit, and a later rewrite of the same block cannot inflate it, so the data is written to the pool only once. Pass the write offset and a new blk_fixed flag to zil_write_state() and, when the write is an aligned multiple of a fixed block size, choose WR_INDIRECT regardless of zfs_immediate_write_sz. Volumes set blk_fixed=B_TRUE, since volblocksize is immutable; the file path passes B_FALSE and keeps its current behavior. The dedicated-SLOG and special-vdev decisions are left in place, so a log device still keeps these writes in the ZIL for latency. Measured on a 16K-volblocksize zvol with sync=always, 64 aligned 16K O_DIRECT sync writes: zil_itx_indirect_count rises from 0 to 63, and the duplicate commit-time write is gone. Partial writes and pools with a dedicated SLOG are unaffected. Signed-off-by: MorganaFuture <103630661+MorganaFuture@users.noreply.github.com> Closes openzfs#17677
6d3952f to
7dc9b1a
Compare
Verify that a synchronous write covering an aligned volblocksize block on a zvol is stored indirectly: full-block sync writes must increment the objset's zil_itx_indirect_count, sub-block writes must not, and a dedicated SLOG must keep even full-block writes in the log. Because this changes which writes are logged indirectly, the test also freezes a dedicated pool, writes full blocks, then exports and re-imports it so the WR_INDIRECT records are claimed and replayed; the zvol contents must be byte-for-byte identical afterwards. This guards the actual risk of the change: an indirectly logged block that cannot be read back at import. The test drives aligned block writes with dd oflag=direct, which is Linux-only, so it is recorded as an expected skip on FreeBSD. Exercises the fix for the zvol write amplification in openzfs#17677. Signed-off-by: MorganaFuture <103630661+MorganaFuture@users.noreply.github.com> Closes openzfs#17677
7dc9b1a to
620a4bc
Compare
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
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.
Motivation and Context
Closes #17677.
On a zvol with
sync=always(e.g. a VM disk image), a synchronous write thatexactly covers an aligned volblocksize block is copied into the ZIL
(
WR_COPIED) and then written again at TXG commit — the data hits the pooltwice. For small-volblocksize zvols under a sync-heavy workload this doubles the
write volume.
Description
Store aligned full-block sync writes indirectly (
WR_INDIRECT) so the blockreaches the pool only once.
zil_write_state()gainsoffset/blk_fixedparameters; for a zvol (fixed volblocksize) a write whose offset and length are
whole-block aligned takes the indirect path via
dmu_sync(), and the log recordjust references the block.
The existing heuristics are preserved for everything else: a dedicated SLOG
still keeps full-block writes in the log, sub-block and misaligned writes are
unchanged, and the ZPL path is untouched (
blk_fixed = B_FALSE).How Has This Been Tested?
New
zvol_misc_fullblockZTS case: full-block sync writes increase the objset'szil_itx_indirect_count, sub-block writes do not, and a dedicated SLOG keepseven full-block writes in the log. Because this changes which writes are logged
indirectly, the test also freezes a dedicated pool, writes full blocks, then
exports and re-imports it and checks the zvol is byte-for-byte identical — so the
WR_INDIRECTrecords are actually claimed and replayed correctly. Ran on Linux(Ubuntu, aarch64,
--enable-debug).This is a draft — I'd like reviewer guidance on the design before polishing:
so it overrides
zfs_immediate_write_sz/logbias=latency. On a smallvolblocksize zvol without a SLOG this trades halved write volume for more (and
more random) main-pool IOPS. Should it be gated behind a module parameter or a
size floor?
(
dmu_syncdisables dedup). Are those acceptable, or should the predicate benarrowed?
Types of changes
Checklist:
Signed-off-by.