Merged
Conversation
…return types
This commit enhances error handling throughout the codebase by:
1. **Function Signature Changes**: Modified async functions containing
submit_io_cmd_unified() loops to return Result<(), UblkError> instead
of () for better error propagation.
2. **Simplified Error Handling**: Replaced verbose match statements with
the ? operator for cleaner, more idiomatic Rust error handling:
- Before: match submit_io_cmd_unified() { Ok(f) => f.await, Err(e) => break }
- After: submit_io_cmd_unified()?.await
3. **Function Extraction**: Extracted anonymous async closures into proper
named functions with Result return types for better maintainability:
- examples/ramdisk.rs: Created io_task() -> Result<(), UblkError>
- examples/loop.rs: Created lo_io_task() -> Result<(), UblkError>
- examples/null.rs: Created null_io_task() -> Result<(), UblkError>
- tests/basic.rs: Created test_io_task(), test_auto_reg_io_task(), test_ramdisk_io_task()
- src/ctrl.rs: Updated io_async_fn() -> Result<(), UblkError>
4. **Import Fixes**: Added missing UblkError imports to fix compilation errors.
Benefits:
- Cleaner, more readable error handling code
- Consistent error propagation patterns across the codebase
- Early return on errors via ? operator
- Better separation of concerns with named functions
- Improved maintainability and debuggability
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
…eturn on failure
This commit enhances error handling for submit_fetch_commands_unified() callsites
by implementing graceful failure handling instead of proceeding to wait_and_handle_io()
after failures.
**Key Changes:**
1. **Replace Panic-Prone Pattern**: Changed from:
```rust
.submit_fetch_commands_unified(...)
.unwrap() // <- Would panic on failure
.wait_and_handle_io(handler);
```
To:
```rust
match .submit_fetch_commands_unified(...) {
Ok(q) => q.wait_and_handle_io(handler),
Err(e) => {
log::error!("submit_fetch_commands_unified failed: {}", e);
return; // <- Return instead of proceeding
}
}
```
2. **Prevent Broken I/O Loops**: Functions now return immediately on
submit_fetch_commands_unified() failure instead of entering potentially
broken I/O handling loops.
3. **Improved Error Logging**: All failures are logged with descriptive
error messages for better debugging.
**Files Modified:**
- examples/null.rs: Fixed sync queue handler
- examples/loop.rs: Fixed sync queue handler
- tests/basic.rs: Fixed 4 test handler instances
- src/ctrl.rs: Fixed test queue handler
**Benefits:**
- Prevents crashes from .unwrap() failures
- Avoids wait_and_handle_io() execution with failed queue setup
- Provides clean shutdown with proper error reporting
- Improves system stability and debuggability
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
- Make submit_io_cmd_unified() private (no longer exported) - Add submit_io_prep_cmd() for UBLK_U_IO_FETCH_REQ operations - Add submit_io_commit_cmd() for UBLK_U_IO_COMMIT_AND_FETCH_REQ operations - Convert all users to use new API functions - Optimize by moving submit_io_prep_cmd() out of loops for better performance - Update examples, tests, and documentation to use new API Signed-off-by: Ming Lei <tom.leiming@gmail.com>
…plify error handling - Make submit_io_prep_cmd() and submit_io_commit_cmd() async functions returning Result<i32, UblkError> - Automatically convert UBLK_IO_RES_ABORT to UblkError::QueueIsDown in both functions - Simplify all users to use ? operator instead of manual UBLK_IO_RES_ABORT checks - Update examples, tests, and documentation to use simplified error handling pattern - Suppress QueueIsDown error logging in q_async_fn() as it's expected during shutdown Benefits: - Cleaner code with standard Rust error handling patterns - No more manual UBLK_IO_RES_ABORT checks required - Consistent error propagation using ? operator - Reduced boilerplate in user code - Quieter logs during normal shutdown operations Signed-off-by: Ming Lei <tom.leiming@gmail.com>
- Add Option<&IoBuf> parameter to submit_io_prep_cmd() for automatic buffer registration - Move q.register_io_buf() logic into submit_io_prep_cmd() to simplify API usage - Update all users to pass IoBuf and remove manual register_io_buf calls: - Updated examples (null.rs, loop.rs, ramdisk.rs) to use new API pattern - Updated tests/basic.rs to handle both IoBuf and AutoReg cases - Updated src/ctrl.rs test function - Updated README.md example and documentation - Fix doctests in src/io.rs to use new 4-parameter signature - Maintain backward compatibility for AutoReg cases by passing None Benefits: - Simplified API - users no longer need manual register_io_buf() calls - Better ergonomics - common registration pattern handled automatically - Cleaner code - removed repetitive registration calls throughout codebase Signed-off-by: Ming Lei <tom.leiming@gmail.com>
…ified() - Move automatic buffer registration into submit_fetch_commands_unified() for BufDescList::Slices - Remove manual regiser_io_bufs() calls from all users: - Updated examples/null.rs sync handler to use simplified API - Updated examples/loop.rs to remove redundant registration call - Updated src/ctrl.rs test function to use new pattern - Update documentation to explain automatic buffer registration behavior - Maintain backward compatibility for AutoReg cases (no buffer registration needed) Benefits: - Simplified API - users no longer need manual regiser_io_bufs() calls - Better ergonomics - common registration pattern handled automatically in unified function - Single responsibility - submit_fetch_commands_unified() handles complete initialization workflow - Cleaner code - eliminated repetitive .regiser_io_bufs().submit_fetch_commands_unified() chaining Signed-off-by: Ming Lei <tom.leiming@gmail.com>
…iptor alignment - Add comprehensive verification to ensure BufDesc aligns with provided IoBuf: - BufDesc::Slice with Some(io_buf): Verify slice points to same memory as IoBuf - BufDesc::Slice with None: Ensure slice is empty (user_copy mode) - BufDesc::AutoReg with Some(io_buf): Return error (mutually exclusive) - BufDesc::AutoReg with None: Valid usage for zero-copy operations - ZonedAppendLba/RawAddress: No specific verification needed - Update documentation to explain verification behavior and error conditions - Add test_submit_io_prep_cmd_verification() to validate verification logic - Return -EINVAL error for misaligned buffer descriptors and IoBuf instances Benefits: - Prevents user errors by catching mismatched buffer descriptors at runtime - Better debugging with clear error messages for alignment issues - Type safety ensuring AutoReg and IoBuf aren't used together - Early error detection preventing potential memory corruption - Maintains high performance for correct usage patterns Signed-off-by: Ming Lei <tom.leiming@gmail.com>
- Add async-lock dependency for Semaphore support - Add buf_reg_semaphore to UblkQueue for coordination between async tasks - Add buf_reg_counter for optimized O(1) registration tracking - Implement wait_for_all_buffer_registrations() using semaphore.acquire() - Only check all buffers when counter reaches queue depth (optimization) - Skip synchronization for AutoReg operations (kernel-managed buffers) - Add permits when all buffers registered or for batch operations - Fix zero-copy test timeouts by bypassing wait for AutoReg operations - Update tests to verify counter behavior and semaphore coordination This approach eliminates circular dependencies between async tasks while maintaining proper synchronization, ensuring all buffer registrations complete before prep commands proceed. Performance is maintained with O(1) registration tracking and selective synchronization. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
…tests Update error handling patterns in async task spawns to use pattern matching instead of if-let, filtering out expected QueueIsDown errors during shutdown. Changes: - examples/null.rs: Filter QueueIsDown errors in null_io_task logging - examples/loop.rs: Filter QueueIsDown errors in lo_io_task logging - examples/ramdisk.rs: Filter QueueIsDown errors in io_task logging - tests/basic.rs: Filter QueueIsDown errors in test task logging - test_ublk_null_async: test_io_task error handling - __test_ublk_null_zc: test_auto_reg_io_task error handling - __test_ublk_ramdisk: test_ramdisk_io_task error handling This provides cleaner log output by avoiding noise from expected shutdown errors while still capturing genuine error conditions. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
…k fails - Add mlock failure state tracking with UBLK_QUEUE_MLOCK_FAIL flag - Mark mlock failure in register_io_buf() when IoBuf.mlock() fails - Add mlock failure check to submit_fetch_commands_unified() - Add conditional mlock failure check to submit_io_cmd_unified() for FETCH_REQ operations only - Add mlock failure check to __wait_ios() to fail immediately with -EPERM - Prevents unsafe operations when memory locking has failed Signed-off-by: Ming Lei <tom.leiming@gmail.com>
- Add synchronization mechanism using std::sync::Condvar to UblkDev - UblkCtrl::start_dev() and start_dev_async() now wait for all queue buffer registrations to complete - Immediate failure on any mlock errors during buffer registration - Simplified API with mlock_failed parameter in notify_buffer_registration_complete() - Remove total_queues from BufferRegState, pass nr_hw_queues as parameter - Make BufferRegState and buf_reg_sync field pub(crate) for proper encapsulation Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
… APIs Document that UBLK_DEV_F_MLOCK_IO_BUFFER is not supported with deprecated submit_io_cmd() and submit_fetch_commands() methods. Users should migrate to the unified APIs (submit_io_prep_cmd, submit_io_commit_cmd, and submit_fetch_commands_unified) for mlock functionality. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Add buffer address validation to submit_io_commit_cmd() and complete_io_cmd_unified() when UBLK_DEV_F_MLOCK_IO_BUFFER is enabled: - Created validate_mlock_buffer_consistency() helper function for code reuse - Validates that BufDesc::Slice buffer addresses match UblkQueue::bufs[tag] - Ensures mlock'd buffers are used consistently across operations - Added comprehensive documentation explaining the validation constraint - Returns -EINVAL if buffer addresses don't match - Helper function is marked #[inline(always)] for performance This prevents potential memory safety issues by ensuring registered mlock'd buffers are used consistently throughout the I/O lifecycle. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
refactor io code path
improve mlock failure handling