Re: [PATCH] mm: move oom_kill sysctls to their own file

[苏家训 <sujiaxun@uniontech.com>]

在 2022/2/15 15:29, Luis Chamberlain 写道:
> On Tue, Feb 15, 2022 at 11:02:57AM +0800, sujiaxun wrote:
>> kernel/sysctl.c is a kitchen sink where everyone leaves their dirty
>> dishes, this makes it very difficult to maintain.
>>
>> To help with this maintenance let's start by moving sysctls to places
>> where they actually belong.  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.
>>
>> So move the oom_kill sysctls to its own file.
>>
>> Signed-off-by: sujiaxun <sujiaxun@uniontech.com>
>> ---
>>   include/linux/oom.h |  4 ----
>>   kernel/sysctl.c     | 23 -----------------------
>>   mm/oom_kill.c       | 37 ++++++++++++++++++++++++++++++++++---
>>   3 files changed, 34 insertions(+), 30 deletions(-)
>>
>> diff --git a/include/linux/oom.h b/include/linux/oom.h
>> index 2db9a1432511..02d1e7bbd8cd 100644
>> --- a/include/linux/oom.h
>> +++ b/include/linux/oom.h
>> @@ -123,8 +123,4 @@ extern void oom_killer_enable(void);
>>
>>   extern struct task_struct *find_lock_task_mm(struct task_struct *p);
>>
>> -/* sysctls */
>> -extern int sysctl_oom_dump_tasks;
>> -extern int sysctl_oom_kill_allocating_task;
>> -extern int sysctl_panic_on_oom;
>>   #endif /* _INCLUDE_LINUX_OOM_H */
>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>> index 788b9a34d5ab..40d822fbb6d5 100644
>> --- a/kernel/sysctl.c
>> +++ b/kernel/sysctl.c
>> @@ -2352,29 +2352,6 @@ static struct ctl_table vm_table[] = {
>>   		.extra1		= SYSCTL_ZERO,
>>   		.extra2		= SYSCTL_TWO,
>>   	},
>> -	{
>> -		.procname	= "panic_on_oom",
>> -		.data		= &sysctl_panic_on_oom,
>> -		.maxlen		= sizeof(sysctl_panic_on_oom),
>> -		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec_minmax,
>> -		.extra1		= SYSCTL_ZERO,
>> -		.extra2		= SYSCTL_TWO,
>> -	},
>> -	{
>> -		.procname	= "oom_kill_allocating_task",
>> -		.data		= &sysctl_oom_kill_allocating_task,
>> -		.maxlen		= sizeof(sysctl_oom_kill_allocating_task),
>> -		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec,
>> -	},
>> -	{
>> -		.procname	= "oom_dump_tasks",
>> -		.data		= &sysctl_oom_dump_tasks,
>> -		.maxlen		= sizeof(sysctl_oom_dump_tasks),
>> -		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec,
>> -	},
>>   	{
>>   		.procname	= "overcommit_ratio",
>>   		.data		= &sysctl_overcommit_ratio,
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 6b875acabd1e..c720c0710911 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -52,9 +52,35 @@
>>   #define CREATE_TRACE_POINTS
>>   #include <trace/events/oom.h>
>>
>> -int sysctl_panic_on_oom;
>> -int sysctl_oom_kill_allocating_task;
>> -int sysctl_oom_dump_tasks = 1;
>> +static int sysctl_panic_on_oom;
>> +static int sysctl_oom_kill_allocating_task;
>> +static int sysctl_oom_dump_tasks = 1;
>> +
>> +static struct ctl_table vm_oom_kill_table[] = {
>> +	{
>> +		.procname	= "panic_on_oom",
>> +		.data		= &sysctl_panic_on_oom,
>> +		.maxlen		= sizeof(sysctl_panic_on_oom),
>> +		.mode		= 0644,
>> +		.proc_handler	= proc_dointvec_minmax,
>> +		.extra1		= SYSCTL_ZERO,
>> +		.extra2		= SYSCTL_TWO,
>> +	},
>> +	{
>> +		.procname	= "oom_kill_allocating_task",
>> +		.data		= &sysctl_oom_kill_allocating_task,
>> +		.maxlen		= sizeof(sysctl_oom_kill_allocating_task),
>> +		.mode		= 0644,
>> +		.proc_handler	= proc_dointvec,
>> +	},
>> +	{
>> +		.procname	= "oom_dump_tasks",
>> +		.data		= &sysctl_oom_dump_tasks,
>> +		.maxlen		= sizeof(sysctl_oom_dump_tasks),
>> +		.mode		= 0644,
>> +		.proc_handler	= proc_dointvec,
>> +	}
>> +};
>>
>>   /*
>>    * Serializes oom killer invocations (out_of_memory()) from all contexts to
>> @@ -680,6 +706,11 @@ static void wake_oom_reaper(struct task_struct *tsk)
>>   static int __init oom_init(void)
>>   {
>>   	oom_reaper_th = kthread_run(oom_reaper, NULL, "oom_reaper");
>> +
>> +	#ifdef CONFIG_SYSCTL
>> +		register_sysctl_init("vm", vm_oom_kill_table);
>> +	#endif
> 
> PLease avoid the ifdefs and the tab spacing seems very off here.
> 
> Also are you running ./scripts/get_maintainer* ?
> 
>    Luis
> 
Thank you for reviewing, I will revise and resubmit.
I checked the patch using ./scripts/checkpatch.pl and found no errors.