On Fri 06-03-15 15:09:29, Balasubramani Vivekanandan wrote:
> If panic_on_oom=2, then there was no cgroup information printed,
> if system panics due of cgroup memory limit reached. Information is
> only printed when configured to kill task. This patch prints
> information of the cgroup whose limit triggerred panic
Yes the patch makes sense but the changelog is missing information why
this is a problem.
We will see that the OOM happened because of "compulsory panic_on_oom
is enabled" but this doesn't tell the difference between mempolicy and
memcg. And dumping system wide information is plain wrong and more
confusing.
Please mention something like that in the changelog so that the
motivation is clear.
> Signed-off-by: Balasubramani Vivekanandan <balasubramani_vivekanandan@xxxxxxxxxx>
minor fixes/suggestion below, other than that it looks OK to me.
Acked-by: Michal Hocko <mhocko@xxxxxxx>
> ---
> include/linux/oom.h | 3 ++-
> mm/memcontrol.c | 21 ++++++++++++---------
> mm/oom_kill.c | 7 ++++---
> 3 files changed, 18 insertions(+), 13 deletions(-)
>
[...]
> @@ -1442,17 +1442,20 @@ void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p)
> struct mem_cgroup *iter;
> unsigned int i;
>
> - if (!p)
> - return;
> -
> mutex_lock(&oom_info_lock);
> rcu_read_lock();
>
> - pr_info("Task in ");
> - pr_cont_cgroup_path(task_cgroup(p, memory_cgrp_id));
> - pr_cont(" killed as a result of limit of ");
> - pr_cont_cgroup_path(memcg->css.cgroup);
> - pr_cont("\n");
> + if (p) {
> + pr_info("Task in ");
> + pr_cont_cgroup_path(task_cgroup(p, memory_cgrp_id));
> + pr_cont(" killed as a result of limit of ");
> + pr_cont_cgroup_path(memcg->css.cgroup);
> + pr_cont("\n");
> + } else {
> + pr_info("Memory limit reached of cgroup ");
s@of@for@
> + pr_cont_cgroup_path(memcg->css.cgroup);
> + pr_cont("\n");
> + }
and there is no reason to duplicate pr_cont_cgroup_path and the followup
pr_cont becayse both branches are doing the same.
>
> rcu_read_unlock();
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
