Skip to content

Commit 6f08b08

Browse files
doge-rgbCMGS
authored andcommitted
block: Reissue io_uring requests that complete with -EAGAIN
io_uring can complete an O_DIRECT read with -EAGAIN on kernels without commit b08e02045002 ("io_uring/rw: don't gate retry on completion context"), which was merged for Linux 6.14. This happens on XFS for a read that spans several extents when a concurrent allocating write holds the inode lock: the bios for the first extents are already in flight when mapping a later extent fails with -EAGAIN, iomap completes the request from a workqueue, and io_uring does not retry it outside the submitting task. virtio-blk then reports VIRTIO_BLK_S_IOERR to the guest for data that is intact. Reissue a read or write that completes with -EAGAIN or -EINTR once, with IOSQE_ASYNC so that io-wq runs it in blocking mode, where it cannot fail this way again. QEMU resubmits both errors as well. Signed-off-by: doge <me@crackerben.com>
1 parent 5b691d2 commit 6f08b08

1 file changed

Lines changed: 101 additions & 7 deletions

File tree

‎block/src/io/async_io/uring_data_io.rs‎

Lines changed: 101 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::os::fd::{AsRawFd, RawFd};
99
use std::{io, mem};
1010

1111
use io_uring::{IoUring, opcode, squeue, types};
12-
use log::{error, warn};
12+
use log::{debug, error, warn};
1313
use vmm_sys_util::eventfd::EventFd;
1414

1515
use super::common::{duplicate_user_data_error, validate_batch};
@@ -24,6 +24,11 @@ pub struct UringDataIo {
2424
// data operations store `Some(op)` so their iovecs and backing buffers
2525
// remain valid until completion; metadata operations store `None`.
2626
in_flight: HashMap<u64, Option<AsyncIoOperation>>,
27+
// `reissue_fds` holds the file descriptor of each owned data operation
28+
// that has not been reissued.
29+
reissue_fds: HashMap<u64, RawFd>,
30+
// `rw_flags` holds the RWF_* flags of read and write SQEs, set by tests.
31+
rw_flags: i32,
2732
completions: CompletionCommon,
2833
// `needs_submit_retry` is set when SQEs have been published to the ring,
2934
// but the submit syscall failed before confirming kernel ownership.
@@ -42,6 +47,8 @@ impl UringDataIo {
4247
Ok(Self {
4348
io_uring,
4449
in_flight: HashMap::new(),
50+
reissue_fds: HashMap::new(),
51+
rw_flags: 0,
4552
completions,
4653
needs_submit_retry: false,
4754
})
@@ -119,13 +126,15 @@ impl UringDataIo {
119126
let mut batch = batch.into_iter();
120127
while let Some(op) = batch.next() {
121128
let user_data = op.user_data();
122-
let entry = Self::build_entry(fd, &op);
129+
let entry = Self::build_entry(fd, &op, self.rw_flags);
123130
self.in_flight.insert(user_data, Some(op));
131+
self.reissue_fds.insert(user_data, fd);
124132

125133
// SAFETY: the SQ capacity was just checked. Every iovec's pointer is retained in
126134
// self.in_flight before the SQ tail is advanced by sync or drop. in_flight only
127135
// drops the memory after a completion.
128136
if let Err(e) = unsafe { sq.push(&entry) } {
137+
self.reissue_fds.remove(&user_data);
129138
Self::handle_push_failure(
130139
&mut self.in_flight,
131140
&mut self.completions,
@@ -176,17 +185,19 @@ impl UringDataIo {
176185
warn!("io_uring submission queue became full after capacity check: {error:?}");
177186
}
178187

179-
fn build_entry(fd: RawFd, op: &AsyncIoOperation) -> squeue::Entry {
188+
fn build_entry(fd: RawFd, op: &AsyncIoOperation, rw_flags: i32) -> squeue::Entry {
180189
let iovecs = op.iovecs();
181190
let fd = types::Fd(fd);
182191
if op.is_read() {
183192
opcode::Readv::new(fd, iovecs.as_ptr(), iovecs.len() as u32)
184193
.offset(op.offset() as u64)
194+
.rw_flags(rw_flags)
185195
.build()
186196
.user_data(op.user_data())
187197
} else {
188198
opcode::Writev::new(fd, iovecs.as_ptr(), iovecs.len() as u32)
189199
.offset(op.offset() as u64)
200+
.rw_flags(rw_flags)
190201
.build()
191202
.user_data(op.user_data())
192203
}
@@ -218,7 +229,8 @@ impl UringDataIo {
218229
/// Returns the next kernel or injected completion if one is available.
219230
///
220231
/// Consuming a kernel completion returns ownership of any buffer retained
221-
/// by the corresponding operation.
232+
/// by the corresponding operation. A read or write that completes with
233+
/// `-EAGAIN` or `-EINTR` is reissued once instead of being returned.
222234
pub fn next_completion(&mut self) -> Option<AsyncIoCompletion> {
223235
if self.needs_submit_retry {
224236
match self.io_uring.submitter().submit() {
@@ -227,11 +239,39 @@ impl UringDataIo {
227239
}
228240
}
229241

230-
if let Some(entry) = self.io_uring.completion().next() {
231-
let user_data = entry.user_data();
242+
loop {
243+
let Some(cqe) = self.io_uring.completion().next() else {
244+
break;
245+
};
246+
let user_data = cqe.user_data();
247+
let result = cqe.result();
248+
let reissue_fd = self.reissue_fds.remove(&user_data);
249+
if (result == -libc::EAGAIN || result == -libc::EINTR)
250+
&& let Some(fd) = reissue_fd
251+
&& let Some(Some(op)) = self.in_flight.get(&user_data)
252+
{
253+
// IOSQE_ASYNC makes io-wq run the request in blocking mode.
254+
let entry = Self::build_entry(fd, op, self.rw_flags).flags(squeue::Flags::ASYNC);
255+
let (submitter, mut sq, _) = self.io_uring.split();
256+
// SAFETY: the operation and its iovecs stay in self.in_flight
257+
// until its final completion is consumed.
258+
if unsafe { sq.push(&entry) }.is_ok() {
259+
sq.sync();
260+
if let Err(e) = submitter.submit() {
261+
self.needs_submit_retry = true;
262+
warn!("io_uring submit failed after SQE was reissued: {e}");
263+
self.completions.notifier().write(1).unwrap();
264+
}
265+
debug!(
266+
"Reissued io_uring request {user_data:#x} after {}",
267+
io::Error::from_raw_os_error(-result)
268+
);
269+
continue;
270+
}
271+
}
232272
return Some(AsyncIoCompletion::new(
233273
user_data,
234-
entry.result(),
274+
result,
235275
self.in_flight
236276
.remove(&user_data)
237277
.flatten()
@@ -294,6 +334,7 @@ impl Drop for UringDataIo {
294334
#[cfg(test)]
295335
mod tests {
296336
use std::io;
337+
use std::io::Write;
297338
use std::os::fd::AsRawFd;
298339
use std::thread::sleep;
299340
use std::time::Duration;
@@ -392,4 +433,57 @@ mod tests {
392433
completed.sort_unstable();
393434
assert_eq!(completed, (0..batch_len as u64).collect::<Vec<_>>());
394435
}
436+
437+
#[test]
438+
fn uring_reissues_operation_after_eagain() {
439+
let (reader, mut writer) = io::pipe().unwrap();
440+
let mut data_io = UringDataIo::new(8).unwrap();
441+
// RWF_NOWAIT makes a read of an empty pipe complete with -EAGAIN.
442+
data_io.rw_flags = libc::RWF_NOWAIT;
443+
444+
data_io
445+
.submit_operation(
446+
reader.as_raw_fd(),
447+
AsyncIoOperation::read_to_vec(0, OwnedIoBuffer::from_vec(vec![0; 4]), 9),
448+
)
449+
.unwrap();
450+
// Written after the first attempt, so only the reissue can read it.
451+
writer.write_all(b"data").unwrap();
452+
453+
let completion = wait_for_completion(&mut data_io);
454+
if completion.result == -libc::EOPNOTSUPP {
455+
eprintln!("skipping: RWF_NOWAIT not supported on pipes");
456+
return;
457+
}
458+
assert_eq!(completion.user_data, 9);
459+
assert_eq!(completion.result, 4);
460+
assert_eq!(completion.buffer.unwrap().as_slice(), b"data");
461+
assert!(data_io.in_flight.is_empty());
462+
assert!(data_io.reissue_fds.is_empty());
463+
}
464+
465+
#[test]
466+
fn uring_delivers_eagain_after_reissue() {
467+
let (reader, _writer) = io::pipe().unwrap();
468+
let mut data_io = UringDataIo::new(8).unwrap();
469+
data_io.rw_flags = libc::RWF_NOWAIT;
470+
471+
data_io
472+
.submit_operation(
473+
reader.as_raw_fd(),
474+
AsyncIoOperation::read_to_vec(0, OwnedIoBuffer::from_vec(vec![0; 4]), 3),
475+
)
476+
.unwrap();
477+
478+
let completion = wait_for_completion(&mut data_io);
479+
if completion.result == -libc::EOPNOTSUPP {
480+
eprintln!("skipping: RWF_NOWAIT not supported on pipes");
481+
return;
482+
}
483+
assert_eq!(completion.user_data, 3);
484+
assert_eq!(completion.result, -libc::EAGAIN);
485+
assert!(completion.buffer.is_some());
486+
assert!(data_io.in_flight.is_empty());
487+
assert!(data_io.reissue_fds.is_empty());
488+
}
395489
}

0 commit comments

Comments
 (0)