[libvirt] [PATCH v2] Improve on virAtomic implementation

This patch improves the previously added virAtomicInt implementation by using gcc-builtins if possible. The needed builtins are available since GCC >= 4.1. At least the 4.0 docs don't mention them. --- src/util/viratomic.h | 95 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 79 insertions(+), 16 deletions(-) Index: libvirt-acl/src/util/viratomic.h =================================================================== --- libvirt-acl.orig/src/util/viratomic.h +++ libvirt-acl/src/util/viratomic.h @@ -30,6 +30,29 @@ typedef struct _virAtomicInt virAtomicInt; typedef virAtomicInt *virAtomicIntPtr; +# define __VIR_ATOMIC_USES_LOCK + +# if ((__GNUC__ == 4) && (__GNUC_MINOR__ >= 1)) || (__GNUC__ > 4) +# undef __VIR_ATOMIC_USES_LOCK +# endif + +static inline int virAtomicIntInit(virAtomicIntPtr vaip) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; +static inline int virAtomicIntRead(virAtomicIntPtr vaip) + ATTRIBUTE_NONNULL(1); +static inline void virAtomicIntSet(virAtomicIntPtr vaip, int val) + ATTRIBUTE_NONNULL(1); +static inline int virAtomicIntAdd(virAtomicIntPtr vaip, int add) + ATTRIBUTE_NONNULL(1); +static inline int virAtomicIntSub(virAtomicIntPtr vaip, int add) + ATTRIBUTE_NONNULL(1); +static inline int virAtomicIntInc(virAtomicIntPtr vaip) + ATTRIBUTE_NONNULL(1); +static inline int virAtomicIntDec(virAtomicIntPtr vaip) + ATTRIBUTE_NONNULL(1); + +# ifdef __VIR_ATOMIC_USES_LOCK + struct _virAtomicInt { virMutex lock; int value; @@ -42,22 +65,6 @@ virAtomicIntInit(virAtomicIntPtr vaip) return virMutexInit(&vaip->lock); } -static inline void -virAtomicIntSet(virAtomicIntPtr vaip, int value) -{ - virMutexLock(&vaip->lock); - - vaip->value = value; - - virMutexUnlock(&vaip->lock); -} - -static inline int -virAtomicIntRead(virAtomicIntPtr vaip) -{ - return vaip->value; -} - static inline int virAtomicIntAdd(virAtomicIntPtr vaip, int add) { @@ -88,4 +95,60 @@ virAtomicIntSub(virAtomicIntPtr vaip, in return ret; } +# else /* __VIR_ATOMIC_USES_LOCK */ + +struct _virAtomicInt { + int value; +}; + +static inline int +virAtomicIntInit(virAtomicIntPtr vaip) +{ + vaip->value = 0; + return 0; +} + +static inline int +virAtomicIntAdd(virAtomicIntPtr vaip, int add) +{ + return __sync_add_and_fetch(&vaip->value, add); +} + +static inline int +virAtomicIntSub(virAtomicIntPtr vaip, int sub) +{ + return __sync_sub_and_fetch(&vaip->value, sub); +} + +# endif /* __VIR_ATOMIC_USES_LOCK */ + + + +/* common operations that need no locking or build on others */ + + +static inline void +virAtomicIntSet(virAtomicIntPtr vaip, int value) +{ + vaip->value = value; +} + +static inline int +virAtomicIntRead(virAtomicIntPtr vaip) +{ + return *(volatile int *)&vaip->value; +} + +static inline int +virAtomicIntInc(virAtomicIntPtr vaip) +{ + return virAtomicIntAdd(vaip, 1); +} + +static inline int +virAtomicIntDec(virAtomicIntPtr vaip) +{ + return virAtomicIntSub(vaip, 1); +} + #endif /* __VIR_ATOMIC_H */

On 04/20/2012 06:44 AM, Stefan Berger wrote:
This patch improves the previously added virAtomicInt implementation by using gcc-builtins if possible. The needed builtins are available since GCC >= 4.1. At least the 4.0 docs don't mention them.
--- src/util/viratomic.h | 95 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 79 insertions(+), 16 deletions(-)
ACK. Now we can start thinking about eliminating locking for things that just need a refcount :-)

On 04/20/2012 11:56 AM, Laine Stump wrote:
On 04/20/2012 06:44 AM, Stefan Berger wrote:
This patch improves the previously added virAtomicInt implementation by using gcc-builtins if possible. The needed builtins are available since GCC>= 4.1. At least the 4.0 docs don't mention them.
--- src/util/viratomic.h | 95 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 79 insertions(+), 16 deletions(-) ACK.
Now we can start thinking about eliminating locking for things that just need a refcount :-) Of which there aren't too many yet... I'll push this one later today. Thanks.

On Tue, Apr 24, 2012 at 10:09:10AM -0400, Stefan Berger wrote:
On 04/20/2012 11:56 AM, Laine Stump wrote:
On 04/20/2012 06:44 AM, Stefan Berger wrote:
This patch improves the previously added virAtomicInt implementation by using gcc-builtins if possible. The needed builtins are available since GCC>= 4.1. At least the 4.0 docs don't mention them.
--- src/util/viratomic.h | 95 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 79 insertions(+), 16 deletions(-) ACK.
Now we can start thinking about eliminating locking for things that just need a refcount :-) Of which there aren't too many yet... I'll push this one later today. Thanks.
We can't eliminate locking for the objects in general, but we can eliminate locking in places where we only touch the ref-count, while keeping the locks for aall other API calls on the object. This would make like easier for us in a number of places, by avoiding potential lock aqcuisition order inversion problems we've hit in the past - particularly in the async callbacks/event dispatch Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange
-
Laine Stump
-
Stefan Berger