Re: [PATCH v10 00/10] Fix missing fops.owner in Rust DRM/misc abstractions
"Danilo Krummrich" <[email protected]>
| Newsgroups | org.kernel.vger.rust-for-linux,dev.linux.lists.driver-core,dev.linux.lists.nova-gpu,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest,org.kernel.vger.linux-modules,org.kernel.vger.linux-pci,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On Thu Aug 13, 2026 at 1:57 AM CEST, Gary Guo wrote: > On Wed Aug 12, 2026 at 10:53 PM BST, Danilo Krummrich wrote: >> (Cc: Mark) >> >> On Wed Aug 12, 2026 at 6:33 PM CEST, Miguel Ojeda wrote: >>> On Tue, Aug 11, 2026 at 8:41 AM Alvin Sun <[email protected]> wrote: >>>> >>>> The series moves `THIS_MODULE` into the `ModuleMetadata` as a const, threads it >>>> through `#[vtable]` to set `fops.owner` in DRM/miscdevice, and updates configfs >>>> and rnull to use `this_module::<LocalModule>()`. >>> >>> Applied to `rust-next` -- thanks everyone! >> >> This series has a semantic conflict with both the driver-core and the drm-rust >> tree: >> >> @Mark: When you merge driver-core-next after rust-next (which I think is the >> case) then you need to include the diff in [1] into the merge. >> >> In drm-rust-next the build fails with: >> >> error[E0425]: cannot find type `LocalModule` in the crate root >> --> rust/kernel/drm/gem/shmem.rs:628:5 >> | >> 628 | #[vtable] >> | ^^^^^^^^^ not found in the crate root >> | >> = note: this error originates in the attribute macro `vtable` (in Nightly builds, run with -Z macro-backtrace for more info) >> >> which is because the kunit test in rust/kernel/drm/gem/shmem.rs uses the >> #[vtable] macro. >> >> This should be fixed up with a patch on top of this series in rust-next. >> >> I came up with to potential solutions [2] and [3]. I think with the new build >> system we want [3], but I'm not entirely sure this works correctly with the >> current build system in all cases (at least it did survive my tests). >> >> Alternatively, we could just open-code a dummy module as in [2] for now. >> >> [1] driver-core-next merge fixup >> >> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs >> index a4927452016e..17ca504b7f8d 100644 >> --- a/rust/kernel/serdev.rs >> +++ b/rust/kernel/serdev.rs >> @@ -87,7 +87,7 @@ unsafe fn register( >> } >> >> // SAFETY: `sdrv` is guaranteed to be a valid `DriverType`. >> - to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.0) }) >> + to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.as_ptr()) }) > > `module.0` shouldn't be used in the first place, it just happens to be visible > due to the unfortunate placement at crate root. > > Perhaps you can update driver-core tree to use `as_ptr()`? It was already there > and not newly introduced in the series. Heh, I just remembered that patch 1 of this series did fix this in a couple of places and assumed that it was introduced in the same patch without looking further. In that case I can throw in a patch in the driver-core tree; will send it tomorrow. >> } >> >> unsafe fn unregister(sdrv: &Opaque<Self::DriverType>) { >> >> [2] Open-coded dummy module >> >> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs >> index c04e6c5aa7e0..274924cfcc05 100644 >> --- a/rust/kernel/lib.rs >> +++ b/rust/kernel/lib.rs >> @@ -157,6 +157,15 @@ >> /// Prefix to appear before log messages printed from within the `kernel` crate. >> const __LOG_PREFIX: &[u8] = b"rust_kernel\0"; >> >> +/// Dummy module type for `#[vtable]` impl blocks within the kernel crate (e.g. kunit tests). >> +struct LocalModule; >> + >> +impl ModuleMetadata for LocalModule { >> + const NAME: &'static str::CStr = c"rust_kernel"; >> + // SAFETY: `try_module_get`/`module_put` handle null module pointers gracefully. >> + const THIS_MODULE: ThisModule = unsafe { ThisModule::from_ptr(core::ptr::null_mut()) }; >> +} >> + >> #[cfg(not(testlib))] >> #[panic_handler] >> fn panic(info: &core::panic::PanicInfo<'_>) -> ! { > > IMO this is the correct way, also consistent with > > https://lore.kernel.org/rust-for-linux/[email protected]/ > > It might make sense to add this to rust-next. I feel like [3] is the superior solution (just unsure about the build system implications); once we have the new build system some subsystem crates will be actual modules, some will be always built-in. So, in general I think we want to use the module!() macro.