Messages in this thread | | | Date | Thu, 16 Feb 2006 11:19:32 -0800 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH 1/2] fix kill_proc_info() vs CLONE_THREAD race |
| |
On Wed, Feb 15, 2006 at 10:13:24PM +0300, Oleg Nesterov wrote: > There is a window after copy_process() unlocks ->sighand.siglock > and before it adds the new thread to the thread list. > > In that window __group_complete_signal(SIGKILL) will not see the > new thread yet, so this thread will start running while the whole > thread group was supposed to exit.
The fix looks good to me!
> I beleive we have another good reason to place attach_pid(PID/TGID) > under ->sighand.siglock. We can do the same for > > release_task()->__unhash_process() > > de_thread()->switch_exec_pids() > > After that we don't need tasklist_lock to iterate over the thread > list, and we can simplify things, see for example do_sigaction() > or sys_times().
The above proposal would require that we hold siglock during the traversal, correct? Is that reasonable for non-signal-related traversals? Or were you thinking of making this change only for signal code?
Thanx, Paul
Acked-by: <paulmck@us.ibm.com> > Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru> > > --- 2.6.16-rc3/kernel/fork.c~1_KILL 2006-02-15 22:52:07.000000000 +0300 > +++ 2.6.16-rc3/kernel/fork.c 2006-02-15 23:21:51.000000000 +0300 > @@ -1123,8 +1123,8 @@ static task_t *copy_process(unsigned lon > p->real_parent = current; > p->parent = p->real_parent; > > + spin_lock(¤t->sighand->siglock); > if (clone_flags & CLONE_THREAD) { > - spin_lock(¤t->sighand->siglock); > /* > * Important: if an exit-all has been started then > * do not create this new thread - the whole thread > @@ -1162,8 +1162,6 @@ static task_t *copy_process(unsigned lon > */ > p->it_prof_expires = jiffies_to_cputime(1); > } > - > - spin_unlock(¤t->sighand->siglock); > } > > /* > @@ -1189,6 +1187,7 @@ static task_t *copy_process(unsigned lon > > nr_threads++; > total_forks++; > + spin_unlock(¤t->sighand->siglock); > write_unlock_irq(&tasklist_lock); > proc_fork_connector(p); > return p; > - 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/
| |