diff options
author | Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> | 2009-12-15 19:47:12 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2009-12-16 10:20:07 -0500 |
commit | d31f56dbf8bafaacb0c617f9a6f137498d5c7aed (patch) | |
tree | 88d095c2208d27362e58ff7431407040ead9d848 /mm | |
parent | 57f9fd7d25ac9a0d7e3a4ced580e780ab4524e3b (diff) |
memcg: avoid oom-killing innocent task in case of use_hierarchy
task_in_mem_cgroup(), which is called by select_bad_process() to check
whether a task can be a candidate for being oom-killed from memcg's limit,
checks "curr->use_hierarchy"("curr" is the mem_cgroup the task belongs
to).
But this check return true(it's false positive) when:
<some path>/aa use_hierarchy == 0 <- hitting limit
<some path>/aa/00 use_hierarchy == 1 <- the task belongs to
This leads to killing an innocent task in aa/00. This patch is a fix for
this bug. And this patch also fixes the arg for
mem_cgroup_print_oom_info(). We should print information of mem_cgroup
which the task being killed, not current, belongs to.
Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'mm')
-rw-r--r-- | mm/memcontrol.c | 10 | ||||
-rw-r--r-- | mm/oom_kill.c | 13 |
2 files changed, 15 insertions, 8 deletions
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 6273984f2e34..a294b7576070 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c | |||
@@ -760,7 +760,13 @@ int task_in_mem_cgroup(struct task_struct *task, const struct mem_cgroup *mem) | |||
760 | task_unlock(task); | 760 | task_unlock(task); |
761 | if (!curr) | 761 | if (!curr) |
762 | return 0; | 762 | return 0; |
763 | if (curr->use_hierarchy) | 763 | /* |
764 | * We should check use_hierarchy of "mem" not "curr". Because checking | ||
765 | * use_hierarchy of "curr" here make this function true if hierarchy is | ||
766 | * enabled in "curr" and "curr" is a child of "mem" in *cgroup* | ||
767 | * hierarchy(even if use_hierarchy is disabled in "mem"). | ||
768 | */ | ||
769 | if (mem->use_hierarchy) | ||
764 | ret = css_is_ancestor(&curr->css, &mem->css); | 770 | ret = css_is_ancestor(&curr->css, &mem->css); |
765 | else | 771 | else |
766 | ret = (curr == mem); | 772 | ret = (curr == mem); |
@@ -1009,7 +1015,7 @@ void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p) | |||
1009 | static char memcg_name[PATH_MAX]; | 1015 | static char memcg_name[PATH_MAX]; |
1010 | int ret; | 1016 | int ret; |
1011 | 1017 | ||
1012 | if (!memcg) | 1018 | if (!memcg || !p) |
1013 | return; | 1019 | return; |
1014 | 1020 | ||
1015 | 1021 | ||
diff --git a/mm/oom_kill.c b/mm/oom_kill.c index 25c679e0288a..f52481b1c1e5 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c | |||
@@ -356,7 +356,8 @@ static void dump_tasks(const struct mem_cgroup *mem) | |||
356 | } while_each_thread(g, p); | 356 | } while_each_thread(g, p); |
357 | } | 357 | } |
358 | 358 | ||
359 | static void dump_header(gfp_t gfp_mask, int order, struct mem_cgroup *mem) | 359 | static void dump_header(struct task_struct *p, gfp_t gfp_mask, int order, |
360 | struct mem_cgroup *mem) | ||
360 | { | 361 | { |
361 | pr_warning("%s invoked oom-killer: gfp_mask=0x%x, order=%d, " | 362 | pr_warning("%s invoked oom-killer: gfp_mask=0x%x, order=%d, " |
362 | "oom_adj=%d\n", | 363 | "oom_adj=%d\n", |
@@ -365,7 +366,7 @@ static void dump_header(gfp_t gfp_mask, int order, struct mem_cgroup *mem) | |||
365 | cpuset_print_task_mems_allowed(current); | 366 | cpuset_print_task_mems_allowed(current); |
366 | task_unlock(current); | 367 | task_unlock(current); |
367 | dump_stack(); | 368 | dump_stack(); |
368 | mem_cgroup_print_oom_info(mem, current); | 369 | mem_cgroup_print_oom_info(mem, p); |
369 | show_mem(); | 370 | show_mem(); |
370 | if (sysctl_oom_dump_tasks) | 371 | if (sysctl_oom_dump_tasks) |
371 | dump_tasks(mem); | 372 | dump_tasks(mem); |
@@ -440,7 +441,7 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order, | |||
440 | struct task_struct *c; | 441 | struct task_struct *c; |
441 | 442 | ||
442 | if (printk_ratelimit()) | 443 | if (printk_ratelimit()) |
443 | dump_header(gfp_mask, order, mem); | 444 | dump_header(p, gfp_mask, order, mem); |
444 | 445 | ||
445 | /* | 446 | /* |
446 | * If the task is already exiting, don't alarm the sysadmin or kill | 447 | * If the task is already exiting, don't alarm the sysadmin or kill |
@@ -576,7 +577,7 @@ retry: | |||
576 | /* Found nothing?!?! Either we hang forever, or we panic. */ | 577 | /* Found nothing?!?! Either we hang forever, or we panic. */ |
577 | if (!p) { | 578 | if (!p) { |
578 | read_unlock(&tasklist_lock); | 579 | read_unlock(&tasklist_lock); |
579 | dump_header(gfp_mask, order, NULL); | 580 | dump_header(NULL, gfp_mask, order, NULL); |
580 | panic("Out of memory and no killable processes...\n"); | 581 | panic("Out of memory and no killable processes...\n"); |
581 | } | 582 | } |
582 | 583 | ||
@@ -644,7 +645,7 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, | |||
644 | return; | 645 | return; |
645 | 646 | ||
646 | if (sysctl_panic_on_oom == 2) { | 647 | if (sysctl_panic_on_oom == 2) { |
647 | dump_header(gfp_mask, order, NULL); | 648 | dump_header(NULL, gfp_mask, order, NULL); |
648 | panic("out of memory. Compulsory panic_on_oom is selected.\n"); | 649 | panic("out of memory. Compulsory panic_on_oom is selected.\n"); |
649 | } | 650 | } |
650 | 651 | ||
@@ -663,7 +664,7 @@ void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, | |||
663 | 664 | ||
664 | case CONSTRAINT_NONE: | 665 | case CONSTRAINT_NONE: |
665 | if (sysctl_panic_on_oom) { | 666 | if (sysctl_panic_on_oom) { |
666 | dump_header(gfp_mask, order, NULL); | 667 | dump_header(NULL, gfp_mask, order, NULL); |
667 | panic("out of memory. panic_on_oom is selected\n"); | 668 | panic("out of memory. panic_on_oom is selected\n"); |
668 | } | 669 | } |
669 | /* Fall-through */ | 670 | /* Fall-through */ |