Re: [Xen-devel] [PATCH v7 02/17] Add cmpxchg16b support for x86-64

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.09.15 at 10:28, <feng.wu@xxxxxxxxx> wrote:

> --- a/xen/include/asm-x86/x86_64/system.h
> +++ b/xen/include/asm-x86/x86_64/system.h
> @@ -6,6 +6,37 @@
>                                     (unsigned long)(n),sizeof(*(ptr))))
>  
>  /*
> + * Atomic 16 bytes compare and exchange.  Compare OLD with MEM, if
> + * identical, store NEW in MEM.  Return the initial value in MEM.
> + * Success is indicated by comparing RETURN with OLD.
> + *
> + * This function can only be called when cpu_has_cx16 is true.
> + */
> +
> +static always_inline __uint128_t __cmpxchg16b(
> +    volatile void *ptr, const __uint128_t *old, const __uint128_t *new)
> +{
> +    __uint128_t prev;
> +    uint64_t new_high = *new >> 64;
> +    uint64_t new_low = (uint64_t)*new;

Pointless cast.

> +    ASSERT(cpu_has_cx16);
> +
> +    asm volatile ( "lock; cmpxchg16b %3"
> +                   : "=A" (prev)
> +                   : "c" (new_high), "b" (new_low), "m" (*__xg(ptr)), "0" 
> (*old)
> +                 );

The closing parenthesis belongs on the previous line.

> +    return prev;
> +}
> +
> +#define cmpxchg16b(ptr,o,n)                                            \
> +    ( ({ ASSERT(((unsigned long)ptr & 0xF) == 0); }),                  \
> +      (BUILD_BUG_ON(sizeof(*o) != sizeof(__uint128_t))),               \
> +      (BUILD_BUG_ON(sizeof(*n) != sizeof(__uint128_t))),               \
> +      (__cmpxchg16b((ptr), (void *)(o), (void *)(n))) )

Sigh - this is _still_ not properly parenthesized, and evaluates ptr
twice:

#define cmpxchg16b(ptr, o, n) ({                       \
    volatile void *_p = (ptr);                         \
    ASSERT(!((unsigned long)_p & 0xf));                \
    BUILD_BUG_ON(sizeof(*(o)) != sizeof(__uint128_t)); \
    BUILD_BUG_ON(sizeof(*(n)) != sizeof(__uint128_t)); \
    __cmpxchg16b(_p, (void *)(o), (void *)(n));        \
})

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel