[PATCH v2 0/5] <asm/atomic.h> adjustments

[PATCH v2 0/5] <asm/atomic.h> adjustments

[Corneliu ZUZU]

Corneliu ZUZU (5):
  asm-arm/atomic.h: fix arm32|arm64 macros duplication
  asm-x86/atomic.h: minor: proper atomic_inc_and_test() placement
  asm-arm/atomic.h: reorder macros to match x86-side
  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       |  46 +++++++---
 xen/include/asm-x86/atomic.h       | 128 +++------------------------
 xen/include/xen/atomic.h           | 171 +++++++++++++++++++++++++++++++++++++
 5 files changed, 220 insertions(+), 147 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

[PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Corneliu ZUZU]

Move duplicate macros between asm-arm/arm32/atomic.h and asm-arm/arm64/atomic.h
to asm-arm/atomic.h.

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       | 11 +++++++++++
 3 files changed, 11 insertions(+), 22 deletions(-)

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..41d1b6c 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -138,6 +138,17 @@ static inline void _atomic_set(atomic_t *v, int i)
 # error "unknown ARM variant"
 #endif
 
+#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_ATOMIC__ */
 /*
  * Local variables:
-- 
2.5.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Stefano Stabellini]

On Wed, 13 Jul 2016, Corneliu ZUZU wrote:
> Move duplicate macros between asm-arm/arm32/atomic.h and asm-arm/arm64/atomic.h
> to asm-arm/atomic.h.
> 
> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


>  xen/include/asm-arm/arm32/atomic.h | 11 -----------
>  xen/include/asm-arm/arm64/atomic.h | 11 -----------
>  xen/include/asm-arm/atomic.h       | 11 +++++++++++
>  3 files changed, 11 insertions(+), 22 deletions(-)
> 
> 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..41d1b6c 100644
> --- a/xen/include/asm-arm/atomic.h
> +++ b/xen/include/asm-arm/atomic.h
> @@ -138,6 +138,17 @@ static inline void _atomic_set(atomic_t *v, int i)
>  # error "unknown ARM variant"
>  #endif
>  
> +#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_ATOMIC__ */
>  /*
>   * Local variables:
> -- 
> 2.5.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Julien Grall]

Hi Corneliu,

On 13/07/2016 15:18, Corneliu ZUZU wrote:
> Move duplicate macros between asm-arm/arm32/atomic.h and asm-arm/arm64/atomic.h
> to asm-arm/atomic.h.

asm-arm/arm*/atomic.h were a copy from Linux. I don't mind if we 
diverge, however the file xen/arch/arm/README.primitives needs to be 
update to mention the divergence with Linux.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Corneliu ZUZU]

Hi Julien,

On 7/13/2016 10:12 PM, Julien Grall wrote:
> Hi Corneliu,
>
> On 13/07/2016 15:18, Corneliu ZUZU wrote:
>> Move duplicate macros between asm-arm/arm32/atomic.h and 
>> asm-arm/arm64/atomic.h
>> to asm-arm/atomic.h.
>
> asm-arm/arm*/atomic.h were a copy from Linux. I don't mind if we 
> diverge, however the file xen/arch/arm/README.primitives needs to be 
> update to mention the divergence with Linux.
>
> Regards,
>

Thanks for pointing that out, I didn't know these files were referred in 
such a way. What is the purpose of README.LinuxPrimitives and how must I 
update it to align with these patches?

Cheers,
Zuzu C.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Corneliu ZUZU]

On 7/13/2016 10:12 PM, Julien Grall wrote:
> Hi Corneliu,
>
> On 13/07/2016 15:18, Corneliu ZUZU wrote:
>> Move duplicate macros between asm-arm/arm32/atomic.h and 
>> asm-arm/arm64/atomic.h
>> to asm-arm/atomic.h.
>
> asm-arm/arm*/atomic.h were a copy from Linux. I don't mind if we 
> diverge, however the file xen/arch/arm/README.primitives needs to be 
> update to mention the divergence with Linux.
>
> Regards,
>

Julien,

AFAICT the README.LinuxPrimitives file specifies the Linux kernel 
version from which the arm{32,64}/atomic.h files were imported as well 
as the respective commit in the -Linux kernel- tree. I suppose that 
information needn't be updated.
Could you be more specific on how I should modify that file?

Thanks,
Zuzu C.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Julien Grall]

On 14/07/16 06:11, Corneliu ZUZU wrote:
> On 7/13/2016 10:12 PM, Julien Grall wrote:
>> Hi Corneliu,
>>
>> On 13/07/2016 15:18, Corneliu ZUZU wrote:
>>> Move duplicate macros between asm-arm/arm32/atomic.h and
>>> asm-arm/arm64/atomic.h
>>> to asm-arm/atomic.h.
>>
>> asm-arm/arm*/atomic.h were a copy from Linux. I don't mind if we
>> diverge, however the file xen/arch/arm/README.primitives needs to be
>> update to mention the divergence with Linux.
>>
>> Regards,
>>
>
> Julien,
>
> AFAICT the README.LinuxPrimitives file specifies the Linux kernel
> version from which the arm{32,64}/atomic.h files were imported as well
> as the respective commit in the -Linux kernel- tree. I suppose that
> information needn't be updated.
> Could you be more specific on how I should modify that file?

To specify which helpers has been taken from Linux in those files. Until 
now, it was quite easy to figure out that we took all atomic_* helpers.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Corneliu ZUZU]

On 7/14/2016 12:26 PM, Julien Grall wrote:
>
>
> On 14/07/16 06:11, Corneliu ZUZU wrote:
>> On 7/13/2016 10:12 PM, Julien Grall wrote:
>>> Hi Corneliu,
>>>
>>> On 13/07/2016 15:18, Corneliu ZUZU wrote:
>>>> Move duplicate macros between asm-arm/arm32/atomic.h and
>>>> asm-arm/arm64/atomic.h
>>>> to asm-arm/atomic.h.
>>>
>>> asm-arm/arm*/atomic.h were a copy from Linux. I don't mind if we
>>> diverge, however the file xen/arch/arm/README.primitives needs to be
>>> update to mention the divergence with Linux.
>>>
>>> Regards,
>>>
>>
>> Julien,
>>
>> AFAICT the README.LinuxPrimitives file specifies the Linux kernel
>> version from which the arm{32,64}/atomic.h files were imported as well
>> as the respective commit in the -Linux kernel- tree. I suppose that
>> information needn't be updated.
>> Could you be more specific on how I should modify that file?
>
> To specify which helpers has been taken from Linux in those files. 
> Until now, it was quite easy to figure out that we took all atomic_* 
> helpers.
>
> Regards,


Ok, will look into that.

I suppose also adding:

diff -u linux/arch/arm64/include/asm/atomic.h 
xen/include/asm-arm/arm64/atomic.h
diff -u linux/arch/arm/include/asm/atomic.h 
xen/include/asm-arm/arm32/atomic.h

as it's done for the others helps?

Zuzu C.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Julien Grall]

On 14/07/16 11:11, Corneliu ZUZU wrote:
> On 7/14/2016 12:26 PM, Julien Grall wrote:
>>
>>
>> On 14/07/16 06:11, Corneliu ZUZU wrote:
>>> On 7/13/2016 10:12 PM, Julien Grall wrote:
>>>> Hi Corneliu,
>>>>
>>>> On 13/07/2016 15:18, Corneliu ZUZU wrote:
>>>>> Move duplicate macros between asm-arm/arm32/atomic.h and
>>>>> asm-arm/arm64/atomic.h
>>>>> to asm-arm/atomic.h.
>>>>
>>>> asm-arm/arm*/atomic.h were a copy from Linux. I don't mind if we
>>>> diverge, however the file xen/arch/arm/README.primitives needs to be
>>>> update to mention the divergence with Linux.
>>>>
>>>> Regards,
>>>>
>>>
>>> Julien,
>>>
>>> AFAICT the README.LinuxPrimitives file specifies the Linux kernel
>>> version from which the arm{32,64}/atomic.h files were imported as well
>>> as the respective commit in the -Linux kernel- tree. I suppose that
>>> information needn't be updated.
>>> Could you be more specific on how I should modify that file?
>>
>> To specify which helpers has been taken from Linux in those files.
>> Until now, it was quite easy to figure out that we took all atomic_*
>> helpers.
>>
>> Regards,
>
>
> Ok, will look into that.
>
> I suppose also adding:
>
> diff -u linux/arch/arm64/include/asm/atomic.h
> xen/include/asm-arm/arm64/atomic.h
> diff -u linux/arch/arm/include/asm/atomic.h
> xen/include/asm-arm/arm32/atomic.h
>
> as it's done for the others helps?

No, the other files are a verbatim copy of the Linux headers. It is not 
the case here.

Something like:

"Only the following functions were taken from Linux:
   - ...
   - ...
   - ...
"

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Corneliu ZUZU]

On 7/14/2016 1:14 PM, Julien Grall wrote:
>
>
> On 14/07/16 11:11, Corneliu ZUZU wrote:
>> On 7/14/2016 12:26 PM, Julien Grall wrote:
>>>
>>>
>>> On 14/07/16 06:11, Corneliu ZUZU wrote:
>>>> On 7/13/2016 10:12 PM, Julien Grall wrote:
>>>>> Hi Corneliu,
>>>>>
>>>>> On 13/07/2016 15:18, Corneliu ZUZU wrote:
>>>>>> Move duplicate macros between asm-arm/arm32/atomic.h and
>>>>>> asm-arm/arm64/atomic.h
>>>>>> to asm-arm/atomic.h.
>>>>>
>>>>> asm-arm/arm*/atomic.h were a copy from Linux. I don't mind if we
>>>>> diverge, however the file xen/arch/arm/README.primitives needs to be
>>>>> update to mention the divergence with Linux.
>>>>>
>>>>> Regards,
>>>>>
>>>>
>>>> Julien,
>>>>
>>>> AFAICT the README.LinuxPrimitives file specifies the Linux kernel
>>>> version from which the arm{32,64}/atomic.h files were imported as well
>>>> as the respective commit in the -Linux kernel- tree. I suppose that
>>>> information needn't be updated.
>>>> Could you be more specific on how I should modify that file?
>>>
>>> To specify which helpers has been taken from Linux in those files.
>>> Until now, it was quite easy to figure out that we took all atomic_*
>>> helpers.
>>>
>>> Regards,
>>
>>
>> Ok, will look into that.
>>
>> I suppose also adding:
>>
>> diff -u linux/arch/arm64/include/asm/atomic.h
>> xen/include/asm-arm/arm64/atomic.h
>> diff -u linux/arch/arm/include/asm/atomic.h
>> xen/include/asm-arm/arm32/atomic.h
>>
>> as it's done for the others helps?
>
> No, the other files are a verbatim copy of the Linux headers. It is 
> not the case here.
>
> Something like:
>
> "Only the following functions were taken from Linux:
>   - ...
>   - ...
>   - ...
> "
>
> Regards,
>

Well, weren't they all taken from Linux?..I just turned some of the 
macros into inline functions and implemented some missing (no 
additional) ones on the X86-side.

Corneliu.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Julien Grall]

On 14/07/16 17:40, Corneliu ZUZU wrote:
> On 7/14/2016 1:14 PM, Julien Grall wrote:
>>
>>
>> On 14/07/16 11:11, Corneliu ZUZU wrote:
>>> On 7/14/2016 12:26 PM, Julien Grall wrote:
>>>>
>>>>
>>>> On 14/07/16 06:11, Corneliu ZUZU wrote:
>>>>> On 7/13/2016 10:12 PM, Julien Grall wrote:
>>>>>> Hi Corneliu,
>>>>>>
>>>>>> On 13/07/2016 15:18, Corneliu ZUZU wrote:
>>>>>>> Move duplicate macros between asm-arm/arm32/atomic.h and
>>>>>>> asm-arm/arm64/atomic.h
>>>>>>> to asm-arm/atomic.h.
>>>>>>
>>>>>> asm-arm/arm*/atomic.h were a copy from Linux. I don't mind if we
>>>>>> diverge, however the file xen/arch/arm/README.primitives needs to be
>>>>>> update to mention the divergence with Linux.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>
>>>>> Julien,
>>>>>
>>>>> AFAICT the README.LinuxPrimitives file specifies the Linux kernel
>>>>> version from which the arm{32,64}/atomic.h files were imported as well
>>>>> as the respective commit in the -Linux kernel- tree. I suppose that
>>>>> information needn't be updated.
>>>>> Could you be more specific on how I should modify that file?
>>>>
>>>> To specify which helpers has been taken from Linux in those files.
>>>> Until now, it was quite easy to figure out that we took all atomic_*
>>>> helpers.
>>>>
>>>> Regards,
>>>
>>>
>>> Ok, will look into that.
>>>
>>> I suppose also adding:
>>>
>>> diff -u linux/arch/arm64/include/asm/atomic.h
>>> xen/include/asm-arm/arm64/atomic.h
>>> diff -u linux/arch/arm/include/asm/atomic.h
>>> xen/include/asm-arm/arm32/atomic.h
>>>
>>> as it's done for the others helps?
>>
>> No, the other files are a verbatim copy of the Linux headers. It is
>> not the case here.
>>
>> Something like:
>>
>> "Only the following functions were taken from Linux:
>>   - ...
>>   - ...
>>   - ...
>> "
>>
>> Regards,
>>
>
> Well, weren't they all taken from Linux?..I just turned some of the
> macros into inline functions and implemented some missing (no
> additional) ones on the X86-side.

Yes, but you don't know why you have not taken the rest (the macros are 
not in atomic.h anymore...). The goal of this file is to keep track of 
the difference between Linux and Xen of file taken from the former.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 2/5] asm-x86/atomic.h: minor: proper atomic_inc_and_test() placement

[Corneliu ZUZU]

Place atomic_inc_and_test() implementation after atomic_inc().
Also remove unneeded empty line and add a needed one.

Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
---
 xen/include/asm-arm/atomic.h |  1 +
 xen/include/asm-x86/atomic.h | 39 +++++++++++++++++++--------------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 41d1b6c..19a87a5 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -150,6 +150,7 @@ static inline void _atomic_set(atomic_t *v, int i)
 #define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
 
 #endif /* __ARCH_ARM_ATOMIC__ */
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index d246b70..5f9f2dd 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -110,7 +110,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 +216,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 +268,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
-- 
2.5.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 2/5] asm-x86/atomic.h: minor: proper atomic_inc_and_test() placement

[Stefano Stabellini]

On Wed, 13 Jul 2016, Corneliu ZUZU wrote:
> Place atomic_inc_and_test() implementation after atomic_inc().
> Also remove unneeded empty line and add a needed one.
> 
> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>

For the ARM bit, even though it is a bit embarassing acking a one-liner
empty line addition:

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


>  xen/include/asm-arm/atomic.h |  1 +
>  xen/include/asm-x86/atomic.h | 39 +++++++++++++++++++--------------------
>  2 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
> index 41d1b6c..19a87a5 100644
> --- a/xen/include/asm-arm/atomic.h
> +++ b/xen/include/asm-arm/atomic.h
> @@ -150,6 +150,7 @@ static inline void _atomic_set(atomic_t *v, int i)
>  #define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
>  
>  #endif /* __ARCH_ARM_ATOMIC__ */
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
> index d246b70..5f9f2dd 100644
> --- a/xen/include/asm-x86/atomic.h
> +++ b/xen/include/asm-x86/atomic.h
> @@ -110,7 +110,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 +216,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 +268,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
> -- 
> 2.5.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 3/5] asm-arm/atomic.h: reorder macros to match x86-side

[Corneliu ZUZU]

Reorder macro definitions to match x86-side.

Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
---
 xen/include/asm-arm/atomic.h | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 19a87a5..e8f7340 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -138,16 +138,15 @@ static inline void _atomic_set(atomic_t *v, int i)
 # error "unknown ARM variant"
 #endif
 
-#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)
+#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_inc(v)               atomic_add(1, v)
+#define atomic_inc_and_test(v)      (atomic_add_return(1, v) == 0)
+#define atomic_dec(v)               atomic_sub(1, v)
+#define atomic_dec_and_test(v)      (atomic_sub_return(1, v) == 0)
+#define atomic_add_negative(i,v)    (atomic_add_return(i, v) < 0)
 
 #endif /* __ARCH_ARM_ATOMIC__ */
 
-- 
2.5.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 3/5] asm-arm/atomic.h: reorder macros to match x86-side

[Stefano Stabellini]

On Wed, 13 Jul 2016, Corneliu ZUZU wrote:
> Reorder macro definitions to match x86-side.
> 
> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


>  xen/include/asm-arm/atomic.h | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
> index 19a87a5..e8f7340 100644
> --- a/xen/include/asm-arm/atomic.h
> +++ b/xen/include/asm-arm/atomic.h
> @@ -138,16 +138,15 @@ static inline void _atomic_set(atomic_t *v, int i)
>  # error "unknown ARM variant"
>  #endif
>  
> -#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)
> +#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_inc(v)               atomic_add(1, v)
> +#define atomic_inc_and_test(v)      (atomic_add_return(1, v) == 0)
> +#define atomic_dec(v)               atomic_sub(1, v)
> +#define atomic_dec_and_test(v)      (atomic_sub_return(1, v) == 0)
> +#define atomic_add_negative(i,v)    (atomic_add_return(i, v) < 0)
>  
>  #endif /* __ARCH_ARM_ATOMIC__ */
>  
> -- 
> 2.5.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 4/5] asm/atomic.h: common prototyping (add xen/atomic.h)

[Corneliu ZUZU]

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.
Removed outdated comment ("NB. I've [...]").

Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changed since v1:
  * removed comments that were duplicate between asm-x86/atomic.h and
    xen/atomic.h
  * remove outdated comment ("NB. [...]")
  * add atomic_cmpxchg doc-comment
  * don't use yoda condition
---
 xen/include/asm-arm/atomic.h |  45 ++++++++----
 xen/include/asm-x86/atomic.h | 103 +-------------------------
 xen/include/xen/atomic.h     | 171 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 202 insertions(+), 117 deletions(-)
 create mode 100644 xen/include/xen/atomic.h

diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index e8f7340..01af43b 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
@@ -141,12 +133,35 @@ static inline void _atomic_set(atomic_t *v, int i)
 #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_inc(v)               atomic_add(1, v)
-#define atomic_inc_and_test(v)      (atomic_add_return(1, v) == 0)
-#define atomic_dec(v)               atomic_sub(1, v)
-#define atomic_dec_and_test(v)      (atomic_sub_return(1, v) == 0)
-#define atomic_add_negative(i,v)    (atomic_add_return(i, v) < 0)
+static inline int atomic_sub_and_test(int i, atomic_t *v)
+{
+    return atomic_sub_return(i, v) == 0;
+}
+
+static inline void atomic_inc(atomic_t *v)
+{
+    atomic_add(1, v);
+}
+
+static inline int atomic_inc_and_test(atomic_t *v)
+{
+    return atomic_add_return(1, v) == 0;
+}
+
+static inline void atomic_dec(atomic_t *v)
+{
+    atomic_sub(1, v);
+}
+
+static inline int atomic_dec_and_test(atomic_t *v)
+{
+    return atomic_sub_return(1, v) == 0;
+}
+
+static inline int atomic_add_negative(int i, atomic_t *v)
+{
+    return atomic_add_return(i, v) < 0;
+}
 
 #endif /* __ARCH_ARM_ATOMIC__ */
 
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index 5f9f2dd..3e99b03 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,56 +80,21 @@ 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
- *
- * Atomically reads the value of @v.
- */
 static inline int atomic_read(atomic_t *v)
 {
     return read_atomic(&v->counter);
 }
 
-/**
- * _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)
 {
     return v.counter;
 }
 
-/**
- * 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)
 {
     write_atomic(&v->counter, 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)
 {
     v->counter = i;
@@ -139,13 +105,6 @@ static inline int atomic_cmpxchg(atomic_t *v, int old, int new)
     return cmpxchg(&v->counter, old, 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)
 {
     asm volatile (
@@ -154,25 +113,11 @@ static inline void atomic_add(int i, atomic_t *v)
         : "ir" (i), "m" (*(volatile int *)&v->counter) );
 }
 
-/**
- * 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)
 {
     return i + arch_fetch_and_add(&v->counter, i);
 }
 
-/**
- * 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)
 {
     asm volatile (
@@ -181,15 +126,6 @@ static inline void atomic_sub(int i, atomic_t *v)
         : "ir" (i), "m" (*(volatile int *)&v->counter) );
 }
 
-/**
- * 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)
 {
     unsigned char c;
@@ -201,12 +137,6 @@ static inline int atomic_sub_and_test(int i, atomic_t *v)
     return c;
 }
 
-/**
- * atomic_inc - increment atomic variable
- * @v: pointer of type atomic_t
- * 
- * Atomically increments @v by 1.
- */ 
 static inline void atomic_inc(atomic_t *v)
 {
     asm volatile (
@@ -215,14 +145,6 @@ static inline void atomic_inc(atomic_t *v)
         : "m" (*(volatile int *)&v->counter) );
 }
 
-/**
- * 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;
@@ -234,12 +156,6 @@ static inline int atomic_inc_and_test(atomic_t *v)
     return c != 0;
 }
 
-/**
- * atomic_dec - decrement atomic variable
- * @v: pointer of type atomic_t
- * 
- * Atomically decrements @v by 1.
- */ 
 static inline void atomic_dec(atomic_t *v)
 {
     asm volatile (
@@ -248,14 +164,6 @@ static inline void atomic_dec(atomic_t *v)
         : "m" (*(volatile int *)&v->counter) );
 }
 
-/**
- * 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)
 {
     unsigned char c;
@@ -267,15 +175,6 @@ static inline int atomic_dec_and_test(atomic_t *v)
     return c != 0;
 }
 
-/**
- * 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)
 {
     unsigned char c;
diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
new file mode 100644
index 0000000..3517bc9
--- /dev/null
+++ b/xen/include/xen/atomic.h
@@ -0,0 +1,171 @@
+/*
+ * 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__
+
+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
+ * @v: pointer of type atomic_t
+ * @old: old value
+ * @new: new value
+ *
+ * Before calling, @old should be set to @v.
+ * Succeeds if @old == @v (likely), in which case stores @new in @v.
+ * Returns the initial value in @v, hence succeeds when the return value
+ * matches that of @old.
+ *
+ * Sample (tries atomic increment of v until the operation succeeds):
+ *
+ *  while(1)
+ *  {
+ *      int old = v.counter;
+ *      int new = old + 1;
+ *      if ( likely(old == atomic_cmpxchg(&v, old, new)) )
+ *          break; // success!
+ *  }
+ */
+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

Re: [PATCH v2 4/5] asm/atomic.h: common prototyping (add xen/atomic.h)

[Stefano Stabellini]

On Wed, 13 Jul 2016, Corneliu ZUZU wrote:
> 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.
> Removed outdated comment ("NB. I've [...]").
> 
> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Changed since v1:
>   * removed comments that were duplicate between asm-x86/atomic.h and
>     xen/atomic.h
>   * remove outdated comment ("NB. [...]")
>   * add atomic_cmpxchg doc-comment
>   * don't use yoda condition
> ---
>  xen/include/asm-arm/atomic.h |  45 ++++++++----
>  xen/include/asm-x86/atomic.h | 103 +-------------------------
>  xen/include/xen/atomic.h     | 171 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 202 insertions(+), 117 deletions(-)
>  create mode 100644 xen/include/xen/atomic.h
> 
> diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
> index e8f7340..01af43b 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
> @@ -141,12 +133,35 @@ static inline void _atomic_set(atomic_t *v, int i)
>  #define atomic_inc_return(v)        (atomic_add_return(1, v))
>  #define atomic_dec_return(v)        (atomic_sub_return(1, v))

What about atomic_inc_return and atomic_dec_return? Doesn't it make
sense to do this for all of them, since we are at it? I believe there
are also a couple more which are #define'd.


> -#define atomic_sub_and_test(i, v)   (atomic_sub_return(i, v) == 0)
> -#define atomic_inc(v)               atomic_add(1, v)
> -#define atomic_inc_and_test(v)      (atomic_add_return(1, v) == 0)
> -#define atomic_dec(v)               atomic_sub(1, v)
> -#define atomic_dec_and_test(v)      (atomic_sub_return(1, v) == 0)
> -#define atomic_add_negative(i,v)    (atomic_add_return(i, v) < 0)
> +static inline int atomic_sub_and_test(int i, atomic_t *v)
> +{
> +    return atomic_sub_return(i, v) == 0;
> +}
> +
> +static inline void atomic_inc(atomic_t *v)
> +{
> +    atomic_add(1, v);
> +}
> +
> +static inline int atomic_inc_and_test(atomic_t *v)
> +{
> +    return atomic_add_return(1, v) == 0;
> +}
> +
> +static inline void atomic_dec(atomic_t *v)
> +{
> +    atomic_sub(1, v);
> +}
> +
> +static inline int atomic_dec_and_test(atomic_t *v)
> +{
> +    return atomic_sub_return(1, v) == 0;
> +}
> +
> +static inline int atomic_add_negative(int i, atomic_t *v)
> +{
> +    return atomic_add_return(i, v) < 0;
> +}
>  
>  #endif /* __ARCH_ARM_ATOMIC__ */

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 4/5] asm/atomic.h: common prototyping (add xen/atomic.h)

[Corneliu ZUZU]

On 7/13/2016 10:01 PM, Stefano Stabellini wrote:
> On Wed, 13 Jul 2016, Corneliu ZUZU wrote:
>> 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.
>> Removed outdated comment ("NB. I've [...]").
>>
>> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> Changed since v1:
>>    * removed comments that were duplicate between asm-x86/atomic.h and
>>      xen/atomic.h
>>    * remove outdated comment ("NB. [...]")
>>    * add atomic_cmpxchg doc-comment
>>    * don't use yoda condition
>> ---
>>   xen/include/asm-arm/atomic.h |  45 ++++++++----
>>   xen/include/asm-x86/atomic.h | 103 +-------------------------
>>   xen/include/xen/atomic.h     | 171 +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 202 insertions(+), 117 deletions(-)
>>   create mode 100644 xen/include/xen/atomic.h
>>
>> diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
>> index e8f7340..01af43b 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
>> @@ -141,12 +133,35 @@ static inline void _atomic_set(atomic_t *v, int i)
>>   #define atomic_inc_return(v)        (atomic_add_return(1, v))
>>   #define atomic_dec_return(v)        (atomic_sub_return(1, v))
> What about atomic_inc_return and atomic_dec_return? Doesn't it make
> sense to do this for all of them, since we are at it? I believe there
> are also a couple more which are #define'd.

Those don't seem to be implemented for X86 and neither are they referred 
from common code (probably because they really aren't defined for X86).

>> -#define atomic_sub_and_test(i, v)   (atomic_sub_return(i, v) == 0)
>> -#define atomic_inc(v)               atomic_add(1, v)
>> -#define atomic_inc_and_test(v)      (atomic_add_return(1, v) == 0)
>> -#define atomic_dec(v)               atomic_sub(1, v)
>> -#define atomic_dec_and_test(v)      (atomic_sub_return(1, v) == 0)
>> -#define atomic_add_negative(i,v)    (atomic_add_return(i, v) < 0)
>> +static inline int atomic_sub_and_test(int i, atomic_t *v)
>> +{
>> +    return atomic_sub_return(i, v) == 0;
>> +}
>> +
>> +static inline void atomic_inc(atomic_t *v)
>> +{
>> +    atomic_add(1, v);
>> +}
>> +
>> +static inline int atomic_inc_and_test(atomic_t *v)
>> +{
>> +    return atomic_add_return(1, v) == 0;
>> +}
>> +
>> +static inline void atomic_dec(atomic_t *v)
>> +{
>> +    atomic_sub(1, v);
>> +}
>> +
>> +static inline int atomic_dec_and_test(atomic_t *v)
>> +{
>> +    return atomic_sub_return(1, v) == 0;
>> +}
>> +
>> +static inline int atomic_add_negative(int i, atomic_t *v)
>> +{
>> +    return atomic_add_return(i, v) < 0;
>> +}
>>   
>>   #endif /* __ARCH_ARM_ATOMIC__ */

Thanks,
Zuzu C.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 4/5] asm/atomic.h: common prototyping (add xen/atomic.h)

[Andrew Cooper]

On 13/07/16 20:33, Corneliu ZUZU wrote:
> On 7/13/2016 10:01 PM, Stefano Stabellini wrote:
>> On Wed, 13 Jul 2016, Corneliu ZUZU wrote:
>>> 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.
>>> Removed outdated comment ("NB. I've [...]").
>>>
>>> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
>>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> Changed since v1:
>>>    * removed comments that were duplicate between asm-x86/atomic.h and
>>>      xen/atomic.h
>>>    * remove outdated comment ("NB. [...]")
>>>    * add atomic_cmpxchg doc-comment
>>>    * don't use yoda condition
>>> ---
>>>   xen/include/asm-arm/atomic.h |  45 ++++++++----
>>>   xen/include/asm-x86/atomic.h | 103 +-------------------------
>>>   xen/include/xen/atomic.h     | 171
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 202 insertions(+), 117 deletions(-)
>>>   create mode 100644 xen/include/xen/atomic.h
>>>
>>> diff --git a/xen/include/asm-arm/atomic.h
>>> b/xen/include/asm-arm/atomic.h
>>> index e8f7340..01af43b 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
>>> @@ -141,12 +133,35 @@ static inline void _atomic_set(atomic_t *v,
>>> int i)
>>>   #define atomic_inc_return(v)        (atomic_add_return(1, v))
>>>   #define atomic_dec_return(v)        (atomic_sub_return(1, v))
>> What about atomic_inc_return and atomic_dec_return? Doesn't it make
>> sense to do this for all of them, since we are at it? I believe there
>> are also a couple more which are #define'd.
>
> Those don't seem to be implemented for X86 and neither are they
> referred from common code (probably because they really aren't defined
> for X86).

Apologies - this is definitely turning into the rats nest I warned you
it might.

As far as I am concerned, I only think it is important to have the
static inline prototypes for the versions which are actually common
between architectures.  Those are the ones we don't want to break
accidentally.

However, for the helpers which are not common, having them consistently
static inline would be a distinct improvement over mixed
inline/defines.  (static inline functions are superior to macros in many
ways.)

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 4/5] asm/atomic.h: common prototyping (add xen/atomic.h)

[Corneliu ZUZU]

On 7/13/2016 10:49 PM, Andrew Cooper wrote:
> On 13/07/16 20:33, Corneliu ZUZU wrote:
>> On 7/13/2016 10:01 PM, Stefano Stabellini wrote:
>>> On Wed, 13 Jul 2016, Corneliu ZUZU wrote:
>>>> 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.
>>>> Removed outdated comment ("NB. I've [...]").
>>>>
>>>> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
>>>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> ---
>>>> Changed since v1:
>>>>     * removed comments that were duplicate between asm-x86/atomic.h and
>>>>       xen/atomic.h
>>>>     * remove outdated comment ("NB. [...]")
>>>>     * add atomic_cmpxchg doc-comment
>>>>     * don't use yoda condition
>>>> ---
>>>>    xen/include/asm-arm/atomic.h |  45 ++++++++----
>>>>    xen/include/asm-x86/atomic.h | 103 +-------------------------
>>>>    xen/include/xen/atomic.h     | 171
>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 202 insertions(+), 117 deletions(-)
>>>>    create mode 100644 xen/include/xen/atomic.h
>>>>
>>>> diff --git a/xen/include/asm-arm/atomic.h
>>>> b/xen/include/asm-arm/atomic.h
>>>> index e8f7340..01af43b 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
>>>> @@ -141,12 +133,35 @@ static inline void _atomic_set(atomic_t *v,
>>>> int i)
>>>>    #define atomic_inc_return(v)        (atomic_add_return(1, v))
>>>>    #define atomic_dec_return(v)        (atomic_sub_return(1, v))
>>> What about atomic_inc_return and atomic_dec_return? Doesn't it make
>>> sense to do this for all of them, since we are at it? I believe there
>>> are also a couple more which are #define'd.
>> Those don't seem to be implemented for X86 and neither are they
>> referred from common code (probably because they really aren't defined
>> for X86).
> Apologies - this is definitely turning into the rats nest I warned you
> it might.
>
> As far as I am concerned, I only think it is important to have the
> static inline prototypes for the versions which are actually common
> between architectures.  Those are the ones we don't want to break
> accidentally.
>
> However, for the helpers which are not common, having them consistently
> static inline would be a distinct improvement over mixed
> inline/defines.  (static inline functions are superior to macros in many
> ways.)
>
> ~Andrew
>

Absolutely no problem! The rats nest you imagined before this series was 
a different one :P . This really doesn't seem like it would require much 
effort.
What I could also try to do is to actually implement them on X86 as 
well. Would that be preferable?
I think the complete functions that are missing there are: 
atomic_sub_return, atomic_{inc,dec}_return, atomic_xchg, 
__atomic_add_unless. Please confirm this list.

As for how I'd implement them:

     atomic_sub_return() - would "return arch_fetch_and_add(&v->counter, 
-i) - i;" - similarly to what atomic_add_return() does - do?

     atomic_{inc,dec}_return() - would "return 
atomic_{add,sub}_return(1, v)" respectively as on ARM do?

     atomic_xchg() - since xchg() is also available on X86, would 
"return xchg(&((v)->counter), new);" as on ARM do?
                                Stefano, Julien: note that I'd have to 
implement this for ARM64, so, since xchg is -also- available on ARM64, 
would the above implementation also do in that case?

     __atomic_add_unless() - again, everything this depends on in the 
ARM64 version is already available on X86 too; would it also be advised 
to rename it to atomic_add_unless()?
                                                Why the built-in leading 
underscores '__'? Is this function used anywhere?

Thanks,
Zuzu C.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 5/5] fix: make atomic_read() param const

[Corneliu ZUZU]

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>

---
Changed since v1: <nothing>
---
 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 01af43b..25a33cb 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 3e99b03..1729e29 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -80,7 +80,7 @@ void __bad_atomic_size(void);
     }                                                     \
 })
 
-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 3517bc9..d97f91d 100644
--- a/xen/include/xen/atomic.h
+++ b/xen/include/xen/atomic.h
@@ -32,7 +32,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

Re: [PATCH v2 5/5] fix: make atomic_read() param const

[Stefano Stabellini]

On Wed, 13 Jul 2016, 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>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
> Changed since v1: <nothing>
> ---
>  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 01af43b..25a33cb 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 3e99b03..1729e29 100644
> --- a/xen/include/asm-x86/atomic.h
> +++ b/xen/include/asm-x86/atomic.h
> @@ -80,7 +80,7 @@ void __bad_atomic_size(void);
>      }                                                     \
>  })
>  
> -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 3517bc9..d97f91d 100644
> --- a/xen/include/xen/atomic.h
> +++ b/xen/include/xen/atomic.h
> @@ -32,7 +32,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