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 <joel.granados@xxxxxxxxxx>
> [...]
> --- 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
