GenDiskBuilder currently installs block_device_operations with owner set to NULL. The block layer uses this field to pin the driver module while block device file operations are in use. Take the driver's ThisModule in GenDiskBuilder::build() and store the operations table in GenDisk so gendisk->fops stays valid if the disk is moved, such as when rnull keeps it in an Option behind configfs. Update rnull as the in-tree caller. Signed-off-by: Adarsh Das --- drivers/block/rnull/configfs.rs | 1 + drivers/block/rnull/rnull.rs | 3 +- rust/kernel/block/mq.rs | 9 +++-- rust/kernel/block/mq/gen_disk.rs | 58 +++++++++++++++++++------------- 4 files changed, 43 insertions(+), 28 deletions(-) diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs index 7c2eb5c0b722..bba30d590f68 100644 --- a/drivers/block/rnull/configfs.rs +++ b/drivers/block/rnull/configfs.rs @@ -147,6 +147,7 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result { if !guard.powered && power_op { guard.disk = Some(NullBlkDevice::new( + &THIS_MODULE, &guard.name, guard.block_size, guard.rotational, diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs index 0ca8715febe8..4265a133cbf0 100644 --- a/drivers/block/rnull/rnull.rs +++ b/drivers/block/rnull/rnull.rs @@ -46,6 +46,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit { impl NullBlkDevice { fn new( + this_module: &'static ThisModule, name: &CStr, block_size: u32, rotational: bool, @@ -61,7 +62,7 @@ fn new( .logical_block_size(block_size)? .physical_block_size(block_size)? .rotational(rotational) - .build(fmt!("{}", name.to_str()?), tagset, queue_data) + .build(this_module, fmt!("{}", name.to_str()?), tagset, queue_data) } } diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs index 1fd0d54dd549..33561e0f67af 100644 --- a/rust/kernel/block/mq.rs +++ b/rust/kernel/block/mq.rs @@ -8,8 +8,8 @@ //! - Implement [`Operations`] for a type `T`. //! - Create a [`TagSet`]. //! - Create a [`GenDisk`], via the [`GenDiskBuilder`]. -//! - Add the disk to the system by calling [`GenDiskBuilder::build`] passing in -//! the `TagSet` reference. +//! - Add the disk to the system by calling [`GenDiskBuilder::build`], passing in +//! the driver's [`ThisModule`], the disk name, the `TagSet`, and queue data. //! //! The types available in this module that have direct C counterparts are: //! @@ -86,9 +86,12 @@ //! //! let tagset: Arc> = //! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?; +//! # // SAFETY: Dummy `ThisModule` for doctest compilation only. +//! # static THIS_MODULE: ThisModule = +//! # unsafe { ThisModule::from_ptr(core::ptr::null_mut()) }; //! let mut disk = gen_disk::GenDiskBuilder::new() //! .capacity_sectors(4096) -//! .build(fmt!("myblk"), tagset, ())?; +//! .build(&THIS_MODULE, fmt!("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 fc97dd873974..ff079223d31a 100644 --- a/rust/kernel/block/mq/gen_disk.rs +++ b/rust/kernel/block/mq/gen_disk.rs @@ -95,8 +95,12 @@ pub fn capacity_sectors(mut self, capacity: u64) -> Self { } /// Build a new `GenDisk` and add it to the VFS. + /// + /// `this_module` must be the [`ThisModule`] for the kernel module registering + /// the disk. pub fn build( self, + this_module: &'static ThisModule, name: fmt::Arguments<'_>, tagset: Arc>, queue_data: T::QueueData, @@ -125,30 +129,32 @@ pub fn build( ) })?; - const TABLE: bindings::block_device_operations = bindings::block_device_operations { - submit_bio: None, - open: None, - release: None, - ioctl: None, - compat_ioctl: None, - check_events: None, - unlock_native_capacity: None, - getgeo: None, - set_read_only: None, - swap_slot_free_notify: None, - report_zones: None, - devnode: None, - alternative_gpt_sector: None, - get_unique_id: None, - // TODO: Set to `THIS_MODULE`. - owner: core::ptr::null_mut(), - pr_ops: core::ptr::null_mut(), - free_disk: None, - poll_bio: None, - }; - - // SAFETY: `gendisk` is a valid pointer as we initialized it above - unsafe { (*gendisk).fops = &TABLE }; + let fops = KBox::new( + bindings::block_device_operations { + submit_bio: None, + open: None, + release: None, + ioctl: None, + compat_ioctl: None, + check_events: None, + unlock_native_capacity: None, + getgeo: None, + set_read_only: None, + swap_slot_free_notify: None, + report_zones: None, + devnode: None, + alternative_gpt_sector: None, + get_unique_id: None, + owner: this_module.as_ptr(), + pr_ops: core::ptr::null_mut(), + free_disk: None, + poll_bio: None, + }, + GFP_KERNEL, + )?; + + // SAFETY: `gendisk` is a valid pointer as we initialized it above. + unsafe { (*gendisk).fops = core::ptr::from_ref(&*fops) }; let cleanup_failure = ScopeGuard::new_with_data((gendisk, data), |(gendisk, data)| { // SAFETY: `gendisk` came from `__blk_mq_alloc_disk()` above and @@ -193,6 +199,7 @@ pub fn build( // `__blk_mq_alloc_disk` above. Ok(GenDisk { _tagset: tagset, + _fops: fops, gendisk, }) } @@ -206,8 +213,11 @@ pub fn build( /// - `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`. +/// - `self._fops` stores the [`bindings::block_device_operations`] pointed to by +/// `gendisk.fops`. pub struct GenDisk { _tagset: Arc>, + _fops: KBox, gendisk: *mut bindings::gendisk, } -- 2.55.0