Re: [PATCH v6 07/10] rust: configfs: use `LocalModule` for `THIS_MODULE`
Alvin Sun <[email protected]>
| Newsgroups | org.kernel.vger.linux-modules,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-pci,org.kernel.vger.netdev,org.kernel.vger.rust-for-linux |
|---|---|
| Message-ID | <[email protected]> |
On 6/26/26 22:41, Gary Guo wrote: > On Fri Jun 26, 2026 at 3:35 AM BST, Alvin Sun wrote: >> On 6/25/26 22:40, Gary Guo wrote: >>> On Wed Jun 24, 2026 at 4:00 PM BST, Alvin Sun wrote: >>>> Replace the `THIS_MODULE` static reference in the `configfs_attrs!` >>>> macro with `this_module::<LocalModule>()`, and update >>>> rnull to import `LocalModule` instead of `THIS_MODULE`, consistent >>>> with the move of `THIS_MODULE` into the `ModuleMetadata` trait. >>>> >>>> Assisted-by: opencode:glm-5.2 >>>> Reviewed-by: Andreas Hindborg <[email protected]> >>>> Acked-by: Danilo Krummrich <[email protected]> >>>> Signed-off-by: Alvin Sun <[email protected]> >>>> --- >>>> drivers/block/rnull/configfs.rs | 6 ++---- >>>> rust/kernel/configfs.rs | 8 +++++--- >>>> 2 files changed, 7 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs >>>> index c10a55fc58948..b2547ad1e5ddd 100644 >>>> --- a/drivers/block/rnull/configfs.rs >>>> +++ b/drivers/block/rnull/configfs.rs >>>> @@ -1,9 +1,7 @@ >>>> // SPDX-License-Identifier: GPL-2.0 >>>> >>>> -use super::{ >>>> - NullBlkDevice, >>>> - THIS_MODULE, // >>>> -}; >>>> +use super::NullBlkDevice; >>>> +use crate::LocalModule; >>>> use kernel::{ >>>> block::mq::gen_disk::{ >>>> GenDisk, >>>> diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs >>>> index 2339c6467325d..c31d7882e216d 100644 >>>> --- a/rust/kernel/configfs.rs >>>> +++ b/rust/kernel/configfs.rs >>>> @@ -875,7 +875,7 @@ fn as_ptr(&self) -> *const bindings::config_item_type { >>>> /// configfs::Subsystem<Configuration>, >>>> /// Configuration >>>> /// >::new_with_child_ctor::<N,Child>( >>>> -/// &THIS_MODULE, >>>> +/// ::kernel::module::this_module::<crate::LocalModule>(), >>>> /// &CONFIGURATION_ATTRS >>>> /// ); >>>> /// >>>> @@ -1021,7 +1021,8 @@ macro_rules! configfs_attrs { >>>> >>>> static [< $data:upper _TPE >] : $crate::configfs::ItemType<$container, $data> = >>>> $crate::configfs::ItemType::<$container, $data>::new::<N>( >>>> - &THIS_MODULE, &[<$ data:upper _ATTRS >] >>>> + $crate::module::this_module::<LocalModule>(), >>> ^ You only changed one single place. This is still plain `LocalModule`. >> Initially I wrote it as `crate::LocalModule`, but clippy warned about it. So >> instead of putting the crate path in the macro body, I added `use >> crate::LocalModule` in the calling file. >> >> ``` >> warning: `crate` references the macro call's crate >> --> rust/kernel/configfs.rs:1024:59 >> | >> 1024 | ... $crate::module::this_module::<crate::LocalModule>(), >> | ^^^^^ help: >> to reference the macro definition's crate, use: `$crate` >> | >> = help: for further information visit >> https://rust-lang.github.io/rust-clippy/rust-1.94.0/index.html#crate_in_macro_def >> = note: `-W clippy::crate-in-macro-def` implied by `-W clippy::all` >> = help: to override `-W clippy::all` add >> `#[allow(clippy::crate_in_macro_def)]` >> >> warning: 1 warning emitted >> ``` > Clippy has a point about `crate::` being usually wrong in macros, but it is what > we actually want here, so obviously you should allow the warning. > > It is the exact same case in `vtable` macro, just that Clippy is unable to check > proc macros! Thanks for the detailed explanation. I specifically searched for issues related to `crate_in_macro_def`, and this information is very useful to me. Best regards, Alvin > > Best, > Gary > >> Alternatively, `#[allow(clippy::crate_in_macro_def)]` could be added on >> the macro >> definition. Would you suggest that approach?