From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:36845)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1aOkJI-000401-0r
	for qemu-devel@nongnu.org; Thu, 28 Jan 2016 06:00:24 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1aOkJE-0006sN-Mr
	for qemu-devel@nongnu.org; Thu, 28 Jan 2016 06:00:19 -0500
Received: from mx1.redhat.com ([209.132.183.28]:51992)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1aOkJE-0006sH-Fm
	for qemu-devel@nongnu.org; Thu, 28 Jan 2016 06:00:16 -0500
References: <1453976119-24372-1-git-send-email-alex.bennee@linaro.org>
	<1453976119-24372-4-git-send-email-alex.bennee@linaro.org>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <56A9F4BA.1030508@redhat.com>
Date: Thu, 28 Jan 2016 12:00:10 +0100
MIME-Version: 1.0
In-Reply-To: <1453976119-24372-4-git-send-email-alex.bennee@linaro.org>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH v1 3/5] include/qemu/atomic.h: default to
 __atomic functions
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: =?UTF-8?Q?Alex_Benn=c3=a9e?= <alex.bennee@linaro.org>, qemu-devel@nongnu.org
Cc: mttcg@greensocs.com, peter.maydell@linaro.org, mark.burton@greensocs.com, a.rigo@virtualopensystems.com, stefanha@redhat.com, fred.konrad@greensocs.com



On 28/01/2016 11:15, Alex Benn=C3=A9e wrote:
> +/* atomic_mb_read/set semantics map Java volatile variables. They are
> + * less expensive on some platforms (notably POWER & ARM) than fully
> + * sequentially consistent operations.
> + *
> + * As long as they are used as paired operations they are safe to
> + * use. See docs/atomic.txt for more discussion.
> + */
> +
> +#define atomic_mb_read(ptr)                             \
> +    ({                                                  \
> +    typeof(*ptr) _val;                                  \
> +     __atomic_load(ptr, &_val, __ATOMIC_RELAXED);       \
> +     smp_rmb();                                         \
> +    _val;                                               \
> +    })
> +
> +#define atomic_mb_set(ptr, i)  do {                     \
> +    typeof(*ptr) _val =3D (i);                            \
> +    smp_wmb();                                          \
> +    __atomic_store(ptr, &_val, __ATOMIC_RELAXED);       \
> +    smp_mb();                                           \
> +} while(0)

Great... I'll change this to

#if defined(_ARCH_PPC)
#define atomic_mb_read(ptr)                             \
    ({                                                  \
    typeof(*ptr) _val;                                  \
     __atomic_load(ptr, &_val, __ATOMIC_RELAXED);       \
     smp_rmb();                                         \
    _val;                                               \
    })

#define atomic_mb_set(ptr, i)  do {                     \
    typeof(*ptr) _val =3D (i);                            \
    smp_wmb();                                          \
    __atomic_store(ptr, &_val, __ATOMIC_RELAXED);       \
    smp_mb();                                           \
} while(0)
#else
#define atomic_mb_read(ptr)                       \
    ({                                            \
    typeof(*ptr) _val;                            \
     __atomic_load(ptr, &_val, __ATOMIC_SEQ_CST); \
    _val;                                         \
    })

#define atomic_mb_set(ptr, i)  do {               \
    typeof(*ptr) _val =3D (i);                      \
    __atomic_store(ptr, &_val, __ATOMIC_SEQ_CST); \
} while(0)
#endif

since this benefits x86 (which can generate mov/xchg respectively) and
aarch64 (where atomic_mb_read/atomic_mb_set map directly to ldar/stlr).

> +/* Returns the eventual value, failed or not */
> +#define atomic_cmpxchg(ptr, old, new)                                 =
  \
> +    ({                                                                =
  \
> +    typeof(*ptr) _old =3D (old), _new =3D (new);                      =
      \
> +    __atomic_compare_exchange(ptr, &_old, &_new, false,               =
  \
> +                              __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);    =
  \
> +    _old; /* can this race if cmpxchg not used elsewhere? */          =
  \
> +    })

How so?

Paolo