On 09/02/2015 08:55 AM, Peter Zijlstra wrote:
So here goes..
Chris, I'm awfully sorry, but I seem to be Tile challenged.
TileGX seems to define:
#define smp_mb__before_atomic() smp_mb()
#define smp_mb__after_atomic() smp_mb()
However, its atomic_add_return() implementation looks like:
static inline int atomic_add_return(int i, atomic_t *v)
{
int val;
smp_mb(); /* barrier for proper semantics */
val = __insn_fetchadd4((void *)&v->counter, i) + i;
barrier(); /* the "+ i" above will wait on memory */
return val;
}
Which leaves me confused on smp_mb__after_atomic().
Are you concerned about whether it has proper memory
barrier semantics already, i.e. full barriers before and after?
In fact we do have a full barrier before, but then because of the
"+ i" / "barrier()", we know that the only other operation since
the previous mb(), namely the read of v->counter, has
completed after the atomic operation. As a result we can
omit explicitly having a second barrier.
It does seem like all the current memory-order semantics are
correct, unless I'm missing something!
That said, your futex ops seem to lack any memory barrier, so naively
I'd add both, its just that your add_return() confuses me.
So something like this?
diff --git a/arch/tile/include/asm/futex.h b/arch/tile/include/asm/futex.h
index 1a6ef1b69cb1..0a5501b11d02 100644
--- a/arch/tile/include/asm/futex.h
+++ b/arch/tile/include/asm/futex.h
@@ -39,6 +39,7 @@
#ifdef __tilegx__
#define __futex_asm(OP) \
+ smp_mb(); \
asm("1: {" #OP " %1, %3, %4; movei %0, 0 }\n" \
".pushsection .fixup,\"ax\"\n" \
"0: { movei %0, %5; j 9f }\n" \
@@ -48,7 +49,8 @@
".popsection\n" \
"9:" \
: "=r" (ret), "=r" (val), "+m" (*(uaddr)) \
- : "r" (uaddr), "r" (oparg), "i" (-EFAULT))
+ : "r" (uaddr), "r" (oparg), "i" (-EFAULT)); \
+ smp_mb()
#define __futex_set() __futex_asm(exch4)
#define __futex_add() __futex_asm(fetchadd4)
@@ -75,7 +77,10 @@
#define __futex_call(FN) \
{ \
- struct __get_user gu = FN((u32 __force *)uaddr, lock, oparg); \
+ struct __get_user gu; \
+ smp_mb(); \
+ gu = FN((u32 __force *)uaddr, lock, oparg); \
+ /* See smp_mb__after_atomic() */ \
val = gu.val; \
ret = gu.err; \
}
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
--
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/
