diff mbox series

[v5,2/4] kernel: param: initialize module_kset in a pure_initcall

Message ID 20260518-acpi_mod_name-v5-2-705ccc430885@sony.com
State Handled Elsewhere
Headers show
Series Enable sysfs module symlink for more built-in drivers | expand

Commit Message

Shashank Balaji May 18, 2026, 10:19 a.m. UTC
Commit "driver core: platform: set mod_name in driver registration" will set
struct device_driver's mod_name member for platform driver registration. For a
driver to be registered with its mod_name set, module_kset needs to be
initialized, which currently happens in a subsys_initcall in param_sysfs_init().
The tegra cbb drivers register themselves before module_kset init, in a
core_initcall. This works currently because lookup_or_create_module_kobject(),
which dereferences module_kset via kset_find_obj(), is not called if mod_name
is not set, which is the case now.

So in preparation for the commit "driver core: platform: set mod_name in driver registration",
move module_kset init to pure_initcall level, ensuring it happens before tegra
cbb driver registration.

Suggested-by: Gary Guo <gary@garyguo.net>
Co-developed-by: Rahul Bukte <rahul.bukte@sony.com>
Signed-off-by: Rahul Bukte <rahul.bukte@sony.com>
Signed-off-by: Shashank Balaji <shashank.mahadasyam@sony.com>
---
Patch 4 depends on this patch
---
 kernel/params.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Petr Pavlu May 22, 2026, 1:06 p.m. UTC | #1
On 5/18/26 12:19 PM, Shashank Balaji wrote:
> Commit "driver core: platform: set mod_name in driver registration" will set
> struct device_driver's mod_name member for platform driver registration. For a
> driver to be registered with its mod_name set, module_kset needs to be
> initialized, which currently happens in a subsys_initcall in param_sysfs_init().
> The tegra cbb drivers register themselves before module_kset init, in a
> core_initcall. This works currently because lookup_or_create_module_kobject(),
> which dereferences module_kset via kset_find_obj(), is not called if mod_name
> is not set, which is the case now.
> 
> So in preparation for the commit "driver core: platform: set mod_name in driver registration",
> move module_kset init to pure_initcall level, ensuring it happens before tegra
> cbb driver registration.
> 
> Suggested-by: Gary Guo <gary@garyguo.net>
> Co-developed-by: Rahul Bukte <rahul.bukte@sony.com>
> Signed-off-by: Rahul Bukte <rahul.bukte@sony.com>
> Signed-off-by: Shashank Balaji <shashank.mahadasyam@sony.com>
> ---
> Patch 4 depends on this patch
> ---
>  kernel/params.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/params.c b/kernel/params.c
> index 74d620bc2521..ac088d4b09a9 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -957,7 +957,7 @@ static int __init param_sysfs_init(void)
>  
>  	return 0;
>  }
> -subsys_initcall(param_sysfs_init);
> +pure_initcall(param_sysfs_init);
>  
>  /*
>   * param_sysfs_builtin_init - add sysfs version and parameter
> 

The change looks ok to me functionality-wise. Sysfs is initialized
earlier in do_basic_setup() and other code, such as classes_init(),
calls kset_create_and_add() similarly early.

One minor issue is that pure_initcall() was originally intended for
static variable initialization. The file include/linux/init.h says:

| /*
|  * A "pure" initcall has no dependencies on anything else, and purely
|  * initializes variables that couldn't be statically initialized.
|  *
|  * This only exists for built-in code, not for modules.
|  * Keep main.c:initcall_level_names[] in sync.
|  */
| #define pure_initcall(fn)		__define_initcall(fn, 0)

The patch stretches the intended use of pure_initcall() somewhat in this
regard. However, other code already appears to do the same, so I guess
this is ok.

Additionally, I think it would be good to update the comment preceding
param_sysfs_init(). It currently says:

| /*
|  * param_sysfs_init - create "module" kset
|  *
|  * This must be done before the initramfs is unpacked and
|  * request_module() thus becomes possible, because otherwise the
|  * module load would fail in mod_sysfs_init.
|  */

I suggest changing it to something like follows:

This must be done before any driver registration so that when a driver comes
from a built-in module, the driver core can add the module under /sys/module
and create the associated driver symlinks.
Shashank Balaji May 23, 2026, 3:24 a.m. UTC | #2
Hi Petr,

Thanks for the feedback!

On Fri, May 22, 2026 at 03:06:04PM +0200, Petr Pavlu wrote:
> On 5/18/26 12:19 PM, Shashank Balaji wrote:
> > Commit "driver core: platform: set mod_name in driver registration" will set
> > struct device_driver's mod_name member for platform driver registration. For a
> > driver to be registered with its mod_name set, module_kset needs to be
> > initialized, which currently happens in a subsys_initcall in param_sysfs_init().
> > The tegra cbb drivers register themselves before module_kset init, in a
> > core_initcall. This works currently because lookup_or_create_module_kobject(),
> > which dereferences module_kset via kset_find_obj(), is not called if mod_name
> > is not set, which is the case now.
> > 
> > So in preparation for the commit "driver core: platform: set mod_name in driver registration",
> > move module_kset init to pure_initcall level, ensuring it happens before tegra
> > cbb driver registration.
> > 
> > Suggested-by: Gary Guo <gary@garyguo.net>
> > Co-developed-by: Rahul Bukte <rahul.bukte@sony.com>
> > Signed-off-by: Rahul Bukte <rahul.bukte@sony.com>
> > Signed-off-by: Shashank Balaji <shashank.mahadasyam@sony.com>
> > ---
> > Patch 4 depends on this patch
> > ---
> >  kernel/params.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/params.c b/kernel/params.c
> > index 74d620bc2521..ac088d4b09a9 100644
> > --- a/kernel/params.c
> > +++ b/kernel/params.c
> > @@ -957,7 +957,7 @@ static int __init param_sysfs_init(void)
> >  
> >  	return 0;
> >  }
> > -subsys_initcall(param_sysfs_init);
> > +pure_initcall(param_sysfs_init);
> >  
> >  /*
> >   * param_sysfs_builtin_init - add sysfs version and parameter
> > 
> 
> The change looks ok to me functionality-wise. Sysfs is initialized
> earlier in do_basic_setup() and other code, such as classes_init(),
> calls kset_create_and_add() similarly early.
> 
> One minor issue is that pure_initcall() was originally intended for
> static variable initialization. The file include/linux/init.h says:
> 
> | /*
> |  * A "pure" initcall has no dependencies on anything else, and purely
> |  * initializes variables that couldn't be statically initialized.
> |  *
> |  * This only exists for built-in code, not for modules.
> |  * Keep main.c:initcall_level_names[] in sync.
> |  */
> | #define pure_initcall(fn)		__define_initcall(fn, 0)
> 
> The patch stretches the intended use of pure_initcall() somewhat in this
> regard. However, other code already appears to do the same, so I guess
> this is ok.

Ah yeah, I thought of this too, but it seems like everyone else is doing
it. Linus introduced pure_initcall in b3438f8266cb
("Add "pure_initcall" for static variable initialization") with the
comment, and he introduced another user of it in 140d0b2108fa
("Do 'shm_init_ns()' in an early pure_initcall") which already stretches
the intended use as per the comment.

Given that it's just being used for "run me before core_initcall;
early_initcall is too early for me" without any "pureness" requirements, I
suppose the comment is due for a revision?

> Additionally, I think it would be good to update the comment preceding
> param_sysfs_init(). It currently says:
> 
> | /*
> |  * param_sysfs_init - create "module" kset
> |  *
> |  * This must be done before the initramfs is unpacked and
> |  * request_module() thus becomes possible, because otherwise the
> |  * module load would fail in mod_sysfs_init.
> |  */
> 
> I suggest changing it to something like follows:
> 
> This must be done before any driver registration so that when a driver comes
> from a built-in module, the driver core can add the module under /sys/module
> and create the associated driver symlinks.

Thanks for catching this! I'll add it in the next revision.

Thanks,
Shashank
Gary Guo May 27, 2026, 11:19 a.m. UTC | #3
On Mon May 18, 2026 at 11:19 AM BST, Shashank Balaji wrote:
> Commit "driver core: platform: set mod_name in driver registration" will set
> struct device_driver's mod_name member for platform driver registration. For a
> driver to be registered with its mod_name set, module_kset needs to be
> initialized, which currently happens in a subsys_initcall in param_sysfs_init().
> The tegra cbb drivers register themselves before module_kset init, in a
> core_initcall. This works currently because lookup_or_create_module_kobject(),
> which dereferences module_kset via kset_find_obj(), is not called if mod_name
> is not set, which is the case now.
> 
> So in preparation for the commit "driver core: platform: set mod_name in driver registration",
> move module_kset init to pure_initcall level, ensuring it happens before tegra
> cbb driver registration.
> 
> Suggested-by: Gary Guo <gary@garyguo.net>
> Co-developed-by: Rahul Bukte <rahul.bukte@sony.com>
> Signed-off-by: Rahul Bukte <rahul.bukte@sony.com>
> Signed-off-by: Shashank Balaji <shashank.mahadasyam@sony.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
> Patch 4 depends on this patch
> ---
>  kernel/params.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/kernel/params.c b/kernel/params.c
index 74d620bc2521..ac088d4b09a9 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -957,7 +957,7 @@  static int __init param_sysfs_init(void)
 
 	return 0;
 }
-subsys_initcall(param_sysfs_init);
+pure_initcall(param_sysfs_init);
 
 /*
  * param_sysfs_builtin_init - add sysfs version and parameter