On 2025-02-06 21:45:37-0800, Shyam Saini wrote:
> lookup_or_create_module_kobject() is static and marked as __init,
> this is not ideal for global usage.
FYI missing "PATCH" in patch subject.
> Fix this limitation by refactoring and declaring this as global:
> - Refactor it by removing BUG_ON() and 'if else' construct by returning
> early
This does look like an unrelated change, could be in its own patch.
> - Remove static and __init markers from the function and add its
> declaration in module.h
> - Mark this function as "__modinit". To facilitate this, move the
> __modinit macro construct to module.h
>
> Suggested-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
> Signed-off-by: Shyam Saini <shyamsa...@linux.microsoft.com>
> ---
> include/linux/module.h | 8 +++++++
> kernel/params.c | 48 ++++++++++++++++++------------------------
> 2 files changed, 29 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 12f8a7d4fc1c..57d09b4e4385 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -162,6 +162,14 @@ extern void cleanup_module(void);
> #define __INITRODATA_OR_MODULE __INITRODATA
> #endif /*CONFIG_MODULES*/
>
> +#ifdef CONFIG_MODULES
> +#define __modinit
> +#else
> +#define __modinit __init
> +#endif
> +
> +struct module_kobject __modinit * lookup_or_create_module_kobject(const char
> *name);
__init / __modinit is not necessary on the declaration.
You can remove it here and keep the #define private.
> +
> /* Generic info of form tag = "info" */
> #define MODULE_INFO(tag, info) __MODULE_INFO(tag, tag, info)
>
> diff --git a/kernel/params.c b/kernel/params.c
> index 4b43baaf7c83..5d16696b1daa 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -595,12 +595,6 @@ static ssize_t param_attr_store(const struct
> module_attribute *mattr,
> }
> #endif
>
> -#ifdef CONFIG_MODULES
> -#define __modinit
> -#else
> -#define __modinit __init
> -#endif
> -
> #ifdef CONFIG_SYSFS
> void kernel_param_lock(struct module *mod)
> {
> @@ -763,38 +757,38 @@ void destroy_params(const struct kernel_param *params,
> unsigned num)
> params[i].ops->free(params[i].arg);
> }
>
> -static struct module_kobject * __init lookup_or_create_module_kobject(const
> char *name)
> +struct module_kobject __modinit * lookup_or_create_module_kobject(const char
> *name)
> {
> struct module_kobject *mk;
> struct kobject *kobj;
> int err;
>
> kobj = kset_find_obj(module_kset, name);
> - if (kobj) {
> - mk = to_module_kobject(kobj);
> - } else {
> - mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL);
> - BUG_ON(!mk);
> -
> - mk->mod = THIS_MODULE;
> - mk->kobj.kset = module_kset;
> - err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL,
> + if (kobj)
> + return to_module_kobject(kobj);
> +
> + mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL);
> + if (!mk)
> + return NULL;
> +
> + mk->mod = THIS_MODULE;
> + mk->kobj.kset = module_kset;
> + err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL,
> "%s", name);
> #ifdef CONFIG_MODULES
As you are cleaning this up anyways: The #ifdef above should become
if (IS_ENABLED(CONFIG_MODULES))
> - if (!err)
> - err = sysfs_create_file(&mk->kobj, &module_uevent.attr);
> + if (!err)
> + err = sysfs_create_file(&mk->kobj, &module_uevent.attr);
> #endif
> - if (err) {
> - kobject_put(&mk->kobj);
> - pr_crit("Adding module '%s' to sysfs failed (%d), the
> system may be unstable.\n",
> - name, err);
> - return NULL;
> - }
> -
> - /* So that we hold reference in both cases. */
> - kobject_get(&mk->kobj);
> + if (err) {
> + kobject_put(&mk->kobj);
> + pr_crit("Adding module '%s' to sysfs failed (%d), the system
> may be unstable.\n",
> + name, err);
> + return NULL;
> }
>
> + /* So that we hold reference in both cases. */
> + kobject_get(&mk->kobj);
> +
> return mk;
> }
>
> --
> 2.34.1
>
