Re: [RFC,PATCH] Use RCU to protect tasklist for unicast signals

[Oleg Nesterov <oleg@tv-sign.ru>]

Paul E. McKenney wrote:
>
> I have indeed been thinking along these lines, but all of the devil plans
> that I have come up thus far are quite intrusive, and might have some
> performance problems in some situations.  So it seems best to remove
> tasklist_lock in steps:
>
> 1.	Single-recipient catch and ignore cases.
>
> 2.	Single-recipient stop/continue cases.
>
> 3.	Single-recipient fatal cases.
>
> 4.	Single-process multi-threaded stop/continue cases.
>
> 5.	Single-process multi-threaded fatal cases.
>
> 6.	And on to process-group cases.

Paul, I am not an expert at all, but honestly I don't see how
this could be achieved. This lock is heavily overloaded for
quite different purposes. I think that may be it makes sense
to try other steps, for example (random order):

  1. Tasklist protects ->sighand changing (de_thread) - rework
     sighand access/locking.

  2. Tasklist protects reparenting - fix this.

  .......

  N. PTRACE!!! Well, I close my eyes immediately when I see this
     word in the sources.

Only then we can eliminate tasklist locking from signal sending
path. But I don't see the easy way to solve any of these 1 - N
problems.

Currently I don't see how your patch could be "fixed" for SIGCONT
case, except very ugly:

	kill_proc_info(sig)
	{
		p = find_task_by_pid(pid);
		if (sig == SIGCONT)
			read_lock(&tasklist_lock);

		error = group_send_sig_info(...);
		...
	}

But there are other problems too.

Look at __group_complete_signal(), it scans p->pids[PIDTYPE_TGID].pid_list
list to find a a suitable thread. What if 'p' does clone(CLONE_THREAD) now?
Let's look at copy_process(), it does attach_pid(p, PIDTYPE_TGID, p->tgid)
under the lovely tasklist_lock again.

So, I don't beleive we can solve even the simplest case (single-recipient,
non fatal, non stop/cont) without significant locking rework.

I hope that your patch will stimulate this work.

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