Re: [PATCH 1/4] module: remove meaningless 'name' parameter from __MODULE_INFO()

[Petr Pavlu <petr.pavlu@suse.com>]

On 6/6/25 6:10 AM, Masahiro Yamada wrote:
> The symbol names in the .modinfo section are never used and already
> randomized by the __UNIQUE_ID() macro.
> 
> Therefore, the second parameter of  __MODULE_INFO() is meaningless
> and can be removed to simplify the code.
> 
> With this change, the symbol names in the .modinfo section will be
> prefixed with __UNIQUE_ID_modinfo, making it clearer that they
> originate from MODULE_INFO().
> 
> [Before]
> 
>   $ objcopy  -j .modinfo vmlinux.o modinfo.o
>   $ nm -n modinfo.o | head -n10
>   0000000000000000 r __UNIQUE_ID_license560
>   0000000000000011 r __UNIQUE_ID_file559
>   0000000000000030 r __UNIQUE_ID_description558
>   0000000000000074 r __UNIQUE_ID_license580
>   000000000000008e r __UNIQUE_ID_file579
>   00000000000000bd r __UNIQUE_ID_description578
>   00000000000000e6 r __UNIQUE_ID_license581
>   00000000000000ff r __UNIQUE_ID_file580
>   0000000000000134 r __UNIQUE_ID_description579
>   0000000000000179 r __UNIQUE_ID_uncore_no_discover578
> 
> [After]
> 
>   $ objcopy  -j .modinfo vmlinux.o modinfo.o
>   $ nm -n modinfo.o | head -n10
>   0000000000000000 r __UNIQUE_ID_modinfo560
>   0000000000000011 r __UNIQUE_ID_modinfo559
>   0000000000000030 r __UNIQUE_ID_modinfo558
>   0000000000000074 r __UNIQUE_ID_modinfo580
>   000000000000008e r __UNIQUE_ID_modinfo579
>   00000000000000bd r __UNIQUE_ID_modinfo578
>   00000000000000e6 r __UNIQUE_ID_modinfo581
>   00000000000000ff r __UNIQUE_ID_modinfo580
>   0000000000000134 r __UNIQUE_ID_modinfo579
>   0000000000000179 r __UNIQUE_ID_modinfo578
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> [...]
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 92e1420fccdf..81b41cc6a19e 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -164,9 +164,6 @@ extern void cleanup_module(void);
>  
>  struct module_kobject *lookup_or_create_module_kobject(const char *name);
>  
> -/* Generic info of form tag = "info" */
> -#define MODULE_INFO(tag, info) __MODULE_INFO(tag, tag, info)
> -
>  /* For userspace: you can also call me... */
>  #define MODULE_ALIAS(_alias) MODULE_INFO(alias, _alias)
>  
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index bfb85fd13e1f..00166f747e27 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -20,18 +20,19 @@
>  /* Chosen so that structs with an unsigned long line up. */
>  #define MAX_PARAM_PREFIX_LEN (64 - sizeof(unsigned long))
>  
> -#define __MODULE_INFO(tag, name, info)					  \
> -	static const char __UNIQUE_ID(name)[]				  \
> +/* Generic info of form tag = "info" */
> +#define MODULE_INFO(tag, info)					  \
> +	static const char __UNIQUE_ID(modinfo)[]			  \
>  		__used __section(".modinfo") __aligned(1)		  \
>  		= __MODULE_INFO_PREFIX __stringify(tag) "=" info
>  

One nit is that MODULE_INFO() is now defined in moduleparam.h, even
though it is a general macro for adding module information and not
specifically tied to parameters. I realize it is needed in moduleparam.h
and that the dependency is from module.h to moduleparam.h, not the other
way around. We could potentially keep the MODULE_INFO() (in module.h) ->
__MODULE_INFO() (moduleparam.h) split solely for this, but it is
probably unnecessary.

The overall change looks ok to me.

Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

-- 
Thanks,
Petr