Looks good to me:
Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
On Sep 24, 2014, at 11:52 AM, Gurucharan Shetty <shet...@nicira.com> wrote:
> MSVC converts 64 bit read/writes into two instructions (uses 'mov' as
> seen through cl //FAs). So there is a possibility that an interrupt can
> make a 64 bit read/write non-atomic even when 8 byte aligned. So we cannot use
> a simple assignment. Use a full memory barrier function instead.
>
> Reported-by: Jarno Rajahalme <jrajaha...@nicira.com>
> Signed-off-by: Gurucharan Shetty <gshe...@nicira.com>
> ---
> lib/ovs-atomic-msvc.h | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/lib/ovs-atomic-msvc.h b/lib/ovs-atomic-msvc.h
> index e8e4891..c6a7db3 100644
> --- a/lib/ovs-atomic-msvc.h
> +++ b/lib/ovs-atomic-msvc.h
> @@ -97,14 +97,22 @@ atomic_signal_fence(memory_order order)
> *(DST) = (SRC); \
> }
>
> -/* 64 bit writes are atomic on i586 if 64 bit aligned. */
> +/* MSVC converts 64 bit writes into two instructions. So there is
> + * a possibility that an interrupt can make a 64 bit write non-atomic even
> + * when 8 byte aligned. So use InterlockedExchange64().
> + *
> + * For atomic stores, 'consume' and 'acquire' semantics are not valid. But we
> + * are using 'Exchange' to get atomic stores here and we only have
> + * InterlockedExchange64(), InterlockedExchangeNoFence64() and
> + * InterlockedExchange64Acquire() available. So we are forced to use
> + * InterlockedExchange64() which uses full memory barrier for everything
> + * greater than 'memory_order_relaxed'. */
> #define atomic_store64(DST, SRC, ORDER) \
> - if (((size_t) (DST) & (sizeof *(DST) - 1)) \
> - || ORDER == memory_order_seq_cst) { \
> - InterlockedExchange64((int64_t volatile *) (DST), \
> - (int64_t) (SRC)); \
> + if (ORDER == memory_order_relaxed) { \
> + InterlockedExchangeNoFence64((int64_t volatile *) (DST), \
> + (int64_t) (SRC)); \
> } else { \
> - *(DST) = (SRC); \
> + InterlockedExchange64((int64_t volatile *) (DST), (int64_t) (SRC));\
> }
>
> /* Used for 8 and 16 bit variations. */
> @@ -149,16 +157,14 @@ atomic_signal_fence(memory_order order)
> #define atomic_readX(SRC, DST, ORDER) \
> *(DST) = *(SRC);
>
> -/* 64 bit reads are atomic on i586 if 64 bit aligned. */
> +/* MSVC converts 64 bit reads into two instructions. So there is
> + * a possibility that an interrupt can make a 64 bit read non-atomic even
> + * when 8 byte aligned. So use fully memory barrier InterlockedOr64(). */
> #define atomic_read64(SRC, DST, ORDER) \
> - if (((size_t) (SRC) & (sizeof *(SRC) - 1)) == 0) { \
> - *(DST) = *(SRC); \
> - } else { \
> __pragma (warning(push)) \
> __pragma (warning(disable:4047)) \
> - *(DST) = InterlockedOr64((int64_t volatile *) (SRC), 0); \
> - __pragma (warning(pop)) \
> - }
> + *(DST) = InterlockedOr64((int64_t volatile *) (SRC), 0); \
> + __pragma (warning(pop))
>
> #define atomic_read(SRC, DST) \
> atomic_read_explicit(SRC, DST, memory_order_seq_cst)
> --
> 1.7.9.5
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
