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

[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@xxxxxxxxxxxxx>
> ---
>  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