Re: Fw: Re: + add-locking-to-evdev.patch added to -mm tree

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

On 04/07, Paul E. McKenney wrote:
>
> The __kill_fasync() function in turn calls send_sigio(), which
> read-acquires both the fown_struct lock and tasklist_lock, then does
> send_sigio_to_task() for each thread in the task.
> 
> The send_sigio_to_task() function invokes group_send_sig_info(), which
> calls lock_task_sighand(), which expects one of its callers to have
> done an rcu_read_lock().  But I believe that read-holding tasklist_lock
> also suffices.  Oleg, could you please either confirm or educate?  @@@
> 
> (I think this is OK, just been awhile since I dug through the signal
> code.)

I also think this is OK.

The comment above lock_task_sighand() is not very clear. Note that it is
_not_ safe in general to do

	read_lock(tasklist_lock);
	lock_task_sighand(tsk);

even if we know that tsk can't go away (say, get_task_struct()).

However, it is safe to do:

	read_lock(tasklist_lock);
	tsk = find_task_in_pids_database();
	lock_task_sighand(tsk);

"find_task_in_pids_database" means something like find_task_by_pid_type() or
do_each_pid_task() (our case). Because read_lock(tasklist_lock) protects us
from release_task()->__exit_signal() which clears ->sighand _and_ removes tsk
from kernel/pid.c:pid_hash[] "atomically" under write_lock_irq(tasklist_lock).

rcu_read_lock() is OK for both cases. It would be nice to convert send_sigio()
to use RCU, but we also need tasklist_lock for

	- SIGCONT, see sig_needs_tasklist()

	- group wide signals (PIDTYPE_PGID), see the comment in copy_process()
	  above the recalc_sigpending() call.

Oleg.