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

yingelin
2022-02-28T01:41:09Z

在 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
>>>>
>>> .
> .