Re: [PATCH] Remove OOM killer from try_to_free_pages / all_unreclaimable braindamage

[Jesse Barnes <jbarnes@sgi.com>]

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

On Friday, November 05, 2004 12:01 pm, Marcelo Tosatti wrote:
> Hi,
>
> As you know the OOM is very problematic in 2.6 right now - so I went
> to investigate it.
>
> Currently the oom killer is invoked from the task reclaim
> code (try_to_free_pages), which IMO is fundamentally broken,
> because its non deterministic - the chance the OOM killer
> will be triggered increases as the number of tasks inside
> reclaiming increases. And kswapd is freeing pages in parallel,
> which is completly ignored by this approach.
>
> In my opinion the correct approach is to trigger the OOM killer
> when kswapd is unable to free pages. Once that is done, the number
> of tasks inside page reclaim is irrelevant.

That makes sense.

> With this in place I can't see spurious OOM kills - just need to guarantee
> that it reliably OOM kills when we are really out of memory.

That's good.  I can test it on a large machine (hopefully next week).

> Comments?

Sounds good, though we may want to do a couple of more things, we shouldn't 
kill root tasks quite as easily and we should avoid zombies since they may be 
large apps in the process of exiting, and killing them would be bad (iirc 
it'll cause a panic).

Thanks,
Jesse

[-- Attachment #2: oom-fixes.patch --]
[-- Type: text/x-diff, Size: 623 bytes --]

===== mm/oom_kill.c 1.30 vs edited =====
--- 1.30/mm/oom_kill.c	2004-10-13 21:08:29 -07:00
+++ edited/mm/oom_kill.c	2004-11-05 15:32:36 -08:00
@@ -88,7 +88,7 @@
 	 */
 	if (cap_t(p->cap_effective) & CAP_TO_MASK(CAP_SYS_ADMIN) ||
 				p->uid == 0 || p->euid == 0)
-		points /= 4;
+		points /= 10;
 
 	/*
 	 * We don't want to kill a process with direct hardware access.
@@ -120,7 +120,7 @@
 
 	do_posix_clock_monotonic_gettime(&uptime);
 	do_each_thread(g, p)
-		if (p->pid) {
+		if (p->pid && !(p->state & TASK_ZOMBIE)) {
 			unsigned long points = badness(p, uptime.tv_sec);
 			if (points > maxpoints) {
 				chosen = p;