Naveen,
sorry for the terse and neglectful reply yesterday.
Actually, when I re-read the Linus's email, I think he already explained
everything, so let me repeat:
On 12/21, Linus Torvalds wrote:
>
> Did you actually *see* the problem, or was this just from looking at the code?
Yes. Because this code assumes that lock_task_sighand() must not fail.
If it fails, we have a problem which should be fixed.
> We have coredump serialization in exit_mm() that I think *should* make
> this all ok - if we still see p->mm matching our mm, I don't think it
> should be able to get to __exit_signal() and make the sighand go away,
> so the lock_task_sighand() shouldn't ever fail.
Yes, exactly.
Note that if we ignore exec, we do not need lock_task_sighand() at all,
we could simply do spin_lock_irq(p->sighand->siglock).
The caller holds mm->mmap_sem for writing, if we see p->mm == mm it
simply can not pass exit_mm() which does down_read(&mm->mmap_sem), so
this task can not exit.
The problem is, this task can change its ->sighand in de_thread(), that
is why we need lock_task_sighand(). But if it does exec, it can't pass
exec_mmap() by the same reason, we hold mmap_sem.
> > if (p->mm) {
> > if (unlikely(p->mm == mm)) {
> > - lock_task_sighand(p, &flags);
> > - nr += zap_process(p, exit_code);
> > - unlock_task_sighand(p, &flags);
> > + if (lock_task_sighand(p, &flags) {
> > + nr += zap_process(p,
> > exit_code);
But we can't silently skip a process with the same ->mm. We can't even
skip the execing thread task if it is going to change its ->mm, even if
it is single-threaded. Note that exec_mmap() will notice mm->core_state
and fail. So every task with the same mm should be accounted because it
will play with core_state->nr_threads in exit_mm(). And it should be
killed because otherwise coredump_wait() can sleep "forever".
So this is not the right change in any case. If lock_task_sighand() can
fail we should fix something else.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
