* [PATCH 0/2] <asm/atomic.h> adjustments
@ 2016-07-13 11:22 Corneliu ZUZU
2016-07-13 11:23 ` [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h) Corneliu ZUZU
2016-07-13 11:23 ` [PATCH 2/2] fix: make atomic_read() param const Corneliu ZUZU
0 siblings, 2 replies; 10+ messages in thread
From: Corneliu ZUZU @ 2016-07-13 11:22 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Jan Beulich
Corneliu ZUZU (2):
asm/atomic.h: common prototyping (add xen/atomic.h)
fix: make atomic_read() param const
xen/include/asm-arm/arm32/atomic.h | 11 ---
xen/include/asm-arm/arm64/atomic.h | 11 ---
xen/include/asm-arm/atomic.h | 91 +++++++++++++++++++---
xen/include/asm-x86/atomic.h | 51 +++++-------
xen/include/xen/atomic.h | 155 +++++++++++++++++++++++++++++++++++++
5 files changed, 257 insertions(+), 62 deletions(-)
create mode 100644 xen/include/xen/atomic.h
--
2.5.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)
2016-07-13 11:22 [PATCH 0/2] <asm/atomic.h> adjustments Corneliu ZUZU
@ 2016-07-13 11:23 ` Corneliu ZUZU
2016-07-13 12:12 ` Andrew Cooper
2016-07-13 12:14 ` Julien Grall
2016-07-13 11:23 ` [PATCH 2/2] fix: make atomic_read() param const Corneliu ZUZU
1 sibling, 2 replies; 10+ messages in thread
From: Corneliu ZUZU @ 2016-07-13 11:23 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Jan Beulich
Following Andrew Cooper's suggestion, create a common-side <xen/atomic.h> to
establish, among others, prototypes of atomic functions called from common-code.
Done to avoid introducing inconsistencies between arch-side <asm/atomic.h>
headers when we make subtle changes to one of them.
Some arm-side macros had to be turned into inline functions in the process.
Also includes a minor adjustment asm-x86/atomic.h: reorder atomic_inc_and_test()
to follow after atomic_inc().
Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
---
xen/include/asm-arm/arm32/atomic.h | 11 ---
xen/include/asm-arm/arm64/atomic.h | 11 ---
xen/include/asm-arm/atomic.h | 89 ++++++++++++++++++---
xen/include/asm-x86/atomic.h | 49 +++++-------
xen/include/xen/atomic.h | 155 +++++++++++++++++++++++++++++++++++++
5 files changed, 255 insertions(+), 60 deletions(-)
create mode 100644 xen/include/xen/atomic.h
diff --git a/xen/include/asm-arm/arm32/atomic.h b/xen/include/asm-arm/arm32/atomic.h
index 7ec712f..be08ff1 100644
--- a/xen/include/asm-arm/arm32/atomic.h
+++ b/xen/include/asm-arm/arm32/atomic.h
@@ -149,17 +149,6 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u)
#define atomic_xchg(v, new) (xchg(&((v)->counter), new))
-#define atomic_inc(v) atomic_add(1, v)
-#define atomic_dec(v) atomic_sub(1, v)
-
-#define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0)
-#define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0)
-#define atomic_inc_return(v) (atomic_add_return(1, v))
-#define atomic_dec_return(v) (atomic_sub_return(1, v))
-#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0)
-
-#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
-
#endif /* __ARCH_ARM_ARM32_ATOMIC__ */
/*
* Local variables:
diff --git a/xen/include/asm-arm/arm64/atomic.h b/xen/include/asm-arm/arm64/atomic.h
index b49219e..80d07bf 100644
--- a/xen/include/asm-arm/arm64/atomic.h
+++ b/xen/include/asm-arm/arm64/atomic.h
@@ -125,17 +125,6 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u)
return c;
}
-#define atomic_inc(v) atomic_add(1, v)
-#define atomic_dec(v) atomic_sub(1, v)
-
-#define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0)
-#define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0)
-#define atomic_inc_return(v) (atomic_add_return(1, v))
-#define atomic_dec_return(v) (atomic_sub_return(1, v))
-#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0)
-
-#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
-
#endif
/*
* Local variables:
diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 29ab265..8e8c5d1 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -2,6 +2,7 @@
#define __ARCH_ARM_ATOMIC__
#include <xen/config.h>
+#include <xen/atomic.h>
#include <xen/prefetch.h>
#include <asm/system.h>
@@ -95,15 +96,6 @@ void __bad_atomic_size(void);
default: __bad_atomic_size(); break; \
} \
})
-
-/*
- * NB. I've pushed the volatile qualifier into the operations. This allows
- * fast accessors such as _atomic_read() and _atomic_set() which don't give
- * the compiler a fit.
- */
-typedef struct { int counter; } atomic_t;
-
-#define ATOMIC_INIT(i) { (i) }
/*
* On ARM, ordinary assignment (str instruction) doesn't clear the local
@@ -138,6 +130,85 @@ static inline void _atomic_set(atomic_t *v, int i)
# error "unknown ARM variant"
#endif
+#define atomic_inc_return(v) (atomic_add_return(1, v))
+#define atomic_dec_return(v) (atomic_sub_return(1, v))
+
+/**
+ * atomic_sub_and_test - subtract value from variable and test result
+ * @i: integer value to subtract
+ * @v: pointer of type atomic_t
+ *
+ * Atomically subtracts @i from @v and returns
+ * true if the result is zero, or false for all
+ * other cases.
+ */
+static inline int atomic_sub_and_test(int i, atomic_t *v)
+{
+ return 0 == atomic_sub_return(i, v);
+}
+
+/**
+ * atomic_inc - increment atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically increments @v by 1.
+ */
+static inline void atomic_inc(atomic_t *v)
+{
+ atomic_add(1, v);
+}
+
+/**
+ * atomic_inc_and_test - increment and test
+ * @v: pointer of type atomic_t
+ *
+ * Atomically increments @v by 1
+ * and returns true if the result is zero, or false for all
+ * other cases.
+ */
+static inline int atomic_inc_and_test(atomic_t *v)
+{
+ return 0 == atomic_add_return(1, v);
+}
+
+/**
+ * atomic_dec - decrement atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically decrements @v by 1.
+ */
+static inline void atomic_dec(atomic_t *v)
+{
+ atomic_sub(1, v);
+}
+
+/**
+ * atomic_dec_and_test - decrement and test
+ * @v: pointer of type atomic_t
+ *
+ * Atomically decrements @v by 1 and
+ * returns true if the result is 0, or false for all other
+ * cases.
+ */
+static inline int atomic_dec_and_test(atomic_t *v)
+{
+ return 0 == atomic_sub_return(1, v);
+}
+
+/**
+ * atomic_add_negative - add and test if negative
+ * @v: pointer of type atomic_t
+ * @i: integer value to add
+ *
+ * Atomically adds @i to @v and returns true
+ * if the result is negative, or false when
+ * result is greater than or equal to zero.
+ */
+static inline int atomic_add_negative(int i, atomic_t *v)
+{
+ return atomic_add_return(i, v) < 0;
+}
+
#endif /* __ARCH_ARM_ATOMIC__ */
/*
* Local variables:
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index d246b70..9bf4bc1 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -2,6 +2,7 @@
#define __ARCH_X86_ATOMIC__
#include <xen/config.h>
+#include <xen/atomic.h>
#include <asm/system.h>
#define build_read_atomic(name, size, type, reg, barrier) \
@@ -79,15 +80,6 @@ void __bad_atomic_size(void);
} \
})
-/*
- * NB. I've pushed the volatile qualifier into the operations. This allows
- * fast accessors such as _atomic_read() and _atomic_set() which don't give
- * the compiler a fit.
- */
-typedef struct { int counter; } atomic_t;
-
-#define ATOMIC_INIT(i) { (i) }
-
/**
* atomic_read - read atomic variable
* @v: pointer of type atomic_t
@@ -110,7 +102,6 @@ static inline int _atomic_read(atomic_t v)
return v.counter;
}
-
/**
* atomic_set - set atomic variable
* @v: pointer of type atomic_t
@@ -217,6 +208,25 @@ static inline void atomic_inc(atomic_t *v)
}
/**
+ * atomic_inc_and_test - increment and test
+ * @v: pointer of type atomic_t
+ *
+ * Atomically increments @v by 1
+ * and returns true if the result is zero, or false for all
+ * other cases.
+ */
+static inline int atomic_inc_and_test(atomic_t *v)
+{
+ unsigned char c;
+
+ asm volatile (
+ "lock; incl %0; sete %1"
+ : "=m" (*(volatile int *)&v->counter), "=qm" (c)
+ : "m" (*(volatile int *)&v->counter) : "memory" );
+ return c != 0;
+}
+
+/**
* atomic_dec - decrement atomic variable
* @v: pointer of type atomic_t
*
@@ -250,25 +260,6 @@ static inline int atomic_dec_and_test(atomic_t *v)
}
/**
- * atomic_inc_and_test - increment and test
- * @v: pointer of type atomic_t
- *
- * Atomically increments @v by 1
- * and returns true if the result is zero, or false for all
- * other cases.
- */
-static inline int atomic_inc_and_test(atomic_t *v)
-{
- unsigned char c;
-
- asm volatile (
- "lock; incl %0; sete %1"
- : "=m" (*(volatile int *)&v->counter), "=qm" (c)
- : "m" (*(volatile int *)&v->counter) : "memory" );
- return c != 0;
-}
-
-/**
* atomic_add_negative - add and test if negative
* @v: pointer of type atomic_t
* @i: integer value to add
diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
new file mode 100644
index 0000000..5d5c051
--- /dev/null
+++ b/xen/include/xen/atomic.h
@@ -0,0 +1,155 @@
+/*
+ * include/xen/atomic.h
+ *
+ * Common atomic operations entities (atomic_t, function prototypes).
+ * Include _from_ arch-side <asm/atomic.h>.
+ *
+ * Copyright (c) 2016 Bitdefender S.R.L.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __XEN_ATOMIC_H__
+#define __XEN_ATOMIC_H__
+
+/*
+ * NB. I've pushed the volatile qualifier into the operations. This allows
+ * fast accessors such as _atomic_read() and _atomic_set() which don't give
+ * the compiler a fit.
+ */
+typedef struct { int counter; } atomic_t;
+
+#define ATOMIC_INIT(i) { (i) }
+
+/**
+ * atomic_read - read atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically reads the value of @v.
+ */
+static inline int atomic_read(atomic_t *v);
+
+/**
+ * _atomic_read - read atomic variable non-atomically
+ * @v atomic_t
+ *
+ * Non-atomically reads the value of @v
+ */
+static inline int _atomic_read(atomic_t v);
+
+/**
+ * atomic_set - set atomic variable
+ * @v: pointer of type atomic_t
+ * @i: required value
+ *
+ * Atomically sets the value of @v to @i.
+ */
+static inline void atomic_set(atomic_t *v, int i);
+
+/**
+ * _atomic_set - set atomic variable non-atomically
+ * @v: pointer of type atomic_t
+ * @i: required value
+ *
+ * Non-atomically sets the value of @v to @i.
+ */
+static inline void _atomic_set(atomic_t *v, int i);
+
+static inline int atomic_cmpxchg(atomic_t *v, int old, int new);
+
+/**
+ * atomic_add - add integer to atomic variable
+ * @i: integer value to add
+ * @v: pointer of type atomic_t
+ *
+ * Atomically adds @i to @v.
+ */
+static inline void atomic_add(int i, atomic_t *v);
+
+/**
+ * atomic_add_return - add integer and return
+ * @i: integer value to add
+ * @v: pointer of type atomic_t
+ *
+ * Atomically adds @i to @v and returns @i + @v
+ */
+static inline int atomic_add_return(int i, atomic_t *v);
+
+/**
+ * atomic_sub - subtract the atomic variable
+ * @i: integer value to subtract
+ * @v: pointer of type atomic_t
+ *
+ * Atomically subtracts @i from @v.
+ */
+static inline void atomic_sub(int i, atomic_t *v);
+
+/**
+ * atomic_sub_and_test - subtract value from variable and test result
+ * @i: integer value to subtract
+ * @v: pointer of type atomic_t
+ *
+ * Atomically subtracts @i from @v and returns
+ * true if the result is zero, or false for all
+ * other cases.
+ */
+static inline int atomic_sub_and_test(int i, atomic_t *v);
+
+/**
+ * atomic_inc - increment atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically increments @v by 1.
+ */
+static inline void atomic_inc(atomic_t *v);
+
+/**
+ * atomic_inc_and_test - increment and test
+ * @v: pointer of type atomic_t
+ *
+ * Atomically increments @v by 1
+ * and returns true if the result is zero, or false for all
+ * other cases.
+ */
+static inline int atomic_inc_and_test(atomic_t *v);
+
+/**
+ * atomic_dec - decrement atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically decrements @v by 1.
+ */
+static inline void atomic_dec(atomic_t *v);
+
+/**
+ * atomic_dec_and_test - decrement and test
+ * @v: pointer of type atomic_t
+ *
+ * Atomically decrements @v by 1 and
+ * returns true if the result is 0, or false for all other
+ * cases.
+ */
+static inline int atomic_dec_and_test(atomic_t *v);
+
+/**
+ * atomic_add_negative - add and test if negative
+ * @v: pointer of type atomic_t
+ * @i: integer value to add
+ *
+ * Atomically adds @i to @v and returns true
+ * if the result is negative, or false when
+ * result is greater than or equal to zero.
+ */
+static inline int atomic_add_negative(int i, atomic_t *v);
+
+#endif /* __XEN_ATOMIC_H__ */
--
2.5.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] fix: make atomic_read() param const
2016-07-13 11:22 [PATCH 0/2] <asm/atomic.h> adjustments Corneliu ZUZU
2016-07-13 11:23 ` [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h) Corneliu ZUZU
@ 2016-07-13 11:23 ` Corneliu ZUZU
2016-07-13 12:12 ` Andrew Cooper
1 sibling, 1 reply; 10+ messages in thread
From: Corneliu ZUZU @ 2016-07-13 11:23 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Jan Beulich
This wouldn't let me make a param of a function that used atomic_read() const.
Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
---
xen/include/asm-arm/atomic.h | 2 +-
xen/include/asm-x86/atomic.h | 2 +-
xen/include/xen/atomic.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 8e8c5d1..cd86cf0 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -102,7 +102,7 @@ void __bad_atomic_size(void);
* strex/ldrex monitor on some implementations. The reason we can use it for
* atomic_set() is the clrex or dummy strex done on every exception return.
*/
-static inline int atomic_read(atomic_t *v)
+static inline int atomic_read(const atomic_t *v)
{
return *(volatile int *)&v->counter;
}
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index 9bf4bc1..a16f0f6 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -86,7 +86,7 @@ void __bad_atomic_size(void);
*
* Atomically reads the value of @v.
*/
-static inline int atomic_read(atomic_t *v)
+static inline int atomic_read(const atomic_t *v)
{
return read_atomic(&v->counter);
}
diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
index 5d5c051..6cbc6d8 100644
--- a/xen/include/xen/atomic.h
+++ b/xen/include/xen/atomic.h
@@ -37,7 +37,7 @@ typedef struct { int counter; } atomic_t;
*
* Atomically reads the value of @v.
*/
-static inline int atomic_read(atomic_t *v);
+static inline int atomic_read(const atomic_t *v);
/**
* _atomic_read - read atomic variable non-atomically
--
2.5.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)
2016-07-13 11:23 ` [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h) Corneliu ZUZU
@ 2016-07-13 12:12 ` Andrew Cooper
2016-07-13 12:24 ` Corneliu ZUZU
2016-07-13 13:20 ` Corneliu ZUZU
2016-07-13 12:14 ` Julien Grall
1 sibling, 2 replies; 10+ messages in thread
From: Andrew Cooper @ 2016-07-13 12:12 UTC (permalink / raw)
To: Corneliu ZUZU, xen-devel; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich
On 13/07/16 12:23, Corneliu ZUZU wrote:
> Following Andrew Cooper's suggestion, create a common-side <xen/atomic.h> to
> establish, among others, prototypes of atomic functions called from common-code.
> Done to avoid introducing inconsistencies between arch-side <asm/atomic.h>
> headers when we make subtle changes to one of them.
>
> Some arm-side macros had to be turned into inline functions in the process.
>
> Also includes a minor adjustment asm-x86/atomic.h: reorder atomic_inc_and_test()
> to follow after atomic_inc().
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
Thanks for doing this!
> diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
> new file mode 100644
> index 0000000..5d5c051
> --- /dev/null
> +++ b/xen/include/xen/atomic.h
> @@ -0,0 +1,155 @@
> +/*
> + * include/xen/atomic.h
> + *
> + * Common atomic operations entities (atomic_t, function prototypes).
> + * Include _from_ arch-side <asm/atomic.h>.
> + *
> + * Copyright (c) 2016 Bitdefender S.R.L.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __XEN_ATOMIC_H__
> +#define __XEN_ATOMIC_H__
> +
> +/*
> + * NB. I've pushed the volatile qualifier into the operations. This allows
> + * fast accessors such as _atomic_read() and _atomic_set() which don't give
> + * the compiler a fit.
> + */
I would recommend simply dropping this paragraph. Is is very out of date.
> +typedef struct { int counter; } atomic_t;
> +
> +#define ATOMIC_INIT(i) { (i) }
> +
> +/**
> + * atomic_read - read atomic variable
> + * @v: pointer of type atomic_t
> + *
> + * Atomically reads the value of @v.
> + */
> +static inline int atomic_read(atomic_t *v);
> +
> +/**
> + * _atomic_read - read atomic variable non-atomically
> + * @v atomic_t
> + *
> + * Non-atomically reads the value of @v
> + */
> +static inline int _atomic_read(atomic_t v);
> +
> +/**
> + * atomic_set - set atomic variable
> + * @v: pointer of type atomic_t
> + * @i: required value
> + *
> + * Atomically sets the value of @v to @i.
> + */
> +static inline void atomic_set(atomic_t *v, int i);
> +
> +/**
> + * _atomic_set - set atomic variable non-atomically
> + * @v: pointer of type atomic_t
> + * @i: required value
> + *
> + * Non-atomically sets the value of @v to @i.
> + */
> +static inline void _atomic_set(atomic_t *v, int i);
> +
/**
* atomic_cmpxchg - compare and exchange an atomic variable
*...
*/
> +static inline int atomic_cmpxchg(atomic_t *v, int old, int new);
With those changes, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] fix: make atomic_read() param const
2016-07-13 11:23 ` [PATCH 2/2] fix: make atomic_read() param const Corneliu ZUZU
@ 2016-07-13 12:12 ` Andrew Cooper
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Cooper @ 2016-07-13 12:12 UTC (permalink / raw)
To: Corneliu ZUZU, xen-devel; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich
[-- Attachment #1.1: Type: text/plain, Size: 234 bytes --]
On 13/07/16 12:23, Corneliu ZUZU wrote:
> This wouldn't let me make a param of a function that used atomic_read() const.
>
> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
[-- Attachment #1.2: Type: text/html, Size: 787 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)
2016-07-13 11:23 ` [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h) Corneliu ZUZU
2016-07-13 12:12 ` Andrew Cooper
@ 2016-07-13 12:14 ` Julien Grall
2016-07-13 12:27 ` Corneliu ZUZU
1 sibling, 1 reply; 10+ messages in thread
From: Julien Grall @ 2016-07-13 12:14 UTC (permalink / raw)
To: Corneliu ZUZU, xen-devel; +Cc: Andrew Cooper, Stefano Stabellini, Jan Beulich
Hi Corneliu,
On 13/07/2016 12:23, Corneliu ZUZU wrote:
> Following Andrew Cooper's suggestion, create a common-side <xen/atomic.h> to
> establish, among others, prototypes of atomic functions called from common-code.
> Done to avoid introducing inconsistencies between arch-side <asm/atomic.h>
> headers when we make subtle changes to one of them.
>
> Some arm-side macros had to be turned into inline functions in the process.
You forgot to mention that you moved the code from
asm-arm/arm{32,64}/atomic.h to asm-arm/arm.h.
Furthermore, this change should really be a separate patch.
>
> Also includes a minor adjustment asm-x86/atomic.h: reorder atomic_inc_and_test()
> to follow after atomic_inc().
>
> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
[...]
> diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
> index 29ab265..8e8c5d1 100644
> --- a/xen/include/asm-arm/atomic.h
> +++ b/xen/include/asm-arm/atomic.h
> @@ -2,6 +2,7 @@
> #define __ARCH_ARM_ATOMIC__
>
> #include <xen/config.h>
> +#include <xen/atomic.h>
> #include <xen/prefetch.h>
> #include <asm/system.h>
>
> @@ -95,15 +96,6 @@ void __bad_atomic_size(void);
> default: __bad_atomic_size(); break; \
> } \
> })
> -
> -/*
> - * NB. I've pushed the volatile qualifier into the operations. This allows
> - * fast accessors such as _atomic_read() and _atomic_set() which don't give
> - * the compiler a fit.
> - */
> -typedef struct { int counter; } atomic_t;
> -
> -#define ATOMIC_INIT(i) { (i) }
>
> /*
> * On ARM, ordinary assignment (str instruction) doesn't clear the local
> @@ -138,6 +130,85 @@ static inline void _atomic_set(atomic_t *v, int i)
> # error "unknown ARM variant"
> #endif
>
> +#define atomic_inc_return(v) (atomic_add_return(1, v))
> +#define atomic_dec_return(v) (atomic_sub_return(1, v))
> +
> +/**
> + * atomic_sub_and_test - subtract value from variable and test result
> + * @i: integer value to subtract
> + * @v: pointer of type atomic_t
> + *
> + * Atomically subtracts @i from @v and returns
> + * true if the result is zero, or false for all
> + * other cases.
> + */
> +static inline int atomic_sub_and_test(int i, atomic_t *v)
> +{
> + return 0 == atomic_sub_return(i, v);
> +}
Please don't use yoda condition. It's less readable and compiler have
safety nowadays to prevent using "=".
> +
> +/**
> + * atomic_inc - increment atomic variable
> + * @v: pointer of type atomic_t
> + *
> + * Atomically increments @v by 1.
> + */
> +static inline void atomic_inc(atomic_t *v)
> +{
> + atomic_add(1, v);
> +}
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)
2016-07-13 12:12 ` Andrew Cooper
@ 2016-07-13 12:24 ` Corneliu ZUZU
2016-07-13 13:20 ` Corneliu ZUZU
1 sibling, 0 replies; 10+ messages in thread
From: Corneliu ZUZU @ 2016-07-13 12:24 UTC (permalink / raw)
To: Andrew Cooper, xen-devel; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich
On 7/13/2016 3:12 PM, Andrew Cooper wrote:
> On 13/07/16 12:23, Corneliu ZUZU wrote:
>> Following Andrew Cooper's suggestion, create a common-side <xen/atomic.h> to
>> establish, among others, prototypes of atomic functions called from common-code.
>> Done to avoid introducing inconsistencies between arch-side <asm/atomic.h>
>> headers when we make subtle changes to one of them.
>>
>> Some arm-side macros had to be turned into inline functions in the process.
>>
>> Also includes a minor adjustment asm-x86/atomic.h: reorder atomic_inc_and_test()
>> to follow after atomic_inc().
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Nice, didn't know that.
>
>
>> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
> Thanks for doing this!
>
>> diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
>> new file mode 100644
>> index 0000000..5d5c051
>> --- /dev/null
>> +++ b/xen/include/xen/atomic.h
>> @@ -0,0 +1,155 @@
>> +/*
>> + * include/xen/atomic.h
>> + *
>> + * Common atomic operations entities (atomic_t, function prototypes).
>> + * Include _from_ arch-side <asm/atomic.h>.
>> + *
>> + * Copyright (c) 2016 Bitdefender S.R.L.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program; If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef __XEN_ATOMIC_H__
>> +#define __XEN_ATOMIC_H__
>> +
>> +/*
>> + * NB. I've pushed the volatile qualifier into the operations. This allows
>> + * fast accessors such as _atomic_read() and _atomic_set() which don't give
>> + * the compiler a fit.
>> + */
> I would recommend simply dropping this paragraph. Is is very out of date.
Ack.
>
>> +typedef struct { int counter; } atomic_t;
>> +
>> +#define ATOMIC_INIT(i) { (i) }
>> +
>> +/**
>> + * atomic_read - read atomic variable
>> + * @v: pointer of type atomic_t
>> + *
>> + * Atomically reads the value of @v.
>> + */
>> +static inline int atomic_read(atomic_t *v);
>> +
>> +/**
>> + * _atomic_read - read atomic variable non-atomically
>> + * @v atomic_t
>> + *
>> + * Non-atomically reads the value of @v
>> + */
>> +static inline int _atomic_read(atomic_t v);
>> +
>> +/**
>> + * atomic_set - set atomic variable
>> + * @v: pointer of type atomic_t
>> + * @i: required value
>> + *
>> + * Atomically sets the value of @v to @i.
>> + */
>> +static inline void atomic_set(atomic_t *v, int i);
>> +
>> +/**
>> + * _atomic_set - set atomic variable non-atomically
>> + * @v: pointer of type atomic_t
>> + * @i: required value
>> + *
>> + * Non-atomically sets the value of @v to @i.
>> + */
>> +static inline void _atomic_set(atomic_t *v, int i);
>> +
> /**
> * atomic_cmpxchg - compare and exchange an atomic variable
> *...
> */
Ack.
>
>> +static inline int atomic_cmpxchg(atomic_t *v, int old, int new);
> With those changes, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Thanks too for the prompt review,
Zuzu C.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)
2016-07-13 12:14 ` Julien Grall
@ 2016-07-13 12:27 ` Corneliu ZUZU
0 siblings, 0 replies; 10+ messages in thread
From: Corneliu ZUZU @ 2016-07-13 12:27 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Andrew Cooper, Stefano Stabellini, Jan Beulich
Hi Julien,
On 7/13/2016 3:14 PM, Julien Grall wrote:
> Hi Corneliu,
>
> On 13/07/2016 12:23, Corneliu ZUZU wrote:
>> Following Andrew Cooper's suggestion, create a common-side
>> <xen/atomic.h> to
>> establish, among others, prototypes of atomic functions called from
>> common-code.
>> Done to avoid introducing inconsistencies between arch-side
>> <asm/atomic.h>
>> headers when we make subtle changes to one of them.
>>
>> Some arm-side macros had to be turned into inline functions in the
>> process.
>
> You forgot to mention that you moved the code from
> asm-arm/arm{32,64}/atomic.h to asm-arm/arm.h.
>
> Furthermore, this change should really be a separate patch.
Noted, will do.
>
>>
>> Also includes a minor adjustment asm-x86/atomic.h: reorder
>> atomic_inc_and_test()
>> to follow after atomic_inc().
>>
>> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
>
> [...]
>
>> diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
>> index 29ab265..8e8c5d1 100644
>> --- a/xen/include/asm-arm/atomic.h
>> +++ b/xen/include/asm-arm/atomic.h
>> @@ -2,6 +2,7 @@
>> #define __ARCH_ARM_ATOMIC__
>>
>> #include <xen/config.h>
>> +#include <xen/atomic.h>
>> #include <xen/prefetch.h>
>> #include <asm/system.h>
>>
>> @@ -95,15 +96,6 @@ void __bad_atomic_size(void);
>> default: __bad_atomic_size();
>> break; \
>> } \
>> })
>> -
>> -/*
>> - * NB. I've pushed the volatile qualifier into the operations. This
>> allows
>> - * fast accessors such as _atomic_read() and _atomic_set() which
>> don't give
>> - * the compiler a fit.
>> - */
>> -typedef struct { int counter; } atomic_t;
>> -
>> -#define ATOMIC_INIT(i) { (i) }
>>
>> /*
>> * On ARM, ordinary assignment (str instruction) doesn't clear the
>> local
>> @@ -138,6 +130,85 @@ static inline void _atomic_set(atomic_t *v, int i)
>> # error "unknown ARM variant"
>> #endif
>>
>> +#define atomic_inc_return(v) (atomic_add_return(1, v))
>> +#define atomic_dec_return(v) (atomic_sub_return(1, v))
>> +
>> +/**
>> + * atomic_sub_and_test - subtract value from variable and test result
>> + * @i: integer value to subtract
>> + * @v: pointer of type atomic_t
>> + *
>> + * Atomically subtracts @i from @v and returns
>> + * true if the result is zero, or false for all
>> + * other cases.
>> + */
>> +static inline int atomic_sub_and_test(int i, atomic_t *v)
>> +{
>> + return 0 == atomic_sub_return(i, v);
>> +}
>
> Please don't use yoda condition. It's less readable and compiler have
> safety nowadays to prevent using "=".
Oh yeah, given that Xen's compiled with -Wall -Werror that's not
necessary. Ack.
>
>> +
>> +/**
>> + * atomic_inc - increment atomic variable
>> + * @v: pointer of type atomic_t
>> + *
>> + * Atomically increments @v by 1.
>> + */
>> +static inline void atomic_inc(atomic_t *v)
>> +{
>> + atomic_add(1, v);
>> +}
>
> Regards,
Thanks,
Corneliu.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)
2016-07-13 12:12 ` Andrew Cooper
2016-07-13 12:24 ` Corneliu ZUZU
@ 2016-07-13 13:20 ` Corneliu ZUZU
2016-07-13 13:21 ` Andrew Cooper
1 sibling, 1 reply; 10+ messages in thread
From: Corneliu ZUZU @ 2016-07-13 13:20 UTC (permalink / raw)
To: Andrew Cooper, Julien Grall; +Cc: Stefano Stabellini, Jan Beulich, xen-devel
On 7/13/2016 3:12 PM, Andrew Cooper wrote:
> On 13/07/16 12:23, Corneliu ZUZU wrote:
>> +typedef struct { int counter; } atomic_t;
>> +
>> +#define ATOMIC_INIT(i) { (i) }
>> +
>> +/**
>> + * atomic_read - read atomic variable
>> + * @v: pointer of type atomic_t
>> + *
>> + * Atomically reads the value of @v.
>> + */
>> +static inline int atomic_read(atomic_t *v);
>> +
>> +/**
>> + * _atomic_read - read atomic variable non-atomically
>> + * @v atomic_t
>> + *
>> + * Non-atomically reads the value of @v
>> + */
>> +static inline int _atomic_read(atomic_t v);
>> +
>> +/**
>> + * atomic_set - set atomic variable
>> + * @v: pointer of type atomic_t
>> + * @i: required value
>> + *
>> + * Atomically sets the value of @v to @i.
>> + */
>> +static inline void atomic_set(atomic_t *v, int i);
>> +
>> +/**
>> + * _atomic_set - set atomic variable non-atomically
>> + * @v: pointer of type atomic_t
>> + * @i: required value
>> + *
>> + * Non-atomically sets the value of @v to @i.
>> + */
>> +static inline void _atomic_set(atomic_t *v, int i);
>> +
>
Is it ok if I also omit repeating the comments which are already @
xen/atomic.h from the arch-side asm/atomic.h ?
Thanks,
Zuzu C.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)
2016-07-13 13:20 ` Corneliu ZUZU
@ 2016-07-13 13:21 ` Andrew Cooper
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Cooper @ 2016-07-13 13:21 UTC (permalink / raw)
To: Corneliu ZUZU, Julien Grall; +Cc: Stefano Stabellini, Jan Beulich, xen-devel
On 13/07/16 14:20, Corneliu ZUZU wrote:
> On 7/13/2016 3:12 PM, Andrew Cooper wrote:
>> On 13/07/16 12:23, Corneliu ZUZU wrote:
>>> +typedef struct { int counter; } atomic_t;
>>> +
>>> +#define ATOMIC_INIT(i) { (i) }
>>> +
>>> +/**
>>> + * atomic_read - read atomic variable
>>> + * @v: pointer of type atomic_t
>>> + *
>>> + * Atomically reads the value of @v.
>>> + */
>>> +static inline int atomic_read(atomic_t *v);
>>> +
>>> +/**
>>> + * _atomic_read - read atomic variable non-atomically
>>> + * @v atomic_t
>>> + *
>>> + * Non-atomically reads the value of @v
>>> + */
>>> +static inline int _atomic_read(atomic_t v);
>>> +
>>> +/**
>>> + * atomic_set - set atomic variable
>>> + * @v: pointer of type atomic_t
>>> + * @i: required value
>>> + *
>>> + * Atomically sets the value of @v to @i.
>>> + */
>>> +static inline void atomic_set(atomic_t *v, int i);
>>> +
>>> +/**
>>> + * _atomic_set - set atomic variable non-atomically
>>> + * @v: pointer of type atomic_t
>>> + * @i: required value
>>> + *
>>> + * Non-atomically sets the value of @v to @i.
>>> + */
>>> +static inline void _atomic_set(atomic_t *v, int i);
>>> +
>>
>
> Is it ok if I also omit repeating the comments which are already @
> xen/atomic.h from the arch-side asm/atomic.h ?
Yes - that should be fine. No point having identical comments twice.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-07-13 13:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-13 11:22 [PATCH 0/2] <asm/atomic.h> adjustments Corneliu ZUZU
2016-07-13 11:23 ` [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h) Corneliu ZUZU
2016-07-13 12:12 ` Andrew Cooper
2016-07-13 12:24 ` Corneliu ZUZU
2016-07-13 13:20 ` Corneliu ZUZU
2016-07-13 13:21 ` Andrew Cooper
2016-07-13 12:14 ` Julien Grall
2016-07-13 12:27 ` Corneliu ZUZU
2016-07-13 11:23 ` [PATCH 2/2] fix: make atomic_read() param const Corneliu ZUZU
2016-07-13 12:12 ` Andrew Cooper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).