[ltt-dev] [PATCH 0/2] Add uatomic_and and uatomic_or to uatomic library

[ltt-dev] [PATCH 0/2] Add uatomic_and and uatomic_or to uatomic library

[Paolo Bonzini]

This short patch series provides atomic AND and OR operations that
return the previous value of the variable.  Can this go in even if
there are no users currently, to improve completeness of the
userspace atomic ops library?

Thanks,

Paolo

Paolo Bonzini (2):
  use generic-size operations for common implementation of atomic ops
  introduce uatomic_and and uatomic_or

 tests/test_uatomic.c   |    6 +
 urcu/uatomic_generic.h |  244 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 242 insertions(+), 8 deletions(-)

-- 
1.7.3.2

[ltt-dev] [PATCH 1/2] use generic-size operations for common implementation of atomic ops

[Paolo Bonzini]

The definition of _uatomic_cmpxchg is different in x86 and other architectures.
For x86 it is a 4-argument macro, for other architectures it is a 3-argument
function.  The next patch will use uatomic_cmpxchg in uatomic_generic.h
even on x86: adjust existing uses of the underscore-prefixed variant for
consistency.

Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
---
 urcu/uatomic_generic.h |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/urcu/uatomic_generic.h b/urcu/uatomic_generic.h
index f65b398..383ddfa 100644
--- a/urcu/uatomic_generic.h
+++ b/urcu/uatomic_generic.h
@@ -201,7 +201,8 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, int len)
 		oldt = uatomic_read((unsigned char *)addr);
 		do {
 			old = oldt;
-			oldt = _uatomic_cmpxchg(addr, old, old + val, 1);
+			oldt = uatomic_cmpxchg((unsigned char *)addr,
+                                               old, old + val);
 		} while (oldt != old);
 
 		return old + val;
@@ -215,7 +216,8 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, int len)
 		oldt = uatomic_read((unsigned short *)addr);
 		do {
 			old = oldt;
-			oldt = _uatomic_cmpxchg(addr, old, old + val, 2);
+			oldt = uatomic_cmpxchg((unsigned short *)addr,
+                                               old, old + val);
 		} while (oldt != old);
 
 		return old + val;
@@ -228,7 +230,8 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, int len)
 		oldt = uatomic_read((unsigned int *)addr);
 		do {
 			old = oldt;
-			oldt = _uatomic_cmpxchg(addr, old, old + val, 4);
+			oldt = uatomic_cmpxchg((unsigned int *)addr,
+                                               old, old + val);
 		} while (oldt != old);
 
 		return old + val;
@@ -241,7 +244,8 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, int len)
 		oldt = uatomic_read((unsigned long *)addr);
 		do {
 			old = oldt;
-			oldt = _uatomic_cmpxchg(addr, old, old + val, 8);
+			oldt = uatomic_cmpxchg((unsigned long *)addr,
+                                               old, old + val);
 		} while (oldt != old);
 
 		return old + val;
@@ -273,7 +277,8 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
 		oldt = uatomic_read((unsigned char *)addr);
 		do {
 			old = oldt;
-			oldt = _uatomic_cmpxchg(addr, old, val, 1);
+			oldt = uatomic_cmpxchg((unsigned char *)addr,
+                                               old, val);
 		} while (oldt != old);
 
 		return old;
@@ -287,7 +292,8 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
 		oldt = uatomic_read((unsigned short *)addr);
 		do {
 			old = oldt;
-			oldt = _uatomic_cmpxchg(addr, old, val, 2);
+			oldt = uatomic_cmpxchg((unsigned short *)addr,
+                                               old, val);
 		} while (oldt != old);
 
 		return old;
@@ -300,7 +306,8 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
 		oldt = uatomic_read((unsigned int *)addr);
 		do {
 			old = oldt;
-			oldt = _uatomic_cmpxchg(addr, old, val, 4);
+			oldt = uatomic_cmpxchg((unsigned int *)addr,
+                                               old, val);
 		} while (oldt != old);
 
 		return old;
@@ -313,7 +320,8 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
 		oldt = uatomic_read((unsigned long *)addr);
 		do {
 			old = oldt;
-			oldt = _uatomic_cmpxchg(addr, old, val, 8);
+			oldt = uatomic_cmpxchg((unsigned long *)addr,
+                                               old, val);
 		} while (oldt != old);
 
 		return old;
-- 
1.7.3.2

[ltt-dev] [PATCH 1/2] use generic-size operations for common implementation of atomic ops

[Mathieu Desnoyers]

* Paolo Bonzini (pbonzini at redhat.com) wrote:
> The definition of _uatomic_cmpxchg is different in x86 and other architectures.
> For x86 it is a 4-argument macro, for other architectures it is a 3-argument
> function.  The next patch will use uatomic_cmpxchg in uatomic_generic.h
> even on x86: adjust existing uses of the underscore-prefixed variant for
> consistency.

Nope. See 

#if ((BITS_PER_LONG != 64) && defined(CONFIG_RCU_COMPAT_ARCH))

#else

path in uatomic_arch_x86.h

to see why we use this trick.

We remove a dynamic test on these architectures where compatibility mode
is present.

Thanks,

Mathieu

> 
> Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> ---
>  urcu/uatomic_generic.h |   24 ++++++++++++++++--------
>  1 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/urcu/uatomic_generic.h b/urcu/uatomic_generic.h
> index f65b398..383ddfa 100644
> --- a/urcu/uatomic_generic.h
> +++ b/urcu/uatomic_generic.h
> @@ -201,7 +201,8 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, int len)
>  		oldt = uatomic_read((unsigned char *)addr);
>  		do {
>  			old = oldt;
> -			oldt = _uatomic_cmpxchg(addr, old, old + val, 1);
> +			oldt = uatomic_cmpxchg((unsigned char *)addr,
> +                                               old, old + val);
>  		} while (oldt != old);
>  
>  		return old + val;
> @@ -215,7 +216,8 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, int len)
>  		oldt = uatomic_read((unsigned short *)addr);
>  		do {
>  			old = oldt;
> -			oldt = _uatomic_cmpxchg(addr, old, old + val, 2);
> +			oldt = uatomic_cmpxchg((unsigned short *)addr,
> +                                               old, old + val);
>  		} while (oldt != old);
>  
>  		return old + val;
> @@ -228,7 +230,8 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, int len)
>  		oldt = uatomic_read((unsigned int *)addr);
>  		do {
>  			old = oldt;
> -			oldt = _uatomic_cmpxchg(addr, old, old + val, 4);
> +			oldt = uatomic_cmpxchg((unsigned int *)addr,
> +                                               old, old + val);
>  		} while (oldt != old);
>  
>  		return old + val;
> @@ -241,7 +244,8 @@ unsigned long _uatomic_add_return(void *addr, unsigned long val, int len)
>  		oldt = uatomic_read((unsigned long *)addr);
>  		do {
>  			old = oldt;
> -			oldt = _uatomic_cmpxchg(addr, old, old + val, 8);
> +			oldt = uatomic_cmpxchg((unsigned long *)addr,
> +                                               old, old + val);
>  		} while (oldt != old);
>  
>  		return old + val;
> @@ -273,7 +277,8 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
>  		oldt = uatomic_read((unsigned char *)addr);
>  		do {
>  			old = oldt;
> -			oldt = _uatomic_cmpxchg(addr, old, val, 1);
> +			oldt = uatomic_cmpxchg((unsigned char *)addr,
> +                                               old, val);
>  		} while (oldt != old);
>  
>  		return old;
> @@ -287,7 +292,8 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
>  		oldt = uatomic_read((unsigned short *)addr);
>  		do {
>  			old = oldt;
> -			oldt = _uatomic_cmpxchg(addr, old, val, 2);
> +			oldt = uatomic_cmpxchg((unsigned short *)addr,
> +                                               old, val);
>  		} while (oldt != old);
>  
>  		return old;
> @@ -300,7 +306,8 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
>  		oldt = uatomic_read((unsigned int *)addr);
>  		do {
>  			old = oldt;
> -			oldt = _uatomic_cmpxchg(addr, old, val, 4);
> +			oldt = uatomic_cmpxchg((unsigned int *)addr,
> +                                               old, val);
>  		} while (oldt != old);
>  
>  		return old;
> @@ -313,7 +320,8 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
>  		oldt = uatomic_read((unsigned long *)addr);
>  		do {
>  			old = oldt;
> -			oldt = _uatomic_cmpxchg(addr, old, val, 8);
> +			oldt = uatomic_cmpxchg((unsigned long *)addr,
> +                                               old, val);
>  		} while (oldt != old);
>  
>  		return old;
> -- 
> 1.7.3.2
> 
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Paolo Bonzini]

These are useful to flip single bits.

Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
---
 tests/test_uatomic.c   |    6 ++
 urcu/uatomic_generic.h |  220 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 226 insertions(+), 0 deletions(-)

diff --git a/tests/test_uatomic.c b/tests/test_uatomic.c
index 5682655..f14ed56 100644
--- a/tests/test_uatomic.c
+++ b/tests/test_uatomic.c
@@ -39,6 +39,12 @@ do {						\
 	v = uatomic_sub_return(ptr, 1);		\
 	assert(v == 121);			\
 	assert(uatomic_read(ptr) == 121);	\
+	v = uatomic_and(ptr, 0x47);		\
+	assert(v == 121);			\
+	assert(uatomic_read(ptr) == 0x41);	\
+	v = uatomic_or(ptr, 120);		\
+	assert(v == 0x41);			\
+	assert(uatomic_read(ptr) == 121);	\
 } while (0)
 
 int main(int argc, char **argv)
diff --git a/urcu/uatomic_generic.h b/urcu/uatomic_generic.h
index 383ddfa..7f3bc00 100644
--- a/urcu/uatomic_generic.h
+++ b/urcu/uatomic_generic.h
@@ -87,6 +87,74 @@ unsigned long _uatomic_cmpxchg(void *addr, unsigned long old,
 						sizeof(*(addr))))
 
 
+/* uatomic_and */
+
+#ifndef uatomic_and
+static inline __attribute__((always_inline))
+unsigned long _uatomic_and(void *addr, unsigned long val,
+				 int len)
+{
+	switch (len) {
+#ifdef UATOMIC_HAS_ATOMIC_BYTE
+	case 1:
+		return __sync_fetch_and_and_1(addr, val);
+#endif
+#ifdef UATOMIC_HAS_ATOMIC_SHORT
+	case 2:
+		return __sync_fetch_and_and_2(addr, val);
+#endif
+	case 4:
+		return __sync_fetch_and_and_4(addr, val);
+#if (BITS_PER_LONG == 64)
+	case 8:
+		return __sync_fetch_and_and_8(addr, val);
+#endif
+	}
+	_uatomic_link_error();
+	return 0;
+}
+
+
+#define uatomic_and(addr, v)					\
+	((__typeof__(*(addr))) _uatomic_and((addr),		\
+					    (unsigned long)(v),	\
+					    sizeof(*(addr))))
+#endif /* #ifndef uatomic_and */
+
+/* uatomic_or */
+
+#ifndef uatomic_or
+static inline __attribute__((always_inline))
+unsigned long _uatomic_or(void *addr, unsigned long val,
+				 int len)
+{
+	switch (len) {
+#ifdef UATOMIC_HAS_ATOMIC_BYTE
+	case 1:
+		return __sync_fetch_and_or_1(addr, val);
+#endif
+#ifdef UATOMIC_HAS_ATOMIC_SHORT
+	case 2:
+		return __sync_fetch_and_or_2(addr, val);
+#endif
+	case 4:
+		return __sync_fetch_and_or_4(addr, val);
+#if (BITS_PER_LONG == 64)
+	case 8:
+		return __sync_fetch_and_or_8(addr, val);
+#endif
+	}
+	_uatomic_link_error();
+	return 0;
+}
+
+
+#define uatomic_or(addr, v)					\
+	((__typeof__(*(addr))) _uatomic_or((addr),		\
+					   (unsigned long)(v),	\
+					   sizeof(*(addr))))
+#endif /* #ifndef uatomic_or */
+
 /* uatomic_add_return */
 
 #ifndef uatomic_add_return
@@ -186,6 +254,158 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
 
 #else /* #ifndef uatomic_cmpxchg */
 
+#ifndef uatomic_and
+/* uatomic_and */
+
+static inline __attribute__((always_inline))
+unsigned long _uatomic_and(void *addr, unsigned long val, int len)
+{
+	switch (len) {
+#ifdef UATOMIC_HAS_ATOMIC_BYTE
+	case 1:
+	{
+		unsigned char old, oldt;
+
+		oldt = uatomic_read((unsigned char *)addr);
+		do {
+			old = oldt;
+			oldt = uatomic_cmpxchg((unsigned char *)addr,
+                                               old, old & val);
+		} while (oldt != old);
+
+		return old;
+	}
+#endif
+#ifdef UATOMIC_HAS_ATOMIC_SHORT
+	case 2:
+	{
+		unsigned short old, oldt;
+
+		oldt = uatomic_read((unsigned short *)addr);
+		do {
+			old = oldt;
+			oldt = uatomic_cmpxchg((unsigned short *)addr,
+                                               old, old & val);
+		} while (oldt != old);
+
+		return old;
+	}
+#endif
+	case 4:
+	{
+		unsigned int old, oldt;
+
+		oldt = uatomic_read((unsigned int *)addr);
+		do {
+			old = oldt;
+			oldt = uatomic_cmpxchg((unsigned int *)addr,
+                                               old, old & val);
+		} 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((unsigned long *)addr,
+                                               old, old & val);
+		} while (oldt != old);
+
+		return old;
+	}
+#endif
+	}
+	_uatomic_link_error();
+	return 0;
+}
+
+#define uatomic_and(addr, v)					\
+	((__typeof__(*(addr))) _uatomic_and((addr),		\
+					    (unsigned long)(v),	\
+					    sizeof(*(addr))))
+#endif /* #ifndef uatomic_and */
+
+#ifndef uatomic_or
+/* uatomic_or */
+
+static inline __attribute__((always_inline))
+unsigned long _uatomic_or(void *addr, unsigned long val, int len)
+{
+	switch (len) {
+#ifdef UATOMIC_HAS_ATOMIC_BYTE
+	case 1:
+	{
+		unsigned char old, oldt;
+
+		oldt = uatomic_read((unsigned char *)addr);
+		do {
+			old = oldt;
+			oldt = uatomic_cmpxchg((unsigned char *)addr,
+                                               old, old | val);
+		} while (oldt != old);
+
+		return old;
+	}
+#endif
+#ifdef UATOMIC_HAS_ATOMIC_SHORT
+	case 2:
+	{
+		unsigned short old, oldt;
+
+		oldt = uatomic_read((unsigned short *)addr);
+		do {
+			old = oldt;
+			oldt = uatomic_cmpxchg((unsigned short *)addr,
+                                               old, old | val);
+		} while (oldt != old);
+
+		return old;
+	}
+#endif
+	case 4:
+	{
+		unsigned int old, oldt;
+
+		oldt = uatomic_read((unsigned int *)addr);
+		do {
+			old = oldt;
+			oldt = uatomic_cmpxchg((unsigned int *)addr,
+                                               old, old | val);
+		} 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((unsigned long *)addr,
+                                               old, old | val);
+		} while (oldt != old);
+
+		return old;
+	}
+#endif
+	}
+	_uatomic_link_error();
+	return 0;
+}
+
+#define uatomic_or(addr, v)					\
+	((__typeof__(*(addr))) _uatomic_or((addr),		\
+					   (unsigned long)(v),	\
+					   sizeof(*(addr))))
+#endif /* #ifndef uatomic_or */
+
 #ifndef uatomic_add_return
 /* uatomic_add_return */
 
-- 
1.7.3.2

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Mathieu Desnoyers]

* Paolo Bonzini (pbonzini at redhat.com) wrote:
> These are useful to flip single bits.

I'm trying to figure out the semantic of these operations.

are they

uatomic_add / uatomic_or or uatomic_add_return / uatomic_or_return ?

Thanks,

Mathieu

> 
> Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> ---
>  tests/test_uatomic.c   |    6 ++
>  urcu/uatomic_generic.h |  220 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 226 insertions(+), 0 deletions(-)
> 
> diff --git a/tests/test_uatomic.c b/tests/test_uatomic.c
> index 5682655..f14ed56 100644
> --- a/tests/test_uatomic.c
> +++ b/tests/test_uatomic.c
> @@ -39,6 +39,12 @@ do {						\
>  	v = uatomic_sub_return(ptr, 1);		\
>  	assert(v == 121);			\
>  	assert(uatomic_read(ptr) == 121);	\
> +	v = uatomic_and(ptr, 0x47);		\
> +	assert(v == 121);			\
> +	assert(uatomic_read(ptr) == 0x41);	\
> +	v = uatomic_or(ptr, 120);		\
> +	assert(v == 0x41);			\
> +	assert(uatomic_read(ptr) == 121);	\
>  } while (0)
>  
>  int main(int argc, char **argv)
> diff --git a/urcu/uatomic_generic.h b/urcu/uatomic_generic.h
> index 383ddfa..7f3bc00 100644
> --- a/urcu/uatomic_generic.h
> +++ b/urcu/uatomic_generic.h
> @@ -87,6 +87,74 @@ unsigned long _uatomic_cmpxchg(void *addr, unsigned long old,
>  						sizeof(*(addr))))
>  
>  
> +/* uatomic_and */
> +
> +#ifndef uatomic_and
> +static inline __attribute__((always_inline))
> +unsigned long _uatomic_and(void *addr, unsigned long val,
> +				 int len)
> +{
> +	switch (len) {
> +#ifdef UATOMIC_HAS_ATOMIC_BYTE
> +	case 1:
> +		return __sync_fetch_and_and_1(addr, val);
> +#endif
> +#ifdef UATOMIC_HAS_ATOMIC_SHORT
> +	case 2:
> +		return __sync_fetch_and_and_2(addr, val);
> +#endif
> +	case 4:
> +		return __sync_fetch_and_and_4(addr, val);
> +#if (BITS_PER_LONG == 64)
> +	case 8:
> +		return __sync_fetch_and_and_8(addr, val);
> +#endif
> +	}
> +	_uatomic_link_error();
> +	return 0;
> +}
> +
> +
> +#define uatomic_and(addr, v)					\
> +	((__typeof__(*(addr))) _uatomic_and((addr),		\
> +					    (unsigned long)(v),	\
> +					    sizeof(*(addr))))
> +#endif /* #ifndef uatomic_and */
> +
> +/* uatomic_or */
> +
> +#ifndef uatomic_or
> +static inline __attribute__((always_inline))
> +unsigned long _uatomic_or(void *addr, unsigned long val,
> +				 int len)
> +{
> +	switch (len) {
> +#ifdef UATOMIC_HAS_ATOMIC_BYTE
> +	case 1:
> +		return __sync_fetch_and_or_1(addr, val);
> +#endif
> +#ifdef UATOMIC_HAS_ATOMIC_SHORT
> +	case 2:
> +		return __sync_fetch_and_or_2(addr, val);
> +#endif
> +	case 4:
> +		return __sync_fetch_and_or_4(addr, val);
> +#if (BITS_PER_LONG == 64)
> +	case 8:
> +		return __sync_fetch_and_or_8(addr, val);
> +#endif
> +	}
> +	_uatomic_link_error();
> +	return 0;
> +}
> +
> +
> +#define uatomic_or(addr, v)					\
> +	((__typeof__(*(addr))) _uatomic_or((addr),		\
> +					   (unsigned long)(v),	\
> +					   sizeof(*(addr))))
> +#endif /* #ifndef uatomic_or */
> +
>  /* uatomic_add_return */
>  
>  #ifndef uatomic_add_return
> @@ -186,6 +254,158 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
>  
>  #else /* #ifndef uatomic_cmpxchg */
>  
> +#ifndef uatomic_and
> +/* uatomic_and */
> +
> +static inline __attribute__((always_inline))
> +unsigned long _uatomic_and(void *addr, unsigned long val, int len)
> +{
> +	switch (len) {
> +#ifdef UATOMIC_HAS_ATOMIC_BYTE
> +	case 1:
> +	{
> +		unsigned char old, oldt;
> +
> +		oldt = uatomic_read((unsigned char *)addr);
> +		do {
> +			old = oldt;
> +			oldt = uatomic_cmpxchg((unsigned char *)addr,
> +                                               old, old & val);
> +		} while (oldt != old);
> +
> +		return old;
> +	}
> +#endif
> +#ifdef UATOMIC_HAS_ATOMIC_SHORT
> +	case 2:
> +	{
> +		unsigned short old, oldt;
> +
> +		oldt = uatomic_read((unsigned short *)addr);
> +		do {
> +			old = oldt;
> +			oldt = uatomic_cmpxchg((unsigned short *)addr,
> +                                               old, old & val);
> +		} while (oldt != old);
> +
> +		return old;
> +	}
> +#endif
> +	case 4:
> +	{
> +		unsigned int old, oldt;
> +
> +		oldt = uatomic_read((unsigned int *)addr);
> +		do {
> +			old = oldt;
> +			oldt = uatomic_cmpxchg((unsigned int *)addr,
> +                                               old, old & val);
> +		} 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((unsigned long *)addr,
> +                                               old, old & val);
> +		} while (oldt != old);
> +
> +		return old;
> +	}
> +#endif
> +	}
> +	_uatomic_link_error();
> +	return 0;
> +}
> +
> +#define uatomic_and(addr, v)					\
> +	((__typeof__(*(addr))) _uatomic_and((addr),		\
> +					    (unsigned long)(v),	\
> +					    sizeof(*(addr))))
> +#endif /* #ifndef uatomic_and */
> +
> +#ifndef uatomic_or
> +/* uatomic_or */
> +
> +static inline __attribute__((always_inline))
> +unsigned long _uatomic_or(void *addr, unsigned long val, int len)
> +{
> +	switch (len) {
> +#ifdef UATOMIC_HAS_ATOMIC_BYTE
> +	case 1:
> +	{
> +		unsigned char old, oldt;
> +
> +		oldt = uatomic_read((unsigned char *)addr);
> +		do {
> +			old = oldt;
> +			oldt = uatomic_cmpxchg((unsigned char *)addr,
> +                                               old, old | val);
> +		} while (oldt != old);
> +
> +		return old;
> +	}
> +#endif
> +#ifdef UATOMIC_HAS_ATOMIC_SHORT
> +	case 2:
> +	{
> +		unsigned short old, oldt;
> +
> +		oldt = uatomic_read((unsigned short *)addr);
> +		do {
> +			old = oldt;
> +			oldt = uatomic_cmpxchg((unsigned short *)addr,
> +                                               old, old | val);
> +		} while (oldt != old);
> +
> +		return old;
> +	}
> +#endif
> +	case 4:
> +	{
> +		unsigned int old, oldt;
> +
> +		oldt = uatomic_read((unsigned int *)addr);
> +		do {
> +			old = oldt;
> +			oldt = uatomic_cmpxchg((unsigned int *)addr,
> +                                               old, old | val);
> +		} 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((unsigned long *)addr,
> +                                               old, old | val);
> +		} while (oldt != old);
> +
> +		return old;
> +	}
> +#endif
> +	}
> +	_uatomic_link_error();
> +	return 0;
> +}
> +
> +#define uatomic_or(addr, v)					\
> +	((__typeof__(*(addr))) _uatomic_or((addr),		\
> +					   (unsigned long)(v),	\
> +					   sizeof(*(addr))))
> +#endif /* #ifndef uatomic_or */
> +
>  #ifndef uatomic_add_return
>  /* uatomic_add_return */
>  
> -- 
> 1.7.3.2
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Paolo Bonzini]

On 11/12/2010 01:08 PM, Mathieu Desnoyers wrote:
> * Paolo Bonzini (pbonzini at redhat.com) wrote:
>> These are useful to flip single bits.
>
> I'm trying to figure out the semantic of these operations.
>
> are they
>
> uatomic_add / uatomic_or or uatomic_add_return / uatomic_or_return ?

They return the old value, consistently with their names (see also the 
test_uatomic.c change).  There are three possibilities:

- return nothing

- return the old value

- return the new value

Return nothing can be useful because it can be optimized on x86 as "lock 
orl (mem), reg/imm".  However, there are no other return-nothing atomic 
ops in uatomic_*.h so I decided not to provide this.

Returning the new value doesn't make sense for and/or since you cannot 
revert the operation (unlike uatomic_add which can be implemented from 
uatomic_add_return).

So I chose the second.

Paolo

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Mathieu Desnoyers]

* Paolo Bonzini (pbonzini at redhat.com) wrote:
> On 11/12/2010 01:08 PM, Mathieu Desnoyers wrote:
>> * Paolo Bonzini (pbonzini at redhat.com) wrote:
>>> These are useful to flip single bits.
>>
>> I'm trying to figure out the semantic of these operations.
>>
>> are they
>>
>> uatomic_add / uatomic_or or uatomic_add_return / uatomic_or_return ?
>
> They return the old value, consistently with their names (see also the  
> test_uatomic.c change).  There are three possibilities:
>
> - return nothing
>
> - return the old value
>
> - return the new value
>
> Return nothing can be useful because it can be optimized on x86 as "lock  
> orl (mem), reg/imm".  However, there are no other return-nothing atomic  
> ops in uatomic_*.h so I decided not to provide this.
>
> Returning the new value doesn't make sense for and/or since you cannot  
> revert the operation (unlike uatomic_add which can be implemented from  
> uatomic_add_return).

How about we start by implementing uatomic_and/uatomic_or that return
void, and if we ever need uatomic_return_and/uatomic_return_or (see my
other mail), we add them ?

Otherwise we'll be stucked with heavily synchronized operations to
return a value people just don't care about.

By the way, uatomic_add/uatomic_dec don't return anything.

Mathieu

>
> So I chose the second.
>
> Paolo
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Paolo Bonzini]

On 11/12/2010 01:19 PM, Mathieu Desnoyers wrote:
> * Paolo Bonzini (pbonzini at redhat.com) wrote:
>> Return nothing can be useful because it can be optimized on x86 as "lock
>> orl (mem), reg/imm".  However, there are no other return-nothing atomic
>> ops in uatomic_*.h so I decided not to provide this.
>>
>
> How about we start by implementing uatomic_and/uatomic_or that return
> void, and if we ever need uatomic_return_and/uatomic_return_or (see my
> other mail), we add them ?

I am using uatomic_or's return value in my call_rcu-with-futex, but I 
don't need atomic-or-and-exchange really, I can even change it to a load 
followed by an atomic or (it introduces a race but it is benign).  Or I 
can certainly use cmpxchg too.  Updated patches will come.

Should I rename uatomic_add to uatomic_xchg_add too (I prefer this name 
to uatomic_return_add)?

Paolo

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Paolo Bonzini]

On 11/12/2010 01:24 PM, Paolo Bonzini wrote:
> Should I rename uatomic_add to uatomic_xchg_add too (I prefer this name
> to uatomic_return_add)?

Uh, uatomic_add returns void actually.  Then returning void from 
uatomic_{and,or} is indeed more consistent.

Paolo

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Mathieu Desnoyers]

* Paolo Bonzini (pbonzini at redhat.com) wrote:
> On 11/12/2010 01:24 PM, Paolo Bonzini wrote:
>> Should I rename uatomic_add to uatomic_xchg_add too (I prefer this name
>> to uatomic_return_add)?
>
> Uh, uatomic_add returns void actually.  Then returning void from  
> uatomic_{and,or} is indeed more consistent.

Ah! I knew we'd find the culprit!! ;-)

Thanks,

Mathieu


-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Mathieu Desnoyers]

* Mathieu Desnoyers (compudj at krystal.dyndns.org) wrote:
> * Paolo Bonzini (pbonzini at redhat.com) wrote:
> > On 11/12/2010 01:24 PM, Paolo Bonzini wrote:
> >> Should I rename uatomic_add to uatomic_xchg_add too (I prefer this name
> >> to uatomic_return_add)?
> >
> > Uh, uatomic_add returns void actually.  Then returning void from  
> > uatomic_{and,or} is indeed more consistent.
> 
> Ah! I knew we'd find the culprit!! ;-)

BTW, I'd be fine with adding:

uatomic_and/uatomic_or (returning void)

But I wonder if, on any architecture, there is a significantly better
way to implement:

uatomic_xchg_add/uatomic_xchg_or (returning the old value)

other than using a cmpxchg() underneath ? If not, then creating this
extra primitive would be pretty much useless.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Paolo Bonzini]

On 11/12/2010 01:38 PM, Mathieu Desnoyers wrote:
> But I wonder if, on any architecture, there is a significantly better
> way to implement:
>
> uatomic_xchg_add/uatomic_xchg_or (returning the old value)
                ^^^

Assuming you mean "and" (for "add" there is an obvious benefit on x86 
which has XADD), there is some benefit on LL/SC architectures, where you 
can do

	ll  reg, mem
	and temp, reg, val
	sc  mem, temp
	<redo upon lost reservation>
	isync

instead of this more complicated code using cmpxchg:

   1:
	ld  temp1, mem		; normal load of old value
	and temp2, temp1, val	; compute new one
   2:
	ll  reg, mem		; reg = cmpxchg(&x, temp1, temp2)
	cmp reg, temp1
	bne 1b
	sc  mem, temp2
	<redo from 2 upon lost reservation>
	isync

Actually, what you get from uatomic_ppc.h is even a bit worse:

   1:
	ld  temp1, mem		; normal load of old value
	and temp2, temp1, val	; compute new one
   2:
	ll  reg, mem		; reg = cmpxchg(&x, temp1, temp2)
	cmp reg, temp1
	bne 3f
	sc  mem, temp2
	<redo from 2 upon lost reservation>
	isync
   3:
	cmp reg, temp1   	; compiler cannot optimize jump-to-jump
	bne 1b		 	; because "bne 3f" is inside an asm

Paolo

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Mathieu Desnoyers]

* Paolo Bonzini (pbonzini at redhat.com) wrote:
> On 11/12/2010 01:19 PM, Mathieu Desnoyers wrote:
>> * Paolo Bonzini (pbonzini at redhat.com) wrote:
>>> Return nothing can be useful because it can be optimized on x86 as "lock
>>> orl (mem), reg/imm".  However, there are no other return-nothing atomic
>>> ops in uatomic_*.h so I decided not to provide this.
>>>
>>
>> How about we start by implementing uatomic_and/uatomic_or that return
>> void, and if we ever need uatomic_return_and/uatomic_return_or (see my
>> other mail), we add them ?
>
> I am using uatomic_or's return value in my call_rcu-with-futex, but I  
> don't need atomic-or-and-exchange really, I can even change it to a load  
> followed by an atomic or (it introduces a race but it is benign).

If the race does not matter, why do we need the new atomic op in the
first place ? ;)

>  Or I  
> can certainly use cmpxchg too.  Updated patches will come.
>
> Should I rename uatomic_add to uatomic_xchg_add too (I prefer this name  
> to uatomic_return_add)?

Renaming uatomic_add ? I don't understand what you are trying to achieve
here. uatomic_add returns void, doesn't it ?

(generally speaking, I agree that uatomic_xchg_op is nicer than
uatomic_return_op)

Thanks,

Mathieu


-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Paolo Bonzini]

On 11/12/2010 01:29 PM, Mathieu Desnoyers wrote:
> * Paolo Bonzini (pbonzini at redhat.com) wrote:
>> On 11/12/2010 01:19 PM, Mathieu Desnoyers wrote:
>>> * Paolo Bonzini (pbonzini at redhat.com) wrote:
>>>> Return nothing can be useful because it can be optimized on x86
>>>> as "lock orl (mem), reg/imm".  However, there are no other
>>>> return-nothing atomic ops in uatomic_*.h so I decided not to
>>>> provide this.
>>>>
>>>
>>> How about we start by implementing uatomic_and/uatomic_or that
>>> return void, and if we ever need
>>> uatomic_return_and/uatomic_return_or (see my other mail), we add
>>> them ?
>>
>> I am using uatomic_or's return value in my call_rcu-with-futex, but
>> I don't need atomic-or-and-exchange really, I can even change it to
>> a load followed by an atomic or (it introduces a race but it is
>> benign).
>
> If the race does not matter, why do we need the new atomic op in the
> first place ? ;)

Say the "correct" code would be

    atomic x = *p, *p |= FLAG1;   # uatomic_xchg_or
    if (x & ~FLAG2)
        futex_wake (...)

I may try to rewrite it like this:

    x = *p;                       # 1
    atomic *p |= FLAG1;           # 2, uatomic_or
    if (x & ~FLAG2)
        futex_wake (...)

This is racy because something can set or reset FLAG2 between 1 and 2. 
Setting it may cause spurious wakeups, which are okay.  If I know that 
nothing can reset FLAG2 between 1 and 2, correctness is preserved.

Instead if I remove atomicity, I get races on read-modify-write 
operations, and that is much more obviously incorrect.

But I really do not like being tricky (futexes are tricky enough on 
their own), and rather than spending a lot of time proving this correct 
I'd probably just use cmpxchg here.

Paolo

[ltt-dev] [PATCH 2/2] introduce uatomic_and and uatomic_or

[Mathieu Desnoyers]

* Mathieu Desnoyers (compudj at krystal.dyndns.org) wrote:
> * Paolo Bonzini (pbonzini at redhat.com) wrote:
> > These are useful to flip single bits.
> 
> I'm trying to figure out the semantic of these operations.
> 
> are they
> 
> uatomic_add / uatomic_or or uatomic_add_return / uatomic_or_return ?

It looks like they return the previous value of the variable. So for
consistency with "uatomic_add_return", we should return the value after
the modification. However, given that we're changing the value with the
mask in a way that's not reversible, this means we would lose
information for "and" and "or".

One way we could do it is to specify that what we return is the value
before the op like this:

uatomic_return_add()
uatomic_return_or()

The other question is:

do we really want the "uatomic_or/uatomic_and" to return something ?

Thoughts ?

Mathieu

> 
> Thanks,
> 
> Mathieu
> 
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> > ---
> >  tests/test_uatomic.c   |    6 ++
> >  urcu/uatomic_generic.h |  220 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 226 insertions(+), 0 deletions(-)
> > 
> > diff --git a/tests/test_uatomic.c b/tests/test_uatomic.c
> > index 5682655..f14ed56 100644
> > --- a/tests/test_uatomic.c
> > +++ b/tests/test_uatomic.c
> > @@ -39,6 +39,12 @@ do {						\
> >  	v = uatomic_sub_return(ptr, 1);		\
> >  	assert(v == 121);			\
> >  	assert(uatomic_read(ptr) == 121);	\
> > +	v = uatomic_and(ptr, 0x47);		\
> > +	assert(v == 121);			\
> > +	assert(uatomic_read(ptr) == 0x41);	\
> > +	v = uatomic_or(ptr, 120);		\
> > +	assert(v == 0x41);			\
> > +	assert(uatomic_read(ptr) == 121);	\
> >  } while (0)
> >  
> >  int main(int argc, char **argv)
> > diff --git a/urcu/uatomic_generic.h b/urcu/uatomic_generic.h
> > index 383ddfa..7f3bc00 100644
> > --- a/urcu/uatomic_generic.h
> > +++ b/urcu/uatomic_generic.h
> > @@ -87,6 +87,74 @@ unsigned long _uatomic_cmpxchg(void *addr, unsigned long old,
> >  						sizeof(*(addr))))
> >  
> >  
> > +/* uatomic_and */
> > +
> > +#ifndef uatomic_and
> > +static inline __attribute__((always_inline))
> > +unsigned long _uatomic_and(void *addr, unsigned long val,
> > +				 int len)
> > +{
> > +	switch (len) {
> > +#ifdef UATOMIC_HAS_ATOMIC_BYTE
> > +	case 1:
> > +		return __sync_fetch_and_and_1(addr, val);
> > +#endif
> > +#ifdef UATOMIC_HAS_ATOMIC_SHORT
> > +	case 2:
> > +		return __sync_fetch_and_and_2(addr, val);
> > +#endif
> > +	case 4:
> > +		return __sync_fetch_and_and_4(addr, val);
> > +#if (BITS_PER_LONG == 64)
> > +	case 8:
> > +		return __sync_fetch_and_and_8(addr, val);
> > +#endif
> > +	}
> > +	_uatomic_link_error();
> > +	return 0;
> > +}
> > +
> > +
> > +#define uatomic_and(addr, v)					\
> > +	((__typeof__(*(addr))) _uatomic_and((addr),		\
> > +					    (unsigned long)(v),	\
> > +					    sizeof(*(addr))))
> > +#endif /* #ifndef uatomic_and */
> > +
> > +/* uatomic_or */
> > +
> > +#ifndef uatomic_or
> > +static inline __attribute__((always_inline))
> > +unsigned long _uatomic_or(void *addr, unsigned long val,
> > +				 int len)
> > +{
> > +	switch (len) {
> > +#ifdef UATOMIC_HAS_ATOMIC_BYTE
> > +	case 1:
> > +		return __sync_fetch_and_or_1(addr, val);
> > +#endif
> > +#ifdef UATOMIC_HAS_ATOMIC_SHORT
> > +	case 2:
> > +		return __sync_fetch_and_or_2(addr, val);
> > +#endif
> > +	case 4:
> > +		return __sync_fetch_and_or_4(addr, val);
> > +#if (BITS_PER_LONG == 64)
> > +	case 8:
> > +		return __sync_fetch_and_or_8(addr, val);
> > +#endif
> > +	}
> > +	_uatomic_link_error();
> > +	return 0;
> > +}
> > +
> > +
> > +#define uatomic_or(addr, v)					\
> > +	((__typeof__(*(addr))) _uatomic_or((addr),		\
> > +					   (unsigned long)(v),	\
> > +					   sizeof(*(addr))))
> > +#endif /* #ifndef uatomic_or */
> > +
> >  /* uatomic_add_return */
> >  
> >  #ifndef uatomic_add_return
> > @@ -186,6 +254,158 @@ unsigned long _uatomic_exchange(void *addr, unsigned long val, int len)
> >  
> >  #else /* #ifndef uatomic_cmpxchg */
> >  
> > +#ifndef uatomic_and
> > +/* uatomic_and */
> > +
> > +static inline __attribute__((always_inline))
> > +unsigned long _uatomic_and(void *addr, unsigned long val, int len)
> > +{
> > +	switch (len) {
> > +#ifdef UATOMIC_HAS_ATOMIC_BYTE
> > +	case 1:
> > +	{
> > +		unsigned char old, oldt;
> > +
> > +		oldt = uatomic_read((unsigned char *)addr);
> > +		do {
> > +			old = oldt;
> > +			oldt = uatomic_cmpxchg((unsigned char *)addr,
> > +                                               old, old & val);
> > +		} while (oldt != old);
> > +
> > +		return old;
> > +	}
> > +#endif
> > +#ifdef UATOMIC_HAS_ATOMIC_SHORT
> > +	case 2:
> > +	{
> > +		unsigned short old, oldt;
> > +
> > +		oldt = uatomic_read((unsigned short *)addr);
> > +		do {
> > +			old = oldt;
> > +			oldt = uatomic_cmpxchg((unsigned short *)addr,
> > +                                               old, old & val);
> > +		} while (oldt != old);
> > +
> > +		return old;
> > +	}
> > +#endif
> > +	case 4:
> > +	{
> > +		unsigned int old, oldt;
> > +
> > +		oldt = uatomic_read((unsigned int *)addr);
> > +		do {
> > +			old = oldt;
> > +			oldt = uatomic_cmpxchg((unsigned int *)addr,
> > +                                               old, old & val);
> > +		} 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((unsigned long *)addr,
> > +                                               old, old & val);
> > +		} while (oldt != old);
> > +
> > +		return old;
> > +	}
> > +#endif
> > +	}
> > +	_uatomic_link_error();
> > +	return 0;
> > +}
> > +
> > +#define uatomic_and(addr, v)					\
> > +	((__typeof__(*(addr))) _uatomic_and((addr),		\
> > +					    (unsigned long)(v),	\
> > +					    sizeof(*(addr))))
> > +#endif /* #ifndef uatomic_and */
> > +
> > +#ifndef uatomic_or
> > +/* uatomic_or */
> > +
> > +static inline __attribute__((always_inline))
> > +unsigned long _uatomic_or(void *addr, unsigned long val, int len)
> > +{
> > +	switch (len) {
> > +#ifdef UATOMIC_HAS_ATOMIC_BYTE
> > +	case 1:
> > +	{
> > +		unsigned char old, oldt;
> > +
> > +		oldt = uatomic_read((unsigned char *)addr);
> > +		do {
> > +			old = oldt;
> > +			oldt = uatomic_cmpxchg((unsigned char *)addr,
> > +                                               old, old | val);
> > +		} while (oldt != old);
> > +
> > +		return old;
> > +	}
> > +#endif
> > +#ifdef UATOMIC_HAS_ATOMIC_SHORT
> > +	case 2:
> > +	{
> > +		unsigned short old, oldt;
> > +
> > +		oldt = uatomic_read((unsigned short *)addr);
> > +		do {
> > +			old = oldt;
> > +			oldt = uatomic_cmpxchg((unsigned short *)addr,
> > +                                               old, old | val);
> > +		} while (oldt != old);
> > +
> > +		return old;
> > +	}
> > +#endif
> > +	case 4:
> > +	{
> > +		unsigned int old, oldt;
> > +
> > +		oldt = uatomic_read((unsigned int *)addr);
> > +		do {
> > +			old = oldt;
> > +			oldt = uatomic_cmpxchg((unsigned int *)addr,
> > +                                               old, old | val);
> > +		} 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((unsigned long *)addr,
> > +                                               old, old | val);
> > +		} while (oldt != old);
> > +
> > +		return old;
> > +	}
> > +#endif
> > +	}
> > +	_uatomic_link_error();
> > +	return 0;
> > +}
> > +
> > +#define uatomic_or(addr, v)					\
> > +	((__typeof__(*(addr))) _uatomic_or((addr),		\
> > +					   (unsigned long)(v),	\
> > +					   sizeof(*(addr))))
> > +#endif /* #ifndef uatomic_or */
> > +
> >  #ifndef uatomic_add_return
> >  /* uatomic_add_return */
> >  
> > -- 
> > 1.7.3.2
> > 
> > 
> > _______________________________________________
> > ltt-dev mailing list
> > ltt-dev at lists.casi.polymtl.ca
> > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> > 
> 
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com