Skip to contentSkip to conversation

oom_kill() race?

PATCH In review Andres Salomon 3 participants 3 messages Jan 14, 2002

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 oom_kill() race?

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.

Signature · 3 lines
"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>
mm/oom_kill.c +1 -3
diff -urN linux.pristine/mm/oom_kill.c linux/mm/oom_kill.c
@@ -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);
}

#1 lore raw
Quoted 3 lines · Andres Salomon

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

Signature · 4 lines
"Linux holds advantages over the single-vendor commercial OS"
    -- Microsoft's "Competing with Linux" document

http://www.surriel.com/		http://distro.conectiva.com/
#2 lore raw
Quoted 6 lines · Rik van Riel

Rik

Signature · 4 lines
"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.

Signature · 2 lines
// Gianni Tedesco <[email protected]>
80% of all email is a figment of procmails imagination.