linux-kernel - Re: FW: Commit 81a43adae3b9 (locking/mutex: Use acquire/release semantics) causing failures on arm64 (ThunderX)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151211084133.GE6356@twins.programming.kicks-ass.net>
Date:	Fri, 11 Dec 2015 09:41:33 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Andrew Pinski <andrew.pinski@...iumnetworks.com>
Cc:	Will Deacon <will.deacon@....com>,
	Davidlohr Bueso <dbueso@...e.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Ingo Molnar <mingo@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: FW: Commit 81a43adae3b9 (locking/mutex: Use acquire/release
 semantics) causing failures on arm64 (ThunderX)

On Thu, Dec 10, 2015 at 08:51:34PM -0800, Andrew Pinski wrote:

> So looking further I think I understand what is going wrong and why
> c55a6ffa6285e29f874ed403979472631ec70bff is incorrect.

The osq_wait_next() call in osq_lock() is when we fail the lock. This is
effectively trylock() semantics and like for cmpxchg a failed trylock
has no implied barrier semantics.  So from that POV osq_wait_next() does
not need to provide ACQUIRE semantics.

In osq_unlock() there's an xchg() in front, which implies full barriers
and thereby provides RELEASE semantics for that part of osq_unlock(), so
again, from this POV osq_wait_next() does not need to provide RELEASE
semantics.

> The compare and swap inside osq_lock needs to be both release and
> acquire semantics memory barriers because the stores (to node) need to
> be visible to the other cores before the setting of lock->tail
> happens.

I'm a wee bit confused on what exactly you mean. Both stores to @node:

 1) osq_wait_next(): next = xchg(&node->next, NULL)
 2) osq_unlock():    next = xchg(&node->next, NULL)

are xchg() calls which imply full ordering (sequential consistency).

Similarly the store before osq_wait_next() in osq_lock(), namely:

  cmpxchg(&prev->node, node, NULL)

is fully ordered.

So I cannot see any store being delayed past the
atomic_cmpxchg_acquire().

Now you mention 'compare and swap inside osq_lock' which I take to be
the latter; and it _is_ fully ordered.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/