On Mon, Nov 12, 2012 at 5:54 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: > Il 05/11/2012 06:38, Liu Ping Fan ha scritto: >> From: Liu Ping Fan <pingf...@linux.vnet.ibm.com> >> >> If out of global lock, we will be challenged by SMP in low level, >> so need atomic ops. >> >> This file is a wrapper of GCC atomic builtin. > > I still object to this. > > I know it enforces type-safety, but it is incomplete. It doesn't
Although it is incomplete, but the rest cases are rarely used. Linux faces such issue, and the "int" version is enough, so I think we can borrow experience from there. > provide neither atomic accesses to pointers, nor useful operations such > as exchange. It won't be used consistently, because in some places you > just do not have an Atomic value (see both current uses of __sync_* > builtins). > > If you can make it complete, and prove it by using it where __sync_* is For current code, __sync_* is used rarely, I think except the barriers, only two places use it. We can substitute it easily. Regards, Pingfan > used now, or just use gcc builtins directly. > > Paolo > >> >> Signed-off-by: Liu Ping Fan <pingf...@linux.vnet.ibm.com> >> --- >> include/qemu/atomic.h | 63 >> +++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 files changed, 63 insertions(+), 0 deletions(-) >> create mode 100644 include/qemu/atomic.h >> >> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h >> new file mode 100644 >> index 0000000..a9e6d35 >> --- /dev/null >> +++ b/include/qemu/atomic.h >> @@ -0,0 +1,63 @@ >> +/* >> + * Simple wrapper of gcc Atomic-Builtins >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + * >> + */ >> +#ifndef __QEMU_ATOMIC_H >> +#define __QEMU_ATOMIC_H >> + >> +typedef struct Atomic { >> + volatile int counter; >> +} Atomic; >> + >> +static inline void atomic_set(Atomic *v, int i) >> +{ >> + v->counter = i; >> +} >> + >> +static inline int atomic_read(Atomic *v) >> +{ >> + return v->counter; >> +} >> + >> +static inline int atomic_return_and_add(int i, Atomic *v) >> +{ >> + int ret; >> + >> + ret = __sync_fetch_and_add(&v->counter, i); >> + return ret; >> +} >> + >> +static inline int atomic_return_and_sub(int i, Atomic *v) >> +{ >> + int ret; >> + >> + ret = __sync_fetch_and_sub(&v->counter, i); >> + return ret; >> +} >> + >> +/** >> + * * atomic_inc - increment atomic variable >> + * * @v: pointer of type Atomic >> + * * >> + * * Atomically increments @v by 1. >> + * */ >> +static inline void atomic_inc(Atomic *v) >> +{ >> + __sync_fetch_and_add(&v->counter, 1); >> +} >> + >> +/** >> + * * atomic_dec - decrement atomic variable >> + * * @v: pointer of type Atomic >> + * * >> + * * Atomically decrements @v by 1. >> + * */ >> +static inline void atomic_dec(Atomic *v) >> +{ >> + __sync_fetch_and_sub(&v->counter, 1); >> +} >> + >> +#endif >> >