2011-03-04 21:31:25

by Andrei Vagin

Subject: [PATCH rh6] mm: skip zombie in OOM-killer

A parent may wait a memory and zombie will prevent killing another task.

Signed-off-by: Andrey Vagin <[email protected]>
---
mm/oom_kill.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 7dcca55..2fc554e 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -311,7 +311,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
* blocked waiting for another task which itself is waiting
* for memory. Is there a better alternative?
*/
- if (test_tsk_thread_flag(p, TIF_MEMDIE))
+ if (test_tsk_thread_flag(p, TIF_MEMDIE) && p->mm)
return ERR_PTR(-1UL);

/*
--
1.7.1

2011-03-04 23:41:51

by David Rientjes

Subject: Re: [PATCH rh6] mm: skip zombie in OOM-killer

On Sat, 5 Mar 2011, Andrey Vagin wrote:

> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 7dcca55..2fc554e 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -311,7 +311,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> * blocked waiting for another task which itself is waiting
> * for memory. Is there a better alternative?
> */
> - if (test_tsk_thread_flag(p, TIF_MEMDIE))
> + if (test_tsk_thread_flag(p, TIF_MEMDIE) && p->mm)
> return ERR_PTR(-1UL);
>
> /*

I think it would be better to just do

if (!p->mm)
continue;

after the check for oom_unkillable_task() because everything that follows
this really depends on p->mm being non-NULL to actually do anything
useful.

2011-03-05 00:52:13

by Andrei Vagin

Subject: Re: [PATCH rh6] mm: skip zombie in OOM-killer

On 03/05/2011 02:41 AM, David Rientjes wrote:
> On Sat, 5 Mar 2011, Andrey Vagin wrote:
>
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 7dcca55..2fc554e 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -311,7 +311,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
>> * blocked waiting for another task which itself is waiting
>> * for memory. Is there a better alternative?
>> */
>> - if (test_tsk_thread_flag(p, TIF_MEMDIE))
>> + if (test_tsk_thread_flag(p, TIF_MEMDIE)&& p->mm)
>> return ERR_PTR(-1UL);
>>
>> /*
>
> I think it would be better to just do
>
> if (!p->mm)
> continue;
>
> after the check for oom_unkillable_task() because everything that follows
> this really depends on p->mm being non-NULL to actually do anything
> useful.
Yes. You are right. Thank you.

2011-03-06 11:20:52

by KOSAKI Motohiro

Subject: Re: [PATCH rh6] mm: skip zombie in OOM-killer

> On Sat, 5 Mar 2011, Andrey Vagin wrote:
>
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 7dcca55..2fc554e 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -311,7 +311,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > * blocked waiting for another task which itself is waiting
> > * for memory. Is there a better alternative?
> > */
> > - if (test_tsk_thread_flag(p, TIF_MEMDIE))
> > + if (test_tsk_thread_flag(p, TIF_MEMDIE) && p->mm)
> > return ERR_PTR(-1UL);
> >
> > /*
>
> I think it would be better to just do
>
> if (!p->mm)
> continue;
>
> after the check for oom_unkillable_task() because everything that follows
> this really depends on p->mm being non-NULL to actually do anything
> useful.

I'm glad you join to review MM patches. It is worth effort for making
solid kernel. But, please look at a current code at first.

2011-03-08 01:25:40

by KOSAKI Motohiro

Subject: Re: [PATCH rh6] mm: skip zombie in OOM-killer

> A parent may wait a memory and zombie will prevent killing another task.
>
> Signed-off-by: Andrey Vagin <[email protected]>
> ---
> mm/oom_kill.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 7dcca55..2fc554e 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -311,7 +311,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> * blocked waiting for another task which itself is waiting
> * for memory. Is there a better alternative?
> */
> - if (test_tsk_thread_flag(p, TIF_MEMDIE))
> + if (test_tsk_thread_flag(p, TIF_MEMDIE) && p->mm)
> return ERR_PTR(-1UL);

OK. Good catch.

Reviewed-by: KOSAKI Motohiro <[email protected]>