On Wed, 2016-09-07 at 10:17 +0530, Nikunj A Dadhania wrote: > > David Gibson <da...@gibson.dropbear.id.au> writes: > > > > > [ Unknown signature status ] > > On Fri, Sep 02, 2016 at 12:02:55PM +0530, Nikunj A Dadhania wrote: > > > > > > > > > Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com> > > > > This really needs a comment indicating that this implementation isn't > > strictly correct (although probably good enough in practice). > > Sure. And it also does not help if someone uses any store other than > store conditional, that isn't taken care. > > Assumption here is the locking primitives use load with reservation and > store conditional. And no other ld/st variant touch this memory.
This is an incorrect assumption. spin_unlock for example uses a normal store. That being said, you will observe the difference in value which should hopefully make things work... I *hope* we don't have anything that relies on a normal store of the same value as the atomic breaking the reservation, I *think* we might get away with it, but it is indeed fishy. > > Specifically a racing store which happens to store the same value > > which was already in memory should clobber the reservation, but won't > > with this implementation. > > > > I had a long discussion at KVM Forum with Emilio Costa about this, in > > which I discovered just how hard it is to strictly implement > > store-conditional semantics in terms of anything else. So, this is > > probably a reasonable substitute, but we should note the fact that > > it's not 100%. > > I will update the commit log. > > Regards, > Nikunj