Allow users of the rust block device driver API to install private data in
the `GenDisk` structure.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
drivers/block/rnull/rnull.rs | 8 ++++---
rust/kernel/block/mq.rs | 7 +++---
rust/kernel/block/mq/gen_disk.rs | 32 ++++++++++++++++++++++----
rust/kernel/block/mq/operations.rs | 46 ++++++++++++++++++++++++++++++--------
4 files changed, 74 insertions(+), 19 deletions(-)
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 8690ff5f974f..8255236bc550 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -61,14 +61,16 @@ fn new(
.logical_block_size(block_size)?
.physical_block_size(block_size)?
.rotational(rotational)
- .build(fmt!("{}", name.to_str()?), tagset)
+ .build(fmt!("{}", name.to_str()?), tagset, ())
}
}
#[vtable]
impl Operations for NullBlkDevice {
+ type QueueData = ();
+
#[inline(always)]
- fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
+ fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
mq::Request::end_ok(rq)
.map_err(|_e| kernel::error::code::EIO)
// We take no refcounts on the request, so we expect to be able to
@@ -79,5 +81,5 @@ fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
Ok(())
}
- fn commit_rqs() {}
+ fn commit_rqs(_queue_data: ()) {}
}
diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 98fa0d6bc8f7..6e546f4f3d1c 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -69,20 +69,21 @@
//!
//! #[vtable]
//! impl Operations for MyBlkDevice {
+//! type QueueData = ();
//!
-//! fn queue_rq(rq: ARef<Request<Self>>, _is_last: bool) -> Result {
+//! fn queue_rq(_queue_data: (), rq: ARef<Request<Self>>, _is_last: bool) -> Result {
//! Request::end_ok(rq);
//! Ok(())
//! }
//!
-//! fn commit_rqs() {}
+//! fn commit_rqs(_queue_data: ()) {}
//! }
//!
//! let tagset: Arc<TagSet<MyBlkDevice>> =
//! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
//! let mut disk = gen_disk::GenDiskBuilder::new()
//! .capacity_sectors(4096)
-//! .build(format_args!("myblk"), tagset)?;
+//! .build(format_args!("myblk"), tagset, ())?;
//!
//! # Ok::<(), kernel::error::Error>(())
//! ```
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 6b1b846874db..46ec80269970 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -13,6 +13,7 @@
static_lock_class,
str::NullTerminatedFormatter,
sync::Arc,
+ types::{ForeignOwnable, ScopeGuard},
};
use core::fmt::{self, Write};
@@ -98,7 +99,14 @@ pub fn build<T: Operations>(
self,
name: fmt::Arguments<'_>,
tagset: Arc<TagSet<T>>,
+ queue_data: T::QueueData,
) -> Result<GenDisk<T>> {
+ let data = queue_data.into_foreign();
+ let recover_data = ScopeGuard::new(|| {
+ // SAFETY: T::QueueData was created by the call to `into_foreign()` above
+ drop(unsafe { T::QueueData::from_foreign(data) });
+ });
+
// SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed.
let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() };
@@ -113,7 +121,7 @@ pub fn build<T: Operations>(
bindings::__blk_mq_alloc_disk(
tagset.raw_tag_set(),
&mut lim,
- core::ptr::null_mut(),
+ data,
static_lock_class!().as_ptr(),
)
})?;
@@ -167,8 +175,12 @@ pub fn build<T: Operations>(
},
)?;
+ recover_data.dismiss();
+
// INVARIANT: `gendisk` was initialized above.
// INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above.
+ // INVARIANT: `gendisk.queue.queue_data` is set to `data` in the call to
+ // `__blk_mq_alloc_disk` above.
Ok(GenDisk {
_tagset: tagset,
gendisk,
@@ -180,9 +192,10 @@ pub fn build<T: Operations>(
///
/// # Invariants
///
-/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
-/// - `gendisk` was added to the VFS through a call to
-/// `bindings::device_add_disk`.
+/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
+/// - `gendisk` was added to the VFS through a call to
+/// `bindings::device_add_disk`.
+/// - `self.gendisk.queue.queuedata` is initialized by a call to `ForeignOwnable::into_foreign`.
pub struct GenDisk<T: Operations> {
_tagset: Arc<TagSet<T>>,
gendisk: *mut bindings::gendisk,
@@ -194,9 +207,20 @@ unsafe impl<T: Operations + Send> Send for GenDisk<T> {}
impl<T: Operations> Drop for GenDisk<T> {
fn drop(&mut self) {
+ // SAFETY: By type invariant of `Self`, `self.gendisk` points to a valid
+ // and initialized instance of `struct gendisk`, and, `queuedata` was
+ // initialized with the result of a call to
+ // `ForeignOwnable::into_foreign`.
+ let queue_data = unsafe { (*(*self.gendisk).queue).queuedata };
+
// SAFETY: By type invariant, `self.gendisk` points to a valid and
// initialized instance of `struct gendisk`, and it was previously added
// to the VFS.
unsafe { bindings::del_gendisk(self.gendisk) };
+
+ // SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with
+ // a call to `ForeignOwnable::into_foreign` to create `queuedata`.
+ // `ForeignOwnable::from_foreign` is only called here.
+ drop(unsafe { T::QueueData::from_foreign(queue_data) });
}
}
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index c2b98f507bcb..6fb256f55acc 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -6,14 +6,15 @@
use crate::{
bindings,
- block::mq::request::RequestDataWrapper,
- block::mq::Request,
+ block::mq::{request::RequestDataWrapper, Request},
error::{from_result, Result},
prelude::*,
- types::ARef,
+ types::{ARef, ForeignOwnable},
};
use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
+type ForeignBorrowed<'a, T> = <T as ForeignOwnable>::Borrowed<'a>;
+
/// Implement this trait to interface blk-mq as block devices.
///
/// To implement a block device driver, implement this trait as described in the
@@ -26,12 +27,20 @@
/// [module level documentation]: kernel::block::mq
#[macros::vtable]
pub trait Operations: Sized {
+ /// Data associated with the `struct request_queue` that is allocated for
+ /// the `GenDisk` associated with this `Operations` implementation.
+ type QueueData: ForeignOwnable;
+
/// Called by the kernel to queue a request with the driver. If `is_last` is
/// `false`, the driver is allowed to defer committing the request.
- fn queue_rq(rq: ARef<Request<Self>>, is_last: bool) -> Result;
+ fn queue_rq(
+ queue_data: ForeignBorrowed<'_, Self::QueueData>,
+ rq: ARef<Request<Self>>,
+ is_last: bool,
+ ) -> Result;
/// Called by the kernel to indicate that queued requests should be submitted.
- fn commit_rqs();
+ fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
/// Called by the kernel to poll the device for completed requests. Only
/// used for poll queues.
@@ -70,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
/// promise to not access the request until the driver calls
/// `bindings::blk_mq_end_request` for the request.
unsafe extern "C" fn queue_rq_callback(
- _hctx: *mut bindings::blk_mq_hw_ctx,
+ hctx: *mut bindings::blk_mq_hw_ctx,
bd: *const bindings::blk_mq_queue_data,
) -> bindings::blk_status_t {
// SAFETY: `bd.rq` is valid as required by the safety requirement for
@@ -88,10 +97,20 @@ impl<T: Operations> OperationsVTable<T> {
// reference counted by `ARef` until then.
let rq = unsafe { Request::aref_from_raw((*bd).rq) };
+ // SAFETY: `hctx` is valid as required by this function.
+ let queue_data = unsafe { (*(*hctx).queue).queuedata };
+
+ // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
+ // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
+ // `ForeignOwnable::from_foreign()` is only called when the tagset is
+ // 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 { Request::start_unchecked(&rq) };
let ret = T::queue_rq(
+ queue_data,
rq,
// SAFETY: `bd` is valid as required by the safety requirement for
// this function.
@@ -110,9 +129,18 @@ impl<T: Operations> OperationsVTable<T> {
///
/// # Safety
///
- /// This function may only be called by blk-mq C infrastructure.
- unsafe extern "C" fn commit_rqs_callback(_hctx: *mut bindings::blk_mq_hw_ctx) {
- T::commit_rqs()
+ /// This function may only be called by blk-mq C infrastructure. The caller
+ /// must ensure that `hctx` is valid.
+ unsafe extern "C" fn commit_rqs_callback(hctx: *mut bindings::blk_mq_hw_ctx) {
+ // SAFETY: `hctx` is valid as required by this function.
+ let queue_data = unsafe { (*(*hctx).queue).queuedata };
+
+ // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
+ // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
+ // `ForeignOwnable::from_foreign()` is only called when the tagset is
+ // dropped, which happens after we are dropped.
+ let queue_data = unsafe { T::QueueData::borrow(queue_data) };
+ T::commit_rqs(queue_data)
}
/// This function is called by the C kernel. It is not currently
--
2.47.2
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote: > > Allow users of the rust block device driver API to install private data in > the `GenDisk` structure. > > Reviewed-by: Alice Ryhl <aliceryhl@google.com> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org> > --- > drivers/block/rnull/rnull.rs | 8 ++++--- > rust/kernel/block/mq.rs | 7 +++--- > rust/kernel/block/mq/gen_disk.rs | 32 ++++++++++++++++++++++---- > rust/kernel/block/mq/operations.rs | 46 ++++++++++++++++++++++++++++++-------- > 4 files changed, 74 insertions(+), 19 deletions(-) > > diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs > index 8690ff5f974f..8255236bc550 100644 > --- a/drivers/block/rnull/rnull.rs > +++ b/drivers/block/rnull/rnull.rs > @@ -61,14 +61,16 @@ fn new( > .logical_block_size(block_size)? > .physical_block_size(block_size)? > .rotational(rotational) > - .build(fmt!("{}", name.to_str()?), tagset) > + .build(fmt!("{}", name.to_str()?), tagset, ()) > } > } > > #[vtable] > impl Operations for NullBlkDevice { > + type QueueData = (); > + > #[inline(always)] > - fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result { > + fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result { > mq::Request::end_ok(rq) > .map_err(|_e| kernel::error::code::EIO) > // We take no refcounts on the request, so we expect to be able to > @@ -79,5 +81,5 @@ fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result { > Ok(()) > } > > - fn commit_rqs() {} > + fn commit_rqs(_queue_data: ()) {} > } > diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs > index 98fa0d6bc8f7..6e546f4f3d1c 100644 > --- a/rust/kernel/block/mq.rs > +++ b/rust/kernel/block/mq.rs > @@ -69,20 +69,21 @@ > //! > //! #[vtable] > //! impl Operations for MyBlkDevice { > +//! type QueueData = (); > //! > -//! fn queue_rq(rq: ARef<Request<Self>>, _is_last: bool) -> Result { > +//! fn queue_rq(_queue_data: (), rq: ARef<Request<Self>>, _is_last: bool) -> Result { > //! Request::end_ok(rq); > //! Ok(()) > //! } > //! > -//! fn commit_rqs() {} > +//! fn commit_rqs(_queue_data: ()) {} > //! } > //! > //! let tagset: Arc<TagSet<MyBlkDevice>> = > //! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?; > //! let mut disk = gen_disk::GenDiskBuilder::new() > //! .capacity_sectors(4096) > -//! .build(format_args!("myblk"), tagset)?; > +//! .build(format_args!("myblk"), tagset, ())?; > //! > //! # Ok::<(), kernel::error::Error>(()) > //! ``` > diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs > index 6b1b846874db..46ec80269970 100644 > --- a/rust/kernel/block/mq/gen_disk.rs > +++ b/rust/kernel/block/mq/gen_disk.rs > @@ -13,6 +13,7 @@ > static_lock_class, > str::NullTerminatedFormatter, > sync::Arc, > + types::{ForeignOwnable, ScopeGuard}, > }; > use core::fmt::{self, Write}; > > @@ -98,7 +99,14 @@ pub fn build<T: Operations>( > self, > name: fmt::Arguments<'_>, > tagset: Arc<TagSet<T>>, > + queue_data: T::QueueData, > ) -> Result<GenDisk<T>> { > + let data = queue_data.into_foreign(); > + let recover_data = ScopeGuard::new(|| { > + // SAFETY: T::QueueData was created by the call to `into_foreign()` above > + drop(unsafe { T::QueueData::from_foreign(data) }); > + }); > + > // SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed. > let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() }; > > @@ -113,7 +121,7 @@ pub fn build<T: Operations>( > bindings::__blk_mq_alloc_disk( > tagset.raw_tag_set(), > &mut lim, > - core::ptr::null_mut(), > + data, > static_lock_class!().as_ptr(), > ) > })?; > @@ -167,8 +175,12 @@ pub fn build<T: Operations>( > }, > )?; > > + recover_data.dismiss(); > + > // INVARIANT: `gendisk` was initialized above. > // INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above. > + // INVARIANT: `gendisk.queue.queue_data` is set to `data` in the call to > + // `__blk_mq_alloc_disk` above. > Ok(GenDisk { > _tagset: tagset, > gendisk, > @@ -180,9 +192,10 @@ pub fn build<T: Operations>( > /// > /// # Invariants > /// > -/// - `gendisk` must always point to an initialized and valid `struct gendisk`. > -/// - `gendisk` was added to the VFS through a call to > -/// `bindings::device_add_disk`. > +/// - `gendisk` must always point to an initialized and valid `struct gendisk`. > +/// - `gendisk` was added to the VFS through a call to > +/// `bindings::device_add_disk`. > +/// - `self.gendisk.queue.queuedata` is initialized by a call to `ForeignOwnable::into_foreign`. > pub struct GenDisk<T: Operations> { > _tagset: Arc<TagSet<T>>, > gendisk: *mut bindings::gendisk, > @@ -194,9 +207,20 @@ unsafe impl<T: Operations + Send> Send for GenDisk<T> {} > > impl<T: Operations> Drop for GenDisk<T> { > fn drop(&mut self) { > + // SAFETY: By type invariant of `Self`, `self.gendisk` points to a valid > + // and initialized instance of `struct gendisk`, and, `queuedata` was > + // initialized with the result of a call to > + // `ForeignOwnable::into_foreign`. > + let queue_data = unsafe { (*(*self.gendisk).queue).queuedata }; > + > // SAFETY: By type invariant, `self.gendisk` points to a valid and > // initialized instance of `struct gendisk`, and it was previously added > // to the VFS. > unsafe { bindings::del_gendisk(self.gendisk) }; > + > + // SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with > + // a call to `ForeignOwnable::into_foreign` to create `queuedata`. > + // `ForeignOwnable::from_foreign` is only called here. > + drop(unsafe { T::QueueData::from_foreign(queue_data) }); > } > } > diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs > index c2b98f507bcb..6fb256f55acc 100644 > --- a/rust/kernel/block/mq/operations.rs > +++ b/rust/kernel/block/mq/operations.rs > @@ -6,14 +6,15 @@ > > use crate::{ > bindings, > - block::mq::request::RequestDataWrapper, > - block::mq::Request, > + block::mq::{request::RequestDataWrapper, Request}, > error::{from_result, Result}, > prelude::*, > - types::ARef, > + types::{ARef, ForeignOwnable}, > }; > use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering}; > > +type ForeignBorrowed<'a, T> = <T as ForeignOwnable>::Borrowed<'a>; > + > /// Implement this trait to interface blk-mq as block devices. > /// > /// To implement a block device driver, implement this trait as described in the > @@ -26,12 +27,20 @@ > /// [module level documentation]: kernel::block::mq > #[macros::vtable] > pub trait Operations: Sized { > + /// Data associated with the `struct request_queue` that is allocated for > + /// the `GenDisk` associated with this `Operations` implementation. > + type QueueData: ForeignOwnable; > + > /// Called by the kernel to queue a request with the driver. If `is_last` is > /// `false`, the driver is allowed to defer committing the request. > - fn queue_rq(rq: ARef<Request<Self>>, is_last: bool) -> Result; > + fn queue_rq( > + queue_data: ForeignBorrowed<'_, Self::QueueData>, > + rq: ARef<Request<Self>>, > + is_last: bool, > + ) -> Result; > > /// Called by the kernel to indicate that queued requests should be submitted. > - fn commit_rqs(); > + fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>); > > /// Called by the kernel to poll the device for completed requests. Only > /// used for poll queues. > @@ -70,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> { > /// promise to not access the request until the driver calls > /// `bindings::blk_mq_end_request` for the request. > unsafe extern "C" fn queue_rq_callback( > - _hctx: *mut bindings::blk_mq_hw_ctx, > + hctx: *mut bindings::blk_mq_hw_ctx, > bd: *const bindings::blk_mq_queue_data, > ) -> bindings::blk_status_t { > // SAFETY: `bd.rq` is valid as required by the safety requirement for > @@ -88,10 +97,20 @@ impl<T: Operations> OperationsVTable<T> { > // reference counted by `ARef` until then. > let rq = unsafe { Request::aref_from_raw((*bd).rq) }; > > + // SAFETY: `hctx` is valid as required by this function. > + let queue_data = unsafe { (*(*hctx).queue).queuedata }; > + > + // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a isn’t this set on build() ? > + // call to `ForeignOwnable::into_pointer()` to create `queuedata`. into_pointer() ? > + // `ForeignOwnable::from_foreign()` is only called when the tagset is > + // 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 { Request::start_unchecked(&rq) }; > > let ret = T::queue_rq( > + queue_data, > rq, > // SAFETY: `bd` is valid as required by the safety requirement for > // this function. > @@ -110,9 +129,18 @@ impl<T: Operations> OperationsVTable<T> { > /// > /// # Safety > /// > - /// This function may only be called by blk-mq C infrastructure. > - unsafe extern "C" fn commit_rqs_callback(_hctx: *mut bindings::blk_mq_hw_ctx) { > - T::commit_rqs() > + /// This function may only be called by blk-mq C infrastructure. The caller > + /// must ensure that `hctx` is valid. > + unsafe extern "C" fn commit_rqs_callback(hctx: *mut bindings::blk_mq_hw_ctx) { > + // SAFETY: `hctx` is valid as required by this function. > + let queue_data = unsafe { (*(*hctx).queue).queuedata }; > + > + // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a > + // call to `ForeignOwnable::into_pointer()` to create `queuedata`. into_foreign()? > + // `ForeignOwnable::from_foreign()` is only called when the tagset is > + // dropped, which happens after we are dropped. > + let queue_data = unsafe { T::QueueData::borrow(queue_data) }; > + T::commit_rqs(queue_data) > } > > /// This function is called by the C kernel. It is not currently > > -- > 2.47.2 > > > Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
"Daniel Almeida" <daniel.almeida@collabora.com> writes: >> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote: >> >> Allow users of the rust block device driver API to install private data in >> the `GenDisk` structure. >> >> Reviewed-by: Alice Ryhl <aliceryhl@google.com> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org> >> --- >> drivers/block/rnull/rnull.rs | 8 ++++--- >> rust/kernel/block/mq.rs | 7 +++--- >> rust/kernel/block/mq/gen_disk.rs | 32 ++++++++++++++++++++++---- >> rust/kernel/block/mq/operations.rs | 46 ++++++++++++++++++++++++++++++-------- >> 4 files changed, 74 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs >> index 8690ff5f974f..8255236bc550 100644 >> --- a/drivers/block/rnull/rnull.rs >> +++ b/drivers/block/rnull/rnull.rs >> @@ -61,14 +61,16 @@ fn new( >> .logical_block_size(block_size)? >> .physical_block_size(block_size)? >> .rotational(rotational) >> - .build(fmt!("{}", name.to_str()?), tagset) >> + .build(fmt!("{}", name.to_str()?), tagset, ()) >> } >> } >> >> #[vtable] >> impl Operations for NullBlkDevice { >> + type QueueData = (); >> + >> #[inline(always)] >> - fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result { >> + fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result { >> mq::Request::end_ok(rq) >> .map_err(|_e| kernel::error::code::EIO) >> // We take no refcounts on the request, so we expect to be able to >> @@ -79,5 +81,5 @@ fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result { >> Ok(()) >> } >> >> - fn commit_rqs() {} >> + fn commit_rqs(_queue_data: ()) {} >> } >> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs >> index 98fa0d6bc8f7..6e546f4f3d1c 100644 >> --- a/rust/kernel/block/mq.rs >> +++ b/rust/kernel/block/mq.rs >> @@ -69,20 +69,21 @@ >> //! >> //! #[vtable] >> //! impl Operations for MyBlkDevice { >> +//! type QueueData = (); >> //! >> -//! fn queue_rq(rq: ARef<Request<Self>>, _is_last: bool) -> Result { >> +//! fn queue_rq(_queue_data: (), rq: ARef<Request<Self>>, _is_last: bool) -> Result { >> //! Request::end_ok(rq); >> //! Ok(()) >> //! } >> //! >> -//! fn commit_rqs() {} >> +//! fn commit_rqs(_queue_data: ()) {} >> //! } >> //! >> //! let tagset: Arc<TagSet<MyBlkDevice>> = >> //! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?; >> //! let mut disk = gen_disk::GenDiskBuilder::new() >> //! .capacity_sectors(4096) >> -//! .build(format_args!("myblk"), tagset)?; >> +//! .build(format_args!("myblk"), tagset, ())?; >> //! >> //! # Ok::<(), kernel::error::Error>(()) >> //! ``` >> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs >> index 6b1b846874db..46ec80269970 100644 >> --- a/rust/kernel/block/mq/gen_disk.rs >> +++ b/rust/kernel/block/mq/gen_disk.rs >> @@ -13,6 +13,7 @@ >> static_lock_class, >> str::NullTerminatedFormatter, >> sync::Arc, >> + types::{ForeignOwnable, ScopeGuard}, >> }; >> use core::fmt::{self, Write}; >> >> @@ -98,7 +99,14 @@ pub fn build<T: Operations>( >> self, >> name: fmt::Arguments<'_>, >> tagset: Arc<TagSet<T>>, >> + queue_data: T::QueueData, >> ) -> Result<GenDisk<T>> { >> + let data = queue_data.into_foreign(); >> + let recover_data = ScopeGuard::new(|| { >> + // SAFETY: T::QueueData was created by the call to `into_foreign()` above >> + drop(unsafe { T::QueueData::from_foreign(data) }); >> + }); >> + >> // SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed. >> let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() }; >> >> @@ -113,7 +121,7 @@ pub fn build<T: Operations>( >> bindings::__blk_mq_alloc_disk( >> tagset.raw_tag_set(), >> &mut lim, >> - core::ptr::null_mut(), >> + data, >> static_lock_class!().as_ptr(), >> ) >> })?; >> @@ -167,8 +175,12 @@ pub fn build<T: Operations>( >> }, >> )?; >> >> + recover_data.dismiss(); >> + >> // INVARIANT: `gendisk` was initialized above. >> // INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above. >> + // INVARIANT: `gendisk.queue.queue_data` is set to `data` in the call to >> + // `__blk_mq_alloc_disk` above. >> Ok(GenDisk { >> _tagset: tagset, >> gendisk, >> @@ -180,9 +192,10 @@ pub fn build<T: Operations>( >> /// >> /// # Invariants >> /// >> -/// - `gendisk` must always point to an initialized and valid `struct gendisk`. >> -/// - `gendisk` was added to the VFS through a call to >> -/// `bindings::device_add_disk`. >> +/// - `gendisk` must always point to an initialized and valid `struct gendisk`. >> +/// - `gendisk` was added to the VFS through a call to >> +/// `bindings::device_add_disk`. >> +/// - `self.gendisk.queue.queuedata` is initialized by a call to `ForeignOwnable::into_foreign`. >> pub struct GenDisk<T: Operations> { >> _tagset: Arc<TagSet<T>>, >> gendisk: *mut bindings::gendisk, >> @@ -194,9 +207,20 @@ unsafe impl<T: Operations + Send> Send for GenDisk<T> {} >> >> impl<T: Operations> Drop for GenDisk<T> { >> fn drop(&mut self) { >> + // SAFETY: By type invariant of `Self`, `self.gendisk` points to a valid >> + // and initialized instance of `struct gendisk`, and, `queuedata` was >> + // initialized with the result of a call to >> + // `ForeignOwnable::into_foreign`. >> + let queue_data = unsafe { (*(*self.gendisk).queue).queuedata }; >> + >> // SAFETY: By type invariant, `self.gendisk` points to a valid and >> // initialized instance of `struct gendisk`, and it was previously added >> // to the VFS. >> unsafe { bindings::del_gendisk(self.gendisk) }; >> + >> + // SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with >> + // a call to `ForeignOwnable::into_foreign` to create `queuedata`. >> + // `ForeignOwnable::from_foreign` is only called here. >> + drop(unsafe { T::QueueData::from_foreign(queue_data) }); >> } >> } >> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs >> index c2b98f507bcb..6fb256f55acc 100644 >> --- a/rust/kernel/block/mq/operations.rs >> +++ b/rust/kernel/block/mq/operations.rs >> @@ -6,14 +6,15 @@ >> >> use crate::{ >> bindings, >> - block::mq::request::RequestDataWrapper, >> - block::mq::Request, >> + block::mq::{request::RequestDataWrapper, Request}, >> error::{from_result, Result}, >> prelude::*, >> - types::ARef, >> + types::{ARef, ForeignOwnable}, >> }; >> use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering}; >> >> +type ForeignBorrowed<'a, T> = <T as ForeignOwnable>::Borrowed<'a>; >> + >> /// Implement this trait to interface blk-mq as block devices. >> /// >> /// To implement a block device driver, implement this trait as described in the >> @@ -26,12 +27,20 @@ >> /// [module level documentation]: kernel::block::mq >> #[macros::vtable] >> pub trait Operations: Sized { >> + /// Data associated with the `struct request_queue` that is allocated for >> + /// the `GenDisk` associated with this `Operations` implementation. >> + type QueueData: ForeignOwnable; >> + >> /// Called by the kernel to queue a request with the driver. If `is_last` is >> /// `false`, the driver is allowed to defer committing the request. >> - fn queue_rq(rq: ARef<Request<Self>>, is_last: bool) -> Result; >> + fn queue_rq( >> + queue_data: ForeignBorrowed<'_, Self::QueueData>, >> + rq: ARef<Request<Self>>, >> + is_last: bool, >> + ) -> Result; >> >> /// Called by the kernel to indicate that queued requests should be submitted. >> - fn commit_rqs(); >> + fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>); >> >> /// Called by the kernel to poll the device for completed requests. Only >> /// used for poll queues. >> @@ -70,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> { >> /// promise to not access the request until the driver calls >> /// `bindings::blk_mq_end_request` for the request. >> unsafe extern "C" fn queue_rq_callback( >> - _hctx: *mut bindings::blk_mq_hw_ctx, >> + hctx: *mut bindings::blk_mq_hw_ctx, >> bd: *const bindings::blk_mq_queue_data, >> ) -> bindings::blk_status_t { >> // SAFETY: `bd.rq` is valid as required by the safety requirement for >> @@ -88,10 +97,20 @@ impl<T: Operations> OperationsVTable<T> { >> // reference counted by `ARef` until then. >> let rq = unsafe { Request::aref_from_raw((*bd).rq) }; >> >> + // SAFETY: `hctx` is valid as required by this function. >> + let queue_data = unsafe { (*(*hctx).queue).queuedata }; >> + >> + // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a > > isn’t this set on build() ? Yes, you are right. Refactoring happened. > >> + // call to `ForeignOwnable::into_pointer()` to create `queuedata`. > > into_pointer() ? Should be `into_foreign`, will fix. > >> + // `ForeignOwnable::from_foreign()` is only called when the tagset is >> + // 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 { Request::start_unchecked(&rq) }; >> >> let ret = T::queue_rq( >> + queue_data, >> rq, >> // SAFETY: `bd` is valid as required by the safety requirement for >> // this function. >> @@ -110,9 +129,18 @@ impl<T: Operations> OperationsVTable<T> { >> /// >> /// # Safety >> /// >> - /// This function may only be called by blk-mq C infrastructure. >> - unsafe extern "C" fn commit_rqs_callback(_hctx: *mut bindings::blk_mq_hw_ctx) { >> - T::commit_rqs() >> + /// This function may only be called by blk-mq C infrastructure. The caller >> + /// must ensure that `hctx` is valid. >> + unsafe extern "C" fn commit_rqs_callback(hctx: *mut bindings::blk_mq_hw_ctx) { >> + // SAFETY: `hctx` is valid as required by this function. >> + let queue_data = unsafe { (*(*hctx).queue).queuedata }; >> + >> + // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a >> + // call to `ForeignOwnable::into_pointer()` to create `queuedata`. > > into_foreign()? Yep, thanks. > >> + // `ForeignOwnable::from_foreign()` is only called when the tagset is >> + // dropped, which happens after we are dropped. >> + let queue_data = unsafe { T::QueueData::borrow(queue_data) }; >> + T::commit_rqs(queue_data) >> } >> >> /// This function is called by the C kernel. It is not currently >> >> -- >> 2.47.2 >> >> >> > > Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
© 2016 - 2025 Red Hat, Inc.