Skip to content

aio: Squash aio_nowait_supported into fs_info::nowait_works #2893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

xemul
Copy link
Contributor

@xemul xemul commented Aug 1, 2025

When setting up aiocb, the set_nowait() helper checks two bits:

  • global aio_nowait_supported, which comes from CLI option whose default value depends on the kernel uname
  • per-request nowait_works, which is inherited from file that gets it from fs_info object filled based on filesystem type and, again, kernel uname value

This patch removes the global bit, keeps the respective CLI option on reactor config and then updates the per-filesystem fs_info nowait_works with it.

"While at it" drop few unneeded forward declaration.

When setting up aiocb, the set_nowait() helper checks two bits:
- global aio_nowait_supported, which comes from CLI option whose default
  value depends on the kernel uname
- per-request nowait_works, which is inherited from file that gets it
  from fs_info object filled based on filesystem type and, again, kernel
  uname value

This patch removes the global bit, keeps the respective CLI option on
reactor config and then updates the per-filesystem fs_info nowait_works
with it.

"While at it" drop few unneeded forward declaration.

Signed-off-by: Pavel Emelyanov <[email protected]>
@xemul xemul requested review from avikivity and Copilot August 1, 2025 13:08
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors AIO nowait support by consolidating the global aio_nowait_supported variable into the per-filesystem fs_info::nowait_works field. Instead of checking both a global flag and per-filesystem setting, the code now uses only the per-filesystem setting which is updated with the reactor configuration value.

Key changes:

  • Removes the global aio_nowait_supported variable and related forward declarations
  • Adds aio_nowait_works field to reactor_config structure
  • Updates filesystem-specific nowait detection to combine with reactor configuration
  • Simplifies the set_nowait() function to remove the global check

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/unit/file_io_test.cc Updates test to use new reactor test API instead of global variable
src/core/reactor_backend.cc Removes unused forward declaration of global variable
src/core/reactor.cc Removes global variable, adds reactor config field and test API
src/core/file.cc Updates filesystem info to combine with reactor configuration
include/seastar/core/reactor_config.hh Adds aio_nowait_works field to reactor configuration
include/seastar/core/reactor.hh Adds test API for accessing nowait configuration
include/seastar/core/linux-aio.hh Simplifies set_nowait function and removes global variable declaration

@@ -3869,7 +3869,7 @@ reactor_options::reactor_options(program_options::option_group* parent_group)
, blocked_reactor_reports_per_minute(*this, "blocked-reactor-reports-per-minute", 5, "Maximum number of backtraces reported by stall detector per minute")
, blocked_reactor_report_format_oneline(*this, "blocked-reactor-report-format-oneline", true, "Print a simplified backtrace on a single line")
, relaxed_dma(*this, "relaxed-dma", "allow using buffered I/O if DMA is not available (reduces performance)")
, linux_aio_nowait(*this, "linux-aio-nowait", aio_nowait_supported,
, linux_aio_nowait(*this, "linux-aio-nowait", internal::kernel_uname().whitelisted({"4.13"}), // base version where this works
Copy link
Preview

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The inline comment should be moved to a separate line or expanded to better explain the significance of kernel version 4.13 for AIO nowait support.

Suggested change
, linux_aio_nowait(*this, "linux-aio-nowait", internal::kernel_uname().whitelisted({"4.13"}), // base version where this works
// The Linux AIO NOWAIT feature, which allows asynchronous I/O operations to return immediately if they would block,
// was introduced in Linux kernel version 4.13. This option is only enabled on kernels 4.13 and newer.
, linux_aio_nowait(*this, "linux-aio-nowait", internal::kernel_uname().whitelisted({"4.13"}),

Copilot uses AI. Check for mistakes.

@avikivity avikivity merged commit 7543da5 into scylladb:master Aug 1, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants