On Sat, 23 Nov 2013, Davidlohr Bueso wrote: > On Sat, 2013-11-23 at 19:46 -0800, Linus Torvalds wrote: > > On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner <t...@linutronix.de> wrote: > > > > > > Now the question is why we queue the waiter _AFTER_ reading the user > > > space value. The comment in the code is pretty non sensical: > > > > > > * On the other hand, we insert q and release the hash-bucket only > > > * after testing *uaddr. This guarantees that futex_wait() will NOT > > > * absorb a wakeup if *uaddr does not match the desired values > > > * while the syscall executes. > > > > > > There is no reason why we cannot queue _BEFORE_ reading the user space > > > value. We just have to dequeue in all the error handling cases, but > > > for the fast path it does not matter at all. > > > > > > CPU 0 CPU 1 > > > > > > val = *futex; > > > futex_wait(futex, val); > > > > > > spin_lock(&hb->lock); > > > > > > plist_add(hb, self); > > > smp_wmb(); > > > > > > uval = *futex; > > > *futex = newval; > > > futex_wake(); > > > > > > smp_rmb(); > > > if (plist_empty(hb)) > > > return; > > > ... > > > > This would seem to be a nicer approach indeed, without needing the > > extra atomics. > > Yep, I think we can all agree that doing this optization without atomic > ops is a big plus. > > > > > Davidlohr, mind trying Thomas' approach? > > I just took a quick look and it seems pretty straightforward, but not > without some details to consider. We basically have to redo/reorder > futex_wait_setup(), which checks that uval == val, and > futex_wait_queue_me(), which adds the task to the list and blocks. Now, > both futex_wait() and futex_wait_requeue_pi() have this logic, but since > we don't use futex_wake() to wakeup tasks on pi futex_qs, I believe it's > ok to only change futex_wait(), while the order of the uval checking > doesn't matter for futex_wait_requeue_pi() so it can stay as is.
There is no mechanism which prevents a futex_wake() call on the inner futex of the wait_requeue_pi mechanism. So no, we have to change both. futexes are no place for believe. Either you understand them completely or you just leave them alone. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/