Decline Direct I/O reads on a file handle after a benign verify failure - #18844
Decline Direct I/O reads on a file handle after a benign verify failure#18844mkhllr wants to merge 1 commit into
Conversation
A Direct I/O read verifies the block checksum over the caller's buffer after the read completes, to catch the buffer being modified while the read is in flight. When an application recycles its O_DIRECT read buffers across concurrent requests -- QEMU's block layer does this -- a queued read can overwrite the buffer before the previous read's verify runs. The verify then fails even though the data on disk is correct: ZFS discards the direct read, re-reads the block through the ARC, and emits an ereport.fs.zfs.dio_verify_rd. The returned data is correct and the pool stays healthy, but under concurrent load the stream of failed verifies and buffered re-reads is a real cost and can stall the workload (openzfs#18610). Once a file handle hits one of these benign failures -- a DIO read verify that failed but whose buffered re-read then succeeded, proving the on-disk data good and the buffer caller-modified -- decline Direct I/O for reads on that handle for the rest of its life and route them through the existing uncached buffered path. An application that uses a distinct buffer per request never trips the failure and keeps zero-copy Direct I/O; one that recycles buffers stops paying the verify-failure and re-read cost after the first occurrence. zfs_read reports the benign failure outward with a new UIO_DIO_CKSUM_RETRIED uio_extflg bit, set only when the buffered re-read returned success so a genuine on-disk error is never flagged and still self-heals. The Linux zpl layer records it in a small file->private_data struct and, on later reads, sets UIO_DIO_DENY so zfs_setup_direct declines Direct I/O and falls through to the uncached path. This also covers direct=always, since the decline is checked after O_DIRECT is forced on. The checksum verify and all mirror and raidz self-heal are untouched; a genuine on-disk error is still detected and repaired. This is read-side only -- a Direct I/O write verify failure already returns EIO to the application. FreeBSD defines the new bits but does not yet drive them, as VOP_READ has no open file handle to key the per-handle state on. A new ZTS test, functional/direct/dio_read_verify_decline, reads a file on a single O_DIRECT handle while a second thread rewrites the shared buffer and checks that the direct read count stays bounded once the handle declines Direct I/O. It is Linux only, matching where the bits are driven. Signed-off-by: Michael Heller <michael.heller@gmail.com> Closes: openzfs#18610
|
I measured how this interacts with the async Direct I/O path in #18684, since The short version. On that branch a Direct I/O read which fails its verify is This change does not currently reach that path. The decline arms from If #18684 lands, arming the decline from its async completion on ECKSUM looks One mechanical note for whoever merges second: both changes insert at the same |
Motivation and Context
Closes #18610.
A Direct I/O read verifies the block checksum over the caller's buffer after
the read completes, so a buffer that is modified while the read is in flight is
caught. When an application recycles its O_DIRECT read buffers across
concurrent requests (QEMU's block layer does this) a queued read can overwrite
the buffer before the previous read's verify runs. The verify then fails even
though the data on disk is correct: ZFS discards the direct read, re-reads the
block through the ARC, and emits an
ereport.fs.zfs.dio_verify_rd. The datareturned to the application is correct and the pool stays healthy, but under
concurrent load the stream of failed verifies and buffered re-reads is a real
cost and can stall the workload.
#18795 rate-limited the ereport flood, but the re-read storm itself is still
there. #18794 (a per-vdev knob to skip the read verify) was closed because
gating the verify also disables raidz and mirror self-heal.
Description
Once a file handle hits one of these benign failures, a Direct I/O read verify
that failed but whose buffered re-read then succeeded, decline Direct I/O for
reads on that handle for the rest of its life and route them through the
existing uncached buffered path. An application that uses a distinct buffer per
request never trips the failure and keeps zero-copy Direct I/O. One that
recycles buffers stops paying the verify-failure and re-read cost after the
first occurrence. The decline is keyed on the open file handle (Linux
file->private_data), so a single misbehaving handle is declined while otheropeners of the same file are unaffected.
zfs_readreports the benign failure outward with a newUIO_DIO_CKSUM_RETRIEDuio_extflgbit, set only when the buffered re-readreturned success, so a genuine on-disk error is never flagged and still
self-heals. The Linux zpl layer records it in a small
file->private_datastruct and, on later reads, sets
UIO_DIO_DENYsozfs_setup_directdeclinesDirect I/O. This also covers
direct=always, since the decline is checkedafter O_DIRECT is forced on. The checksum verify and all mirror and raidz
self-heal are untouched; a genuine on-disk error is still detected and
repaired.
This is read-side only. A Direct I/O write verify failure already returns EIO
to the application. FreeBSD defines the new bits but does not yet drive them,
as
VOP_READhas no open file handle to key the per-handle state on.The per-handle decline follows the direction settled in the #18610 discussion.
How Has This Been Tested?
New ZTS test
functional/direct/dio_read_verify_decline(Linux). It reads afile on a single O_DIRECT handle while a second thread continuously rewrites
the shared buffer (
manipulate_user_buffer), forcing read verify failures, andchecks that the direct read count stays bounded once the handle declines
Direct I/O. Over 500 requests it records 1 direct read with this change and 500
without it, so the test passes with the change and fails against master. The
existing
direct/dio_read_verifyanddirect/dio_write_verifystill pass.qemu-img bench -d 64(cold ARC each run) records 60
dio_verify_rdevents on master at-c 200000. With this change the same image and workload records 4 events at-c 200000and 3 at-c 400000, so the count stays bounded to a smallconstant as the request rate grows, since each handle arms once and declines
for its remaining life. No
dio_verify_wrevents are seen (this is read-sideonly) and the pool stays healthy.
zpool scrubreports no errors and the file contents are stableacross the run.
of a mirror (
zinject -e corrupt) still produces detected CKSUM errors thatheal from the good leg, confirming self-heal is intact on the buffered path.
make checkstyle(cstyle, shellcheck, checkbashisms) andcommitcheckareclean; the ZFS Test Suite direct group was run with the change applied.
Types of Changes
Checklist
Signed-off-by.