[PATCH] oom_kill() race?

[PATCH] oom_kill() race?

[Andres Salomon]

[-- Attachment #1: Type: text/plain, Size: 469 bytes --]

In oom_kill(), is there any chance the task_struct can be unmapped
between being returned from select_bad_process() (where the tasklist
is locked) and where it walks the tasklist again, looking for threads?

If so, the following patch (against 2.4.17) will clean that up; if not,
ignore this.


-- 
"I think a lot of the basis of the open source movement comes from
  procrastinating students..."
	-- Andrew Tridgell <http://www.linux-mag.com/2001-07/tridgell_04.html>

[-- Attachment #2: oom_fix.diff --]
[-- Type: text/plain, Size: 1312 bytes --]

diff -urN linux.pristine/mm/oom_kill.c linux/mm/oom_kill.c
--- linux.pristine/mm/oom_kill.c	Mon Jan 14 05:51:13 2002
+++ linux/mm/oom_kill.c	Mon Jan 14 05:49:07 2002
@@ -110,8 +110,7 @@
 
 /*
  * Simple selection loop. We chose the process with the highest
- * number of 'points'. We need the locks to make sure that the
- * list of task structs doesn't change while we look the other way.
+ * number of 'points'. We expect the caller will lock the tasklist.
  *
  * (not docbooked, we don't want this one cluttering up the manual)
  */
@@ -121,7 +120,6 @@
 	struct task_struct *p = NULL;
 	struct task_struct *chosen = NULL;
 
-	read_lock(&tasklist_lock);
 	for_each_task(p) {
 		if (p->pid) {
			int points = badness(p);
@@ -131,7 +129,6 @@
 			}
 		}
 	}
-	read_unlock(&tasklist_lock);
 	return chosen;
 }
 
@@ -170,14 +167,16 @@
  */
 static void oom_kill(void)
 {
-	struct task_struct *p = select_bad_process(), *q;
+	struct task_struct *p, *q;
+	
+	read_lock(&tasklist_lock);
+	p = select_bad_process();
 
 	/* Found nothing?!?! Either we hang forever, or we panic. */
 	if (p == NULL)
 		panic("Out of memory and no killable processes...\n");
 
 	/* kill all processes that share the ->mm (i.e. all threads) */
-	read_lock(&tasklist_lock);
 	for_each_task(q) {
 		if(q->mm == p->mm) oom_kill_task(q);
 	}

Re: [PATCH] oom_kill() race?

[Rik van Riel]

On Mon, 14 Jan 2002, Andres Salomon wrote:

> In oom_kill(), is there any chance the task_struct can be unmapped
> between being returned from select_bad_process() (where the tasklist
> is locked) and where it walks the tasklist again, looking for threads?

Indeed you're right.  Thanks for the patch!

regards,

Rik
-- 
"Linux holds advantages over the single-vendor commercial OS"
    -- Microsoft's "Competing with Linux" document

http://www.surriel.com/		http://distro.conectiva.com/

Re: [PATCH] oom_kill() race?

[Gianni Tedesco]

On Mon, 2002-01-14 at 06:34, Rik van Riel wrote:
> Rik
> -- 
> "Linux holds advantages over the single-vendor commercial OS"
>     -- Microsoft's "Competing with Linux" document
> 
> http://www.surriel.com/		http://distro.conectiva.com/

Heh,

Ive got a copy of that document here because my company is a MS partner.
Ironically enough it manages to hang MS word, abiword to the rescue.

I love the whole "Windows is good for the user because it increases your
profit margin" angle.

Fun reading.

-- 
// Gianni Tedesco <gianni@ecsc.co.uk>
80% of all email is a figment of procmails imagination.