lkml.org 
[lkml]   [2015]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: futex atomic vs ordering constraints
From
Date
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



\
 
 \ /
  Last update: 2015-09-02 18:21    [W:0.556 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site