On Thu, 2 Feb 2017, Mateusz Guzik wrote:
On Wed, Feb 01, 2017 at 10:12:57PM +1100, Bruce Evans wrote:
On Mon, 30 Jan 2017, Bruce Evans wrote:
On Mon, 30 Jan 2017, Mateusz Guzik wrote:
Log:
i386: add atomic_fcmpset
Tested by: pho
This is has some bugs and style bugs.
This is still broken. The invalid asm breaks building at least atomic.o
with gcc-4.2.1.
Tested fix:
...
Uh, I ended up with the same fix. Committed in r313080.
Thanks.
Sorry for breakage in the first place.
The semantics of fcmpset seem to be undocumented. On x86, *expect is
updated non-atomically by a store in the output parameter. I think
cmpxchg updates the "a" register atomically, but then the output
parameter causes this to be stored non-atomically to *expect. A better
API would somehow return the "a" register and let the caller store it
if it wants. Ordinary cmpset can be built on this by not storing, and
the caller can do the store atomically to a different place if *expect
is too volatile to be atomic.
The primitive was modeled after atomic_compare_exchange_* from c11
atomics. I don't see what's the benefit of storing the result
separately.
As it is, the primitive fits nicely into loops like "inc not zero".
Like this:
r = *counter;
for (;;) {
if (r == 0)
break;
if (atomic_fcmpset_int(counter, &r, r + 1))
break;
// r we can loop back to re-test r
}
You won't want to ifdef this for SMP, so the i386 implementation has
further bugs like I expected (fcmpset is not implemented in the
CPU_DISABLE_CMPXCHG case).
Maybe just decouple the input parameter from the output parameter. The
following works right (for an amd64 API):
Y static __inline int
Y atomic_xfcmpset_long(volatile u_long *dst, u_long *expect_out, u_long
expect_in,
Y u_long src)
The output parameter is not well named in this or in fcmpset, since
when the comparison fails it holds the compared copy of *dst, not of
*expect_in, and otherwise it is not useful and holds a copy of src.
If the caller doesn't want to use *expect_out, it passes a pointer to an
unused local variable. The compiler can then optimize away the store
since it is not hidden in the asm.
_fcmpset is specifically for callers who want the value back. Ones which
don't can use the _cmpset variant.
The main caller of _xfcmpset would be the _cmpset variantL
static __inline int
atomic_cmpset_int(volatile u_int *dst, u_int expect, u_int src)
{
u_int dummy __unused;
return (atomic_xfcmpset_int(dst, &dummy, expect, src));
}
Actually, _cmpset can be built out of _fcmpset even more easily:
static __inline int
atomic_cmpset_int(volatile u_int *dst, u_int expect, u_int src)
{
return (atomic_fcmpset_int(dst, &expect, src));
}
This has to be a function since a macro would expose *&expect to
clobbering.
Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
