Messages in this thread | | | Date | Mon, 6 Nov 2006 07:31:25 -0500 (EST) | From | Steven Rostedt <> | Subject | Re: PATCH? hrtimer_wakeup: fix a theoretical race wrt rt_mutex_slowlock() |
| |
On Sun, 5 Nov 2006, Oleg Nesterov wrote:
> When task->array != NULL, try_to_wake_up() just goes to "out_running" and sets > task->state = TASK_RUNNING. > > In that case hrtimer_wakeup() does: > > timeout->task = NULL; <----- [1] > > spin_lock(runqueues->lock); > > task->state = TASK_RUNNING; <----- [2] > > from Documentation/memory-barriers.txt > > Memory operations that occur before a LOCK operation may appear to > happen after it completes. > > This means that [2] may be completed before [1], and > > CPU_0 CPU_1 > rt_mutex_slowlock: > > for (;;) { > ... > if (timeout && !timeout->task) > return -ETIMEDOUT; > ... > > schedule(); > hrtimer_wakeup() sets > ... task->state = TASK_RUNNING, > but "timeout->task = NULL" > is not completed > set_current_state(TASK_INTERRUPTIBLE); > } > > we can miss a timeout. > > Of course, this all is scholasticism, this can't happen in practice, but > may be this patch makes sense as a documentation update. > > Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru> > > --- STATS/kernel/hrtimer.c~1_hrtw 2006-10-22 18:24:03.000000000 +0400 > +++ STATS/kernel/hrtimer.c 2006-11-05 22:32:36.000000000 +0300 > @@ -662,9 +662,12 @@ static int hrtimer_wakeup(struct hrtimer > container_of(timer, struct hrtimer_sleeper, timer); > struct task_struct *task = t->task; > > - t->task = NULL; > - if (task) > + if (task) { > + t->task = NULL; > + /* must be visible before task->state = TASK_RUNNING */ > + smp_wmb(); > wake_up_process(task); > + } > > return HRTIMER_NORESTART; > } > >
Ingo, Thomas, could you ack this too, or Nack it with an explaination.
Ingo, you might want to read the whole thread here.
Acked-by: Steven Rostedt <rostedt@goodmis.org>
-- Steve
- 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/
| |