Re: [PATCH v3 1/4] kernel: param: initialize module_kset on-demand

[Shashank Balaji <shashank.mahadasyam@sony.com>]

On Wed, Apr 22, 2026 at 06:49:03PM +0900, Shashank Balaji wrote:
> module_kset is initialized in param_sysfs_init(), a subsys_initcall. A number
> of platform drivers register themselves prior to subsys_initcalls. With an
> upcoming patch ("driver core: platform: set mod_name in driver registration")
> that sets their mod_name in struct device_driver, lookup_or_create_module()
> will be called for those drivers, which calls kset_find_object(module_kset, mod_name).
> This fails because module_kset isn't alive yet.
> 
> Fix this by initializing module_kset on-demand in lookup_or_create_module().
> Retain the param_sysfs_init() subsys_initcall to ensure that module_kset is
> live after subsys_initcalls (assuming no OOM) for any users who may need it,
> on the off chance that it wasn't init'd on-demand because of no
> pre-subsys_initcall drivers.
> 
> This on-demand path can trigger before subsys_initcall. kset_create_and_add()
> be should safe in those contexts because the allocator is up and running by then,
> no userspace to start uevent helper or listen to a uevent socket.
> 
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 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 3 depends on this patch.
> ---
>  kernel/params.c | 41 +++++++++++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/kernel/params.c b/kernel/params.c
> index 74d620bc2521..f25d6fda159c 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -745,6 +745,26 @@ void module_param_sysfs_remove(struct module *mod)
>  }
>  #endif
>  
> +static int uevent_filter(const struct kobject *kobj)
> +{
> +	const struct kobj_type *ktype = get_ktype(kobj);
> +
> +	if (ktype == &module_ktype)
> +		return 1;
> +	return 0;
> +}
> +
> +static const struct kset_uevent_ops module_uevent_ops = {
> +	.filter = uevent_filter,
> +};
> +
> +static struct kset *__init_or_module ensure_module_kset(void)
> +{
> +	if (!module_kset)
> +		module_kset = kset_create_and_add("module", &module_uevent_ops, NULL);
> +	return module_kset;
> +}

Sashiko's review [1]:

	Could this cause a race condition if multiple threads try to initialize
	module_kset concurrently?

	If asynchronous driver registration triggers this path concurrently before
	subsys_initcalls, is it possible for both threads to see module_kset as NULL:

	Thread 1
	    if (!module_kset)
		module_kset = kset_create_and_add("module", &module_uevent_ops, NULL);
		/* Succeeds and assigns a valid kset */

	Thread 2
	    if (!module_kset)
		module_kset = kset_create_and_add("module", &module_uevent_ops, NULL);
		/* Fails due to duplicate name and returns NULL */

	If Thread 2 overwrites module_kset with NULL after Thread 1 succeeds, wouldn't
	this orphan the kset created by Thread 1 and cause all subsequent callers to
	fail? Does this initialization need synchronization to prevent this?

While all pre-subsys_initcall platform driver registration happens
synchronously now, on the boot cpu, asynchronous registration may be
introduced in the future which would silently break this patch. Either
way, it would be better to be safe with a mutex.

I also noticed another problem with this patch: kset_create_and_add() is
called as long as module_kset is not set. So in the case of OOM, init
will be attempted as long as it succeeds, even beyond subsys_initcall.
While the current behaviour is to init only in subsys_initcall.

Both of these can be fixed with a DO_ONCE_SLEEPABLE-based
initialization.

[1] https://sashiko.dev/#/patchset/20260422-acpi_mod_name-v3-0-a184eff9ff6f@sony.com?part=1