On Thu, 12 Apr 2012, Michal Hocko wrote:
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 46bf2ed5..a9df008 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -171,23 +171,10 @@ static bool oom_unkillable_task(struct task_struct *p,
> return false;
> }
>
> -/**
> - * oom_badness - heuristic function to determine which candidate task to kill
> - * @p: task struct of which task we should calculate
> - * @totalpages: total present RAM allowed for page allocation
> - *
> - * The heuristic for determining which task to kill is made to be as simple and
> - * predictable as possible. The goal is to return the highest value for the
> - * task consuming the most memory to avoid subsequent oom failures.
> - */
> -unsigned int oom_badness(struct task_struct *p, struct mem_cgroup *memcg,
> +/* can be used only for tasks which are killable as per oom_unkillable_task */
> +static unsigned int __oom_badness(struct task_struct *p, struct mem_cgroup *memcg,
> const nodemask_t *nodemask, unsigned long totalpages)
> {
> - long points;
> -
> - if (oom_unkillable_task(p, memcg, nodemask))
> - return 0;
> -
> p = find_lock_task_mm(p);
> if (!p)
> return 0;
> @@ -239,6 +226,26 @@ unsigned int oom_badness(struct task_struct *p, struct mem_cgroup *memcg,
> return (points < 1000) ? points : 1000;
> }
>
> +/**
> + * oom_badness - heuristic function to determine which candidate task to kill
> + * @p: task struct of which task we should calculate
> + * @totalpages: total present RAM allowed for page allocation
> + *
> + * The heuristic for determining which task to kill is made to be as simple and
> + * predictable as possible. The goal is to return the highest value for the
> + * task consuming the most memory to avoid subsequent oom failures.
> + */
> +unsigned int oom_badness(struct task_struct *p, struct mem_cgroup *memcg,
> + const nodemask_t *nodemask, unsigned long totalpages)
> +{
> + long points;
> +
> + if (oom_unkillable_task(p, memcg, nodemask))
> + return 0;
> +
> + return __oom_badness(p, memcg, nodemask, totalpages);
> +}
> +
> /*
> * Determine the type of allocation constraint.
> */
> @@ -366,7 +373,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> }
> }
>
> - points = oom_badness(p, memcg, nodemask, totalpages);
> + points = __oom_badness(p, memcg, nodemask, totalpages);
> if (points > *ppoints) {
> chosen = p;
> *ppoints = points;
No, the way I had it written is correct: the above unnecessarily checks
for membership in a memcg or intersection with a set of allowable nodes
for child threads in oom_kill_process(). With a lot of children and with
a CONFIG_NODES_SHIFT significantly large (the prerequisite for this patch
to make any difference), that's too costly to do.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>