From: Minchan Kim on
On Fri, 2010-01-22 at 15:23 +0900, KAMEZAWA Hiroyuki wrote:
> updated. thank you for review.
>
> The patch is onto mmotm-Jan15 (depends on mm-count-lowmem-rss.patch)
> Tested on x86-64/SMP + debug module(to allocated lowmem), works well.
>
> ==
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
>
> Default oom-killer uses badness calculation based on process's vm_size
> and some amounts of heuristics. Some users see proc->oom_score and
> proc->oom_adj to control oom-killed tendency under their server.
>
> Now, we know oom-killer don't work ideally in some situaion, in PCs. Some
> enhancements are demanded. But such enhancements for oom-killer makes
> incomaptibility to oom-controls in enterprise world. So, this patch
> adds sysctl for extensions for oom-killer. Main purpose is for
> making a chance for wider test for new scheme.
>
> One cause of OOM-Killer is memory shortage in lower zones.
> (If memory is enough, lowmem_reserve_ratio works well. but..)
> I saw lowmem-oom frequently on x86-32 and sometimes on ia64 in
> my cusotmer support jobs. If we just see process's vm_size at oom,
> we can never kill a process which has lowmem.
> At last, there will be an oom-serial-killer.
>
> Now, we have per-mm lowmem usage counter. We can make use of it
> to select a good victim.
>
> This patch does
> - add sysctl for new bahavior.
> - add CONSTRAINT_LOWMEM to oom's constraint type.
> - pass constraint to __badness()
> - change calculation based on constraint. If CONSTRAINT_LOWMEM,
> use low_rss instead of vmsize.
>
> Changelog 2010/01/22:
> - added sysctl
> - fixed !CONFIG_MMU
> - fixed fs/proc/base.c breakacge.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> ---
> Documentation/sysctl/vm.txt | 16 ++++++++
> fs/proc/base.c | 5 +-
> include/linux/oom.h | 1
> kernel/sysctl.c | 10 ++++-
> mm/oom_kill.c | 87 ++++++++++++++++++++++++++++++++------------
> 5 files changed, 94 insertions(+), 25 deletions(-)
>
> Index: mmotm-2.6.33-Jan15/include/linux/oom.h
> ===================================================================
> --- mmotm-2.6.33-Jan15.orig/include/linux/oom.h
> +++ mmotm-2.6.33-Jan15/include/linux/oom.h
> @@ -20,6 +20,7 @@ struct notifier_block;
> */
> enum oom_constraint {
> CONSTRAINT_NONE,
> + CONSTRAINT_LOWMEM,
> CONSTRAINT_CPUSET,
> CONSTRAINT_MEMORY_POLICY,
> };

<snip>
> @@ -475,7 +511,7 @@ void mem_cgroup_out_of_memory(struct mem
>
> read_lock(&tasklist_lock);
> retry:
> - p = select_bad_process(&points, mem);
> + p = select_bad_process(&points, mem, CONSTRAINT_NONE);

Why do you fix this with only CONSTRAINT_NONE?
I think we can know CONSTRAINT_LOWMEM with gfp_mask in here.

Any problem?

Otherwise, Looks good to me. :)

Reviewed-by: Minchan Kim <minchan.kim(a)gmail.com>

--
Kind regards,
Minchan Kim

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: KAMEZAWA Hiroyuki on
Minchan Kim wrote:
> On Fri, 2010-01-22 at 15:23 +0900, KAMEZAWA Hiroyuki wrote:
>> updated. thank you for review.

>> CONSTRAINT_MEMORY_POLICY,
>> };
>
> <snip>
>> @@ -475,7 +511,7 @@ void mem_cgroup_out_of_memory(struct mem
>>
>> read_lock(&tasklist_lock);
>> retry:
>> - p = select_bad_process(&points, mem);
>> + p = select_bad_process(&points, mem, CONSTRAINT_NONE);
>
> Why do you fix this with only CONSTRAINT_NONE?
> I think we can know CONSTRAINT_LOWMEM with gfp_mask in here.
>
memcg is just for accounting anon/file pages. Then, it's never
cause lowmem oom problem (any memory is ok for memcg).



> Any problem?
>
> Otherwise, Looks good to me. :)
>
> Reviewed-by: Minchan Kim <minchan.kim(a)gmail.com>
>
Thank you.

-Kame

> --
> Kind regards,
> Minchan Kim
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Minchan Kim on
2010/1/23 KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>:
>> <snip>
>>> @@ -475,7 +511,7 @@ void mem_cgroup_out_of_memory(struct mem
>>>
>>>      read_lock(&tasklist_lock);
>>>  retry:
>>> -    p = select_bad_process(&points, mem);
>>> +    p = select_bad_process(&points, mem, CONSTRAINT_NONE);
>>
>> Why do you fix this with only CONSTRAINT_NONE?
>> I think we can know CONSTRAINT_LOWMEM with gfp_mask in here.
>>
> memcg is just for accounting anon/file pages. Then, it's never
> cause lowmem oom problem (any memory is ok for memcg).

Okay. Thanks for the explanation.
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on
On Mon, 25 Jan 2010 15:15:03 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com> wrote:

> -unsigned long badness(struct task_struct *p, unsigned long uptime)
> +unsigned long badness(struct task_struct *p, unsigned long uptime,
> + int constraint)

And badness() should be renamed to something better (eg, oom_badness), and
the declaration should be placed in a mm-specific header file.

Yes, the code was already like that. But please don't leave crappiness in
place when you come across it - take the opportunity to fix it up.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: KAMEZAWA Hiroyuki on
On Tue, 26 Jan 2010 15:16:06 -0800
Andrew Morton <akpm(a)linux-foundation.org> wrote:

> On Mon, 25 Jan 2010 15:15:03 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com> wrote:
>
> > -unsigned long badness(struct task_struct *p, unsigned long uptime)
> > +unsigned long badness(struct task_struct *p, unsigned long uptime,
> > + int constraint)
>
> And badness() should be renamed to something better (eg, oom_badness), and
> the declaration should be placed in a mm-specific header file.
>
> Yes, the code was already like that. But please don't leave crappiness in
> place when you come across it - take the opportunity to fix it up.
>
Sure. I'll write v4.

-Kame

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/