cube(2): the walk skips the records the cursor already returned
`v3_enum` built its batch with `seen: cursor`, and `Batch::offer` increments
before comparing (`seen += 1; if seen <= cursor { skip }`), so nothing was ever
skipped: every call re-served the space from its first record while `out_cursor`
advanced by the records returned. The contract's only end signal is a cursor that
stops moving, so the walk never ended — a listing looped inside the front-end
until the kernel's OOM killer took that process, twice, at ~6.4 GiB anon.
Every other Batch site already starts `seen` at 0, and two of them carry comments
describing exactly this trap; this one was the lone outlier. It only shows on an
**addressed** image whose space has log edits, because the merge path re-walks
from the space's start and has nothing but `seen` to skip with — the no-edits path
skips by index arithmetic (`first + cursor`) and was already right. That is why no
gate saw it: the enumeration gate's store is packed.
So each path now skips exactly once — the merge path through `offer`'s `seen`
starting at 0, the no-edits path through its arithmetic with the batch told to
skip nothing. Doing both would skip twice and lose records. The arithmetic cursor
is added saturating, because a wrapped sum would land back near the first record
and re-serve the walk: the same endless listing this exists to avoid.
Verified: a copy of the box's own addressed store walks in QEMU on this kernel to
`12 space(s), 69665 record(s)` — 69,641 image records plus 24 unfolded log edits —
and terminates, where before every batch repeated the space from its start.
This commit is contained in:
@@ -3026,21 +3026,44 @@ unsafe fn v3_enum(
|
||||
|
||||
// SAFETY: the shim guarantees `cap` writable bytes at `buf`.
|
||||
let out = unsafe { core::slice::from_raw_parts_mut(buf, cap) };
|
||||
let mut batch = Batch {
|
||||
out,
|
||||
written: 0,
|
||||
returned: 0,
|
||||
seen: cursor,
|
||||
cursor,
|
||||
too_big: 0,
|
||||
with_space: false,
|
||||
};
|
||||
|
||||
if edits.is_empty() {
|
||||
/*
|
||||
* The two paths skip the already-returned records in two different ways, and each must do it
|
||||
* exactly once.
|
||||
*
|
||||
* A batch's skip is `offer`'s `seen`, which counts the records it has been shown and starts at
|
||||
* 0 — the cursor-th record is the first one offered that is returned. The merge path below has
|
||||
* nothing but that count: it re-walks the space from its start every batch, because the log's
|
||||
* edits interleave and the index position of the cursor-th match is not arithmetic.
|
||||
*
|
||||
* The no-edits path does not need it: it starts at the index entry the cursor names
|
||||
* (`first + cursor`), which *is* the skip, exact and in one step. So it hands `offer` a cursor
|
||||
* of 0 and lets the arithmetic do it — doing both would skip twice and lose records.
|
||||
*
|
||||
* What this used to do instead, on the merge path, was start `seen` *at* the cursor. `offer`
|
||||
* compares after incrementing, so the first record it saw counted as `cursor + 1`, nothing was
|
||||
* ever skipped, and every call re-served the space from its first record while `out_cursor`
|
||||
* advanced by the records returned. A walk whose only end signal is a cursor that stops moving
|
||||
* therefore never ended: a listing looped until the caller ran out of memory.
|
||||
*/
|
||||
let (written, returned, too_big) = if edits.is_empty() {
|
||||
let mut batch = Batch {
|
||||
out,
|
||||
written: 0,
|
||||
returned: 0,
|
||||
seen: 0,
|
||||
cursor: 0,
|
||||
too_big: 0,
|
||||
with_space: false,
|
||||
};
|
||||
// Straight through the index, from the record the cursor names, reading a page of index
|
||||
// entries at a time and then the span of values they point at.
|
||||
//
|
||||
// The cursor is the caller's number, so it is added the saturating way: a wrapped sum
|
||||
// would land back near the space's first record and re-serve the walk from there — the
|
||||
// same endless listing this arithmetic exists to avoid.
|
||||
let mut entry = KVVec::<u8>::new();
|
||||
let mut at = first + cursor;
|
||||
let mut at = first.saturating_add(cursor);
|
||||
while at < first + records {
|
||||
let (value_off, value_len, flags) = match image.index_entry(at, &mut entry) {
|
||||
Ok(triple) => triple,
|
||||
@@ -3059,7 +3082,17 @@ unsafe fn v3_enum(
|
||||
}
|
||||
at += 1;
|
||||
}
|
||||
(batch.written, batch.returned, batch.too_big)
|
||||
} else {
|
||||
let mut batch = Batch {
|
||||
out,
|
||||
written: 0,
|
||||
returned: 0,
|
||||
seen: 0,
|
||||
cursor,
|
||||
too_big: 0,
|
||||
with_space: false,
|
||||
};
|
||||
// The log's edits interleave, so the merged order is what the cursor counts. Stream the
|
||||
// space's index entries rather than all of them: the space is what is being listed.
|
||||
let mut entry = KVVec::<u8>::new();
|
||||
@@ -3116,17 +3149,18 @@ unsafe fn v3_enum(
|
||||
_ => break,
|
||||
}
|
||||
}
|
||||
}
|
||||
(batch.written, batch.returned, batch.too_big)
|
||||
};
|
||||
|
||||
// SAFETY: both out-pointers are writable under this function's contract.
|
||||
unsafe {
|
||||
if batch.too_big > 0 && batch.written == 0 {
|
||||
*out_len = batch.too_big as u64;
|
||||
if too_big > 0 && written == 0 {
|
||||
*out_len = too_big as u64;
|
||||
*out_cursor = cursor;
|
||||
return -34; // -ERANGE
|
||||
}
|
||||
*out_len = batch.written as u64;
|
||||
*out_cursor = cursor + batch.returned;
|
||||
*out_len = written as u64;
|
||||
*out_cursor = cursor + returned;
|
||||
}
|
||||
0
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user