7960: feat: add SQPOLL support for io_uring#72
Conversation
This adds `uring_setup_sqpoll` to the Runtime `Builder`, allowing the io_uring driver to be configured with SQPOLL enabled. This offloads submission queue polling to a kernel thread, which can significantly reduce syscall overhead. - Added `uring_setup_sqpoll` to `tokio::runtime::Builder`. - Updated `UringContext` to sync the submission queue and check `need_wakeup`. - Added tests to verify `sqpoll` in both single-thread and multi-thread runtimes.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances Tokio's Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughThis pull request adds support for configuring SQ poll idle timeout in Tokio's io_uring driver. A new builder method ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for SQPOLL in io_uring, which can improve performance by reducing syscall overhead. The changes introduce a new uring_setup_sqpoll method to the runtime Builder and correctly wire it through to the io_uring driver initialization. The implementation also includes logic to avoid unnecessary syscalls when SQPOLL is enabled. New integration tests are added to verify the functionality for both single-threaded and multi-threaded runtimes.
My main feedback is to address a small amount of code duplication in builder.rs to improve maintainability. Otherwise, the changes look good.
| #[cfg(all(tokio_unstable, feature = "io-uring", target_os = "linux"))] | ||
| if let Some(idle) = self.uring_setup_sqpoll { | ||
| if let Some(io) = driver_handle.io.as_ref() { | ||
| io.setup_uring_sqpoll(idle); | ||
| } | ||
| } |
There was a problem hiding this comment.
This block of code is identical to the one in build_current_thread_runtime_components (lines 1607-1612). To avoid duplication and improve maintainability, consider extracting this logic into a private helper function.
For example, you could add a helper method like this:
#[cfg(all(tokio_unstable, feature = "io-uring", target_os = "linux"))]
fn setup_uring_sqpoll(&self, driver_handle: &driver::Handle) {
if let Some(idle) = self.uring_setup_sqpoll {
if let Some(io) = driver_handle.io.as_ref() {
io.setup_uring_sqpoll(idle);
}
}
}And then call it from both build_current_thread_runtime_components and build_threaded_runtime.
There was a problem hiding this comment.
value:good-to-have; category:bug; feedback: The Gemini AI reviewer is correct! The same snippet of code is used in two places. It would be good to extract it to a helper function and reuse it. This would prevent any eventual regressions in the future if a bug is fixed in only one of the duplicated snippets.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tokio/src/runtime/builder.rs (1)
1758-1779: Documentation could mention privilege requirements and error behavior.The SQPOLL feature typically requires elevated privileges (CAP_SYS_ADMIN or kernel 5.11+ with specific configs). Consider documenting:
- That SQPOLL may fail to initialize on systems without proper privileges
- What happens when initialization fails (falls back to regular io_uring or returns error?)
📝 Suggested documentation enhancement
/// Enables SQPOLL for the io_uring driver and sets the idle timeout. /// /// When SQPOLL is enabled, a kernel thread is created to poll the /// submission queue. This can reduce syscall overhead. /// + /// # Requirements + /// + /// SQPOLL mode may require elevated privileges depending on kernel version. + /// On Linux 5.11+, unprivileged users may use SQPOLL if the system is configured + /// to allow it. On older kernels, `CAP_SYS_ADMIN` is typically required. + /// + /// If SQPOLL initialization fails due to permission issues, the runtime + /// will return an error during build. + /// /// # Examples🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tokio/src/runtime/builder.rs` around lines 1758 - 1779, The doc comment for Builder::uring_setup_sqpoll should be expanded to mention that enabling SQPOLL may require elevated privileges (e.g., CAP_SYS_ADMIN) or specific kernel configuration (kernel ≥5.11+ and proper io_uring options) and to state the observable behavior when SQPOLL initialization fails; update the docs on uring_setup_sqpoll (and nearby enable_io_uring/build examples) to explicitly note the privilege/kernel requirements and whether the runtime will fall back to non-SQPOLL io_uring or cause build()/runtime initialization to return an error so callers know what to expect at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tokio/src/runtime/builder.rs`:
- Around line 1758-1779: The doc comment for Builder::uring_setup_sqpoll should
be expanded to mention that enabling SQPOLL may require elevated privileges
(e.g., CAP_SYS_ADMIN) or specific kernel configuration (kernel ≥5.11+ and proper
io_uring options) and to state the observable behavior when SQPOLL
initialization fails; update the docs on uring_setup_sqpoll (and nearby
enable_io_uring/build examples) to explicitly note the privilege/kernel
requirements and whether the runtime will fall back to non-SQPOLL io_uring or
cause build()/runtime initialization to return an error so callers know what to
expect at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 706acee0-06a2-4a20-9b8f-ee214e719a25
📒 Files selected for processing (4)
tokio/src/runtime/builder.rstokio/src/runtime/driver.rstokio/src/runtime/io/driver/uring.rstokio/tests/fs_uring_sqpoll.rs
Contributing to TokioThanks for your help improving Tokio! We are so happy to have you! There are opportunities to contribute to Tokio at any level. It doesn't matter if No contribution is too small and all contributions are valued. See the contributing guidelines to get started. Code of ConductThe Tokio project adheres to the Rust Code of Conduct. This describes Need Help?Reach out to us on the Discord server for any concern not covered in this guide. LTS guaranteesIn Tokio ≥1.0.0, each LTS release comes with the guarantee of at least one year of The goal of these guarantees is to provide stability to the ecosystem. Minimum Supported Rust Version (MSRV)
Versioning PolicyWith Tokio ≥1.0.0:
This is as defined by Semantic Versioning 2.0. |
|
test simple body again |
PR Review: Add SQPOLL support for io_uringline2 |
🤖 Augment PR SummarySummary: Adds optional SQPOLL configuration for Tokio’s io_uring-backed filesystem I/O on Linux (unstable), allowing users to enable a kernel SQ polling thread and set its idle timeout. Changes:
Technical Notes: The SQPOLL setup is applied prior to first io_uring initialization (which is lazy), and the submission path uses 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| cfg.timer_flavor = TimerFlavor::Traditional; | ||
| let (driver, driver_handle) = driver::Driver::new(cfg)?; | ||
|
|
||
| #[cfg(all(tokio_unstable, feature = "io-uring", target_os = "linux"))] |
There was a problem hiding this comment.
The cfg here doesn’t match the conditions under which the io_uring pieces (including setup_uring_sqpoll) are actually compiled (cfg_io_uring! also requires feature = "rt" and feature = "fs"). As written, building with tokio_unstable + io-uring on Linux but without fs can try to compile this call path and fail due to missing symbols.
Severity: high
Other Locations
tokio/src/runtime/builder.rs:1819
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
value:useful; category:bug; feedback: The Augment AI reviewer is correct! All io_uring related functionalities should be gated behind the same cfg!. Otherwise some methods may be visible in some config but not usable due to others being missing in this config.
| } | ||
|
|
||
| let uring = IoUring::new(DEFAULT_RING_SIZE)?; | ||
| let uring = if let Some(idle) = self.sqpoll_idle { |
There was a problem hiding this comment.
setup_sqpoll can fail with EPERM on systems that allow regular io_uring but don’t permit SQPOLL; since check_and_init treats EPERM as “io_uring unavailable”, this would disable io_uring entirely when SQPOLL is requested. Consider distinguishing SQPOLL permission errors from “io_uring blocked” errors so non-SQPOLL io_uring can still be used.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
value:valid-but-wont-fix; category:bug; feedback: The Augment AI reviewer is нот correct! If the SQPOLL setup fails then the error should be returned to the caller. Then it can try to setup io_uring without SQPOLL. The Tokio library should not try to do this automatically without letting know the developer.
PR Review: Add SQPOLL support for io_uringOverall, this is a solid feature addition. The SQPOLL submission optimization in Correctness Issues1. Tests may not exercise io_uring or SQPOLL at all The tests call 2. Tests do not handle SQPOLL unavailability
3. Potential busy-spin during The existing Drop loop: while !self.ring_mut().submission().is_empty() {
self.submit().expect("Internal error when dropping driver");
}When SQPOLL is active and Design Issues4. Inconsistent The Consequences:
These cfg guards should be aligned. Also consider whether requiring 5. Post-hoc
Documentation6. The current docs only mention "can reduce syscall overhead." Please also document:
Minor
|
value:good-to-have; category:documentation; feedback: The CodeRabbit AI is correct! The documentation should be updated with any relevant information about the minimum Linux kernel version, any required privileges, etc. This will help the developers to decide whether they could use the SQPOLL functionality or not. |
value:annoying; category:bug; feedback: The Claude AI reviewer is not correct! |
value:useful; category:bug; feedback: The Claude AI reviewer is correct! All io_uring related functionalities should be gated behind the same cfg!. Otherwise some methods may be visible in some config but not usable due to others being missing in this config. |
2 participants
7960: To review by AI