[PATCH] de_thread: eliminate unneccessary sighand locking

[Oleg Nesterov]

while switching current->sighand de_thread does:

	write_lock_irq(&tasklist_lock);
	spin_lock(&oldsighand->siglock);
	spin_lock(&newsighand->siglock);

	current->sighand = newsighand;
	recalc_sigpending();

Is these 2 sighand locks are really needed?

At this moment we already zapped other threads, so nobody
can access newsighand via current->. And we are holding
tasklist_lock, so other processes can't send signals to us
or use our ->sighand in any way.

oldsighand can be seen from CLONE_SIGHAND processes, but
we are not using oldsighand in any way, so this lock seems
to be unneeded too.

The only possibility that I can imagine is that some process
does:
	read_lock(tasklist_lock);
	task = find_task();
	spin_lock(task->sighand->siglock);
	read_unlock(tasklist_lock);
	play with task->signal

Is this possible/allowed?

And why do we need recalc_sigpending() ? We are not changing
->pending or ->blocked, just ->sighand.

Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>

--- 2.6.12/fs/exec.c~	2005-05-09 16:37:16.000000000 +0400
+++ 2.6.12/fs/exec.c	2005-06-20 00:03:24.000000000 +0400
@@ -758,14 +758,7 @@ no_thread_group:
 		       sizeof(newsighand->action));
 
 		write_lock_irq(&tasklist_lock);
-		spin_lock(&oldsighand->siglock);
-		spin_lock(&newsighand->siglock);
-
 		current->sighand = newsighand;
-		recalc_sigpending();
-
-		spin_unlock(&newsighand->siglock);
-		spin_unlock(&oldsighand->siglock);
 		write_unlock_irq(&tasklist_lock);
 
 		if (atomic_dec_and_test(&oldsighand->count))
-
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/