linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Nick Piggin <npiggin@suse.de>
To: David Rientjes <rientjes@google.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	anfei <anfei.zhou@gmail.com>,
	nishimura@mxp.nes.nec.co.jp,
	Balbir Singh <balbir@linux.vnet.ibm.com>,
	linux-mm@kvack.org
Subject: Re: [patch -mm] memcg: make oom killer a no-op when no killable task can be found
Date: Fri, 23 Apr 2010 01:39:56 +1000	[thread overview]
Message-ID: <20100422153956.GY5683@laptop> (raw)
In-Reply-To: <alpine.DEB.2.00.1004220326130.19785@chino.kir.corp.google.com>

On Thu, Apr 22, 2010 at 03:28:38AM -0700, David Rientjes wrote:
> On Thu, 22 Apr 2010, Nick Piggin wrote:
> 
> > Oh actually what happened with the pagefault OOM / panic on oom thing?
> > We were talking around in circles about that too.
> > 
> 
> The oom killer rewrite attempts to kill current first, if possible, and 
> then will panic if panic_on_oom is set before falling back to selecting a 
> victim.

See, this is what we want to avoid. If the user sets panic_on_oom,
it is because they want the system to panic on oom. Not to kill
tasks and try to continue. The user does not know or care in the
slightest about "page fault oom". So I don't know why you think this
is a good idea.


>  This is consistent with all other architectures such as powerpc 
> that currently do not use pagefault_out_of_memory().  If all architectures 
> are eventually going to be converted to using pagefault_out_of_memory() 

Yes, architectures are going to be converted, it has already been
agreed, I dropped the ball and lazily hoped the arch people would do it.
But further work done should be to make it consistent in the right way,
not the wrong way.


> with additional work on top of -mm, it would be possible to define 
> consistent panic_on_oom semantics for this case.  I welcome such an 
> addition since I believe it's a natural extension of panic_on_oom, but I 
> believe it should be done consistently so the sysctl doesn't have 
> different semantics depending on the underlying arch.

It's simply a bug rather than intentional semantics. "pagefault oom"
is basically a meaningless semantic for the user.

Let's do a deal. I'll split up the below patch and send it to arch
maintainers, and you don't change the sysctl interface or "fix" the
pagefault oom path.

--
Index: linux-2.6/arch/alpha/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/alpha/mm/fault.c
+++ linux-2.6/arch/alpha/mm/fault.c
@@ -188,16 +188,10 @@ do_page_fault(unsigned long address, uns
 	/* We ran out of memory, or some other thing happened to us that
 	   made us unable to handle the page fault gracefully.  */
  out_of_memory:
-	if (is_global_init(current)) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
-	printk(KERN_ALERT "VM: killing process %s(%d)\n",
-	       current->comm, task_pid_nr(current));
 	if (!user_mode(regs))
 		goto no_context;
-	do_group_exit(SIGKILL);
+	pagefault_out_of_memory();
+	return;
 
  do_sigbus:
 	/* Send a sigbus, regardless of whether we were in kernel
Index: linux-2.6/arch/avr32/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/avr32/mm/fault.c
+++ linux-2.6/arch/avr32/mm/fault.c
@@ -211,15 +211,10 @@ no_context:
 	 */
 out_of_memory:
 	up_read(&mm->mmap_sem);
-	if (is_global_init(current)) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
-	printk("VM: Killing process %s\n", tsk->comm);
-	if (user_mode(regs))
-		do_group_exit(SIGKILL);
-	goto no_context;
+	pagefault_out_of_memory();
+	if (!user_mode(regs))
+		goto no_context;
+	return;
 
 do_sigbus:
 	up_read(&mm->mmap_sem);
Index: linux-2.6/arch/cris/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/cris/mm/fault.c
+++ linux-2.6/arch/cris/mm/fault.c
@@ -245,10 +245,10 @@ do_page_fault(unsigned long address, str
 
  out_of_memory:
 	up_read(&mm->mmap_sem);
-	printk("VM: killing process %s\n", tsk->comm);
-	if (user_mode(regs))
-		do_exit(SIGKILL);
-	goto no_context;
+	if (!user_mode(regs))
+		goto no_context;
+	pagefault_out_of_memory();
+	return;
 
  do_sigbus:
 	up_read(&mm->mmap_sem);
Index: linux-2.6/arch/frv/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/frv/mm/fault.c
+++ linux-2.6/arch/frv/mm/fault.c
@@ -257,10 +257,10 @@ asmlinkage void do_page_fault(int datamm
  */
  out_of_memory:
 	up_read(&mm->mmap_sem);
-	printk("VM: killing process %s\n", current->comm);
-	if (user_mode(__frame))
-		do_group_exit(SIGKILL);
-	goto no_context;
+	if (!user_mode(__frame))
+		goto no_context;
+	pagefault_out_of_memory();
+	return;
 
  do_sigbus:
 	up_read(&mm->mmap_sem);
Index: linux-2.6/arch/ia64/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/ia64/mm/fault.c
+++ linux-2.6/arch/ia64/mm/fault.c
@@ -276,13 +276,7 @@ ia64_do_page_fault (unsigned long addres
 
   out_of_memory:
 	up_read(&mm->mmap_sem);
-	if (is_global_init(current)) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
-	printk(KERN_CRIT "VM: killing process %s\n", current->comm);
-	if (user_mode(regs))
-		do_group_exit(SIGKILL);
-	goto no_context;
+	if (!user_mode(regs))
+		goto no_context;
+	pagefault_out_of_memory();
 }
Index: linux-2.6/arch/m32r/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/m32r/mm/fault.c
+++ linux-2.6/arch/m32r/mm/fault.c
@@ -271,15 +271,10 @@ no_context:
  */
 out_of_memory:
 	up_read(&mm->mmap_sem);
-	if (is_global_init(tsk)) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
-	printk("VM: killing process %s\n", tsk->comm);
 	if (error_code & ACE_USERMODE)
-		do_group_exit(SIGKILL);
-	goto no_context;
+		goto no_context;
+	pagefault_out_of_memory();
+	return;
 
 do_sigbus:
 	up_read(&mm->mmap_sem);
Index: linux-2.6/arch/m68k/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/m68k/mm/fault.c
+++ linux-2.6/arch/m68k/mm/fault.c
@@ -180,15 +180,10 @@ good_area:
  */
 out_of_memory:
 	up_read(&mm->mmap_sem);
-	if (is_global_init(current)) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
-
-	printk("VM: killing process %s\n", current->comm);
-	if (user_mode(regs))
-		do_group_exit(SIGKILL);
+	if (!user_mode(regs))
+		goto no_context;
+	pagefault_out_of_memory();
+	return;
 
 no_context:
 	current->thread.signo = SIGBUS;
Index: linux-2.6/arch/microblaze/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/microblaze/mm/fault.c
+++ linux-2.6/arch/microblaze/mm/fault.c
@@ -273,16 +273,11 @@ bad_area_nosemaphore:
  * us unable to handle the page fault gracefully.
  */
 out_of_memory:
-	if (current->pid == 1) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
 	up_read(&mm->mmap_sem);
-	printk(KERN_WARNING "VM: killing process %s\n", current->comm);
-	if (user_mode(regs))
-		do_exit(SIGKILL);
-	bad_page_fault(regs, address, SIGKILL);
+	if (!user_mode(regs))
+		bad_page_fault(regs, address, SIGKILL);
+	else
+		pagefault_out_of_memory();
 	return;
 
 do_sigbus:
Index: linux-2.6/arch/mn10300/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/mn10300/mm/fault.c
+++ linux-2.6/arch/mn10300/mm/fault.c
@@ -338,11 +338,10 @@ no_context:
  */
 out_of_memory:
 	up_read(&mm->mmap_sem);
-	monitor_signal(regs);
-	printk(KERN_ALERT "VM: killing process %s\n", tsk->comm);
-	if ((fault_code & MMUFCR_xFC_ACCESS) == MMUFCR_xFC_ACCESS_USR)
-		do_exit(SIGKILL);
-	goto no_context;
+	if ((fault_code & MMUFCR_xFC_ACCESS) != MMUFCR_xFC_ACCESS_USR)
+		goto no_context;
+	pagefault_out_of_memory();
+	return;
 
 do_sigbus:
 	up_read(&mm->mmap_sem);
Index: linux-2.6/arch/parisc/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/parisc/mm/fault.c
+++ linux-2.6/arch/parisc/mm/fault.c
@@ -264,8 +264,7 @@ no_context:
 
   out_of_memory:
 	up_read(&mm->mmap_sem);
-	printk(KERN_CRIT "VM: killing process %s\n", current->comm);
-	if (user_mode(regs))
-		do_group_exit(SIGKILL);
-	goto no_context;
+	if (!user_mode(regs))
+		goto no_context;
+	pagefault_out_of_memory();
 }
Index: linux-2.6/arch/powerpc/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/powerpc/mm/fault.c
+++ linux-2.6/arch/powerpc/mm/fault.c
@@ -359,15 +359,10 @@ bad_area_nosemaphore:
  */
 out_of_memory:
 	up_read(&mm->mmap_sem);
-	if (is_global_init(current)) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
-	printk("VM: killing process %s\n", current->comm);
-	if (user_mode(regs))
-		do_group_exit(SIGKILL);
-	return SIGKILL;
+	if (!user_mode(regs))
+		return SIGKILL;
+	pagefault_out_of_memory();
+	return 0;
 
 do_sigbus:
 	up_read(&mm->mmap_sem);
Index: linux-2.6/arch/score/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/score/mm/fault.c
+++ linux-2.6/arch/score/mm/fault.c
@@ -167,15 +167,10 @@ no_context:
 	*/
 out_of_memory:
 	up_read(&mm->mmap_sem);
-	if (is_global_init(tsk)) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
-	printk("VM: killing process %s\n", tsk->comm);
-	if (user_mode(regs))
-		do_group_exit(SIGKILL);
-	goto no_context;
+	if (!user_mode(regs))
+		goto no_context;
+	pagefault_out_of_memory();
+	return;
 
 do_sigbus:
 	up_read(&mm->mmap_sem);
Index: linux-2.6/arch/sh/mm/fault_32.c
===================================================================
--- linux-2.6.orig/arch/sh/mm/fault_32.c
+++ linux-2.6/arch/sh/mm/fault_32.c
@@ -290,15 +290,10 @@ no_context:
  */
 out_of_memory:
 	up_read(&mm->mmap_sem);
-	if (is_global_init(current)) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
-	printk("VM: killing process %s\n", tsk->comm);
-	if (user_mode(regs))
-		do_group_exit(SIGKILL);
-	goto no_context;
+	if (!user_mode(regs))
+		goto no_context;
+	pagefault_out_of_memory();
+	return;
 
 do_sigbus:
 	up_read(&mm->mmap_sem);
Index: linux-2.6/arch/sh/mm/tlbflush_64.c
===================================================================
--- linux-2.6.orig/arch/sh/mm/tlbflush_64.c
+++ linux-2.6/arch/sh/mm/tlbflush_64.c
@@ -294,22 +294,11 @@ no_context:
  * us unable to handle the page fault gracefully.
  */
 out_of_memory:
-	if (is_global_init(current)) {
-		panic("INIT out of memory\n");
-		yield();
-		goto survive;
-	}
-	printk("fault:Out of memory\n");
 	up_read(&mm->mmap_sem);
-	if (is_global_init(current)) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
-	printk("VM: killing process %s\n", tsk->comm);
-	if (user_mode(regs))
-		do_group_exit(SIGKILL);
-	goto no_context;
+	if (!user_mode(regs))
+		goto no_context;
+	pagefault_out_of_memory();
+	return;
 
 do_sigbus:
 	printk("fault:Do sigbus\n");
Index: linux-2.6/arch/xtensa/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/xtensa/mm/fault.c
+++ linux-2.6/arch/xtensa/mm/fault.c
@@ -146,15 +146,10 @@ bad_area:
 	 */
 out_of_memory:
 	up_read(&mm->mmap_sem);
-	if (is_global_init(current)) {
-		yield();
-		down_read(&mm->mmap_sem);
-		goto survive;
-	}
-	printk("VM: killing process %s\n", current->comm);
-	if (user_mode(regs))
-		do_group_exit(SIGKILL);
-	bad_page_fault(regs, address, SIGKILL);
+	if (!user_mode(regs))
+		bad_page_fault(regs, address, SIGKILL);
+	else
+		pagefault_out_of_memory();
 	return;
 
 do_sigbus:

--
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-04-22 15:40 UTC|newest]

Thread overview: 116+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-24 16:25 [PATCH] oom killer: break from infinite loop Anfei Zhou
2010-03-25  2:51 ` KOSAKI Motohiro
2010-03-26 22:08 ` Andrew Morton
2010-03-26 22:33   ` Oleg Nesterov
2010-03-28 14:55     ` anfei
2010-03-28 16:28       ` Oleg Nesterov
2010-03-28 21:21         ` David Rientjes
2010-03-29 11:21           ` Oleg Nesterov
2010-03-29 20:49             ` [patch] oom: give current access to memory reserves if it has been killed David Rientjes
2010-03-30 15:46               ` Oleg Nesterov
2010-03-30 20:26                 ` David Rientjes
2010-03-31 17:58                   ` Oleg Nesterov
2010-03-31 20:47                     ` Oleg Nesterov
2010-04-01  8:35                       ` David Rientjes
2010-04-01  8:57                         ` [patch -mm] oom: hold tasklist_lock when dumping tasks David Rientjes
2010-04-01 14:27                           ` Oleg Nesterov
2010-04-01 19:16                             ` David Rientjes
2010-04-01 13:59                         ` [patch] oom: give current access to memory reserves if it has been killed Oleg Nesterov
2010-04-01 19:12                           ` David Rientjes
2010-04-02 11:14                             ` Oleg Nesterov
2010-04-02 18:30                               ` [PATCH -mm 0/4] oom: linux has threads Oleg Nesterov
2010-04-02 18:31                                 ` [PATCH -mm 1/4] oom: select_bad_process: check PF_KTHREAD instead of !mm to skip kthreads Oleg Nesterov
2010-04-02 19:05                                   ` David Rientjes
2010-04-02 18:32                                 ` [PATCH -mm 2/4] oom: select_bad_process: PF_EXITING check should take ->mm into account Oleg Nesterov
2010-04-06 11:42                                   ` anfei
2010-04-06 12:18                                     ` Oleg Nesterov
2010-04-06 13:05                                       ` anfei
2010-04-06 13:38                                         ` Oleg Nesterov
2010-04-02 18:32                                 ` [PATCH -mm 3/4] oom: introduce find_lock_task_mm() to fix !mm false positives Oleg Nesterov
2010-04-02 18:33                                 ` [PATCH -mm 4/4] oom: oom_forkbomb_penalty: move thread_group_cputime() out of task_lock() Oleg Nesterov
2010-04-02 19:04                                   ` David Rientjes
2010-04-05 14:23                                 ` [PATCH -mm] oom: select_bad_process: never choose tasks with badness == 0 Oleg Nesterov
2010-04-02 19:02                               ` [patch] oom: give current access to memory reserves if it has been killed David Rientjes
2010-04-02 19:14                                 ` Oleg Nesterov
2010-04-02 19:46                                   ` David Rientjes
2010-04-02 19:54                                     ` [patch -mm] oom: exclude tasks with badness score of 0 from being selected David Rientjes
2010-04-02 21:04                                       ` Oleg Nesterov
2010-04-02 21:22                                         ` [patch -mm v2] " David Rientjes
2010-04-02 20:55                                     ` [patch] oom: give current access to memory reserves if it has been killed Oleg Nesterov
2010-03-31 21:07                     ` David Rientjes
2010-03-31 22:50                       ` Oleg Nesterov
2010-03-31 23:30                         ` Oleg Nesterov
2010-03-31 23:48                           ` David Rientjes
2010-04-01 14:39                             ` Oleg Nesterov
2010-04-01 18:58                               ` David Rientjes
2010-04-01  8:25                         ` David Rientjes
2010-04-01 15:26                           ` Oleg Nesterov
2010-04-08 21:08                             ` David Rientjes
2010-04-09 12:38                               ` Oleg Nesterov
2010-03-30 16:39               ` [PATCH] oom: fix the unsafe proc_oom_score()->badness() call Oleg Nesterov
2010-03-30 17:43                 ` [PATCH -mm] proc: don't take ->siglock for /proc/pid/oom_adj Oleg Nesterov
2010-03-30 20:30                   ` David Rientjes
2010-03-31  9:17                     ` Oleg Nesterov
2010-03-31 18:59                     ` Oleg Nesterov
2010-03-31 21:14                       ` David Rientjes
2010-03-31 23:00                         ` Oleg Nesterov
2010-04-01  8:32                           ` David Rientjes
2010-04-01 15:37                             ` Oleg Nesterov
2010-04-01 19:04                               ` David Rientjes
2010-03-30 20:32                 ` [PATCH] oom: fix the unsafe proc_oom_score()->badness() call David Rientjes
2010-03-31  9:16                   ` Oleg Nesterov
2010-03-31 20:17                     ` Oleg Nesterov
2010-04-01  7:41                       ` David Rientjes
2010-04-01 13:13                         ` [PATCH 0/1] oom: fix the unsafe usage of badness() in proc_oom_score() Oleg Nesterov
2010-04-01 13:13                           ` [PATCH 1/1] " Oleg Nesterov
2010-04-01 19:03                             ` David Rientjes
2010-03-29 14:06           ` [PATCH] oom killer: break from infinite loop anfei
2010-03-29 20:01             ` David Rientjes
2010-03-30 14:29               ` anfei
2010-03-30 20:29                 ` David Rientjes
2010-03-31  0:57                   ` KAMEZAWA Hiroyuki
2010-03-31  6:07                     ` David Rientjes
2010-03-31  6:13                       ` KAMEZAWA Hiroyuki
2010-03-31  6:30                         ` Balbir Singh
2010-03-31  6:31                           ` KAMEZAWA Hiroyuki
2010-03-31  7:04                             ` David Rientjes
2010-03-31  6:32                           ` David Rientjes
2010-03-31  7:08                             ` [patch -mm] memcg: make oom killer a no-op when no killable task can be found David Rientjes
2010-03-31  7:08                               ` KAMEZAWA Hiroyuki
2010-03-31  8:04                               ` Balbir Singh
2010-03-31 10:38                                 ` David Rientjes
2010-04-04 23:28                               ` David Rientjes
2010-04-05 21:30                                 ` Andrew Morton
2010-04-05 22:40                                   ` David Rientjes
2010-04-05 22:49                                     ` Andrew Morton
2010-04-05 23:01                                       ` David Rientjes
2010-04-06 12:08                                         ` KOSAKI Motohiro
2010-04-06 21:47                                           ` David Rientjes
2010-04-07  0:20                                             ` KAMEZAWA Hiroyuki
2010-04-07 13:29                                               ` KOSAKI Motohiro
2010-04-08 18:05                                                 ` David Rientjes
2010-04-21 19:17                                                   ` Andrew Morton
2010-04-21 22:04                                                     ` David Rientjes
2010-04-22  0:23                                                       ` KAMEZAWA Hiroyuki
2010-04-22  8:34                                                         ` David Rientjes
2010-04-27 22:58                                                       ` [patch -mm] oom: reintroduce and deprecate oom_kill_allocating_task David Rientjes
2010-04-28  0:57                                                         ` KAMEZAWA Hiroyuki
2010-04-22  7:23                                                     ` [patch -mm] memcg: make oom killer a no-op when no killable task can be found Nick Piggin
2010-04-22  7:25                                                       ` KAMEZAWA Hiroyuki
2010-04-22 10:09                                                         ` Nick Piggin
2010-04-22 10:27                                                           ` KAMEZAWA Hiroyuki
2010-04-22 21:11                                                             ` David Rientjes
2010-04-22 10:28                                                           ` David Rientjes
2010-04-22 15:39                                                             ` Nick Piggin [this message]
2010-04-22 21:09                                                               ` David Rientjes
2010-05-04 23:55                                                     ` David Rientjes
2010-04-08 17:36                                               ` David Rientjes
2010-04-02 10:17           ` [PATCH] oom killer: break from infinite loop Mel Gorman
2010-04-04 23:26             ` David Rientjes
2010-04-05 10:47               ` Mel Gorman
2010-04-06 22:40                 ` David Rientjes
2010-03-29 11:31         ` anfei
2010-03-29 11:46           ` Oleg Nesterov
2010-03-29 12:09             ` anfei
2010-03-28  2:46 ` David Rientjes
  -- strict thread matches above, loose matches on Subject: below --
2010-05-04 23:51 [patch -mm] memcg: make oom killer a no-op when no killable task can be found 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=20100422153956.GY5683@laptop \
    --to=npiggin@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=anfei.zhou@gmail.com \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-mm@kvack.org \
    --cc=nishimura@mxp.nes.nec.co.jp \
    --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;
as well as URLs for NNTP newsgroup(s).