[PATCH] sysctl_panic_on_oom broken

[PATCH] sysctl_panic_on_oom broken

[Larry Woodman]

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

out_of_memory() does not panic when sysctl_panic_on_oom is set
if constrained_alloc() does not return CONSTRAINT_NONE.  Instead,
out_of_memory() kills the current process whenever constrained_alloc()
returns either CONSTRAINT_MEMORY_POLICY or CONSTRAINT_CPUSET.
This patch fixes this problem:

Signed-off-by: Larry Woodman <lwoodman@redhat.com>




[-- Attachment #2: panic_on_oom.patch --]
[-- Type: text/plain, Size: 724 bytes --]

--- linux-2.6.18.noarch/mm/oom_kill.c.orig
+++ linux-2.6.18.noarch/mm/oom_kill.c
@@ -431,6 +437,9 @@ void out_of_memory(struct zonelist *zone
 	cpuset_lock();
 	read_lock(&tasklist_lock);
 
+	/* check if we are going to panic before enything else... */
+	if (sysctl_panic_on_oom)
+		panic("out of memory. panic_on_oom is selected\n");
 	/*
 	 * Check if there were limitations on the allocation (only relevant for
 	 * NUMA) that may require different handling.
@@ -447,8 +456,6 @@ void out_of_memory(struct zonelist *zone
 		break;
 
 	case CONSTRAINT_NONE:
-		if (sysctl_panic_on_oom)
-			panic("out of memory. panic_on_oom is selected\n");
 retry:
 		/*
 		 * Rambo mode: Shoot down a process and hope it solves whatever

Re: [PATCH] sysctl_panic_on_oom broken

[Christoph Lameter]

On Tue, 17 Apr 2007, Larry Woodman wrote:

> out_of_memory() does not panic when sysctl_panic_on_oom is set
> if constrained_alloc() does not return CONSTRAINT_NONE.  Instead,
> out_of_memory() kills the current process whenever constrained_alloc()
> returns either CONSTRAINT_MEMORY_POLICY or CONSTRAINT_CPUSET.
> This patch fixes this problem:

It recreates the old problem that we OOM while we still have memory 
in other parts of the system.

Re: [PATCH] sysctl_panic_on_oom broken

[Larry Woodman]

On Tue, 2007-04-17 at 14:39 -0700, Christoph Lameter wrote:

> 
> It recreates the old problem that we OOM while we still have memory 
> in other parts of the system.

How, by the time we get here we have already decided we are going to
OOMkill or panic.  This change just obeys sysctl_panic_on_oom before
killing current.

-----------------------------------------------------------------------
        /* check if we are going to panic before enything else... */
        if (sysctl_panic_on_oom)
                panic("out of memory. panic_on_oom is selected\n");
        /*
         * Check if there were limitations on the allocation
         * NUMA) that may require different handling.
         */
        switch (constrained_alloc(zonelist, gfp_mask)) {
        case CONSTRAINT_MEMORY_POLICY:
                oom_kill_process(current, points,
                                "No available memory (MPOL_BIND)");
                break;

        case CONSTRAINT_CPUSET:
                oom_kill_process(current, points,
                                "No available memory in cpuset");
                break;

        case CONSTRAINT_NONE:
retry:
                /*
                 * Rambo mode: Shoot down a process and hope
                 * issues we may have.
                 */
                p = select_bad_process(&points);

                if (PTR_ERR(p) == -1UL)
                        goto out;

                /* Found nothing?!?! Either we hang or we panic. */

Re: [PATCH] sysctl_panic_on_oom broken

[Christoph Lameter]

On Tue, 17 Apr 2007, Larry Woodman wrote:

> On Tue, 2007-04-17 at 14:39 -0700, Christoph Lameter wrote:
> 
> > 
> > It recreates the old problem that we OOM while we still have memory 
> > in other parts of the system.
> 
> How, by the time we get here we have already decided we are going to
> OOMkill or panic.  This change just obeys sysctl_panic_on_oom before
> killing current.

If you panic then there is no reason to kill current. Lets say you
have a 10 node system and the application attempt to allocate from node 8 
which has no reclaimable memory then the application has a problem not the 
system as a whole.

Re: [PATCH] sysctl_panic_on_oom broken

[Yasunori Goto]

> On Tue, 17 Apr 2007, Larry Woodman wrote:
> 
> > out_of_memory() does not panic when sysctl_panic_on_oom is set
> > if constrained_alloc() does not return CONSTRAINT_NONE.  Instead,
> > out_of_memory() kills the current process whenever constrained_alloc()
> > returns either CONSTRAINT_MEMORY_POLICY or CONSTRAINT_CPUSET.
> > This patch fixes this problem:
> 
> It recreates the old problem that we OOM while we still have memory 
> in other parts of the system.

Hmm. User's expectation is failover of clustering ASAP by panic.
Even if free memory remain due to cpuset/mempolicy setting,
some people may want failover soon.

Of course some other people don't want panic if free memory remain.
I think it depends on user.

If panic_on_oom is 1, only panic if mempolicy/cpuset is not used.
And if panic_on_oom is 2, panic on all case.
This might be desirable.

Bye.


-- 
Yasunori Goto 

Re: [PATCH] sysctl_panic_on_oom broken

[Christoph Lameter]

On Wed, 18 Apr 2007, Yasunori Goto wrote:

> If panic_on_oom is 1, only panic if mempolicy/cpuset is not used.
> And if panic_on_oom is 2, panic on all case.
> This might be desirable.

Sounds good. Add some documentation mentioned that this may panic your 
system if there is still plenty of memory available.