Re: [libvirt] [PATCH V11 2/7] Support for atomic operations on integers

2012-04-19 Thread Daniel Veillard 
On Tue, Apr 17, 2012 at 10:44:03AM -0400, Stefan Berger wrote:
 For threading support, add atomic add and sub operations working on
 integers. Base this on locking support provided by virMutex.
 
 ---
  src/util/viratomic.h |   91 
 +++
  1 file changed, 91 insertions(+)
 
 Index: libvirt-acl/src/util/viratomic.h
 ===
 --- /dev/null
 +++ libvirt-acl/src/util/viratomic.h
 @@ -0,0 +1,91 @@
 +/*
 + * viratomic.h: atomic integer operations
 + *
 + * Copyright (C) 2012 IBM Corporation
 + *
 + * Authors:
 + * Stefan Berger stef...@linux.vnet.ibm.com
 + *
 + * This library is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU Lesser General Public
 + * License as published by the Free Software Foundation; either
 + * version 2.1 of the License, or (at your option) any later version.
 + *
 + * This library is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * Lesser General Public License for more details.
 + *
 + * You should have received a copy of the GNU Lesser General Public
 + * License along with this library; if not, write to the Free Software
 + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
 + *
 + */
 +
 +#ifndef __VIR_ATOMIC_H__
 +# define __VIR_ATOMIC_H__
 +
 +# include threads.h
 +
 +typedef struct _virAtomicInt virAtomicInt;
 +typedef virAtomicInt *virAtomicIntPtr;
 +
 +struct _virAtomicInt {
 +virMutex lock;
 +int value;
 +};
 +
 +static inline int
 +virAtomicIntInit(virAtomicIntPtr vaip)
 +{
 +vaip-value = 0;
 +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)
 +{
 +int ret;
 +
 +virMutexLock(vaip-lock);
 +
 +vaip-value += add;
 +ret = vaip-value;
 +
 +virMutexUnlock(vaip-lock);
 +
 +return ret;
 +}
 +
 +static inline int
 +virAtomicIntSub(virAtomicIntPtr vaip, int sub)
 +{
 +int ret;
 +
 +virMutexLock(vaip-lock);
 +
 +vaip-value -= sub;
 +ret = vaip-value;
 +
 +virMutexUnlock(vaip-lock);
 +
 +return ret;
 +}
 +
 +#endif /* __VIR_ATOMIC_H */

  To be honnest, I'm not too fond of inline functions, but we already
rely on them (though for accessors mostly). In that case it's
reasonnable to use them,

  ACK, but it itches me a bit

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH V11 2/7] Support for atomic operations on integers

2012-04-19 Thread Eric Blake 
On 04/17/2012 08:44 AM, Stefan Berger wrote:
 For threading support, add atomic add and sub operations working on
 integers. Base this on locking support provided by virMutex.
 
 ---
  src/util/viratomic.h |   91 
 +++
  1 file changed, 91 insertions(+)
 
 Index: libvirt-acl/src/util/viratomic.h

 +typedef struct _virAtomicInt virAtomicInt;
 +typedef virAtomicInt *virAtomicIntPtr;
 +
 +struct _virAtomicInt {
 +virMutex lock;
 +int value;
 +};

virMutex is very heavyweight.  I'd love it if we could use gcc
primitives and/or MSVC libc primitives (where they are known to be
available), for a lighter-weight implementation on platforms that
support it.  See this patch proposal (now a year old!) for some ideas
that we should fold in before 0.9.12:

https://www.redhat.com/archives/libvir-list/2011-April/msg00368.html

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH V11 2/7] Support for atomic operations on integers

2012-04-19 Thread Stefan Berger 

On 04/19/2012 02:08 PM, Eric Blake wrote:

On 04/17/2012 08:44 AM, Stefan Berger wrote:

For threading support, add atomic add and sub operations working on
integers. Base this on locking support provided by virMutex.


virMutex is very heavyweight.  I'd love it if we could use gcc
primitives and/or MSVC libc primitives (where they are known to be
available), for a lighter-weight implementation on platforms that
support it.  See this patch proposal (now a year old!) for some ideas
that we should fold in before 0.9.12:

https://www.redhat.com/archives/libvir-list/2011-April/msg00368.html

Well, I wasn't aware of this previous proposal and had actually looked 
in pthread for some form of atomic support.
I already pushed this patch now. We can add some compiler- and 
architecture-specific #ifdef's around the few existing functions. So the 
existing implementation probably has a place but can be improved upon.


Stefan


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH V11 2/7] Support for atomic operations on integers

2012-04-19 Thread Eric Blake 
On 04/19/2012 12:19 PM, Stefan Berger wrote:
 On 04/19/2012 02:08 PM, Eric Blake wrote:
 On 04/17/2012 08:44 AM, Stefan Berger wrote:
 For threading support, add atomic add and sub operations working on
 integers. Base this on locking support provided by virMutex.

 virMutex is very heavyweight.  I'd love it if we could use gcc
 primitives and/or MSVC libc primitives (where they are known to be
 available), for a lighter-weight implementation on platforms that
 support it.  See this patch proposal (now a year old!) for some ideas
 that we should fold in before 0.9.12:

 https://www.redhat.com/archives/libvir-list/2011-April/msg00368.html

 Well, I wasn't aware of this previous proposal and had actually looked
 in pthread for some form of atomic support.
 I already pushed this patch now. We can add some compiler- and
 architecture-specific #ifdef's around the few existing functions. So the
 existing implementation probably has a place but can be improved upon.

No problem with the current implementation (don't go reverting anything
just because I came late to the review party!); I'm okay with future
patches for the improvements, especially since they will merely be
optimizations.  But I'm hoping we can get those optimizations in the
tree before 0.9.12, rather than later, now that it is fresh on our minds.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH V11 2/7] Support for atomic operations on integers

2012-04-17 Thread Stefan Berger 
For threading support, add atomic add and sub operations working on
integers. Base this on locking support provided by virMutex.

---
 src/util/viratomic.h |   91 +++
 1 file changed, 91 insertions(+)

Index: libvirt-acl/src/util/viratomic.h
===
--- /dev/null
+++ libvirt-acl/src/util/viratomic.h
@@ -0,0 +1,91 @@
+/*
+ * viratomic.h: atomic integer operations
+ *
+ * Copyright (C) 2012 IBM Corporation
+ *
+ * Authors:
+ * Stefan Berger stef...@linux.vnet.ibm.com
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ */
+
+#ifndef __VIR_ATOMIC_H__
+# define __VIR_ATOMIC_H__
+
+# include threads.h
+
+typedef struct _virAtomicInt virAtomicInt;
+typedef virAtomicInt *virAtomicIntPtr;
+
+struct _virAtomicInt {
+virMutex lock;
+int value;
+};
+
+static inline int
+virAtomicIntInit(virAtomicIntPtr vaip)
+{
+vaip-value = 0;
+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)
+{
+int ret;
+
+virMutexLock(vaip-lock);
+
+vaip-value += add;
+ret = vaip-value;
+
+virMutexUnlock(vaip-lock);
+
+return ret;
+}
+
+static inline int
+virAtomicIntSub(virAtomicIntPtr vaip, int sub)
+{
+int ret;
+
+virMutexLock(vaip-lock);
+
+vaip-value -= sub;
+ret = vaip-value;
+
+virMutexUnlock(vaip-lock);
+
+return ret;
+}
+
+#endif /* __VIR_ATOMIC_H */

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list