Re: [patch 06/18] oom: avoid sending exiting tasks a SIGKILL

[Oleg Nesterov <oleg@redhat.com>]

On 06/09, David Rientjes wrote:
>
> On Wed, 9 Jun 2010, Oleg Nesterov wrote:
>
> > David, currently I do not know how the code looks with all patches
> > applied, could you please confirm there is no problem here? I am
> > looking at Linus's tree,
> >
> > 	mem_cgroup_out_of_memory:
> >
> > 		 p = select_bad_process();
> > 		 oom_kill_process(p);
> >
>
> mem_cgroup_out_of_memory() does this under tasklist_lock:
>
> retry:
> 	p = select_bad_process(&points, mem, CONSTRAINT_MEMCG, NULL);
> 	if (!p || PTR_ERR(p) == -1UL)
> 		goto out;
>
> 	if (oom_kill_process(p, gfp_mask, 0, points, mem,
> 				"Memory cgroup out of memory"))
> 		goto retry;
> out:
> 	...
>
> > Now, again, select_bad_process() can return the dead group-leader
> > of the memory-hog-thread-group.
> >
>
> select_bad_process() already has:
>
> 	if ((p->flags & PF_EXITING) && p->mm) {
> 		if (p != current)
> 			return ERR_PTR(-1UL);
>
> 		chosen = p;
> 		*ppoints = ULONG_MAX;
> 	}
>
> so we can disregard the check for p == current

Not sure I understand... We can just ignore this check, in this case
p->mm == NULL.

> in this case since it would
> not be allocating memory without p->mm.

This thread will not allocate the memory, yes. But its sub-threads can.
And select_bad_process() can constantly return the same (dead) thread P,
badness() inspects ->mm under find_lock_task_mm() which finds the thread
with the valid ->mm.

OK. Probably this doesn't matter. I don't know if task_in_mem_cgroup(task)
was fixed or not, but currently it also looks at task->mm and thus have
the same boring problem: it is trivial to make the memory-hog process
invisible to oom. Unless I missed something, of course.

Oleg.

--
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>