Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 56 additions & 1 deletion crates/windows-ioring-sys/CHECKLIST.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ own dated groups, M8-M10
and M15-M18
[here](COMPLETED-CHECKLIST.md#moved-2026-08-30----m15-through-m18-the-testing-strategy-response-to-eight-defects).

**Only `M6+` remains, and it is parked rather than pending** -- see the `M{n}+` convention: it is gated work
**`M20` is pending; `M6+` is parked rather than pending** -- see the `M{n}+` convention: it is gated work
with no current obligation, not an unfinished milestone. `M19` below is complete and awaits archival with the
next group.

Expand Down Expand Up @@ -74,6 +74,61 @@ the API whose breaking change 0.2.0 is being cut for, and it is reachable with n
and D-45 is added to its table of shipped defects of this shape.
**Swept the count restatements too:** that file said "three defects" in four places and is now four, which
is the restatement drift the repository's own conventions warn about.

## M20 -- Repairs from the 2026-08-30 NUMA-sharding measurement

Queued from
[DESIGN-SESSION-2026-08-30-numa-sharded-io-execution-domains.md](../../design-sessions/DESIGN-SESSION-2026-08-30-numa-sharded-io-execution-domains.md),
which measured a shipping ARM laptop and found the L3 heuristic's justification does not hold there. These
are documentation and policy repairs only; **no defect was found in `ring_copy`** -- `Policy::select`
already degrades to a whole-machine domain and reports it, which an initial reading of the session got
wrong and the code corrected.

The design questions the session opened are deliberately **not** queued here. It is still open, and its
conclusions belong to it until it converges.

- [ ] **M20.1** -- Correct the L3 heuristic's justification in
[DESIGN-NOTES.md](DESIGN-NOTES.md). It currently says the last-level-cache domain "is meaningful on Intel
and ARM too, where the NUMA node often is not." **Measured counter-example:** a Snapdragon X2 Elite
(X2E80100, Qualcomm Oryon; 12 cores, no SMT) reports **zero** L3 cache domains -- `L3CacheSize = 0` from
WMI, and `GetLogicalProcessorInformationEx` yields L1 and L2 only, with L2 forming two domains of six
processors that agree with the two `Module` domains. The claim that L3 is meaningful on ARM is false on a
shipping part. Keep the finding that L3 beats the NUMA node; restate the rule as **the outermost cache
level that actually partitions the machine**, and say what happens when no such level is reported. Sweep
every restatement of the L3 rule per the repository's blast-radius convention, including the README and
`ring_copy`'s `policy.rs` doc comments, not only the one sentence quoted above.

- [ ] **M20.2** -- Record the measurement itself as a decision in
[DESIGN-NOTES.md](DESIGN-NOTES.md), so the next reader inherits the datapoint rather than re-measuring:
an ARM Windows laptop with no L3 at all, and zero `Win32_NumaNode` instances, is the *common* consumer
shape now rather than an exotic one. This is the ARM sibling of the existing zero-NUMA-node VM
observation and belongs beside it.

- [ ] **M20.3** -- Make `ring_copy`'s degraded-fallback path observable in a test. The whole-machine
fallback in `Policy::select` is the branch every zero-relation machine takes, and this session was the
first time anyone confirmed it runs. Assert both halves on a synthetic topology: that a policy whose
relation is absent returns one whole-machine domain with `degraded = true`, and that a policy whose
relation is present is **not** flagged degraded -- the second half matters because a test of the first
alone would pass against a function that always degrades.

- [ ] **M20.4** -- Correct "What is not reachable" in [DESIGN-NOTES.md](DESIGN-NOTES.md). It says mapping a
file handle to its backing device's NUMA node "has no clean user-mode path" and "means walking volume to
disk to device instance and reading `DEVPKEY_Device_Numa_Node`". **That is wrong on mechanism.**
`FSCTL_QUERY_VOLUME_NUMA_INFO` is documented in the IFS docs, takes a handle to a **file or directory**
directly, and returns `FSCTL_QUERY_VOLUME_NUMA_INFO_OUTPUT { ULONG NumaNode }`. No walking required.
The **conclusion survives for a better reason**, and that is the point of the rewrite: the documented
meaning is the node the *volume* resides on, not where the file's extents live, so it cannot answer
"which ring should this file's I/O go to" even when it succeeds; and it is absent whenever the device
advertised no proximity domain. Record `GetNumaNodeNumberFromHandle` as the other path -- a wrapper over
`NtQueryInformationFile` with `FileNumaNodeInformation` (class 53) -- and that PHNT and the WDK mark that
class **reserved for system use**, so this crate must not build on it. State plainly that no published
measurement of either call succeeding on an ordinary NTFS data file could be found, and cite
[file-handle-numa-spike.rs](design-sessions/spikes/file-handle-numa-spike.rs) as the unrun instrument.
**Blocked on hardware, not on a decision:** settling it needs a multi-node machine with storage whose
PDO advertises a proximity domain. Write the correction now (the documentation defect is independent of
the measurement) and leave the empirical question open.


## M6+ -- Model B: explicit-thread delivery and affinity

Parked, not pending. Deferred by the engineer's explicit direction during the 2026-08-22 design session,
Expand Down
19 changes: 18 additions & 1 deletion crates/windows-ioring-sys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,24 @@ windows-threadpool-sys = { version = "0.1.3", path = "../windows-threadpool-sys"
# crate rather than raw `GetLogicalProcessorInformationEx` calls. M7's
# ring-copy sample additionally needs the `serde` feature, to load a fed-in
# topology description (D-10 in windows-topology-sys's DESIGN-NOTES.md).
windows-topology-sys = { version = "0.2.0", path = "../windows-topology-sys", features = [
#
# **Path-only, with no `version`, deliberately.** Cargo omits a versionless
# dev-dependency from the published manifest entirely -- verified by packaging
# this crate and reading the result -- and that is the right outcome here. A
# `version` would have been published as a requirement nothing ever checks:
# publish verification builds the library alone, not examples or tests, so the
# pin was never exercised at the one moment it was supposed to matter.
#
# What it *did* do was break the workspace. Cargo enforces `path` + `version`
# agreement at every build regardless of dependency kind, so this pin reading
# "0.1.0" against a `windows-topology-sys` bumped to 0.2.0 failed `cargo
# metadata` for every crate here -- turning a release of an unrelated crate into
# a red `main`. Deleting it removes the last of those.
#
# The examples still build from a checkout, which is where anyone reads them.
# They are not buildable from the packaged tarball, which they were only ever
# incidentally.
windows-topology-sys = { path = "../windows-topology-sys", features = [
"serde",
] }
# M7's ring-copy sample deserializes a fed-in topology description (--topology).
Expand Down
2 changes: 1 addition & 1 deletion crates/windows-ioring-sys/PLANS.md

Large diffs are not rendered by default.

16 changes: 15 additions & 1 deletion crates/windows-ioring-sys/UNRESOLVED-TEST-FAILURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,18 @@ Pre-existing failures that do not block an unrelated commit, recorded per the re
checklist-execution rules. When one is resolved, move its entry into a sibling
[RESOLVED-TEST-FAILURES.md](RESOLVED-TEST-FAILURES.md) (append-only) rather than deleting it.

None currently.
## `flush_barrier::a_covering_flush_waits_for_preceding_writes_and_an_unordered_one_does_not`

**Flaky under a full-workspace run, green in isolation.** Observed once during
`cargo test --workspace --all-features` on 2026-09-03 (2899 of 2900 passed); the
same test re-run on its own with `--test flush_barrier` passes.

Not caused by the change that observed it, which touched
`windows-topology-sys` only. The test measures real I/O ordering, so it is
sensitive to load: a full workspace run has every other suite competing for the
disk, and the window this test asserts is a timing one.

Recorded rather than fixed because the failure mode -- a load-sensitive
assertion in a real-I/O test -- needs a decision about whether the test should
be made load-independent or marked as serial, and that is not this change's
scope. It has not been seen to fail in CI.
12 changes: 12 additions & 0 deletions crates/windows-ioring-sys/src/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,18 @@ impl<B: IoBufMut> RegisteredBuffers<B> {
}

/// Whether this registration holds no buffers.
///
/// Always `false` in practice, and a mutation run reports the constant as
/// surviving for that reason rather than for want of a test. Nothing in
/// this crate rejects an empty vector, but the kernel refuses the
/// submission with `E_INVALIDARG`, so a caller never holds an empty
/// registration. `windows_refuses_an_empty_buffer_registration` pins that
/// platform behaviour; if a future Windows accepts it, that test fails and
/// this becomes reachable.
///
/// Kept because it is half of the `len`/`is_empty` pair every Rust
/// collection surface offers, and because "cannot happen today" is a
/// weaker claim than "cannot happen".
#[must_use]
pub fn is_empty(&self) -> bool {
self.buffers.is_empty()
Expand Down
89 changes: 89 additions & 0 deletions crates/windows-ioring-sys/src/batch/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,3 +446,92 @@ fn dropping_a_registration_with_work_outstanding_is_refused() {
// already in progress would abort instead of failing the test.
drop(buffers);
}

#[test]
fn require_refuses_an_op_the_ring_does_not_support() {
// `Batch::require -> Ok(())` survived: every op this crate names is
// genuinely supported on any real host these tests run on, so nothing
// distinguished the real check from one that always passes.
// `set_supported_ops_for_test` constructs a ring that lacks one, so the
// refusal has something to refuse.
let mut ring = IoRing::new(8, 8).expect("create ring");
ring.set_supported_ops_for_test(&[crate::Op::Nop]);
let batch = Batch::new(&mut ring);

let error = batch
.require(crate::Op::Read)
.expect_err("Read was left out of the constructed capability set");
assert_eq!(error.kind(), std::io::ErrorKind::Unsupported);

batch
.require(crate::Op::Nop)
.expect("Nop is in the constructed capability set");
}

#[test]
fn the_debug_rendering_names_the_registration_and_its_identity() {
// `<impl Debug for PendingBufferRegistration<B>>::fmt -> Ok(Default::default())`
// survived: that mutation writes nothing to the formatter, so the
// rendering comes back empty regardless of what the registration holds.
let mut ring = IoRing::new(8, 8).expect("create ring");
let mut batch = Batch::new(&mut ring);
let pending = batch
.register_buffers(vec![vec![0_u8; 64]])
.expect("queue buffer registration");
let rendering = format!("{pending:?}");
assert!(
rendering.contains("PendingBufferRegistration"),
"got {rendering}"
);
assert!(
rendering.contains(&pending.user_data().to_string()),
"the operation's identity must appear: {rendering}"
);
}

#[test]
fn windows_refuses_an_empty_buffer_registration() {
// Written while chasing `RegisteredBuffers::is_empty -> false`, which a
// mutation run reports as surviving. It survives because the state it would
// misreport **cannot be reached**: nothing in this crate rejects an empty
// vector -- `register_buffers` only checks that the count fits a `u32` and
// that the ring has no prior table -- but the kernel refuses the submission
// with `E_INVALIDARG`, so no caller ever holds an empty registration and
// `is_empty` never has occasion to return `true`.
//
// That makes the mutant unreachable rather than untested, and manufacturing
// an in-crate struct literal to kill it would assert a shape the API cannot
// produce. What is worth pinning is the platform behaviour itself, because
// it is undocumented, it is the reason the accessor looks untested, and a
// future version that started accepting empty registrations would change
// which states this crate can be in.
let mut ring = IoRing::new(8, 8).expect("create ring");
let mut batch = Batch::new(&mut ring);
let pending = batch
.register_buffers(Vec::<Vec<u8>>::new())
.expect("this crate queues it; the refusal comes from the kernel");
let user_data = pending.user_data();
batch.submit_and_wait(1, 5_000).expect("submit");

let completion = crate::ring::pop_within(&mut ring, "the registration's completion");
assert_eq!(completion.user_data(), user_data);

let Err(error) = pending
.claim_if(&completion)
.expect("its own completion is accepted")
else {
panic!("Windows accepted an empty buffer registration; is_empty is now reachable");
};
// Asserted through the crate's own downcast rather than against
// `Display`'s text: the point is which HRESULT the kernel returned, and
// matching a formatted string would also fail if the message were reworded,
// reporting a documentation change as a platform change.
let code = crate::IoRingErrorExt::as_ioring_error(&error)
.expect("a kernel refusal carries an HRESULT")
.code();
assert_eq!(
code,
windows_sys::Win32::Foundation::E_INVALIDARG,
"the kernel must refuse an empty registration with E_INVALIDARG, got {error}"
);
}
15 changes: 15 additions & 0 deletions crates/windows-ioring-sys/src/error/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,21 @@ fn code_reports_the_raw_value() {
assert_eq!(error.code(), IORING_E_SUBMISSION_QUEUE_FULL);
}

#[test]
fn the_debug_rendering_names_the_type_and_the_code() {
// `<impl Debug for IoRingError>::fmt -> Ok(Default::default())` survived:
// that mutation writes nothing to the formatter, so `format!("{error:?}")`
// comes back empty. The real rendering names the type and prints the code
// in hex, neither of which an empty string can satisfy.
let error = IoRingError::new(IORING_E_SUBMISSION_QUEUE_FULL);
let rendering = format!("{error:?}");
assert!(rendering.contains("IoRingError"), "got {rendering}");
assert!(
rendering.contains(&format!("{:08X}", IORING_E_SUBMISSION_QUEUE_FULL as u32)),
"the code must appear in hex: {rendering}"
);
}

// --- named conditions and predicates (M10.5, D-30) ---

#[test]
Expand Down
19 changes: 19 additions & 0 deletions crates/windows-ioring-sys/src/event_delivery/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,25 @@ fn a_scope_reports_the_rings_static_properties() {
assert_eq!(scope.info().expect("query info").submission_queue_size, 8);
}

#[test]
fn a_scope_reflects_a_ring_that_genuinely_lacks_support() {
// `RingScope::supports -> true` survived: the test above only ever asks
// about an op the host genuinely supports, so the honest forwarder and
// the constant agree everywhere a real host could answer. Restricting the
// ring's capability set before wrapping it constructs the disagreement --
// the same seam `Batch::require`'s own gap needed.
let mut ring = IoRing::new(8, 8).expect("create ring");
ring.set_supported_ops_for_test(&[Op::Nop]);
let delivery = EventDelivery::new(ring, |_completion| {}, None).expect("wire event delivery");

let scope = delivery.scope();
assert!(scope.supports(Op::Nop));
assert!(
!scope.supports(Op::Read),
"Read was left out of the constructed capability set"
);
}

#[test]
fn a_scope_reports_registration_counts_that_change_with_registrations() {
let ring = IoRing::new(8, 8).expect("create ring");
Expand Down
Loading
Loading