Skip to content

Decline Direct I/O reads on a file handle after a benign verify failure - #18844

Open
mkhllr wants to merge 1 commit into
openzfs:masterfrom
mkhllr:dio-read-fd-flag
Open

Decline Direct I/O reads on a file handle after a benign verify failure#18844
mkhllr wants to merge 1 commit into
openzfs:masterfrom
mkhllr:dio-read-fd-flag

Conversation

@mkhllr

@mkhllr mkhllr commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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 data
returned 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 other
openers of the same file are unaffected.

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

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 a
file on a single O_DIRECT handle while a second thread continuously rewrites
the shared buffer (manipulate_user_buffer), forcing read verify failures, and
checks 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_verify and direct/dio_write_verify still pass.

  • Storm: reading a recycled buffer over an lz4 image with qemu-img bench -d 64
    (cold ARC each run) records 60 dio_verify_rd events on master at
    -c 200000. With this change the same image and workload records 4 events at
    -c 200000 and 3 at -c 400000, so the count stays bounded to a small
    constant as the request rate grows, since each handle arms once and declines
    for its remaining life. No dio_verify_wr events are seen (this is read-side
    only) and the pool stays healthy.
  • Integrity: zpool scrub reports no errors and the file contents are stable
    across the run.
  • Self-heal: with the change loaded, injecting checksum corruption on one leg
    of a mirror (zinject -e corrupt) still produces detected CKSUM errors that
    heal from the good leg, confirming self-heal is intact on the buffered path.
  • make checkstyle (cstyle, shellcheck, checkbashisms) and commitcheck are
    clean; the ZFS Test Suite direct group was run with the change applied.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)

Checklist

  • My code follows the OpenZFS code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

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
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 23, 2026
@behlendorf
behlendorf self-requested a review July 24, 2026 22:27
@mkhllr mkhllr mentioned this pull request Jul 27, 2026
14 tasks
@mkhllr

mkhllr commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

I measured how this interacts with the async Direct I/O path in #18684, since
both changes touch zpl_iter_read(). Full numbers are in
#18684 (comment).

The short version. On that branch a Direct I/O read which fails its verify is
completed in ZIO taskq context without the submitter's mm, so it cannot re-read
through the ARC and returns EIO. With concurrent libaio reads at depth 16 all
landing in one buffer while another thread rewrites it, 403 of 3200 requests
came back EIO on that branch, where the same workload on master returns correct
data every time. That happens with or without this change.

This change does not currently reach that path. The decline arms from
UIO_DIO_CKSUM_RETRIED, which zfs_read() sets once the buffered re-read has
come back clean, and the async completion has no such re-read, so the flag is
never set and the handle never declines. None of it affects master, where
zfs_read() is the only read path: with async disabled the same build takes
verify failures from 20 down to 1.

If #18684 lands, arming the decline from its async completion on ECKSUM looks
like the natural extension, and it is the piece that would keep a recycling
reader working there: one EIO followed by buffered reads that succeed, rather
than a permanent stream of them for the life of the handle. Declining stays
safe when the checksum failure is genuine, since the buffered path still
detects and self-heals real corruption. I have offered to write that against
the branch.

One mechanical note for whoever merges second: both changes insert at the same
point in zpl_iter_read(), so it is a small rebase either way. The decline
check wants to run first, with a declined handle skipping async dispatch, since
a handle that has stopped doing Direct I/O should not take the async Direct I/O
path either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Code Review Needed Ready for review and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spurious dio_verify_rd on concurrent DIO reads of lz4-compressed records (read-path only, non-corrupting)

2 participants