* [patch -mm] mm, oom: prefer thread group leaders for display purposes
@ 2014-01-16 2:40 David Rientjes
2014-01-16 7:05 ` Johannes Weiner
0 siblings, 1 reply; 7+ messages in thread
From: David Rientjes @ 2014-01-16 2:40 UTC (permalink / raw)
To: Andrew Morton
Cc: Johannes Weiner, Michal Hocko, KAMEZAWA Hiroyuki, cgroups,
linux-mm, linux-kernel
When two threads have the same badness score, it's preferable to kill the
thread group leader so that the actual process name is printed to the
kernel log rather than the thread group name which may be shared amongst
several processes.
This was the behavior when select_bad_process() used to do
for_each_process(), but it now iterates threads instead and leads to
ambiguity.
Signed-off-by: David Rientjes <rientjes@google.com>
---
mm/memcontrol.c | 18 +++++++++++-------
mm/oom_kill.c | 12 ++++++++----
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a815686..b482f49 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1841,13 +1841,17 @@ static void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
break;
};
points = oom_badness(task, memcg, NULL, totalpages);
- if (points > chosen_points) {
- if (chosen)
- put_task_struct(chosen);
- chosen = task;
- chosen_points = points;
- get_task_struct(chosen);
- }
+ if (points < chosen_points)
+ continue;
+ /* Prefer thread group leaders for display purposes */
+ if (points == chosen_points &&
+ thread_group_leader(chosen))
+ continue;
+
+ if (chosen)
+ put_task_struct(chosen);
+ chosen = task;
+ chosen_points = points;
}
css_task_iter_end(&it);
}
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 054ff47..1dca3d8 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -327,10 +327,14 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
break;
};
points = oom_badness(p, NULL, nodemask, totalpages);
- if (points > chosen_points) {
- chosen = p;
- chosen_points = points;
- }
+ if (points < chosen_points)
+ continue;
+ /* Prefer thread group leaders for display purposes */
+ if (points == chosen_points && thread_group_leader(chosen))
+ continue;
+
+ chosen = p;
+ chosen_points = points;
}
if (chosen)
get_task_struct(chosen);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [patch -mm] mm, oom: prefer thread group leaders for display purposes
2014-01-16 2:40 [patch -mm] mm, oom: prefer thread group leaders for display purposes David Rientjes
@ 2014-01-16 7:05 ` Johannes Weiner
2014-01-16 7:45 ` David Rientjes
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Weiner @ 2014-01-16 7:05 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Michal Hocko, KAMEZAWA Hiroyuki, cgroups, linux-mm,
linux-kernel
On Wed, Jan 15, 2014 at 06:40:41PM -0800, David Rientjes wrote:
> When two threads have the same badness score, it's preferable to kill the
> thread group leader so that the actual process name is printed to the
> kernel log rather than the thread group name which may be shared amongst
> several processes.
>
> This was the behavior when select_bad_process() used to do
> for_each_process(), but it now iterates threads instead and leads to
> ambiguity.
>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
> mm/memcontrol.c | 18 +++++++++++-------
> mm/oom_kill.c | 12 ++++++++----
> 2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a815686..b482f49 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1841,13 +1841,17 @@ static void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
> break;
> };
> points = oom_badness(task, memcg, NULL, totalpages);
> - if (points > chosen_points) {
> - if (chosen)
> - put_task_struct(chosen);
> - chosen = task;
> - chosen_points = points;
> - get_task_struct(chosen);
Where did that GET go?
> - }
> + if (points < chosen_points)
> + continue;
> + /* Prefer thread group leaders for display purposes */
> + if (points == chosen_points &&
> + thread_group_leader(chosen))
> + continue;
> +
> + if (chosen)
> + put_task_struct(chosen);
...boom.
> + chosen = task;
> + chosen_points = points;
> }
> css_task_iter_end(&it);
> }
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 054ff47..1dca3d8 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -327,10 +327,14 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> break;
> };
> points = oom_badness(p, NULL, nodemask, totalpages);
> - if (points > chosen_points) {
> - chosen = p;
> - chosen_points = points;
> - }
> + if (points < chosen_points)
> + continue;
> + /* Prefer thread group leaders for display purposes */
> + if (points == chosen_points && thread_group_leader(chosen))
> + continue;
> +
> + chosen = p;
> + chosen_points = points;
> }
> if (chosen)
> get_task_struct(chosen);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [patch -mm] mm, oom: prefer thread group leaders for display purposes
2014-01-16 7:05 ` Johannes Weiner
@ 2014-01-16 7:45 ` David Rientjes
2014-01-16 7:46 ` [patch v2 " David Rientjes
0 siblings, 1 reply; 7+ messages in thread
From: David Rientjes @ 2014-01-16 7:45 UTC (permalink / raw)
To: Johannes Weiner
Cc: Andrew Morton, Michal Hocko, KAMEZAWA Hiroyuki, cgroups, linux-mm,
linux-kernel
On Thu, 16 Jan 2014, Johannes Weiner wrote:
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index a815686..b482f49 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -1841,13 +1841,17 @@ static void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
> > break;
> > };
> > points = oom_badness(task, memcg, NULL, totalpages);
> > - if (points > chosen_points) {
> > - if (chosen)
> > - put_task_struct(chosen);
> > - chosen = task;
> > - chosen_points = points;
> > - get_task_struct(chosen);
>
> Where did that GET go?
>
No idea, good catch! This patch was doomed from the GET-go.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 7+ messages in thread* [patch v2 -mm] mm, oom: prefer thread group leaders for display purposes
2014-01-16 7:45 ` David Rientjes
@ 2014-01-16 7:46 ` David Rientjes
[not found] ` <alpine.DEB.2.02.1401152345330.14407-X6Q0R45D7oAcqpCFd4KODRPsWskHk0ljAL8bYrjMMd8@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: David Rientjes @ 2014-01-16 7:46 UTC (permalink / raw)
To: Andrew Morton
Cc: Johannes Weiner, Michal Hocko, KAMEZAWA Hiroyuki, cgroups,
linux-mm, linux-kernel
When two threads have the same badness score, it's preferable to kill the
thread group leader so that the actual process name is printed to the
kernel log rather than the thread group name which may be shared amongst
several processes.
This was the behavior when select_bad_process() used to do
for_each_process(), but it now iterates threads instead and leads to
ambiguity.
Signed-off-by: David Rientjes <rientjes@google.com>
---
v2: fixes missing get_task_struct() found by Johannes.
mm/memcontrol.c | 19 ++++++++++++-------
mm/oom_kill.c | 12 ++++++++----
2 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a815686..d69c4b3 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1841,13 +1841,18 @@ static void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
break;
};
points = oom_badness(task, memcg, NULL, totalpages);
- if (points > chosen_points) {
- if (chosen)
- put_task_struct(chosen);
- chosen = task;
- chosen_points = points;
- get_task_struct(chosen);
- }
+ if (points < chosen_points)
+ continue;
+ /* Prefer thread group leaders for display purposes */
+ if (points == chosen_points &&
+ thread_group_leader(chosen))
+ continue;
+
+ if (chosen)
+ put_task_struct(chosen);
+ chosen = task;
+ chosen_points = points;
+ get_task_struct(chosen);
}
css_task_iter_end(&it);
}
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 054ff47..1dca3d8 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -327,10 +327,14 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
break;
};
points = oom_badness(p, NULL, nodemask, totalpages);
- if (points > chosen_points) {
- chosen = p;
- chosen_points = points;
- }
+ if (points < chosen_points)
+ continue;
+ /* Prefer thread group leaders for display purposes */
+ if (points == chosen_points && thread_group_leader(chosen))
+ continue;
+
+ chosen = p;
+ chosen_points = points;
}
if (chosen)
get_task_struct(chosen);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-01-17 9:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-16 2:40 [patch -mm] mm, oom: prefer thread group leaders for display purposes David Rientjes
2014-01-16 7:05 ` Johannes Weiner
2014-01-16 7:45 ` David Rientjes
2014-01-16 7:46 ` [patch v2 " David Rientjes
[not found] ` <alpine.DEB.2.02.1401152345330.14407-X6Q0R45D7oAcqpCFd4KODRPsWskHk0ljAL8bYrjMMd8@public.gmane.org>
2014-01-16 14:21 ` Michal Hocko
2014-01-16 22:49 ` David Rientjes
2014-01-17 9:17 ` Michal Hocko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox