Block device drivers need to invoke `blk_mq_start_request` on a request to indicate that they have started processing the request. This function may only be called once after a request has been issued to a driver. For Rust block device drivers, the Rust abstractions handle this call. However, in some situations a driver may want to control when a request is started. Thus, expose the start method to Rust block device drivers. To ensure the method is not called more than once, introduce a type state for `Request`. Requests are issued as `IdleRequest` and transition to `Request` when the `start` method is called. Signed-off-by: Andreas Hindborg --- drivers/block/rnull/rnull.rs | 3 +- rust/kernel/block/mq.rs | 5 +- rust/kernel/block/mq/operations.rs | 13 ++-- rust/kernel/block/mq/request.rs | 155 ++++++++++++++++++++++++++++++------- 4 files changed, 136 insertions(+), 40 deletions(-) diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs index aa59ede72e495..034ddc06eabf9 100644 --- a/drivers/block/rnull/rnull.rs +++ b/drivers/block/rnull/rnull.rs @@ -575,9 +575,10 @@ fn new_request_data() -> impl PinInit { fn queue_rq( hw_data: Pin<&SpinLock>, this: Pin<&Self>, - mut rq: Owned>, + rq: Owned>, _is_last: bool, ) -> Result { + let mut rq = rq.start(); let mut sectors = rq.sectors(); Self::handle_bad_blocks(this.get_ref(), &mut rq, &mut sectors)?; diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs index 3eab3ca8f5480..ab493bd91af4c 100644 --- a/rust/kernel/block/mq.rs +++ b/rust/kernel/block/mq.rs @@ -88,10 +88,10 @@ //! fn queue_rq( //! _hw_data: (), //! _queue_data: (), -//! rq: Owned>, +//! rq: Owned>, //! _is_last: bool //! ) -> Result { -//! rq.end_ok(); +//! rq.start().end_ok(); //! Ok(()) //! } //! @@ -130,6 +130,7 @@ pub mod tag_set; pub use operations::Operations; +pub use request::IdleRequest; pub use request::Request; pub use request::RequestTimerHandle; pub use tag_set::TagSet; diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs index 6641c340d87f4..fb75d65f67071 100644 --- a/rust/kernel/block/mq/operations.rs +++ b/rust/kernel/block/mq/operations.rs @@ -6,13 +6,13 @@ use crate::{ bindings, - block::mq::{request::RequestDataWrapper, Request}, + block::mq::{request::RequestDataWrapper, IdleRequest, Request}, error::{from_result, Result}, prelude::*, sync::{aref::ARef, atomic::ordering, Refcount}, types::{ForeignOwnable, Owned}, }; -use core::{marker::PhantomData, ptr::NonNull}; +use core::marker::PhantomData; use pin_init::PinInit; type ForeignBorrowed<'a, T> = ::Borrowed<'a>; @@ -60,7 +60,7 @@ pub trait Operations: Sized { fn queue_rq( hw_data: ForeignBorrowed<'_, Self::HwData>, queue_data: ForeignBorrowed<'_, Self::QueueData>, - rq: Owned>, + rq: Owned>, is_last: bool, ) -> Result; @@ -132,14 +132,14 @@ impl OperationsVTable { == 0 ); + // INVARIANT: By C API contract, `bd.rq` has not been started yet. // SAFETY: // - By API contract, we own the request. // - By the safety requirements of this function, `request` is a valid // `struct request` and the private data is properly initialized. // - `rq` will be alive until `blk_mq_end_request` is called and is // reference counted by until then. - let mut rq = - unsafe { Owned::from_raw(NonNull::>::new_unchecked((*bd).rq.cast())) }; + let rq = unsafe { IdleRequest::from_raw((*bd).rq) }; // SAFETY: The safety requirement for this function ensure that `hctx` // is valid and that `driver_data` was produced by a call to @@ -155,9 +155,6 @@ impl OperationsVTable { // dropped, which happens after we are dropped. let queue_data = unsafe { T::QueueData::borrow(queue_data) }; - // SAFETY: We have exclusive access and we just set the refcount above. - unsafe { rq.start_unchecked() }; - let ret = T::queue_rq( hw_data, queue_data, diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs index 38289b9f966fa..f6addd20624a9 100644 --- a/rust/kernel/block/mq/request.rs +++ b/rust/kernel/block/mq/request.rs @@ -15,13 +15,111 @@ time::hrtimer::{ HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, HrTimerMode, HrTimerPointer, }, - types::{Opaque, Ownable, OwnableRefCounted, Owned}, + types::{ForeignOwnable, Opaque, Ownable, OwnableRefCounted, Owned}, }; -use core::{ffi::c_void, marker::PhantomData, ptr::NonNull}; +use core::{ffi::c_void, marker::PhantomData, ops::Deref, ptr::NonNull}; use crate::block::bio::Bio; use crate::block::bio::BioIterator; +/// A [`Request`] that a driver has not yet begun to process. +/// +/// A driver can convert an `IdleRequest` to a [`Request`] by calling [`IdleRequest::start`]. +/// +/// # Invariants +/// +/// - This request has not been started yet. +#[repr(transparent)] +pub struct IdleRequest(RequestInner); + +impl IdleRequest { + /// Mark the request as processing. + /// + /// This converts the [`IdleRequest`] into a [`Request`]. + pub fn start(self: Owned) -> Owned> { + // SAFETY: By type invariant `self.0.0` is a valid request. Because we have an `Owned<_>`, + // the refcount is zero. + let mut request = unsafe { Request::from_raw(self.0 .0.get()) }; + + debug_assert!( + request + .wrapper_ref() + .refcount() + .as_atomic() + .load(ordering::Acquire) + == 0 + ); + + // SAFETY: We have exclusive access and the refcount is 0. By type invariant `request` was + // not started yet. + unsafe { request.start_unchecked() }; + + request + } + + /// Create a [`Self`] from a raw request pointer. + /// + /// # Safety + /// + /// - The request pointed to by `ptr` must satisfythe invariants of both [`Request`] and + /// [`Self`]. + /// - The refcount of the request pointed to by `ptr` must be 0. + pub(crate) unsafe fn from_raw(ptr: *mut bindings::request) -> Owned { + // SAFETY: By function safety requirements, `ptr` is valid for use as an `IdleRequest`. + unsafe { Owned::from_raw(NonNull::::new_unchecked(ptr.cast())) } + } +} + +// SAFETY: The `release` implementation leaks the `IdleRequest`, which is a valid state for a +// [`Request`] with refcount 0. +unsafe impl Ownable for IdleRequest { + unsafe fn release(_this: NonNull) {} +} + +impl Deref for IdleRequest { + type Target = RequestInner; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +pub struct RequestInner(Opaque, PhantomData); + +impl RequestInner { + /// Get the command identifier for the request + pub fn command(&self) -> u32 { + // SAFETY: By C API contract and type invariant, `cmd_flags` is valid for read + unsafe { (*self.0.get()).cmd_flags & ((1 << bindings::REQ_OP_BITS) - 1) } + } + + /// Get the target sector for the request. + #[inline(always)] + pub fn sector(&self) -> u64 { + // SAFETY: By type invariant of `Self`, `self.0` is valid and live. + unsafe { (*self.0.get()).__sector } + } + + /// Get the size of the request in number of sectors. + #[inline(always)] + pub fn sectors(&self) -> u32 { + self.bytes() >> crate::block::SECTOR_SHIFT + } + + /// Get the size of the request in bytes. + #[inline(always)] + pub fn bytes(&self) -> u32 { + // SAFETY: By type invariant of `Self`, `self.0` is valid and live. + unsafe { (*self.0.get()).__data_len } + } + + /// Borrow the queue data from the request queue associated with this request. + pub fn queue_data(&self) -> ::Borrowed<'_> { + // SAFETY: By type invariants of `Request`, `self.0` is a valid request. + unsafe { T::QueueData::borrow((*(*self.0.get()).q).queuedata) } + } +} + /// A wrapper around a blk-mq [`struct request`]. This represents an IO request. /// /// # Implementation details @@ -58,9 +156,28 @@ /// [`struct request`]: srctree/include/linux/blk-mq.h /// #[repr(transparent)] -pub struct Request(Opaque, PhantomData); +pub struct Request(RequestInner); + +impl Deref for Request { + type Target = RequestInner; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} impl Request { + /// Create a `Owned` from a request pointer. + /// + /// # Safety + /// + /// - `ptr` must satisfy invariants of `Request`. + /// - The refcount of the request pointed to by `ptr` must be 0. + pub(crate) unsafe fn from_raw(ptr: *mut bindings::request) -> Owned { + // SAFETY: By function safety requirements, `ptr` is valid for use as `Owned`. + unsafe { Owned::from_raw(NonNull::::new_unchecked(ptr.cast())) } + } + /// Create an [`ARef`] from a [`struct request`] pointer. /// /// # Safety @@ -78,12 +195,6 @@ pub(crate) unsafe fn aref_from_raw(ptr: *mut bindings::request) -> ARef { unsafe { ARef::from_raw(NonNull::new_unchecked(ptr.cast())) } } - /// Get the command identifier for the request - pub fn command(&self) -> u32 { - // SAFETY: By C API contract and type invariant, `cmd_flags` is valid for read - unsafe { (*self.0.get()).cmd_flags & ((1 << bindings::REQ_OP_BITS) - 1) } - } - /// Complete the request by scheduling `Operations::complete` for /// execution. /// @@ -106,7 +217,7 @@ pub fn complete(this: ARef) { pub fn bio(&self) -> Option<&Bio> { // SAFETY: By type invariant of `Self`, `self.0` is valid and the deref // is safe. - let ptr = unsafe { (*self.0.get()).bio }; + let ptr = unsafe { (*self.0 .0.get()).bio }; // SAFETY: By C API contract, if `bio` is not null it will have a // positive refcount at least for the duration of the lifetime of // `&self`. @@ -118,7 +229,7 @@ pub fn bio(&self) -> Option<&Bio> { pub fn bio_mut(&mut self) -> Option<&mut Bio> { // SAFETY: By type invariant of `Self`, `self.0` is valid and the deref // is safe. - let ptr = unsafe { (*self.0.get()).bio }; + let ptr = unsafe { (*self.0 .0.get()).bio }; // SAFETY: By C API contract, if `bio` is not null it will have a // positive refcount at least for the duration of the lifetime of // `&self`. @@ -132,25 +243,11 @@ pub fn bio_iter_mut<'a>(self: &'a mut Owned) -> BioIterator<'a> { // `NonNull::new` will return `None` if the pointer is null. BioIterator { // SAFETY: By type invariant `self.0` is a valid `struct request`. - bio: NonNull::new(unsafe { (*self.0.get()).bio.cast() }), + bio: NonNull::new(unsafe { (*self.0 .0.get()).bio.cast() }), _p: PhantomData, } } - /// Get the target sector for the request. - #[inline(always)] - pub fn sector(&self) -> u64 { - // SAFETY: By type invariant of `Self`, `self.0` is valid and live. - unsafe { (*self.0.get()).__sector } - } - - /// Get the size of the request in number of sectors. - #[inline(always)] - pub fn sectors(&self) -> u32 { - // SAFETY: By type invariant of `Self`, `self.0` is valid and live. - (unsafe { (*self.0.get()).__data_len }) >> crate::block::SECTOR_SHIFT - } - /// Return a pointer to the [`RequestDataWrapper`] stored in the private area /// of the request structure. /// @@ -289,10 +386,10 @@ impl Owned> { /// `self.wrapper_ref().refcount() == 0`. /// /// This can only be called once in the request life cycle. - pub(crate) unsafe fn start_unchecked(&mut self) { + pub unsafe fn start_unchecked(&mut self) { // SAFETY: By type invariant, `self.0` is a valid `struct request` and // we have exclusive access. - unsafe { bindings::blk_mq_start_request(self.0.get()) }; + unsafe { bindings::blk_mq_start_request(self.0 .0.get()) }; } /// Notify the block layer that the request has been completed without errors. @@ -302,7 +399,7 @@ pub fn end_ok(self) { /// Notify the block layer that the request has been completed. pub fn end(self, status: u8) { - let request_ptr = self.0.get().cast(); + let request_ptr = self.0 .0.get().cast(); core::mem::forget(self); // SAFETY: By type invariant, `this.0` was a valid `struct request`. The // existence of `self` guarantees that there are no `ARef`s pointing to -- 2.51.2