Re: [PATCH] signals: do_tkill: don't use tasklist_lock

From: Roland McGrath
Date: Fri Mar 07 2008 - 14:32:57 EST


> Convert do_tkill() to use rcu_read_lock() + lock_task_sighand() to avoid
> taking tasklist lock.

That part looks good.

> Note that we don't return an error if lock_task_sighand() fails, we
> pretend the task dies after receiving the signal. Otherwise, we should
> fight with the nasty races with mt-exec without having any advantage.

To clarify, this is not a change from the existing behavior.
So your change is fine regardless of this issue.

The case you have in mind is that p was the old group_leader
being replaced by another thread that exec'd, right?

It is the most obscure of nits, but I think it can be wrong to drop a
signal in this case. If it's a fatal signal (especially SIGKILL),
then either the thread group should be killed or the call should
return an error.

For the exec case, if p->sighand is cleared that means the
release_task(leader) call at the end of de_thread started. So by
now, the pid has been transferred to the exec'ing thread. If we just
restart the lookup, it will find the new thread (or not, and we can
return -ESRCH).

I'm inclined to do that, but it certainly should be a second patch
after this one.


Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/