On Fri, Dec 11, 2015 at 12:18:00PM +0000, Will Deacon wrote: > On Fri, Dec 11, 2015 at 01:13:19PM +0100, Peter Zijlstra wrote: > > On Fri, Dec 11, 2015 at 12:04:19PM +0000, Will Deacon wrote: > > > I think Andrew meant the atomic_xchg_acquire at the start of osq_lock, > > > as opposed to "compare and swap". In which case, it does look like > > > there's a bug here because there is nothing to order the initialisation > > > of the node fields with publishing of the node, whether that's > > > indirectly as a result of setting the tail to the current CPU or > > > directly as a result of the WRITE_ONCE. > > > > Agreed, this does indeed look like a bug. If confirmed please write a > > shiny changelog and I'll queue asap. > > Yup. I've failed to reproduce the issue locally, so we'll need to wait > for Andrew and/or David to get back to us first.
While we're there, the acquire in osq_wait_next() seems somewhat ill documented too. I _think_ we need ACQUIRE semantics there because we want to strictly order the lock-unqueue A,B,C steps and we get that with: A: SC B: ACQ C: Relaxed Similarly for unlock we want the WRITE_ONCE to happen after osq_wait_next, but in that case we can even rely on the control dependency there. As noted in a previous email, the ACQUIRE for osq_wait_next() does not come from its use in lock since its on the fail path, and trylock failure doesn't imply any barriers. Not should it have RELEASE semantics for its use in unlock, since we already have that covered by the xchg() done prior. -- 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/
