Messages in this thread | | | Date | Sun, 06 Nov 2005 15:01:42 +0300 | From | Oleg Nesterov <> | Subject | Re: [PATCH] Fixes for RCU handling of task_struct |
| |
"Paul E. McKenney" wrote: > > So the idea is to error out of send_sigqueue() so that posix_timer_event() > will instead call send_group_sigqueeue(). But that could suffer from > the same race if the new leader thread also exits -- or if the exiting > thread was the leader thread to begin with.
The case when leader exits is ok. If it is the only (last) thread - it will call exit_itimers(). If not - it (or sys_wait4 from parent) will not call release_task(), but will stay TASK_ZOMBIE with valid ->signal/sighand until the last thread in same thread group exits (and call exit_itimers).
> But once send_group_sigqueue() read-acquires tasklist_lock, threads > and processes must stay put. So it should be possible to follow the > ->group_leader chain at that point.
Not quite so, I think. See below.
> Except that the group leader could do an exec(), right? If it does so, > it must do so before tasklist_lock is read-acquired. So the nightmare > case is where all but one thread exits, and then that one thread does > and exec().
... and that thread is not group leader. Actually, it does not matter if other threads exited or not, execing thread will kill other threads.
> If this case can really happen, we want to drop the signal > on the floor, right?
I think yes.
> diff -urpNa -X dontdiff linux-2.6.14-mm0-fix/kernel/signal.c linux-2.6.14-mm0-fix-2/kernel/signal.c > --- linux-2.6.14-mm0-fix/kernel/signal.c 2005-11-04 17:23:40.000000000 -0800 > +++ linux-2.6.14-mm0-fix-2/kernel/signal.c 2005-11-05 15:05:38.000000000 -0800 > @@ -1408,6 +1408,11 @@ send_sigqueue(int sig, struct sigqueue * > > retry: > sh = rcu_dereference(p->sighand); > + if (sh == NULL) { > + /* We raced with pthread_exit()... */ > + ret = -1; > + goto out_err; > + }
I lost the plot. Because I can't apply this and previous patches (rejects) and can't imagine how send_sigqueue() looks now. I think this is ok, but we also need to re-check ->signal != NULL after lock(->sighand) or check PF_EXITING (iirc ve do have such check).
> @@ -1474,7 +1479,8 @@ send_group_sigqueue(int sig, struct sigq > BUG_ON(!(q->flags & SIGQUEUE_PREALLOC)); > > read_lock(&tasklist_lock); > - /* Since it_lock is held, p->sighand cannot be NULL. */ > + while (p->group_leader != p) > + p = p->group_leader;
No, this is definitely not right. de_thread() does not change leader->group_leader when non-leader execs, so p->group_leader == p always.
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/
| |