[PATCH 2/6] relax sig_needs_tasklist()

[PATCH 2/6] relax sig_needs_tasklist()

[Oleg Nesterov]

handle_stop_signal() does not need tasklist_lock for
SIG_KERNEL_STOP_MASK signals anymore.

Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>

--- 2.6.16-rc3/kernel/signal.c~2_RELAX	2006-02-23 00:36:49.000000000 +0300
+++ 2.6.16-rc3/kernel/signal.c	2006-02-23 01:22:45.000000000 +0300
@@ -146,8 +146,7 @@ static kmem_cache_t *sigqueue_cachep;
 #define sig_kernel_stop(sig) \
 		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_STOP_MASK))
 
-#define sig_needs_tasklist(sig) \
-		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_STOP_MASK | M(SIGCONT)))
+#define sig_needs_tasklist(sig)	((sig) == SIGCONT)
 
 #define sig_user_defined(t, signr) \
 	(((t)->sighand->action[(signr)-1].sa.sa_handler != SIG_DFL) &&	\

Re: [PATCH 2/6] relax sig_needs_tasklist()

[Eric W. Biederman]

Oleg Nesterov <oleg@tv-sign.ru> writes:

> handle_stop_signal() does not need tasklist_lock for
> SIG_KERNEL_STOP_MASK signals anymore.

Small question.

If I read the code correctly the only thing handle_stop_signal needs
the tasklist_lock for is to protect task->parent, for the
do_notify_parent_cldstop(...) case.

If this is correct.  I think I see a path to kill read_lock(&tasklist_lock)
completely. 

- Protect task->parent with the rcu_read_lock && task_lock().
- Use the rcu forms of list_add/list_del on the tasklist.
- replace read_lock(&tasklist_lock) with rcu_read_lock().
- Make tasklist_lock a simple spin lock.

Comments?

Eric

Re: [PATCH 2/6] relax sig_needs_tasklist()

[Oleg Nesterov]

Eric W. Biederman wrote:
>
> Oleg Nesterov <oleg@tv-sign.ru> writes:
>
> > handle_stop_signal() does not need tasklist_lock for
> > SIG_KERNEL_STOP_MASK signals anymore.
>
> Small question.
>
> If I read the code correctly the only thing handle_stop_signal needs
> the tasklist_lock for is to protect task->parent, for the
> do_notify_parent_cldstop(...) case.

Yes, exactly.

> If this is correct.  I think I see a path to kill read_lock(&tasklist_lock)
> completely.
>
> - Protect task->parent with the rcu_read_lock && task_lock().
> - Use the rcu forms of list_add/list_del on the tasklist.
> - replace read_lock(&tasklist_lock) with rcu_read_lock().
> - Make tasklist_lock a simple spin lock.
>
> Comments?

I must admit, I am not brave enough to even think about this
now :)

I already thought about protecting ->parent with task_lock(),
but I can't find a reasonable solution.

As for handle_stop_signal(), there is another problem.
do_notify_parent_cldstop takes ->parent's sighand->siglock, so
the caller drops child's. And this is possible only because we
are holding tasklist_lock.

Somehow we need to lock both the parent and the child, and what
if child does ptrace on it's ->real_parent?

Oleg.