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?
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.