Repository navigation
fix(iouring): fix the accept-direct SQE and gate fixed files behind an explicit opt-in (#541) - #553
Merged
Conversation
…n explicit opt-in (#541) prepMultishotAcceptDirect built on prepMultishotAccept, which sets SOCK_NONBLOCK|SOCK_CLOEXEC, and then wrote IORING_FILE_INDEX_ALLOC into file_index. io_accept_prep rejects a fixed file slot combined with SOCK_CLOEXEC with -EINVAL — a direct descriptor lives in the ring's file table, not the process fd table, so close-on-exec is meaningless for it. So the runtime probe failed on every kernel, and the failure was attributed to the kernel: "the kernel registered files but refuses ACCEPT_DIRECT (seen on 6.6.10-cix aarch64). Treat as unsupported." It reproduces on 7.0.12 aarch64 too, because it was our SQE. Dropping SOCK_CLOEXEC flips fixed_files=false to true on the same kernel and container with nothing else changed. That one-line fix would have been a trap. cs.fixedFile has never been true anywhere, so every branch gated on it is unexecuted code, and an audit of those branches — four surfaces, each finding attacked by an independent reviewer — found eleven defects and refuted none. The worst is the DEFAULT receive path: prepRecv has no fixed-file variant and never touches the flags byte, and the single-shot branch is the default (the buffer ring is only built with CELERIS_IOURING_MULTISHOT_RECV=1), so every connection would arm a recv against a raw fd equal to its slot index. Low slots fail with -ENOTSOCK; higher ones collide with real sockets in the process — another worker's listen fd, a driver database connection — and the ring reads their bytes. prepCloseDirect also omits the +1 its own comment documents, and w.conns is indexed by two conflicting namespaces. So the SQE is fixed and the feature is now held off by an explicit gate instead. Leaving it to the -EINVAL was not safe: that depends on a kernel continuing to reject a malformed SQE, and one that tolerated it would silently switch the whole broken path on. celeris#541 carries the readiness checklist; finishing the feature means working through it and deleting the gate, not finding the flags bug and assuming that was all. Also fixes a log that had become actively misleading: both startup lines reported tier.SupportsFixedFiles(), the CAPABILITY, so they printed fixed_files=true while the feature was off. They now report the effective value through the same helper the gate uses, so the two cannot disagree. Verified: default run logs fixed_files=false and no warning; with CELERIS_IOURING_FIXED_FILES=1 it logs fixed_files=true plus a per-worker warning that the path is incomplete. The test's load-bearing case is tier support WITHOUT the opt-in — exactly what the SQE fix newly makes reachable — and it fails without the gate.
FumingPower3925
deleted the
fix/iouring-fixed-files-explicit-gate-541
branch
September 9, 2026 21:25
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.
Refs #541 — deliberately does not close it; that issue is now the readiness checklist.
The bug
prepMultishotAcceptDirectbuilt onprepMultishotAccept, which setsSOCK_NONBLOCK|SOCK_CLOEXEC, then wroteIORING_FILE_INDEX_ALLOCintofile_index.io_accept_preprejects a fixed file slot combined withSOCK_CLOEXECwith-EINVAL: a directdescriptor lives in the ring's file table, not the process fd table, so close-on-exec is meaningless
for it.
The probe therefore failed on every kernel, and the code blamed the kernel:
It reproduces on 7.0.12 aarch64 as well. Dropping
SOCK_CLOEXECflipsfixed_files=falsetotrueon the same kernel and container with nothing else changed.
Why the one-line fix alone would have been a trap
cs.fixedFilehas never been true anywhere, so every branch gated on it is unexecuted code. Afour-surface audit, each finding attacked by an independent reviewer, found eleven defects and
refuted none. The worst is the default receive path:
prepRecvhas no fixed-file variant andnever touches the flags byte, and the single-shot branch is the default (the buffer ring is only
built with
CELERIS_IOURING_MULTISHOT_RECV=1). Every connection would arm a recv against a raw fdequal to its slot index — low slots give
-ENOTSOCK, higher ones collide with real sockets in theprocess and the ring reads their bytes.
prepCloseDirectalso omits the+1its own commentdocuments, and
w.connsis indexed by two conflicting namespaces.The decision
Fix the SQE, and hold the feature off with an explicit gate rather than an accident.
Leaving it to the
-EINVALwas not safe: it depends on a kernel continuing to reject a malformedSQE, and one that tolerated it would silently switch the whole broken path on. Finishing the feature
now means working through #541's checklist and deleting the gate — not finding the flags bug and
assuming that was all.
Also: a log that had become misleading
Both startup lines reported
tier.SupportsFixedFiles(), the capability, so they printedfixed_files=truewhile the feature was off. They now report the effective value through the samehelper the gate uses, so the two cannot disagree. This caught me during verification — I read
fixed_files=trueand thought my gate had failed.Verified
fixed_files=false, no warningCELERIS_IOURING_FIXED_FILES=1fixed_files=true+ per-worker "this path is INCOMPLETE" warningThe test's load-bearing case is tier support without the opt-in — precisely what the SQE fix
newly makes reachable — and it fails without the gate:
Full
engine/iouringsuite green (100.7s), lint clean.