Skip to content

Commit 945f907

Browse files
committed
wp: harden io_uring submit accounting
Root cause: the host and P2P FileIOLayer io_uring paths counted a request as pending when an SQE was prepared, then ignored the return from io_uring_submit. Under resident-dense paging, many small expert reads and heavy eviction can fill the CQ/SQ pressure window at depth 8/16. If io_uring_submit returned -EBUSY/-EAGAIN/0 or otherwise submitted fewer entries than prepared, the pager still waited for req_ids that were not actually kernel-owned. The targeted waiter then blocked in io_cqring_wait with no disk progress, matching the observed idle-GPU warmup hang. Depth 4 avoids the pressure window often enough, and the all-tensors-paged 64 MB slot mode has much lower request churn, so it does not trip the same accounting skew. Fix: track prepared-but-not-yet-submitted req_ids separately from kernel-owned pending requests. flush now retries interrupted submit, drains ready CQEs into the existing demux buffer on -EBUSY/-EAGAIN/zero progress, and only reports ErrorNoSubmit completions for req_ids that cannot be submitted. reap_raw also flushes any prepared SQEs before blocking and rechecks pending so it never calls io_uring_wait_cqe for requests that have already been converted to terminal buffered results. The same contract is applied to the P2P ring. Regression: added a depth-1 batched FileIOLayer targeted-wait test to exercise chunked submit plus wait_for_req routing without model load or inference. Local build lacks liburing, so the test runs through SyncPread fallback here; the io_uring code path compiled in the target but still needs validation on the GPU/liburing host. Human validation required: rerun resident-dense warmup/decode with WP_RESIDENT_DENSE=1 and WP_IOURING_DEPTH/WP_PREFETCH_DEPTH at 8 and 16. The server should become ready and show continued page-in/eviction progress instead of blocking in io_cqring_wait. Assisted-by: Codex
1 parent 042ef45 commit 945f907

3 files changed

Lines changed: 242 additions & 9 deletions

File tree

src/weight-pager/wp-file-io-p2p.cpp

Lines changed: 98 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <cerrno>
66
#include <cstdlib>
77
#include <cstring>
8+
#include <deque>
89
#include <memory>
910
#include <vector>
1011
#include <unistd.h>
@@ -103,7 +104,7 @@ class IoUringP2PFileIOLayer : public FileIOLayer {
103104

104105
struct io_uring_sqe * sqe = io_uring_get_sqe(&ring_);
105106
if (sqe == nullptr) {
106-
io_uring_submit(&ring_);
107+
flush_submissions_();
107108
sqe = io_uring_get_sqe(&ring_);
108109
}
109110
if (sqe == nullptr) {
@@ -114,18 +115,19 @@ class IoUringP2PFileIOLayer : public FileIOLayer {
114115
io_uring_prep_read(sqe, fd_idx, mapped_dst, (unsigned) size, (off_t) offset);
115116
sqe->flags |= IOSQE_FIXED_FILE;
116117
sqe->user_data = req_id;
118+
pending_submit_.push_back(req_id);
117119
++pending_;
118120
return true;
119121
}
120122

121123
void flush() override {
124+
if (ring_ok_ && !pending_submit_.empty()) {
125+
flush_submissions_();
126+
}
122127
if (!p2p_enabled_ && pending_ == 0) {
123128
host_->flush();
124129
return;
125130
}
126-
if (ring_ok_ && pending_ > 0) {
127-
io_uring_submit(&ring_);
128-
}
129131
}
130132

131133
// Reap one raw completion for the FileIOLayer base demux. Drains the P2P
@@ -136,6 +138,13 @@ class IoUringP2PFileIOLayer : public FileIOLayer {
136138
// buffering happen in the base.
137139
bool reap_raw_(int timeout_ms, IoResult & out) override {
138140
if (ring_ok_ && pending_ > 0) {
141+
if (!pending_submit_.empty()) {
142+
flush_submissions_();
143+
}
144+
if (pending_ == 0) {
145+
return false;
146+
}
147+
139148
struct io_uring_cqe * cqe = nullptr;
140149
int ret = 0;
141150
if (timeout_ms < 0) {
@@ -241,6 +250,90 @@ class IoUringP2PFileIOLayer : public FileIOLayer {
241250
IoUringP2PFileIOLayer(std::vector<int> fds, std::unique_ptr<FileIOLayer> host)
242251
: fds_(std::move(fds)), host_(std::move(host)) {}
243252

253+
void synthesize_submit_failures_(int err) {
254+
while (!pending_submit_.empty()) {
255+
const uint64_t req_id = pending_submit_.front();
256+
pending_submit_.pop_front();
257+
258+
IoResult r;
259+
r.req_id = req_id;
260+
r.status = IoStatus::ErrorNoSubmit;
261+
r.bytes_read = err;
262+
ready_[req_id] = r;
263+
--pending_;
264+
}
265+
}
266+
267+
bool flush_submissions_() {
268+
if (!ring_ok_ || pending_submit_.empty()) {
269+
return true;
270+
}
271+
272+
while (!pending_submit_.empty()) {
273+
int ret = 0;
274+
do {
275+
ret = io_uring_submit(&ring_);
276+
} while (ret == -EINTR);
277+
278+
if (ret == -EBUSY || ret == -EAGAIN || ret == 0) {
279+
bool drained = false;
280+
while (reap_ready_cqe_()) {
281+
drained = true;
282+
}
283+
if (drained) {
284+
continue;
285+
}
286+
}
287+
if (ret < 0) {
288+
switch_to_host_errno_("io_uring submit failed", -ret);
289+
synthesize_submit_failures_(ret);
290+
return false;
291+
}
292+
if (ret == 0) {
293+
switch_to_host_("io_uring submit made no progress", 0);
294+
synthesize_submit_failures_(-EAGAIN);
295+
return false;
296+
}
297+
298+
for (int i = 0; i < ret && !pending_submit_.empty(); ++i) {
299+
pending_submit_.pop_front();
300+
}
301+
}
302+
return true;
303+
}
304+
305+
bool reap_ready_cqe_() {
306+
if (pending_ == 0) {
307+
return false;
308+
}
309+
310+
struct io_uring_cqe * cqe = nullptr;
311+
int ret = io_uring_peek_cqe(&ring_, &cqe);
312+
if (ret == -EAGAIN || ret == -EINTR || cqe == nullptr) {
313+
return false;
314+
}
315+
if (ret < 0) {
316+
return false;
317+
}
318+
319+
IoResult r;
320+
r.req_id = cqe->user_data;
321+
const int res = cqe->res;
322+
io_uring_cqe_seen(&ring_, cqe);
323+
--pending_;
324+
325+
if (res < 0) {
326+
r.status = IoStatus::ErrorIo;
327+
r.bytes_read = res;
328+
switch_to_host_errno_("read failed", -res);
329+
} else {
330+
r.status = IoStatus::Ok;
331+
r.bytes_read = res;
332+
}
333+
ready_[r.req_id] = r;
334+
return true;
335+
}
336+
244337
bool init_(int queue_depth, const FileIOP2PConfig & cfg) {
245338
int rt_version = 0;
246339
if (hipRuntimeGetVersion(&rt_version) == hipSuccess) {
@@ -386,6 +479,7 @@ class IoUringP2PFileIOLayer : public FileIOLayer {
386479
bool ring_ok_ = false;
387480
bool files_registered_ = false;
388481
int pending_ = 0;
482+
std::deque<uint64_t> pending_submit_;
389483
struct io_uring ring_ {};
390484
void * libhsa_ = nullptr;
391485
HsaExportDmaBufFn hsa_export_ = nullptr;

src/weight-pager/wp-file-io.cpp

Lines changed: 97 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -233,21 +233,21 @@ class IoUringAsyncFileIO : public FileIOLayer {
233233
struct io_uring_sqe * sqe = io_uring_get_sqe(&ring_);
234234
if (sqe == nullptr) {
235235
// Ring full: flush and retry once.
236-
io_uring_submit(&ring_);
236+
flush_submissions_();
237237
sqe = io_uring_get_sqe(&ring_);
238238
if (sqe == nullptr) return false;
239239
}
240240
// Use registered-file index (faster than passing raw fds).
241241
io_uring_prep_read(sqe, fd_idx, dst, (unsigned) size, (off_t) offset);
242242
sqe->flags |= IOSQE_FIXED_FILE;
243243
sqe->user_data = req_id;
244+
pending_submit_.push_back(req_id);
244245
++pending_;
245246
return true;
246247
}
247248

248249
void flush() override {
249-
if (!ring_ok_ || pending_ == 0) return;
250-
io_uring_submit(&ring_);
250+
flush_submissions_();
251251
}
252252

253253
// Reap one completion from the ring. The demux/routing (buffering foreign
@@ -265,6 +265,12 @@ class IoUringAsyncFileIO : public FileIOLayer {
265265
if (pending_ == 0) {
266266
return false; // nothing in flight
267267
}
268+
if (!pending_submit_.empty()) {
269+
flush_submissions_();
270+
}
271+
if (pending_ == 0) {
272+
return false;
273+
}
268274

269275
struct io_uring_cqe * cqe = nullptr;
270276
int ret = 0;
@@ -347,28 +353,113 @@ class IoUringAsyncFileIO : public FileIOLayer {
347353
struct io_uring_sqe * sqe = io_uring_get_sqe(&ring_);
348354
if (sqe == nullptr) {
349355
// SQ ring full mid-batch — flush what we've prepped, then retry once.
350-
io_uring_submit(&ring_);
356+
flush_submissions_();
351357
sqe = io_uring_get_sqe(&ring_);
352358
if (sqe == nullptr) break;
353359
}
354360
io_uring_prep_read(sqe, r.fd_idx, r.dst, (unsigned) r.size, (off_t) r.offset);
355361
sqe->flags |= IOSQE_FIXED_FILE;
356362
sqe->user_data = r.req_id;
363+
pending_submit_.push_back(r.req_id);
357364
++pending_;
358365
++n_queued;
359366
}
360367
// Pass 2: single io_uring_submit for the batch. This is the MAD-235
361368
// win — one syscall covers N expert-prefetches for the MoE layer
362369
// instead of N separate submits.
363370
if (n_queued > 0) {
364-
io_uring_submit(&ring_);
371+
flush_submissions_();
365372
}
366373
return n_queued;
367374
}
368375

369376
private:
370377
explicit IoUringAsyncFileIO(std::vector<int> fds) : fds_(std::move(fds)) {}
371378

379+
void synthesize_submit_failures_(int err) {
380+
while (!pending_submit_.empty()) {
381+
const uint64_t req_id = pending_submit_.front();
382+
pending_submit_.pop_front();
383+
384+
IoResult r;
385+
r.req_id = req_id;
386+
r.status = IoStatus::ErrorNoSubmit;
387+
r.bytes_read = err;
388+
ready_[req_id] = r;
389+
--pending_;
390+
}
391+
}
392+
393+
bool flush_submissions_() {
394+
if (!ring_ok_ || pending_submit_.empty()) {
395+
return true;
396+
}
397+
398+
while (!pending_submit_.empty()) {
399+
int ret = 0;
400+
do {
401+
ret = io_uring_submit(&ring_);
402+
} while (ret == -EINTR);
403+
404+
if (ret == -EBUSY || ret == -EAGAIN || ret == 0) {
405+
bool drained = false;
406+
while (reap_ready_cqe_()) {
407+
drained = true;
408+
}
409+
if (drained) {
410+
continue;
411+
}
412+
}
413+
if (ret < 0) {
414+
LLAMA_LOG_WARN("wp::IoUringAsyncFileIO: io_uring_submit failed: %s\n",
415+
strerror(-ret));
416+
synthesize_submit_failures_(ret);
417+
return false;
418+
}
419+
if (ret == 0) {
420+
LLAMA_LOG_WARN("wp::IoUringAsyncFileIO: io_uring_submit made no progress with %zu SQEs pending\n",
421+
pending_submit_.size());
422+
synthesize_submit_failures_(-EAGAIN);
423+
return false;
424+
}
425+
426+
for (int i = 0; i < ret && !pending_submit_.empty(); ++i) {
427+
pending_submit_.pop_front();
428+
}
429+
}
430+
return true;
431+
}
432+
433+
bool reap_ready_cqe_() {
434+
if (pending_ == 0) {
435+
return false;
436+
}
437+
438+
struct io_uring_cqe * cqe = nullptr;
439+
int ret = io_uring_peek_cqe(&ring_, &cqe);
440+
if (ret == -EAGAIN || ret == -EINTR || cqe == nullptr) {
441+
return false;
442+
}
443+
if (ret < 0) {
444+
return false;
445+
}
446+
447+
IoResult r;
448+
r.req_id = cqe->user_data;
449+
const int res = cqe->res;
450+
if (res < 0) {
451+
r.status = IoStatus::ErrorIo;
452+
r.bytes_read = res;
453+
} else {
454+
r.status = IoStatus::Ok;
455+
r.bytes_read = res;
456+
}
457+
io_uring_cqe_seen(&ring_, cqe);
458+
--pending_;
459+
ready_[r.req_id] = r;
460+
return true;
461+
}
462+
372463
bool init_(int queue_depth) {
373464
int ret = io_uring_queue_init(queue_depth, &ring_, 0);
374465
if (ret < 0) {
@@ -397,6 +488,7 @@ class IoUringAsyncFileIO : public FileIOLayer {
397488
}
398489

399490
std::vector<int> fds_;
491+
std::deque<uint64_t> pending_submit_;
400492
bool ring_ok_ = false;
401493
bool files_registered_ = false;
402494
int pending_ = 0;

tests/test-weight-pager.cpp

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1383,6 +1383,52 @@ static int test_file_io_submit_batch_partial_failure() {
13831383
return fails;
13841384
}
13851385

1386+
static int test_file_io_submit_batch_depth_one_targeted_waits() {
1387+
int fails = 0;
1388+
1389+
char path[] = "/tmp/wp-test-batch-depth1-XXXXXX";
1390+
int fd = mkstemp(path);
1391+
if (fd < 0) {
1392+
std::fprintf(stderr, " FAIL: %s: mkstemp failed: %s\n", __func__, std::strerror(errno));
1393+
return 1;
1394+
}
1395+
1396+
constexpr size_t N = 16384;
1397+
std::vector<uint8_t> pattern(N);
1398+
for (size_t i = 0; i < N; ++i) pattern[i] = (uint8_t) ((i * 19 + 11) & 0xff);
1399+
ssize_t w = write(fd, pattern.data(), N);
1400+
EXPECT_EQ_INT((size_t) w, N, "wrote pattern");
1401+
1402+
std::vector<int> fds = { fd };
1403+
auto layer = wp::create_file_io(std::move(fds), /*prefer_async=*/true, 1);
1404+
EXPECT(layer != nullptr, "create_file_io non-null");
1405+
if (!layer) { unlink(path); return fails; }
1406+
1407+
std::vector<std::vector<uint8_t>> dst(8, std::vector<uint8_t>(1024));
1408+
std::vector<wp::FileIOBatchRequest> reqs;
1409+
reqs.reserve(dst.size());
1410+
for (size_t i = 0; i < dst.size(); ++i) {
1411+
reqs.push_back({ 1000 + i, 0, (uint64_t) (i * 1536), 1024, dst[i].data() });
1412+
}
1413+
1414+
int n_ok = layer->submit_batch(reqs);
1415+
EXPECT_EQ_INT(n_ok, (int) reqs.size(), "all depth-1 batch entries accepted");
1416+
1417+
for (size_t i = 0; i < reqs.size(); ++i) {
1418+
wp::IoResult r = layer->wait_for_req(reqs[i].req_id, /*timeout_ms=*/5000);
1419+
EXPECT(r.status == wp::IoStatus::Ok, "targeted wait returns Ok");
1420+
EXPECT_EQ_INT(r.req_id, reqs[i].req_id, "targeted wait req_id");
1421+
EXPECT_EQ_INT(r.bytes_read, 1024, "targeted wait bytes");
1422+
EXPECT(std::memcmp(dst[i].data(), pattern.data() + i * 1536, 1024) == 0,
1423+
"targeted wait content");
1424+
}
1425+
EXPECT_EQ_INT(layer->pending(), 0, "no pending after targeted waits");
1426+
1427+
layer.reset();
1428+
unlink(path);
1429+
return fails;
1430+
}
1431+
13861432
// ---------------------------------------------------------------------------
13871433
// Completion demux — targeted waits must never drop a sibling's completion
13881434
// ---------------------------------------------------------------------------
@@ -1670,6 +1716,7 @@ int main() {
16701716
{ "file_io_advise_prefetch", test_file_io_advise_prefetch },
16711717
{ "file_io_submit_batch", test_file_io_submit_batch },
16721718
{ "file_io_submit_batch_partial", test_file_io_submit_batch_partial_failure },
1719+
{ "file_io_submit_batch_depth_one_targeted_waits", test_file_io_submit_batch_depth_one_targeted_waits },
16731720
{ "file_io_demux_no_cross_drain", test_file_io_demux_no_cross_drain },
16741721
{ "compute_advise_ranges", test_compute_advise_ranges },
16751722
{ "is_uma_archname", test_is_uma_archname },

0 commit comments

Comments
 (0)