Messages in this thread Patch in this message | | | Subject | Re: futex atomic vs ordering constraints | From | Chris Metcalf <> | Date | Wed, 2 Sep 2015 12:10:58 -0400 |
| |
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
| |