From d4de451d459f7df2d9f87ad848e4eafe7e1227f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 16:53:55 +0100 Subject: [PATCH 1/8] core: enable rustix/io_uring with io_uring feature --- core/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Cargo.toml b/core/Cargo.toml index fc1f88fe2..c0c12a0a3 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -22,7 +22,7 @@ json = [ "dep:pest_derive", ] uuid = ["dep:uuid"] -io_uring = ["dep:io-uring"] +io_uring = ["dep:io-uring", "rustix/io_uring"] [target.'cfg(target_os = "linux")'.dependencies] io-uring = { version = "0.6.1", optional = true } From 7808665c924d1ba79247290cbded517f92c39b53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 16:56:18 +0100 Subject: [PATCH 2/8] core: make MAX_IOVECS u32 instead of usize, to match the type expected by io_uring --- core/io/io_uring.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/io/io_uring.rs b/core/io/io_uring.rs index 3278d61b9..a499320a4 100644 --- a/core/io/io_uring.rs +++ b/core/io/io_uring.rs @@ -10,7 +10,7 @@ use std::os::unix::io::AsRawFd; use std::rc::Rc; use thiserror::Error; -const MAX_IOVECS: usize = 128; +const MAX_IOVECS: u32 = 128; const SQPOLL_IDLE: u32 = 1000; #[derive(Debug, Error)] @@ -44,7 +44,7 @@ struct WrappedIOUring { struct InnerUringIO { ring: WrappedIOUring, - iovecs: [iovec; MAX_IOVECS], + iovecs: [iovec; MAX_IOVECS as usize], next_iovec: usize, } @@ -52,10 +52,10 @@ impl UringIO { pub fn new() -> Result { let ring = match io_uring::IoUring::builder() .setup_sqpoll(SQPOLL_IDLE) - .build(MAX_IOVECS as u32) + .build(MAX_IOVECS) { Ok(ring) => ring, - Err(_) => io_uring::IoUring::new(MAX_IOVECS as u32)?, + Err(_) => io_uring::IoUring::new(MAX_IOVECS)?, }; let inner = InnerUringIO { ring: WrappedIOUring { @@ -67,7 +67,7 @@ impl UringIO { iovecs: [iovec { iov_base: std::ptr::null_mut(), iov_len: 0, - }; MAX_IOVECS], + }; MAX_IOVECS as usize], next_iovec: 0, }; debug!("Using IO backend 'io-uring'"); @@ -82,7 +82,7 @@ impl InnerUringIO { let iovec = &mut self.iovecs[self.next_iovec]; iovec.iov_base = buf as *mut std::ffi::c_void; iovec.iov_len = len; - self.next_iovec = (self.next_iovec + 1) % MAX_IOVECS; + self.next_iovec = (self.next_iovec + 1) % MAX_IOVECS as usize; iovec } } From 7b5e5efd14544abe0a81cee69cb09fa128e2f3e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 16:58:43 +0100 Subject: [PATCH 3/8] core/io/unix: replace libc calls and types with their rustix counterparts --- core/io/unix.rs | 82 +++++++++++++++++++++---------------------------- 1 file changed, 35 insertions(+), 47 deletions(-) diff --git a/core/io/unix.rs b/core/io/unix.rs index db0e85ab1..e8390dec8 100644 --- a/core/io/unix.rs +++ b/core/io/unix.rs @@ -3,15 +3,17 @@ use crate::io::common; use crate::Result; use super::{Completion, File, OpenFlags, IO}; -use libc::{c_short, fcntl, flock, F_SETLK}; use log::{debug, trace}; use polling::{Event, Events, Poller}; -use rustix::fd::{AsFd, AsRawFd}; -use rustix::fs::OpenOptionsExt; -use rustix::io::Errno; +use rustix::{ + fd::{AsFd, AsRawFd}, + fs, + fs::{FlockOperation, OpenOptionsExt}, + io::Errno, +}; use std::cell::RefCell; use std::collections::HashMap; -use std::io::{Read, Seek, Write}; +use std::io::{ErrorKind, Read, Seek, Write}; use std::rc::Rc; pub struct UnixIO { @@ -136,55 +138,41 @@ pub struct UnixFile { impl File for UnixFile { fn lock_file(&self, exclusive: bool) -> Result<()> { - let fd = self.file.borrow().as_raw_fd(); - let flock = flock { - l_type: if exclusive { - libc::F_WRLCK as c_short - } else { - libc::F_RDLCK as c_short - }, - l_whence: libc::SEEK_SET as c_short, - l_start: 0, - l_len: 0, // Lock entire file - l_pid: 0, - }; - + let fd = self.file.borrow(); + let fd = fd.as_fd(); // F_SETLK is a non-blocking lock. The lock will be released when the file is closed // or the process exits or after an explicit unlock. - let lock_result = unsafe { fcntl(fd, F_SETLK, &flock) }; - if lock_result == -1 { - let err = std::io::Error::last_os_error(); - if err.kind() == std::io::ErrorKind::WouldBlock { - return Err(LimboError::LockingError( - "Failed locking file. File is locked by another process".to_string(), - )); + fs::fcntl_lock( + fd, + if exclusive { + FlockOperation::LockExclusive } else { - return Err(LimboError::LockingError(format!( - "Failed locking file, {}", - err - ))); - } - } + FlockOperation::LockShared + }, + ) + .map_err(|e| { + let io_error = std::io::Error::from(e); + let message = match io_error.kind() { + ErrorKind::WouldBlock => { + "Failed locking file. File is locked by another process".to_string() + } + _ => format!("Failed locking file, {}", io_error), + }; + LimboError::LockingError(message) + })?; + Ok(()) } fn unlock_file(&self) -> Result<()> { - let fd = self.file.borrow().as_raw_fd(); - let flock = flock { - l_type: libc::F_UNLCK as c_short, - l_whence: libc::SEEK_SET as c_short, - l_start: 0, - l_len: 0, - l_pid: 0, - }; - - let unlock_result = unsafe { fcntl(fd, F_SETLK, &flock) }; - if unlock_result == -1 { - return Err(LimboError::LockingError(format!( + let fd = self.file.borrow(); + let fd = fd.as_fd(); + fs::fcntl_lock(fd, FlockOperation::Unlock).map_err(|e| { + LimboError::LockingError(format!( "Failed to release file lock: {}", - std::io::Error::last_os_error() - ))); - } + std::io::Error::from(e) + )) + })?; Ok(()) } @@ -263,7 +251,7 @@ impl File for UnixFile { fn sync(&self, c: Rc) -> Result<()> { let file = self.file.borrow(); - let result = rustix::fs::fsync(file.as_fd()); + let result = fs::fsync(file.as_fd()); match result { std::result::Result::Ok(()) => { trace!("fsync"); From b1e8f2da735cd6addee02da169dfaef15e10e7d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 17:00:05 +0100 Subject: [PATCH 4/8] core/io/unix: minor formatting --- core/io/unix.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/core/io/unix.rs b/core/io/unix.rs index e8390dec8..035c9f29d 100644 --- a/core/io/unix.rs +++ b/core/io/unix.rs @@ -88,8 +88,8 @@ impl IO for UnixIO { } } }; - match result { - std::result::Result::Ok(n) => { + return match result { + Ok(n) => { match &cf { CompletionCallback::Read(_, ref c, _) => { c.complete(0); @@ -98,12 +98,10 @@ impl IO for UnixIO { c.complete(n as i32); } } - return Ok(()); + Ok(()) } - Err(e) => { - return Err(e.into()); - } - } + Err(e) => Err(e.into()), + }; } } Ok(()) @@ -132,7 +130,7 @@ enum CompletionCallback { pub struct UnixFile { file: Rc>, - poller: Rc>, + poller: Rc>, callbacks: Rc>>, } @@ -187,7 +185,7 @@ impl File for UnixFile { rustix::io::pread(file.as_fd(), buf.as_mut_slice(), pos as u64) }; match result { - std::result::Result::Ok(n) => { + Ok(n) => { trace!("pread n: {}", n); // Read succeeded immediately c.complete(0); @@ -224,7 +222,7 @@ impl File for UnixFile { rustix::io::pwrite(file.as_fd(), buf.as_slice(), pos as u64) }; match result { - std::result::Result::Ok(n) => { + Ok(n) => { trace!("pwrite n: {}", n); // Read succeeded immediately c.complete(n as i32); @@ -253,7 +251,7 @@ impl File for UnixFile { let file = self.file.borrow(); let result = fs::fsync(file.as_fd()); match result { - std::result::Result::Ok(()) => { + Ok(()) => { trace!("fsync"); c.complete(0); Ok(()) @@ -264,7 +262,7 @@ impl File for UnixFile { fn size(&self) -> Result { let file = self.file.borrow(); - Ok(file.metadata().unwrap().len()) + Ok(file.metadata()?.len()) } } From b146f5d4cba52648942d5b4d03b66ae649a50371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 17:06:11 +0100 Subject: [PATCH 5/8] core/io/io_uring: replace nix and libc calls with their rustix counterparts. core: remove dependency on nix. We keep depending on libc, though, because crate io_uring requires libc's iovec. --- core/io/io_uring.rs | 81 ++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 46 deletions(-) diff --git a/core/io/io_uring.rs b/core/io/io_uring.rs index a499320a4..4ff94519f 100644 --- a/core/io/io_uring.rs +++ b/core/io/io_uring.rs @@ -1,11 +1,13 @@ use super::{common, Completion, File, OpenFlags, IO}; use crate::{LimboError, Result}; -use libc::{c_short, fcntl, flock, iovec, F_SETLK}; use log::{debug, trace}; -use nix::fcntl::{FcntlArg, OFlag}; +use rustix::fs::{self, FlockOperation, OFlags}; +use rustix::io_uring::iovec; use std::cell::RefCell; use std::collections::HashMap; use std::fmt; +use std::io::ErrorKind; +use std::os::fd::AsFd; use std::os::unix::io::AsRawFd; use std::rc::Rc; use thiserror::Error; @@ -136,12 +138,12 @@ impl IO for UringIO { .open(path)?; // Let's attempt to enable direct I/O. Not all filesystems support it // so ignore any errors. - let fd = file.as_raw_fd(); + let fd = file.as_fd(); if direct { - match nix::fcntl::fcntl(fd, FcntlArg::F_SETFL(OFlag::O_DIRECT)) { - Ok(_) => {}, + match fs::fcntl_setfl(fd, OFlags::DIRECT) { + Ok(_) => {} Err(error) => debug!("Error {error:?} returned when setting O_DIRECT flag to read file. The performance of the system may be affected"), - }; + } } let uring_file = Rc::new(UringFile { io: self.inner.clone(), @@ -199,52 +201,39 @@ pub struct UringFile { impl File for UringFile { fn lock_file(&self, exclusive: bool) -> Result<()> { - let fd = self.file.as_raw_fd(); - let flock = flock { - l_type: if exclusive { - libc::F_WRLCK as c_short - } else { - libc::F_RDLCK as c_short - }, - l_whence: libc::SEEK_SET as c_short, - l_start: 0, - l_len: 0, // Lock entire file - l_pid: 0, - }; - + let fd = self.file.as_fd(); // F_SETLK is a non-blocking lock. The lock will be released when the file is closed // or the process exits or after an explicit unlock. - let lock_result = unsafe { fcntl(fd, F_SETLK, &flock) }; - if lock_result == -1 { - let err = std::io::Error::last_os_error(); - if err.kind() == std::io::ErrorKind::WouldBlock { - return Err(LimboError::LockingError( - "File is locked by another process".into(), - )); + fs::fcntl_lock( + fd, + if exclusive { + FlockOperation::LockExclusive } else { - return Err(LimboError::IOError(err)); - } - } + FlockOperation::LockShared + }, + ) + .map_err(|e| { + let io_error = std::io::Error::from(e); + let message = match io_error.kind() { + ErrorKind::WouldBlock => { + "Failed locking file. File is locked by another process".to_string() + } + _ => format!("Failed locking file, {}", io_error), + }; + LimboError::LockingError(message) + })?; + Ok(()) } fn unlock_file(&self) -> Result<()> { - let fd = self.file.as_raw_fd(); - let flock = flock { - l_type: libc::F_UNLCK as c_short, - l_whence: libc::SEEK_SET as c_short, - l_start: 0, - l_len: 0, - l_pid: 0, - }; - - let unlock_result = unsafe { fcntl(fd, F_SETLK, &flock) }; - if unlock_result == -1 { - return Err(LimboError::LockingError(format!( + let fd = self.file.as_fd(); + fs::fcntl_lock(fd, FlockOperation::Unlock).map_err(|e| { + LimboError::LockingError(format!( "Failed to release file lock: {}", - std::io::Error::last_os_error() - ))); - } + std::io::Error::from(e) + )) + })?; Ok(()) } @@ -261,7 +250,7 @@ impl File for UringFile { let len = buf.len(); let buf = buf.as_mut_ptr(); let iovec = io.get_iovec(buf, len); - io_uring::opcode::Readv::new(fd, iovec, 1) + io_uring::opcode::Readv::new(fd, iovec as *const iovec as *const libc::iovec, 1) .offset(pos as u64) .build() .user_data(io.ring.get_key()) @@ -282,7 +271,7 @@ impl File for UringFile { let buf = buffer.borrow(); trace!("pwrite(pos = {}, length = {})", pos, buf.len()); let iovec = io.get_iovec(buf.as_ptr(), buf.len()); - io_uring::opcode::Writev::new(fd, iovec, 1) + io_uring::opcode::Writev::new(fd, iovec as *const iovec as *const libc::iovec, 1) .offset(pos as u64) .build() .user_data(io.ring.get_key()) From 5e9cb58f04436d24c3bcbe5f0d0e5c3007f8d657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sun, 12 Jan 2025 14:11:37 +0100 Subject: [PATCH 6/8] core/io/io_uring: remove unnecessary path prefix for log macros, and replace one unwrap with ? --- core/io/io_uring.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/io/io_uring.rs b/core/io/io_uring.rs index 4ff94519f..14a6bd83c 100644 --- a/core/io/io_uring.rs +++ b/core/io/io_uring.rs @@ -91,7 +91,7 @@ impl InnerUringIO { impl WrappedIOUring { fn submit_entry(&mut self, entry: &io_uring::squeue::Entry, c: Rc) { - log::trace!("submit_entry({:?})", entry); + trace!("submit_entry({:?})", entry); self.pending.insert(entry.get_user_data(), c); unsafe { self.ring @@ -111,7 +111,7 @@ impl WrappedIOUring { // NOTE: This works because CompletionQueue's next function pops the head of the queue. This is not normal behaviour of iterators let entry = self.ring.completion().next(); if entry.is_some() { - log::trace!("get_completion({:?})", entry); + trace!("get_completion({:?})", entry); // consumed an entry from completion queue, update pending_ops self.pending_ops -= 1; } @@ -292,7 +292,7 @@ impl File for UringFile { } fn size(&self) -> Result { - Ok(self.file.metadata().unwrap().len()) + Ok(self.file.metadata()?.len()) } } From 2f90a065337698c5b176f2cd97ca92620823ba19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Mon, 13 Jan 2025 21:03:05 +0100 Subject: [PATCH 7/8] core/io/unix: replace O_NONBLOCK flag from libc with equivalent from rustix --- core/io/unix.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/io/unix.rs b/core/io/unix.rs index 035c9f29d..c021c4566 100644 --- a/core/io/unix.rs +++ b/core/io/unix.rs @@ -7,8 +7,7 @@ use log::{debug, trace}; use polling::{Event, Events, Poller}; use rustix::{ fd::{AsFd, AsRawFd}, - fs, - fs::{FlockOperation, OpenOptionsExt}, + fs::{self, FlockOperation, OFlags, OpenOptionsExt}, io::Errno, }; use std::cell::RefCell; @@ -38,7 +37,7 @@ impl IO for UnixIO { trace!("open_file(path = {})", path); let file = std::fs::File::options() .read(true) - .custom_flags(libc::O_NONBLOCK) + .custom_flags(OFlags::NONBLOCK.bits() as i32) .write(true) .create(matches!(flags, OpenFlags::Create)) .open(path)?; From cca3846f950c262c6a92c28980df155d94ef658b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Mon, 13 Jan 2025 21:15:36 +0100 Subject: [PATCH 8/8] core: Previous commits didn't actually remove nix as dependency, so do that here --- Cargo.lock | 1 - core/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 915456422..621727df3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1217,7 +1217,6 @@ dependencies = [ "miette", "mimalloc", "mockall", - "nix 0.29.0", "pest", "pest_derive", "polling", diff --git a/core/Cargo.toml b/core/Cargo.toml index c0c12a0a3..0daa58c0d 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -40,7 +40,6 @@ fallible-iterator = "0.3.0" hex = "0.4.3" libc = "0.2.155" log = "0.4.20" -nix = { version = "0.29.0", features = ["fs"] } sieve-cache = "0.1.4" sqlite3-parser = { path = "../vendored/sqlite3-parser" } thiserror = "1.0.61"