handle_stop_signal() does not need tasklist_lock for
SIG_KERNEL_STOP_MASK signals anymore.
Signed-off-by: Oleg Nesterov <[email protected]>
--- 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) && \
Oleg Nesterov <[email protected]> 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
Eric W. Biederman wrote:
>
> Oleg Nesterov <[email protected]> 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.