On 5/9/25 14:54, Joel Granados wrote:
> Move module sysctl (modprobe_path and modules_disabled) out of sysctl.c
> and into the modules subsystem. Make the modprobe_path variable static
> as it no longer needs to be exported. Remove module.h from the includes
> in sysctl as it no longer uses any module exported variables.
> 
> This is part of a greater effort to move ctl tables into their
> respective subsystems which will reduce the merge conflicts in
> kernel/sysctl.c.
> 
> Signed-off-by: Joel Granados <[email protected]>
> [...]
> --- a/kernel/module/kmod.c
> +++ b/kernel/module/kmod.c
> @@ -60,7 +60,7 @@ static DEFINE_SEMAPHORE(kmod_concurrent_max, 
> MAX_KMOD_CONCURRENT);
>  /*
>       modprobe_path is set via /proc/sys.
>  */
> -char modprobe_path[KMOD_PATH_LEN] = CONFIG_MODPROBE_PATH;
> +static char modprobe_path[KMOD_PATH_LEN] = CONFIG_MODPROBE_PATH;
>  
>  static void free_modprobe_argv(struct subprocess_info *info)
>  {
> @@ -177,3 +177,33 @@ int __request_module(bool wait, const char *fmt, ...)
>       return ret;
>  }
>  EXPORT_SYMBOL(__request_module);
> +
> +#ifdef CONFIG_MODULES
> +static const struct ctl_table kmod_sysctl_table[] = {
> +     {
> +             .procname       = "modprobe",
> +             .data           = &modprobe_path,
> +             .maxlen         = KMOD_PATH_LEN,
> +             .mode           = 0644,
> +             .proc_handler   = proc_dostring,
> +     },
> +     {
> +             .procname       = "modules_disabled",
> +             .data           = &modules_disabled,
> +             .maxlen         = sizeof(int),
> +             .mode           = 0644,
> +             /* only handle a transition from default "0" to "1" */
> +             .proc_handler   = proc_dointvec_minmax,
> +             .extra1         = SYSCTL_ONE,
> +             .extra2         = SYSCTL_ONE,
> +     },

This is minor.. but the file kernel/module/kmod.c contains the logic to
request direct modprobe invocation by the kernel. Registering the
modprobe_path sysctl here is appropriate. However, the modules_disabled
setting affects the entire module loader so I don't think it's best to
register it here.

I suggest keeping a single table for the module sysctl values but moving
it to kernel/module/main.c. This means the variable modprobe_path must
retain external linkage, on the other hand, modules_disabled can be made
static.

-- 
Thanks,
Petr

> +};
> +
> +static int __init init_kmod_sysctl(void)
> +{
> +     register_sysctl_init("kernel", kmod_sysctl_table);
> +     return 0;
> +}
> +
> +subsys_initcall(init_kmod_sysctl);
> +#endif

Reply via email to