linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Nick Piggin <npiggin@suse.de>
To: David Rientjes <rientjes@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Balbir Singh <balbir@linux.vnet.ibm.com>,
	Lubos Lunak <l.lunak@suse.cz>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch -mm 7/9 v2] oom: replace sysctls with quick mode
Date: Tue, 16 Feb 2010 17:28:33 +1100	[thread overview]
Message-ID: <20100216062833.GB5723@laptop> (raw)
In-Reply-To: <alpine.DEB.2.00.1002151419120.26927@chino.kir.corp.google.com>

On Mon, Feb 15, 2010 at 02:20:18PM -0800, David Rientjes wrote:
> Two VM sysctls, oom dump_tasks and oom_kill_allocating_task, were
> implemented for very large systems to avoid excessively long tasklist
> scans.  The former suppresses helpful diagnostic messages that are
> emitted for each thread group leader that are candidates for oom kill
> including their pid, uid, vm size, rss, oom_adj value, and name; this
> information is very helpful to users in understanding why a particular
> task was chosen for kill over others.  The latter simply kills current,
> the task triggering the oom condition, instead of iterating through the
> tasklist looking for the worst offender.
> 
> Both of these sysctls are combined into one for use on the aforementioned
> large systems: oom_kill_quick.  This disables the now-default
> oom_dump_tasks and kills current whenever the oom killer is called.
> 
> The oom killer rewrite is the perfect opportunity to combine both sysctls
> into one instead of carrying around the others for years to come for
> nothing else than legacy purposes.

I just don't understand this either. There appears to be simply no
performance or maintainability reason to change this.

> 
> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  Documentation/sysctl/vm.txt |   44 +++++-------------------------------------
>  include/linux/oom.h         |    3 +-
>  kernel/sysctl.c             |   13 ++---------
>  mm/oom_kill.c               |    9 +++----
>  4 files changed, 14 insertions(+), 55 deletions(-)
> 
> diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
> --- a/Documentation/sysctl/vm.txt
> +++ b/Documentation/sysctl/vm.txt
> @@ -43,9 +43,8 @@ Currently, these files are in /proc/sys/vm:
>  - nr_pdflush_threads
>  - nr_trim_pages         (only if CONFIG_MMU=n)
>  - numa_zonelist_order
> -- oom_dump_tasks
>  - oom_forkbomb_thres
> -- oom_kill_allocating_task
> +- oom_kill_quick
>  - overcommit_memory
>  - overcommit_ratio
>  - page-cluster
> @@ -470,27 +469,6 @@ this is causing problems for your system/application.
>  
>  ==============================================================
>  
> -oom_dump_tasks
> -
> -Enables a system-wide task dump (excluding kernel threads) to be
> -produced when the kernel performs an OOM-killing and includes such
> -information as pid, uid, tgid, vm size, rss, cpu, oom_adj score, and
> -name.  This is helpful to determine why the OOM killer was invoked
> -and to identify the rogue task that caused it.
> -
> -If this is set to zero, this information is suppressed.  On very
> -large systems with thousands of tasks it may not be feasible to dump
> -the memory state information for each one.  Such systems should not
> -be forced to incur a performance penalty in OOM conditions when the
> -information may not be desired.
> -
> -If this is set to non-zero, this information is shown whenever the
> -OOM killer actually kills a memory-hogging task.
> -
> -The default value is 0.
> -
> -==============================================================
> -
>  oom_forkbomb_thres
>  
>  This value defines how many children with a seperate address space a specific
> @@ -511,22 +489,12 @@ The default value is 1000.
>  
>  ==============================================================
>  
> -oom_kill_allocating_task
> -
> -This enables or disables killing the OOM-triggering task in
> -out-of-memory situations.
> -
> -If this is set to zero, the OOM killer will scan through the entire
> -tasklist and select a task based on heuristics to kill.  This normally
> -selects a rogue memory-hogging task that frees up a large amount of
> -memory when killed.
> -
> -If this is set to non-zero, the OOM killer simply kills the task that
> -triggered the out-of-memory condition.  This avoids the expensive
> -tasklist scan.
> +oom_kill_quick
>  
> -If panic_on_oom is selected, it takes precedence over whatever value
> -is used in oom_kill_allocating_task.
> +When enabled, this will always kill the task that triggered the oom killer, i.e.
> +the task that attempted to allocate memory that could not be found.  It also
> +suppresses the tasklist dump to the kernel log whenever the oom killer is
> +called.  Typically set on systems with an extremely large number of tasks.
>  
>  The default value is 0.
>  
> diff --git a/include/linux/oom.h b/include/linux/oom.h
> --- a/include/linux/oom.h
> +++ b/include/linux/oom.h
> @@ -63,8 +63,7 @@ static inline void oom_killer_enable(void)
>  }
>  /* for sysctl */
>  extern int sysctl_panic_on_oom;
> -extern int sysctl_oom_kill_allocating_task;
> -extern int sysctl_oom_dump_tasks;
> +extern int sysctl_oom_kill_quick;
>  extern int sysctl_oom_forkbomb_thres;
>  
>  #endif /* __KERNEL__*/
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -941,16 +941,9 @@ static struct ctl_table vm_table[] = {
>  		.proc_handler	= proc_dointvec,
>  	},
>  	{
> -		.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),
> +		.procname	= "oom_kill_quick",
> +		.data		= &sysctl_oom_kill_quick,
> +		.maxlen		= sizeof(sysctl_oom_kill_quick),
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec,
>  	},
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -32,9 +32,8 @@
>  #include <linux/security.h>
>  
>  int sysctl_panic_on_oom;
> -int sysctl_oom_kill_allocating_task;
> -int sysctl_oom_dump_tasks;
>  int sysctl_oom_forkbomb_thres = DEFAULT_OOM_FORKBOMB_THRES;
> +int sysctl_oom_kill_quick;
>  static DEFINE_SPINLOCK(zone_scan_lock);
>  
>  /*
> @@ -402,7 +401,7 @@ static void dump_header(struct task_struct *p, gfp_t gfp_mask, int order,
>  	dump_stack();
>  	mem_cgroup_print_oom_info(mem, p);
>  	show_mem();
> -	if (sysctl_oom_dump_tasks)
> +	if (!sysctl_oom_kill_quick)
>  		dump_tasks(mem);
>  }
>  
> @@ -609,9 +608,9 @@ static void __out_of_memory(gfp_t gfp_mask, int order, unsigned long totalpages,
>  	struct task_struct *p;
>  	unsigned int points;
>  
> -	if (sysctl_oom_kill_allocating_task)
> +	if (sysctl_oom_kill_quick)
>  		if (!oom_kill_process(current, gfp_mask, order, 0, totalpages,
> -			NULL, "Out of memory (oom_kill_allocating_task)"))
> +			NULL, "Out of memory (quick mode)"))
>  			return;
>  retry:
>  	/*

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2010-02-16  6:28 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-15 22:19 [patch -mm 0/9 v2] oom killer rewrite David Rientjes
2010-02-15 22:20 ` [patch -mm 1/9 v2] oom: filter tasks not sharing the same cpuset David Rientjes
2010-02-16  6:14   ` Nick Piggin
2010-02-15 22:20 ` [patch -mm 2/9 v2] oom: sacrifice child with highest badness score for parent David Rientjes
2010-02-16  6:15   ` Nick Piggin
2010-02-15 22:20 ` [patch -mm 3/9 v2] oom: select task from tasklist for mempolicy ooms David Rientjes
2010-02-23  6:31   ` Balbir Singh
2010-02-23  8:17     ` David Rientjes
2010-02-15 22:20 ` [patch -mm 4/9 v2] oom: remove compulsory panic_on_oom mode David Rientjes
2010-02-16  0:00   ` KAMEZAWA Hiroyuki
2010-02-16  0:14     ` David Rientjes
2010-02-16  0:23       ` KAMEZAWA Hiroyuki
2010-02-16  9:02         ` David Rientjes
2010-02-16 23:42           ` KAMEZAWA Hiroyuki
2010-02-16 23:54             ` David Rientjes
2010-02-17  0:01               ` KAMEZAWA Hiroyuki
2010-02-17  0:31                 ` David Rientjes
2010-02-17  0:41                   ` KAMEZAWA Hiroyuki
2010-02-17  0:54                     ` David Rientjes
2010-02-17  1:03                       ` KAMEZAWA Hiroyuki
2010-02-17  1:58                       ` David Rientjes
2010-02-17  2:13                         ` KAMEZAWA Hiroyuki
2010-02-17  2:23                           ` KAMEZAWA Hiroyuki
2010-02-17  2:37                             ` David Rientjes
2010-02-17  2:28                           ` David Rientjes
2010-02-17  2:34                             ` KAMEZAWA Hiroyuki
2010-02-17  2:58                               ` David Rientjes
2010-02-17  3:21                                 ` KAMEZAWA Hiroyuki
2010-02-17  9:11                                   ` David Rientjes
2010-02-17  9:52                                     ` Nick Piggin
2010-02-17 22:04                                       ` David Rientjes
2010-02-22  5:31                               ` Daisuke Nishimura
2010-02-22  6:15                                 ` KAMEZAWA Hiroyuki
2010-02-22 11:42                                   ` Daisuke Nishimura
2010-02-22 20:59                                     ` David Rientjes
2010-02-22 23:51                                     ` KAMEZAWA Hiroyuki
2010-02-22 20:55                                   ` David Rientjes
2010-02-17  2:19                         ` KOSAKI Motohiro
2010-02-16  6:20   ` Nick Piggin
2010-02-16  6:59     ` David Rientjes
2010-02-16  7:20       ` Nick Piggin
2010-02-16  7:53         ` David Rientjes
2010-02-16  8:08           ` Nick Piggin
2010-02-16  8:10             ` KAMEZAWA Hiroyuki
2010-02-16  8:42             ` David Rientjes
2010-02-15 22:20 ` [patch -mm 5/9 v2] oom: badness heuristic rewrite David Rientjes
2010-02-15 22:20 ` [patch -mm 6/9 v2] oom: deprecate oom_adj tunable David Rientjes
2010-02-15 22:28   ` Alan Cox
2010-02-15 22:35     ` David Rientjes
2010-02-15 22:20 ` [patch -mm 7/9 v2] oom: replace sysctls with quick mode David Rientjes
2010-02-16  6:28   ` Nick Piggin [this message]
2010-02-16  8:58     ` David Rientjes
2010-02-15 22:20 ` [patch -mm 8/9 v2] oom: avoid oom killer for lowmem allocations David Rientjes
2010-02-15 23:57   ` KAMEZAWA Hiroyuki
2010-02-16  0:10     ` David Rientjes
2010-02-16  0:21       ` KAMEZAWA Hiroyuki
2010-02-16  1:13         ` [patch] mm: add comment about deprecation of __GFP_NOFAIL David Rientjes
2010-02-16  1:26           ` KAMEZAWA Hiroyuki
2010-02-16  7:03             ` David Rientjes
2010-02-16  7:23               ` Nick Piggin
2010-02-16  5:32       ` [patch -mm 8/9 v2] oom: avoid oom killer for lowmem allocations KOSAKI Motohiro
2010-02-16  7:29         ` David Rientjes
2010-02-16  6:44       ` Nick Piggin
2010-02-16  7:41         ` David Rientjes
2010-02-16  7:53           ` Nick Piggin
2010-02-16  8:25             ` David Rientjes
2010-02-16 23:48               ` KAMEZAWA Hiroyuki
2010-02-17  0:03                 ` David Rientjes
2010-02-17  0:03                   ` KAMEZAWA Hiroyuki
2010-02-17  0:21                     ` David Rientjes
2010-02-23 11:24                       ` Balbir Singh
2010-02-23 21:12                         ` David Rientjes
2010-02-15 22:20 ` [patch -mm 9/9 v2] oom: remove unnecessary code and cleanup David Rientjes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100216062833.GB5723@laptop \
    --to=npiggin@suse.de \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=l.lunak@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox