Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH] rust: block: set GenDisk block_device_operations.owner to THIS_MODULE
@ 2026-08-05 19:20 Adarsh Das
  2026-08-06  8:36 ` [PATCH v2] " Adarsh Das
  0 siblings, 1 reply; 4+ messages in thread
From: Adarsh Das @ 2026-08-05 19:20 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, linux-block, rust-for-linux,
	linux-kernel, Adarsh Das

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 <adarshdas950@gmail.com>
---
 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<Self, Error> {
 
 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<T>`].
 //! - Create a [`GenDisk<T>`], 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<TagSet<MyBlkDevice>> =
 //!     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<T: Operations>(
         self,
+        this_module: &'static ThisModule,
         name: fmt::Arguments<'_>,
         tagset: Arc<TagSet<T>>,
         queue_data: T::QueueData,
@@ -125,30 +129,32 @@ pub fn build<T: Operations>(
             )
         })?;
 
-        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<T: Operations>(
         // `__blk_mq_alloc_disk` above.
         Ok(GenDisk {
             _tagset: tagset,
+            _fops: fops,
             gendisk,
         })
     }
@@ -206,8 +213,11 @@ pub fn build<T: Operations>(
 ///  - `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<T: Operations> {
     _tagset: Arc<TagSet<T>>,
+    _fops: KBox<bindings::block_device_operations>,
     gendisk: *mut bindings::gendisk,
 }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-06 10:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 19:20 [PATCH] rust: block: set GenDisk block_device_operations.owner to THIS_MODULE Adarsh Das
2026-08-06  8:36 ` [PATCH v2] " Adarsh Das
2026-08-06  9:08   ` Andreas Hindborg
2026-08-06 10:40   ` [PATCH v3] " Adarsh Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox