Re: [PATCH sysctl-next] kernel/kexec_core: move kexec_core sysctls into its own file

[yingelin <yingelin@huawei.com>]

在 2022/2/24 10:35, Baoquan He 写道:
> On 02/24/22 at 09:04am, yingelin wrote:
>> 在 2022/2/23 16:30, Baoquan He 写道:
>>> On 02/23/22 at 11:03am, yingelin wrote:
>>>> This move the kernel/kexec_core.c respective sysctls to its own file.
>>> Hmm, why is the move needed?
>>>
>>> With my understanding, sysctls are all put in kernel/sysctl.c,
>>> why is kexec special?
>> kernel/sysctl.c is a kitchen sink where everyone leaves their dirty dishes,
>>
>> this makes it very difficult to maintain. The proc sysctl maintainers do not
>> want to
>>
>> know what sysctl knobs you wish to add for your own piece of code, we
>>
>> just care about the core logic.
>>
>> This patch moves the kexec sysctls to the place where they actually belong
>> to help
> That seems to be an issue everything related to sysctl are all added to
> kernel/sysctl.c. Do you have a pointer that someone complained about it
> and people agree to scatter them into their own component code?

I'm sorry to reply you too late, the link is

https://lkml.kernel.org/r/20220226031054.47DF8C340E7@smtp.kernel.org

> I understand your concern now, I am personally not confused by that
> maybe because I haven't got stuff adding or changing into sysctls. My
> concern is if we only care and move kexec knob, or we have plan to try
> to move all of them. If there's some background information or
> discussion with a link, that would be helpful.

Yeah, we are going to move all sysctls to their own places, in fact, all 
the filesystem

sysctls are moved out already. I'm sorry I didn't express it clearly. 
I'll fix it in v2 patch.

>
> Thanks
> Baoquan
>
>> with this maintenance.
>>
>>>> Signed-off-by: yingelin <yingelin@huawei.com>
>>>> ---
>>>>    kernel/kexec_core.c | 20 ++++++++++++++++++++
>>>>    kernel/sysctl.c     | 13 -------------
>>>>    2 files changed, 20 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>>>> index 68480f731192..e57339d49439 100644
>>>> --- a/kernel/kexec_core.c
>>>> +++ b/kernel/kexec_core.c
>>>> @@ -936,6 +936,26 @@ int kimage_load_segment(struct kimage *image,
>>>>    struct kimage *kexec_image;
>>>>    struct kimage *kexec_crash_image;
>>>>    int kexec_load_disabled;
>>>> +static struct ctl_table kexec_core_sysctls[] = {
>>>> +	{
>>>> +		.procname	= "kexec_load_disabled",
>>>> +		.data		= &kexec_load_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,
>>>> +	},
>>>> +	{ }
>>>> +};
>>>> +
>>>> +static int __init kexec_core_sysctl_init(void)
>>>> +{
>>>> +	register_sysctl_init("kernel", kexec_core_sysctls);
>>>> +	return 0;
>>>> +}
>>>> +late_initcall(kexec_core_sysctl_init);
>>>>    /*
>>>>     * No panic_cpu check version of crash_kexec().  This function is called
>>>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>>>> index ae5e59396b5d..00e97c6d6576 100644
>>>> --- a/kernel/sysctl.c
>>>> +++ b/kernel/sysctl.c
>>>> @@ -61,7 +61,6 @@
>>>>    #include <linux/capability.h>
>>>>    #include <linux/binfmts.h>
>>>>    #include <linux/sched/sysctl.h>
>>>> -#include <linux/kexec.h>
>>>>    #include <linux/bpf.h>
>>>>    #include <linux/mount.h>
>>>>    #include <linux/userfaultfd_k.h>
>>>> @@ -1839,18 +1838,6 @@ static struct ctl_table kern_table[] = {
>>>>    		.proc_handler	= tracepoint_printk_sysctl,
>>>>    	},
>>>>    #endif
>>>> -#ifdef CONFIG_KEXEC_CORE
>>>> -	{
>>>> -		.procname	= "kexec_load_disabled",
>>>> -		.data		= &kexec_load_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,
>>>> -	},
>>>> -#endif
>>>>    #ifdef CONFIG_MODULES
>>>>    	{
>>>>    		.procname	= "modprobe",
>>>> -- 
>>>> 2.26.2
>>>>
>>> .
> .