Repository navigation
Follow-up: wait-reg + send-bundle helpers, SQE_MIXED/SQ_REWIND fixes - #18
Closed
PetarKirov wants to merge 5 commits into
Closed
PetarKirov wants to merge 5 commits into
PetarKirov wants to merge 5 commits into
Conversation
SQ_REWIND rings compact unsubmitted SQEs back to slot zero after a partial submit. The previous compaction assigned SubmissionEntry values, which only copies the 64-byte SQE header. That is sufficient for ordinary SQEs, but it loses the trailing payload on SQE128 rings. Real URING_CMD128 users can rely on those bytes, so copy the full runtime slot width instead. Add a regression that combines SQ_REWIND and SQE128, forces a partial submit, and verifies the 128-byte slot payload survives compaction before the remaining SQE is submitted.
The public putWith documentation says the queue entry is cleaned before the user callback fills it, but the implementation reused the mmap slot as-is. That lets stale flags, personality ids, or address fields leak into callbacks that only set a minimal operation. Clear the full runtime slot before putWith and custom-op fill write into a reused queue slot. For SQE_MIXED staging, start the temporary SubmissionEntry from init instead of void, then route through the 128-aware put path. Also clear both physical slots when a 128-byte operation is inserted into an SQE_MIXED ring, so the second slot cannot retain stale payload. Add regression tests that poison a reused SQE slot and verify putWith/custom-op insertion zeroes untouched fields.
REGISTER_MEM_REGION wait arguments require a page-aligned user address and a page-sized backing region, and the kernel only accepts WAIT_ARG registration while the ring is disabled. The previous wrapper documented those constraints but still let callers make invalid syscalls with ordinary D slices. Validate the R_DISABLED setup flag, page-aligned address, and rounded region size before issuing the registration. Add WaitRegRegion and allocWaitRegRegion so callers can allocate mmap-backed wait entries without duplicating the page-size and munmap lifecycle details. Update the registered-wait tests to use the helper for the real round trip, and add explicit guards for non-R_DISABLED rings and stack-backed slices.
prepSendBundle now deliberately mirrors liburing: it prepares the bundled SEND operation but leaves IOSQE_BUFFER_SELECT and buf_group to the caller. That is correct for parity, but it removed the ergonomic path that selected a provided-buffer group in one call. Add prepSendBundleSelect as an explicit convenience helper. It delegates to prepSendBundle, then sets BUFFER_SELECT and buf_group for callers that want the common provided-buffer-ring setup without hand-editing the SQE. Cover both helpers with smoke tests so the liburing-compatible surface and the convenience surface stay distinct.
Mirror dub.sdl's targetPath "build" and targetName "during-test-betterC" so Makefile and dub builds produce equivalent outputs. Also fix the silly fetch version to match the resolved 1.1.1 path.
PetarKirov
marked this pull request as draft
May 20, 2026 07:05
Contributor
Author
|
I've switched this PR to draft, as I'd like to rework some of the changes. I'll change it back when I feel it's ready |
tchaloupka
pushed a commit
that referenced
this pull request
Oct 9, 2026
SQ_REWIND rings compact unsubmitted SQEs back to slot zero after a partial submit. The previous compaction assigned SubmissionEntry values, which only copies the 64-byte SQE header. That is sufficient for ordinary SQEs, but it loses the trailing payload on SQE128 rings. Real URING_CMD128 users can rely on those bytes, so copy the full runtime slot width instead. Add a regression that combines SQ_REWIND and SQE128, forces a partial submit, and verifies the 128-byte slot payload survives compaction before the remaining SQE is submitted. [tchaloupka: fix test payload offset (cmd starts at 48, not 64), copySlot via memcpy] (cherry picked from PR #18, commit 58b9df2)
tchaloupka
pushed a commit
that referenced
this pull request
Oct 9, 2026
The public putWith documentation says the queue entry is cleaned before the user callback fills it, but the implementation reused the mmap slot as-is. That lets stale flags, personality ids, or address fields leak into callbacks that only set a minimal operation. Clear the full runtime slot before putWith and custom-op fill write into a reused queue slot. For SQE_MIXED staging, start the temporary SubmissionEntry from init instead of void, then route through the 128-aware put path. Also clear both physical slots when a 128-byte operation is inserted into an SQE_MIXED ring, so the second slot cannot retain stale payload. Add regression tests that poison a reused SQE slot and verify putWith/custom-op insertion zeroes untouched fields. [tchaloupka: memset instead of byte loops; put(entry) clears only what the copy doesn't overwrite] (cherry picked from PR #18, commit 82d0109)
Owner
|
Thanks! The changes from this PR were applied to master (with some adjustments) and released in v0.6.0:
|
Contributor
Author
|
Thanks for taking this through the finish line @tchaloupka! |
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.
Summary
Follow-up polish to the liburing 2.9 / Linux 7.1-rc4 catch-up landed in #16, #17, and the two most recent master commits (85a24ad "finish liburing 2.9 / Linux 7.1-rc4 sync", ee98273 "correctness bugs in the liburing-2.9 / 7.1-rc4 work"). Two more correctness fixes against the new SQE_MIXED / SQ_REWIND storage paths, two convenience helpers that round out the new register/op surface, and one build-infra alignment with
dub.sdl.What's in here
Correctness fixes (continuation of
ee98273's pass):fix: preserve full SQE slots during rewind compaction(58b9df2) —SQ_REWINDcompaction was assigningSubmissionEntryvalues, which only copies the 64-byte SQE header and silently drops the trailing payload onSQE128rings. RealURING_CMD128users can depend on those bytes. Regression test combinesSQ_REWIND+SQE128, forces a partial submit, and verifies the 128-byte payload survives.fix: clear SQE slots before staged insertion(82d0109) — theputWithcontract documents that the SQE is cleared before the user callback fills it, but the implementation reused the mmap slot as-is, leaking stale flags, personality ids, or address fields into callbacks that only set a minimal op. Now clears the full runtimeslot (both physical slots for 128-byte ops on
SQE_MIXEDrings) beforeputWith/ custom-op fill. Regression poisons a reused slot and asserts untouched fields come back zero.New helpers:
feat: add safe wait-reg region allocation(cdbc65a) —REGISTER_MEM_REGIONwait args require a page-aligned user address, a page-sized backing region, and anR_DISABLEDring. The previous wrapper documented those constraints but still let callers makeinvalid syscalls with ordinary D slices. Adds
WaitRegRegion/allocWaitRegRegionfor mmap-backed lifecycle, validates the three preconditions, and guards against stack-backed slices / non-disabled rings in tests.feat: add buffered send-bundle convenience helper(56dd155) —prepSendBundlemirrors liburing strictly: it leavesIOSQE_BUFFER_SELECTandbuf_groupto the caller. That removed the ergonomic one-call path for provided-buffer-ring setup. AddsprepSendBundleSelectas an explicit convenience helper layered on top, with smoke tests covering both surfaces.Build:
build(make): route artifacts to build/ and align with dub.sdl(1585b4c) — Makefile now mirrorsdub.sdl'stargetPath "build"andtargetName "during-test-betterC"somakeanddubproduce equivalent outputs. Also fixes thedub fetch sillyversion to the resolved1.1.1path.Test plan
dub test -- --threads=1— all unit tests pass, including the four new regression tests (rewind+SQE128 payload, putWith slot clearing, custom-op slot clearing, wait-reg page-alignment guards) and the two new helper smoke tests.make test—during-testbuilds intobuild/and runs green.make testBC—build/during-test-betterCbuilds and runs green.dub build --config=betterC— green.6.18.17; kernel-gated tests stay gated viacheckKernelVersion().Test environment
Same as #16: NixOS 25.11, kernel
6.18.17x86_64, AMD Ryzen 9 7940HX, LDC 1.41.0 (DMD v2.111.0 frontend, LLVM 18.1.8), dub 1.39.0. Toolchain pinned by theflake.nixintroduced in #16.