From mboxrd@z Thu Jan 1 00:00:00 1970 From: compudj@krystal.dyndns.org (Mathieu Desnoyers) Date: Wed, 17 Feb 2010 22:10:21 -0500 Subject: [ltt-dev] [PATCH 06/12] add uatomic_defaults.h, use it for default definitions In-Reply-To: <1266260686-17588-7-git-send-email-pbonzini@redhat.com> References: <1266260686-17588-1-git-send-email-pbonzini@redhat.com> <1266260686-17588-7-git-send-email-pbonzini@redhat.com> Message-ID: <20100218031021.GC11338@Krystal> * Paolo Bonzini (pbonzini at redhat.com) wrote: > uatomic_defaults.h can be included by uatomic_arch_*.h to provide useful Maybe rebrand defaults -> generic here too ? More comments inline. > default definitions. uatomic_arch_*.h can define whatever builtins > it wants to override, then uatomic_defaults.h will provide what is not > already defined, as follows: > > - uatomic_cmpxchg will use __sync_val_compare_and_swap builtins; > > - uatomic_add_return will use __sync_fetch_and_add if uatomic_arch_*.h > did not provide a definition of uatomic_cmpxchg. If it did, we assume > __sync builtins are buggy or otherwise undesirable on this platform, > so uatomic_defaults.h will implement uatomic_add_return in terms of > uatomic_cmpxchg too. > > - uatomic_xchg is like uatomic_add_return. However, since GCC does > not provide an adequate builtin, it needs to use a compare-and-swap > loop using __sync_bool_compare_and_swap if uatomic_cmpxchg is not > provided. > > - uatomic_sub_return/uatomic_add/uatomic_sub will be implemented > in terms of uatomic_add_return; > > - uatomic_inc/uatomic_dec will be implemented in terms of uatomic_add. > > After this patch, uatomic_defaults.h is already used for the latter two > categories. > > The hunk in tests/test_uatomic.c is only needed for bisectability > and will be removed later. > > Signed-off-by: Paolo Bonzini > --- > Makefile.am | 2 +- > tests/test_uatomic.c | 2 + > urcu/uatomic_arch_ppc.h | 15 +-- > urcu/uatomic_arch_s390.h | 15 +-- > urcu/uatomic_arch_sparc64.h | 15 +-- > urcu/uatomic_arch_x86.h | 26 ++--- > urcu/uatomic_defaults.h | 262 +++++++++++++++++++++++++++++++++++++++++++ > 7 files changed, 280 insertions(+), 57 deletions(-) > create mode 100644 urcu/uatomic_defaults.h > > diff --git a/Makefile.am b/Makefile.am > index 2ede9e0..c91dfe9 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -6,7 +6,7 @@ SUBDIRS = . tests > > include_HEADERS = urcu.h $(top_srcdir)/urcu-*.h > nobase_dist_include_HEADERS = urcu/compiler.h urcu/hlist.h urcu/list.h \ > - urcu/rculist.h urcu/system.h urcu/urcu-futex.h > + urcu/rculist.h urcu/system.h urcu/urcu-futex.h urcu/uatomic_defaults.h > nobase_nodist_include_HEADERS = urcu/arch.h urcu/uatomic_arch.h urcu/config.h > > EXTRA_DIST = $(top_srcdir)/urcu/arch_*.h $(top_srcdir)/urcu/uatomic_arch_*.h \ > diff --git a/tests/test_uatomic.c b/tests/test_uatomic.c > index 68cb6df..c0f36fe 100644 > --- a/tests/test_uatomic.c > +++ b/tests/test_uatomic.c > @@ -1,5 +1,7 @@ > #include > #include > + > +#define UATOMIC_NO_LINK_ERROR > #include > > #if (defined(__i386__) || defined(__x86_64__)) > diff --git a/urcu/uatomic_arch_ppc.h b/urcu/uatomic_arch_ppc.h > index 8da192e..b42bfdb 100644 > --- a/urcu/uatomic_arch_ppc.h > +++ b/urcu/uatomic_arch_ppc.h > @@ -47,9 +47,6 @@ extern "C" { > > #define ILLEGAL_INSTR ".long 0xd00d00" > > -#define uatomic_set(addr, v) STORE_SHARED(*(addr), (v)) > -#define uatomic_read(addr) LOAD_SHARED(*(addr)) > - > /* > * Using a isync as second barrier for exchange to provide acquire semantic. > * According to uatomic_ops/sysdeps/gcc/powerpc.h, the documentation is "fairly > @@ -225,18 +222,10 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, > (unsigned long)(v), \ > sizeof(*(addr)))) > > -/* uatomic_sub_return, uatomic_add, uatomic_sub, uatomic_inc, uatomic_dec */ > - > -#define uatomic_sub_return(addr, v) uatomic_add_return((addr), -(v)) > - > -#define uatomic_add(addr, v) (void)uatomic_add_return((addr), (v)) > -#define uatomic_sub(addr, v) (void)uatomic_sub_return((addr), (v)) > - > -#define uatomic_inc(addr) uatomic_add((addr), 1) > -#define uatomic_dec(addr) uatomic_add((addr), -1) > - > #ifdef __cplusplus > } > #endif > > +#include > + > #endif /* _URCU_ARCH_UATOMIC_PPC_H */ > diff --git a/urcu/uatomic_arch_s390.h b/urcu/uatomic_arch_s390.h > index 614867f..8f1523c 100644 > --- a/urcu/uatomic_arch_s390.h > +++ b/urcu/uatomic_arch_s390.h > @@ -78,9 +78,6 @@ struct __uatomic_dummy { > }; > #define __hp(x) ((struct __uatomic_dummy *)(x)) > > -#define uatomic_set(addr, v) STORE_SHARED(*(addr), (v)) > -#define uatomic_read(addr) LOAD_SHARED(*(addr)) > - > /* xchg */ > > static inline __attribute__((always_inline)) > @@ -208,18 +205,10 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, int len) > (unsigned long)(v), \ > sizeof(*(addr)))) > > -/* uatomic_sub_return, uatomic_add, uatomic_sub, uatomic_inc, uatomic_dec */ > - > -#define uatomic_sub_return(addr, v) uatomic_add_return((addr), -(v)) > - > -#define uatomic_add(addr, v) (void)uatomic_add_return((addr), (v)) > -#define uatomic_sub(addr, v) (void)uatomic_sub_return((addr), (v)) > - > -#define uatomic_inc(addr) uatomic_add((addr), 1) > -#define uatomic_dec(addr) uatomic_add((addr), -1) > - > #ifdef __cplusplus > } > #endif > > +#include > + > #endif /* _URCU_UATOMIC_ARCH_S390_H */ > diff --git a/urcu/uatomic_arch_sparc64.h b/urcu/uatomic_arch_sparc64.h > index d443d4f..0c16eaf 100644 > --- a/urcu/uatomic_arch_sparc64.h > +++ b/urcu/uatomic_arch_sparc64.h > @@ -39,9 +39,6 @@ extern "C" { > #define BITS_PER_LONG (__SIZEOF_LONG__ * 8) > #endif > > -#define uatomic_set(addr, v) STORE_SHARED(*(addr), (v)) > -#define uatomic_read(addr) LOAD_SHARED(*(addr)) > - > /* cmpxchg */ > > static inline __attribute__((always_inline)) > @@ -169,18 +166,10 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, int len) > (unsigned long)(v), \ > sizeof(*(addr)))) > > -/* uatomic_sub_return, uatomic_add, uatomic_sub, uatomic_inc, uatomic_dec */ > - > -#define uatomic_sub_return(addr, v) uatomic_add_return((addr), -(v)) > - > -#define uatomic_add(addr, v) (void)uatomic_add_return((addr), (v)) > -#define uatomic_sub(addr, v) (void)uatomic_sub_return((addr), (v)) > - > -#define uatomic_inc(addr) uatomic_add((addr), 1) > -#define uatomic_dec(addr) uatomic_add((addr), -1) > - > #ifdef __cplusplus > } > #endif > > +#include > + > #endif /* _URCU_ARCH_UATOMIC_PPC_H */ > diff --git a/urcu/uatomic_arch_x86.h b/urcu/uatomic_arch_x86.h > index 3bfe86d..8a81995 100644 > --- a/urcu/uatomic_arch_x86.h > +++ b/urcu/uatomic_arch_x86.h > @@ -49,7 +49,11 @@ struct __uatomic_dummy { > #define __hp(x) ((struct __uatomic_dummy *)(x)) > > #define _uatomic_set(addr, v) STORE_SHARED(*(addr), (v)) > + > +#if 0 > +/* Read is atomic even in compat mode */ > #define _uatomic_read(addr) LOAD_SHARED(*(addr)) > +#endif Maybe we should fix this by removing it entirely, just leaving the comment below in the "compat_" block ? > > /* cmpxchg */ > > @@ -176,7 +180,7 @@ unsigned long __uatomic_exchange(void *addr, unsigned long val, int len) > ((__typeof__(*(addr))) __uatomic_exchange((addr), (unsigned long)(v), \ > sizeof(*(addr)))) > > -/* uatomic_add_return, uatomic_sub_return */ > +/* uatomic_add_return */ > > static inline __attribute__((always_inline)) > unsigned long __uatomic_add_return(void *addr, unsigned long val, > @@ -241,9 +245,7 @@ unsigned long __uatomic_add_return(void *addr, unsigned long val, > (unsigned long)(v), \ > sizeof(*(addr)))) > > -#define _uatomic_sub_return(addr, v) _uatomic_add_return((addr), -(v)) > - > -/* uatomic_add, uatomic_sub */ > +/* uatomic_add */ > > static inline __attribute__((always_inline)) > void __uatomic_add(void *addr, unsigned long val, int len) > @@ -297,8 +299,6 @@ void __uatomic_add(void *addr, unsigned long val, int len) > #define _uatomic_add(addr, v) \ > (__uatomic_add((addr), (unsigned long)(v), sizeof(*(addr)))) > > -#define _uatomic_sub(addr, v) _uatomic_add((addr), -(v)) > - > > /* uatomic_inc */ > > @@ -449,24 +449,17 @@ extern unsigned long _compat_uatomic_xchg(void *addr, > (unsigned long)(v), \ > sizeof(*(addr)))) > > -#define compat_uatomic_sub_return(addr, v) \ > - compat_uatomic_add_return((addr), -(v)) > #define compat_uatomic_add(addr, v) \ > ((void)compat_uatomic_add_return((addr), (v))) > -#define compat_uatomic_sub(addr, v) \ > - ((void)compat_uatomic_sub_return((addr), (v))) > #define compat_uatomic_inc(addr) \ > (compat_uatomic_add((addr), 1)) > #define compat_uatomic_dec(addr) \ > - (compat_uatomic_sub((addr), 1)) > + (compat_uatomic_add((addr), -1)) > > #else > #define UATOMIC_COMPAT(insn) (_uatomic_##insn) > #endif > > -/* Read is atomic even in compat mode */ We could say: /* Read is atomic even in compat mode. Use generic version. */ > -#define uatomic_read(addr) _uatomic_read(addr) > - > #define uatomic_set(addr, v) \ > UATOMIC_COMPAT(set(addr, v)) > #define uatomic_cmpxchg(addr, old, _new) \ > @@ -475,10 +468,7 @@ extern unsigned long _compat_uatomic_xchg(void *addr, > UATOMIC_COMPAT(xchg(addr, v)) > #define uatomic_add_return(addr, v) \ > UATOMIC_COMPAT(add_return(addr, v)) > -#define uatomic_sub_return(addr, v) \ > - UATOMIC_COMPAT(sub_return(addr, v)) > #define uatomic_add(addr, v) UATOMIC_COMPAT(add(addr, v)) > -#define uatomic_sub(addr, v) UATOMIC_COMPAT(sub(addr, v)) > #define uatomic_inc(addr) UATOMIC_COMPAT(inc(addr)) > #define uatomic_dec(addr) UATOMIC_COMPAT(dec(addr)) > > @@ -486,4 +476,6 @@ extern unsigned long _compat_uatomic_xchg(void *addr, > } > #endif > > +#include > + > #endif /* _URCU_ARCH_UATOMIC_X86_H */ > diff --git a/urcu/uatomic_defaults.h b/urcu/uatomic_defaults.h > new file mode 100644 > index 0000000..93467dd > --- /dev/null > +++ b/urcu/uatomic_defaults.h > @@ -0,0 +1,262 @@ > +#ifndef _URCU_UATOMIC_GCC_H > +#define _URCU_UATOMIC_GCC_H > + > +/* > + * Copyright (c) 1991-1994 by Xerox Corporation. All rights reserved. > + * Copyright (c) 1996-1999 by Silicon Graphics. All rights reserved. > + * Copyright (c) 1999-2004 Hewlett-Packard Development Company, L.P. > + * Copyright (c) 2009 Mathieu Desnoyers > + * Copyright (c) 2010 Paolo Bonzini > + * > + * THIS MATERIAL IS PROVIDED AS IS, WITH ABSOLUTELY NO WARRANTY EXPRESSED > + * OR IMPLIED. ANY USE IS AT YOUR OWN RISK. > + * > + * Permission is hereby granted to use or copy this program > + * for any purpose, provided the above notices are retained on all copies. > + * Permission to modify the code and to distribute modified code is granted, > + * provided the above notices are retained, and a notice that the code was > + * modified is included with the above copyright notice. > + * > + * Code inspired from libuatomic_ops-1.2, inherited in part from the > + * Boehm-Demers-Weiser conservative garbage collector. > + */ > + > +#include > +#include > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#ifndef BITS_PER_LONG > +#define BITS_PER_LONG (__SIZEOF_LONG__ * 8) > +#endif > + > +#ifndef uatomic_set > +#define uatomic_set(addr, v) STORE_SHARED(*(addr), (v)) > +#endif > + > +#ifndef uatomic_read > +#define uatomic_read(addr) LOAD_SHARED(*(addr)) > +#endif > + > +#if !defined __OPTIMIZE__ || defined UATOMIC_NO_LINK_ERROR > +static inline __attribute__((always_inline)) > +void _uatomic_link_error() > +{ > +#ifdef ILLEGAL_INSTR > + /* generate an illegal instruction. Cannot catch this with linker tricks > + * when optimizations are disabled. */ > + __asm__ __volatile__(ILLEGAL_INSTR); > +#else > + __builtin_trap (); coding style: __builtin_trap(); > +#endif > +} > +#else > +extern void _uatomic_link_error (); > +#endif Please add comments after these nested #else and #endif to make it easier to match the nesting. e.g. #ifdef A #ifdef B #else /* #ifdef B */ #endif /* #else #ifdef B */ #endif /* #ifdef A */ > + > +/* cmpxchg */ > + > +#ifndef uatomic_cmpxchg > +static inline __attribute__((always_inline)) > +unsigned long _uatomic_cmpxchg(void *addr, unsigned long old, > + unsigned long _new, int len) > +{ > + switch (len) { > + case 4: > + return __sync_val_compare_and_swap_4(addr, old, _new); > +#if (BITS_PER_LONG == 64) > + case 8: > + return __sync_val_compare_and_swap_8(addr, old, _new); > +#endif > + } > + _uatomic_link_error(); > + return 0; > +} > + > + > +#define uatomic_cmpxchg(addr, old, _new) \ > + ((__typeof__(*(addr))) _uatomic_cmpxchg((addr), (unsigned long)(old),\ > + (unsigned long)(_new), \ > + sizeof(*(addr)))) > + > + > +/* uatomic_add_return */ > + > +#ifndef uatomic_add_return > +static inline __attribute__((always_inline)) > +unsigned long _uatomic_add_return(void *addr, unsigned long val, > + int len) > +{ > + switch (len) { > + case 4: > + return __sync_add_and_fetch_4(addr, val); > +#if (BITS_PER_LONG == 64) > + case 8: > + return __sync_add_and_fetch_8(addr, val); > +#endif > + } > + _uatomic_link_error(); > + return 0; > +} > + > + > +#define uatomic_add_return(addr, v) \ > + ((__typeof__(*(addr))) _uatomic_add_return((addr), \ > + (unsigned long)(v), \ > + sizeof(*(addr)))) > +#endif > + > +#ifndef uatomic_xchg > +/* xchg */ > + > +static inline __attribute__((always_inline)) > +unsigned long _uatomic_exchange(void *addr, unsigned long val, int len) > +{ > + switch (len) { > + case 4: > + { > + unsigned int old; > + > + do > + old = uatomic_read((unsigned int *)addr); > + while (!__sync_bool_compare_and_swap_4(addr, old, val)); I really prefer do { one liner; } while (cond); for some reason. (less error prone) > + > + return old; > + } > +#if (BITS_PER_LONG == 64) > + case 8: > + { > + unsigned long old; > + > + do > + old = uatomic_read((unsigned long *)addr); > + while (!__sync_bool_compare_and_swap_8(addr, old, val)); > + Same here. > + return old; > + } > +#endif > + } > + _uatomic_link_error(); > + return 0; > +} > + > +#define uatomic_xchg(addr, v) \ > + ((__typeof__(*(addr))) _uatomic_exchange((addr), (unsigned long)(v), \ > + sizeof(*(addr)))) > +#endif > + > +#else #else ... Thanks, Mathieu > + > +#ifndef uatomic_add_return > +/* uatomic_add_return */ > + > +static inline __attribute__((always_inline)) > +unsigned long _uatomic_add_return(void *addr, unsigned long val, int len) > +{ > + switch (len) { > + case 4: > + { > + unsigned int old, oldt; > + > + oldt = uatomic_read((unsigned int *)addr); > + do { > + old = oldt; > + oldt = _uatomic_cmpxchg(addr, old, old + val, 4); > + } while (oldt != old); > + > + return old + val; > + } > +#if (BITS_PER_LONG == 64) > + case 8: > + { > + unsigned long old, oldt; > + > + oldt = uatomic_read((unsigned long *)addr); > + do { > + old = oldt; > + oldt = _uatomic_cmpxchg(addr, old, old + val, 8); > + } while (oldt != old); > + > + return old + val; > + } > +#endif > + } > + _uatomic_link_error(); > + return 0; > +} > + > +#define uatomic_add_return(addr, v) \ > + ((__typeof__(*(addr))) _uatomic_add_return((addr), \ > + (unsigned long)(v), \ > + sizeof(*(addr)))) > +#endif > + > +#ifndef uatomic_xchg > +/* xchg */ > + > +static inline __attribute__((always_inline)) > +unsigned long _uatomic_exchange(void *addr, unsigned long val, int len) > +{ > + switch (len) { > + case 4: > + { > + unsigned int old, oldt; > + > + oldt = uatomic_read((unsigned int *)addr); > + do { > + old = oldt; > + oldt = _uatomic_cmpxchg(addr, old, val, 4); > + } while (oldt != old); > + > + return old; > + } > +#if (BITS_PER_LONG == 64) > + case 8: > + { > + unsigned long old, oldt; > + > + oldt = uatomic_read((unsigned long *)addr); > + do { > + old = oldt; > + oldt = _uatomic_cmpxchg(addr, old, val, 8); > + } while (oldt != old); > + > + return old; > + } > +#endif > + } > + _uatomic_link_error(); > + return 0; > +} > + > +#define uatomic_xchg(addr, v) \ > + ((__typeof__(*(addr))) _uatomic_exchange((addr), (unsigned long)(v), \ > + sizeof(*(addr)))) > +#endif > + > +#endif > + > +/* uatomic_sub_return, uatomic_add, uatomic_sub, uatomic_inc, uatomic_dec */ > + > +#ifndef uatomic_add > +#define uatomic_add(addr, v) (void)uatomic_add_return((addr), (v)) > +#endif > + > +#define uatomic_sub_return(addr, v) uatomic_add_return((addr), -(v)) > +#define uatomic_sub(addr, v) uatomic_add((addr), -(v)) > + > +#ifndef uatomic_inc > +#define uatomic_inc(addr) uatomic_add((addr), 1) > +#endif > + > +#ifndef uatomic_dec > +#define uatomic_dec(addr) uatomic_add((addr), -1) > +#endif > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif /* _URCU_UATOMIC_GCC_H */ > -- > 1.6.6 > > > > _______________________________________________ > ltt-dev mailing list > ltt-dev at lists.casi.polymtl.ca > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev > -- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68