Messages in this thread Patch in this message | | | Date | Mon, 15 Dec 2003 00:08:27 +0100 (CET) | From | Ingo Molnar <> | Subject | Re: [patch] Re: Problem with exiting threads under NPTL |
| |
On Sun, 14 Dec 2003, Linus Torvalds wrote:
> Note that both of these micro-optimizations are equally likely to > _hurt_.
agreed. Patch without these changes reverted is attached. (this is pretty close to your last patch.)
just one nit wrt. the first micro-optimization:
> So setting variables earlier tends to actually pessimize code [...]
in this specific case the critical section only sets the variable, and the lifetime of the variable necessiates a stackslot anyway, on x86. So moving the initialization outside the lock shouldnt pessimise the generated code, in fact it could even free up a register, due to the compiler being able to do:
movl $1, 0x12(%esp)
due to the clear register spill, instead of using up a register due to the initialization being too close to the setting to 1.
But i agree, due to worse readability alone it is not worth having this change. And on non-x86 it could even result in a spilled register.
(the nonlinear code at the end of the function was clearly a bad idea.)
Ingo
--- kernel/exit.c.orig +++ kernel/exit.c @@ -50,8 +50,10 @@ static void __unhash_process(struct task void release_task(struct task_struct * p) { task_t *leader; + int zap_leader; struct dentry *proc_dentry; - + +repeat: BUG_ON(p->state < TASK_ZOMBIE); atomic_dec(&p->user->processes); @@ -70,10 +72,22 @@ void release_task(struct task_struct * p * group, and the leader is zombie, then notify the * group leader's parent process. (if it wants notification.) */ + zap_leader = 0; leader = p->group_leader; - if (leader != p && thread_group_empty(leader) && - leader->state == TASK_ZOMBIE && leader->exit_signal != -1) + if (leader != p && thread_group_empty(leader) && leader->state == TASK_ZOMBIE) { + BUG_ON(leader->exit_signal == -1); do_notify_parent(leader, leader->exit_signal); + /* + * If we were the last child thread and the leader has + * exited already, and the leader's parent ignores SIGCHLD, + * then we are the one who should release the leader. + * + * do_notify_parent() will have marked it self-reaping in + * that case. + */ + if (leader->exit_signal == -1) + zap_leader = 1; + } p->parent->cutime += p->utime + p->cutime; p->parent->cstime += p->stime + p->cstime; @@ -88,6 +102,15 @@ void release_task(struct task_struct * p proc_pid_flush(proc_dentry); release_thread(p); put_task_struct(p); + + /* + * Do this outside the tasklist lock. The reference to the + * leader is safe. There's no recursion possible, since + * the leader of the leader is itself: + */ + p = leader; + if (zap_leader) + goto repeat; } /* we are using it only for SMP init */ - 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/
| |