Messages in this thread | | | Date | Tue, 2 Sep 2014 19:39:10 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 1/1] do_exit(): Solve possibility of BUG() due to race with try_to_wake_up() |
| |
On Tue, Sep 02, 2014 at 06:47:14PM +0200, Oleg Nesterov wrote:
> But since I already wrote v2 yesterday, let me show it anyway. Perhaps > you will notice something wrong immediately... > > So, once again, this patch adds the ugly "goto" into schedule(). OTOH, > it removes the ugly spin_unlock_wait(pi_lock).
But schedule() is called _far_ more often than exit(). It would be really good not to have to do that.
> TASK_DEAD can die. The only valid user is schedule_debug(), trivial to > change. The usage of TASK_DEAD in task_numa_fault() is wrong in any case. > > In fact, I think that the next change can change exit_schedule() to use > PREEMPT_ACTIVE, and then we can simply remove the TASK_DEAD check in > schedule_debug().
So you worry about concurrent wakeups vs setting TASK_DEAD and thereby loosing it, right?
Would not something like:
spin_lock_irq(¤t->pi_lock); __set_current_state(TASK_DEAD); spin_unlock_irq(¤t->pi_lock);
Not be race free and similarly expensive to the smp_mb() we have there now?
> - BUG(); > - /* Avoid "noreturn function does return". */ > - for (;;) > - cpu_relax(); /* For when BUG is null */
> +void exit_schedule(void) > +{ > + current->state = TASK_DEAD; /* TODO: kill TASK_DEAD altogether */ > + task_rq(current)->prev_dead = true; > + __schedule(); > + BUG();
you lost that for loop.
| |