lkml.org 
[lkml]   [2005]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-11-06 11:52    [W:0.073 / U:1.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site