Re: [Qemu-devel] [PATCH RFC 3/4] target-ppc: use atomic_cmpxchg for ld/st reservation

[David Gibson]

On Wed, Sep 07, 2016 at 08:13:31AM +0100, Alex Bennée wrote:
> 
> David Gibson <address@hidden> writes:
> 
> > On Wed, Sep 07, 2016 at 10:17:42AM +0530, Nikunj A Dadhania wrote:
> >> David Gibson <address@hidden> writes:
> >>
> >> > [ Unknown signature status ]
> >> > On Fri, Sep 02, 2016 at 12:02:55PM +0530, Nikunj A Dadhania wrote:
> >> >> Signed-off-by: Nikunj A Dadhania <address@hidden>
> >> >
> >> > 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.
> >
> > So, a) I don't think this really relies on that: an ordinary store
> > (assuming it changes the value) will still get picked up the cmpxchg.
> > Well.. at least after a suitable memory barrier.  Matching memory
> > models between emulated and host cpus is a whole other kettle of fish.
> 
> Have you seen Pranith's memory barrier patches?

I have not.

> 
> >
> > I think this does matter, IIRC a kernel spin unlock on ppc is a
> > barrier + plain store, no load locked or store conditional.
> >
> >> > 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
> >>
> 
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature