* [PATCH 0/2] x86: atomic.h polishing
@ 2017-06-09 12:19 Jan Beulich
2017-06-09 12:43 ` [PATCH 1/2] x86: drop unused barrier parameter from build_{read, write}_atomic() Jan Beulich
2017-06-09 12:44 ` [PATCH 2/2] x86: consolidate atomic build_*() macros Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Jan Beulich @ 2017-06-09 12:19 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
1: drop unused barrier parameter from build_{read,write}_atomic()
2: consolidate atomic build_*() macros
Signed-off-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] x86: drop unused barrier parameter from build_{read, write}_atomic()
2017-06-09 12:19 [PATCH 0/2] x86: atomic.h polishing Jan Beulich
@ 2017-06-09 12:43 ` Jan Beulich
2017-06-09 18:28 ` Andrew Cooper
2017-06-09 12:44 ` [PATCH 2/2] x86: consolidate atomic build_*() macros Jan Beulich
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2017-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 2431 bytes --]
Also take the opportunity and make an attempt at making the macro
definitions readable. Drop pointless casts while doing so.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -4,15 +4,19 @@
#include <xen/atomic.h>
#include <asm/system.h>
-#define build_read_atomic(name, size, type, reg, barrier) \
+#define build_read_atomic(name, size, type, reg) \
static inline type name(const volatile type *addr) \
-{ type ret; asm volatile("mov" size " %1,%0":reg (ret) \
-:"m" (*(volatile type *)addr) barrier); return ret; }
+{ \
+ type ret; \
+ asm volatile ( "mov" size " %1,%0" : reg (ret) : "m" (*addr) ); \
+ return ret; \
+}
-#define build_write_atomic(name, size, type, reg, barrier) \
+#define build_write_atomic(name, size, type, reg) \
static inline void name(volatile type *addr, type val) \
-{ asm volatile("mov" size " %1,%0": "=m" (*(volatile type *)addr) \
-:reg (val) barrier); }
+{ \
+ asm volatile ( "mov" size " %1,%0" : "=m" (*addr) : reg (val) ); \
+}
#define build_add_sized(name, size, type, reg) \
static inline void name(volatile type *addr, type val) \
@@ -22,15 +26,15 @@ static inline void name(volatile type *a
: reg (val)); \
}
-build_read_atomic(read_u8_atomic, "b", uint8_t, "=q", )
-build_read_atomic(read_u16_atomic, "w", uint16_t, "=r", )
-build_read_atomic(read_u32_atomic, "l", uint32_t, "=r", )
-build_read_atomic(read_u64_atomic, "q", uint64_t, "=r", )
-
-build_write_atomic(write_u8_atomic, "b", uint8_t, "q", )
-build_write_atomic(write_u16_atomic, "w", uint16_t, "r", )
-build_write_atomic(write_u32_atomic, "l", uint32_t, "r", )
-build_write_atomic(write_u64_atomic, "q", uint64_t, "r", )
+build_read_atomic(read_u8_atomic, "b", uint8_t, "=q")
+build_read_atomic(read_u16_atomic, "w", uint16_t, "=r")
+build_read_atomic(read_u32_atomic, "l", uint32_t, "=r")
+build_read_atomic(read_u64_atomic, "q", uint64_t, "=r")
+
+build_write_atomic(write_u8_atomic, "b", uint8_t, "q")
+build_write_atomic(write_u16_atomic, "w", uint16_t, "r")
+build_write_atomic(write_u32_atomic, "l", uint32_t, "r")
+build_write_atomic(write_u64_atomic, "q", uint64_t, "r")
build_add_sized(add_u8_sized, "b", uint8_t, "qi")
build_add_sized(add_u16_sized, "w", uint16_t, "ri")
[-- Attachment #2: x86-rw-atomic-no-barrier.patch --]
[-- Type: text/plain, Size: 2496 bytes --]
x86: drop unused barrier parameter from build_{read,write}_atomic()
Also take the opportunity and make an attempt at making the macro
definitions readable. Drop pointless casts while doing so.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -4,15 +4,19 @@
#include <xen/atomic.h>
#include <asm/system.h>
-#define build_read_atomic(name, size, type, reg, barrier) \
+#define build_read_atomic(name, size, type, reg) \
static inline type name(const volatile type *addr) \
-{ type ret; asm volatile("mov" size " %1,%0":reg (ret) \
-:"m" (*(volatile type *)addr) barrier); return ret; }
+{ \
+ type ret; \
+ asm volatile ( "mov" size " %1,%0" : reg (ret) : "m" (*addr) ); \
+ return ret; \
+}
-#define build_write_atomic(name, size, type, reg, barrier) \
+#define build_write_atomic(name, size, type, reg) \
static inline void name(volatile type *addr, type val) \
-{ asm volatile("mov" size " %1,%0": "=m" (*(volatile type *)addr) \
-:reg (val) barrier); }
+{ \
+ asm volatile ( "mov" size " %1,%0" : "=m" (*addr) : reg (val) ); \
+}
#define build_add_sized(name, size, type, reg) \
static inline void name(volatile type *addr, type val) \
@@ -22,15 +26,15 @@ static inline void name(volatile type *a
: reg (val)); \
}
-build_read_atomic(read_u8_atomic, "b", uint8_t, "=q", )
-build_read_atomic(read_u16_atomic, "w", uint16_t, "=r", )
-build_read_atomic(read_u32_atomic, "l", uint32_t, "=r", )
-build_read_atomic(read_u64_atomic, "q", uint64_t, "=r", )
-
-build_write_atomic(write_u8_atomic, "b", uint8_t, "q", )
-build_write_atomic(write_u16_atomic, "w", uint16_t, "r", )
-build_write_atomic(write_u32_atomic, "l", uint32_t, "r", )
-build_write_atomic(write_u64_atomic, "q", uint64_t, "r", )
+build_read_atomic(read_u8_atomic, "b", uint8_t, "=q")
+build_read_atomic(read_u16_atomic, "w", uint16_t, "=r")
+build_read_atomic(read_u32_atomic, "l", uint32_t, "=r")
+build_read_atomic(read_u64_atomic, "q", uint64_t, "=r")
+
+build_write_atomic(write_u8_atomic, "b", uint8_t, "q")
+build_write_atomic(write_u16_atomic, "w", uint16_t, "r")
+build_write_atomic(write_u32_atomic, "l", uint32_t, "r")
+build_write_atomic(write_u64_atomic, "q", uint64_t, "r")
build_add_sized(add_u8_sized, "b", uint8_t, "qi")
build_add_sized(add_u16_sized, "w", uint16_t, "ri")
[-- Attachment #3: 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] 6+ messages in thread
* [PATCH 2/2] x86: consolidate atomic build_*() macros
2017-06-09 12:19 [PATCH 0/2] x86: atomic.h polishing Jan Beulich
2017-06-09 12:43 ` [PATCH 1/2] x86: drop unused barrier parameter from build_{read, write}_atomic() Jan Beulich
@ 2017-06-09 12:44 ` Jan Beulich
2017-06-09 18:45 ` Andrew Cooper
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2017-06-09 12:44 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 3381 bytes --]
Use a single macro to define both read and write inline functions.
Avoid redundant inputs (including quotes - use stringification
instead). Generalize "add" to ease eventual addition of other
artihmetic operations.
At once correct the artihmetic asm()'s output constraint (needs to be
"+" instead of "="), its 64-bit immediate one, and permit suitable
immediates for writes.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -4,46 +4,41 @@
#include <xen/atomic.h>
#include <asm/system.h>
-#define build_read_atomic(name, size, type, reg) \
-static inline type name(const volatile type *addr) \
+#define build_rw_atomic(width, size, reg, imm) \
+static inline uint ## width ## _t \
+read_u ## width ## _atomic(const volatile uint ## width ## _t *addr) \
{ \
- type ret; \
- asm volatile ( "mov" size " %1,%0" : reg (ret) : "m" (*addr) ); \
+ uint ## width ## _t ret; \
+ asm volatile ( "mov" #size " %1,%0" : "=" #reg (ret) : "m" (*addr) ); \
return ret; \
+} \
+static inline void \
+write_u ## width ## _atomic(volatile uint ## width ## _t *addr, \
+ uint ## width ## _t val) \
+{ \
+ asm volatile ( "mov" #size " %1,%0" : "=m" (*addr) : #reg #imm (val) ); \
}
-#define build_write_atomic(name, size, type, reg) \
-static inline void name(volatile type *addr, type val) \
+#define build_arith_sized(op, width, size, input) \
+static inline void \
+op ## _u ## width ## _sized(volatile uint ## width ## _t *addr, \
+ uint ## width ## _t val) \
{ \
- asm volatile ( "mov" size " %1,%0" : "=m" (*addr) : reg (val) ); \
+ asm volatile ( #op #size " %1,%0" : "+m" (*addr) : #input (val) ); \
}
-#define build_add_sized(name, size, type, reg) \
- static inline void name(volatile type *addr, type val) \
- { \
- asm volatile("add" size " %1,%0" \
- : "=m" (*addr) \
- : reg (val)); \
- }
-
-build_read_atomic(read_u8_atomic, "b", uint8_t, "=q")
-build_read_atomic(read_u16_atomic, "w", uint16_t, "=r")
-build_read_atomic(read_u32_atomic, "l", uint32_t, "=r")
-build_read_atomic(read_u64_atomic, "q", uint64_t, "=r")
-
-build_write_atomic(write_u8_atomic, "b", uint8_t, "q")
-build_write_atomic(write_u16_atomic, "w", uint16_t, "r")
-build_write_atomic(write_u32_atomic, "l", uint32_t, "r")
-build_write_atomic(write_u64_atomic, "q", uint64_t, "r")
-
-build_add_sized(add_u8_sized, "b", uint8_t, "qi")
-build_add_sized(add_u16_sized, "w", uint16_t, "ri")
-build_add_sized(add_u32_sized, "l", uint32_t, "ri")
-build_add_sized(add_u64_sized, "q", uint64_t, "ri")
-
-#undef build_read_atomic
-#undef build_write_atomic
-#undef build_add_sized
+build_rw_atomic( 8, b, q, i)
+build_rw_atomic(16, w, r, i)
+build_rw_atomic(32, l, r, i)
+build_rw_atomic(64, q, r, e)
+
+build_arith_sized(add, 8, b, qi)
+build_arith_sized(add, 16, w, ri)
+build_arith_sized(add, 32, l, ri)
+build_arith_sized(add, 64, q, re)
+
+#undef build_rw_atomic
+#undef build_arith_sized
void __bad_atomic_size(void);
[-- Attachment #2: x86-rw-atomic-fold.patch --]
[-- Type: text/plain, Size: 3419 bytes --]
x86: consolidate atomic build_*() macros
Use a single macro to define both read and write inline functions.
Avoid redundant inputs (including quotes - use stringification
instead). Generalize "add" to ease eventual addition of other
artihmetic operations.
At once correct the artihmetic asm()'s output constraint (needs to be
"+" instead of "="), its 64-bit immediate one, and permit suitable
immediates for writes.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -4,46 +4,41 @@
#include <xen/atomic.h>
#include <asm/system.h>
-#define build_read_atomic(name, size, type, reg) \
-static inline type name(const volatile type *addr) \
+#define build_rw_atomic(width, size, reg, imm) \
+static inline uint ## width ## _t \
+read_u ## width ## _atomic(const volatile uint ## width ## _t *addr) \
{ \
- type ret; \
- asm volatile ( "mov" size " %1,%0" : reg (ret) : "m" (*addr) ); \
+ uint ## width ## _t ret; \
+ asm volatile ( "mov" #size " %1,%0" : "=" #reg (ret) : "m" (*addr) ); \
return ret; \
+} \
+static inline void \
+write_u ## width ## _atomic(volatile uint ## width ## _t *addr, \
+ uint ## width ## _t val) \
+{ \
+ asm volatile ( "mov" #size " %1,%0" : "=m" (*addr) : #reg #imm (val) ); \
}
-#define build_write_atomic(name, size, type, reg) \
-static inline void name(volatile type *addr, type val) \
+#define build_arith_sized(op, width, size, input) \
+static inline void \
+op ## _u ## width ## _sized(volatile uint ## width ## _t *addr, \
+ uint ## width ## _t val) \
{ \
- asm volatile ( "mov" size " %1,%0" : "=m" (*addr) : reg (val) ); \
+ asm volatile ( #op #size " %1,%0" : "+m" (*addr) : #input (val) ); \
}
-#define build_add_sized(name, size, type, reg) \
- static inline void name(volatile type *addr, type val) \
- { \
- asm volatile("add" size " %1,%0" \
- : "=m" (*addr) \
- : reg (val)); \
- }
-
-build_read_atomic(read_u8_atomic, "b", uint8_t, "=q")
-build_read_atomic(read_u16_atomic, "w", uint16_t, "=r")
-build_read_atomic(read_u32_atomic, "l", uint32_t, "=r")
-build_read_atomic(read_u64_atomic, "q", uint64_t, "=r")
-
-build_write_atomic(write_u8_atomic, "b", uint8_t, "q")
-build_write_atomic(write_u16_atomic, "w", uint16_t, "r")
-build_write_atomic(write_u32_atomic, "l", uint32_t, "r")
-build_write_atomic(write_u64_atomic, "q", uint64_t, "r")
-
-build_add_sized(add_u8_sized, "b", uint8_t, "qi")
-build_add_sized(add_u16_sized, "w", uint16_t, "ri")
-build_add_sized(add_u32_sized, "l", uint32_t, "ri")
-build_add_sized(add_u64_sized, "q", uint64_t, "ri")
-
-#undef build_read_atomic
-#undef build_write_atomic
-#undef build_add_sized
+build_rw_atomic( 8, b, q, i)
+build_rw_atomic(16, w, r, i)
+build_rw_atomic(32, l, r, i)
+build_rw_atomic(64, q, r, e)
+
+build_arith_sized(add, 8, b, qi)
+build_arith_sized(add, 16, w, ri)
+build_arith_sized(add, 32, l, ri)
+build_arith_sized(add, 64, q, re)
+
+#undef build_rw_atomic
+#undef build_arith_sized
void __bad_atomic_size(void);
[-- Attachment #3: 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] 6+ messages in thread
* Re: [PATCH 1/2] x86: drop unused barrier parameter from build_{read, write}_atomic()
2017-06-09 12:43 ` [PATCH 1/2] x86: drop unused barrier parameter from build_{read, write}_atomic() Jan Beulich
@ 2017-06-09 18:28 ` Andrew Cooper
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2017-06-09 18:28 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 09/06/17 13:43, Jan Beulich wrote:
> Also take the opportunity and make an attempt at making the macro
> definitions readable. Drop pointless casts while doing so.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, although as I've
said before ...
>
> --- a/xen/include/asm-x86/atomic.h
> +++ b/xen/include/asm-x86/atomic.h
> @@ -4,15 +4,19 @@
> #include <xen/atomic.h>
> #include <asm/system.h>
>
> -#define build_read_atomic(name, size, type, reg, barrier) \
> +#define build_read_atomic(name, size, type, reg) \
> static inline type name(const volatile type *addr) \
> -{ type ret; asm volatile("mov" size " %1,%0":reg (ret) \
> -:"m" (*(volatile type *)addr) barrier); return ret; }
> +{ \
> + type ret; \
> + asm volatile ( "mov" size " %1,%0" : reg (ret) : "m" (*addr) ); \
> + return ret; \
> +}
... this style of \'s is still less readable than lining them out of the
way on the right hand side.
Actually, for the sake of a couple of lines, I don't think having these
definitions obfuscated by macros is helpful in the first place.
~Andrew
>
> -#define build_write_atomic(name, size, type, reg, barrier) \
> +#define build_write_atomic(name, size, type, reg) \
> static inline void name(volatile type *addr, type val) \
> -{ asm volatile("mov" size " %1,%0": "=m" (*(volatile type *)addr) \
> -:reg (val) barrier); }
> +{ \
> + asm volatile ( "mov" size " %1,%0" : "=m" (*addr) : reg (val) ); \
> +}
>
> #define build_add_sized(name, size, type, reg) \
> static inline void name(volatile type *addr, type val) \
> @@ -22,15 +26,15 @@ static inline void name(volatile type *a
> : reg (val)); \
> }
>
> -build_read_atomic(read_u8_atomic, "b", uint8_t, "=q", )
> -build_read_atomic(read_u16_atomic, "w", uint16_t, "=r", )
> -build_read_atomic(read_u32_atomic, "l", uint32_t, "=r", )
> -build_read_atomic(read_u64_atomic, "q", uint64_t, "=r", )
> -
> -build_write_atomic(write_u8_atomic, "b", uint8_t, "q", )
> -build_write_atomic(write_u16_atomic, "w", uint16_t, "r", )
> -build_write_atomic(write_u32_atomic, "l", uint32_t, "r", )
> -build_write_atomic(write_u64_atomic, "q", uint64_t, "r", )
> +build_read_atomic(read_u8_atomic, "b", uint8_t, "=q")
> +build_read_atomic(read_u16_atomic, "w", uint16_t, "=r")
> +build_read_atomic(read_u32_atomic, "l", uint32_t, "=r")
> +build_read_atomic(read_u64_atomic, "q", uint64_t, "=r")
> +
> +build_write_atomic(write_u8_atomic, "b", uint8_t, "q")
> +build_write_atomic(write_u16_atomic, "w", uint16_t, "r")
> +build_write_atomic(write_u32_atomic, "l", uint32_t, "r")
> +build_write_atomic(write_u64_atomic, "q", uint64_t, "r")
>
> build_add_sized(add_u8_sized, "b", uint8_t, "qi")
> build_add_sized(add_u16_sized, "w", uint16_t, "ri")
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] x86: consolidate atomic build_*() macros
2017-06-09 12:44 ` [PATCH 2/2] x86: consolidate atomic build_*() macros Jan Beulich
@ 2017-06-09 18:45 ` Andrew Cooper
2017-06-12 6:31 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2017-06-09 18:45 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 09/06/17 13:44, Jan Beulich wrote:
> Use a single macro to define both read and write inline functions.
> Avoid redundant inputs (including quotes - use stringification
> instead). Generalize "add" to ease eventual addition of other
> artihmetic operations.
>
> At once correct the artihmetic asm()'s output constraint (needs to be
> "+" instead of "="), its 64-bit immediate one, and permit suitable
> immediates for writes.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
I can appreciate what you are trying to do here, but it is a step
backwards in terms of readability. At least in the past, if you needed
to find the implementation of read_u16_atomic, you could grep for it.
Its now completely hidden from any tools.
~Andrew
>
> --- a/xen/include/asm-x86/atomic.h
> +++ b/xen/include/asm-x86/atomic.h
> @@ -4,46 +4,41 @@
> #include <xen/atomic.h>
> #include <asm/system.h>
>
> -#define build_read_atomic(name, size, type, reg) \
> -static inline type name(const volatile type *addr) \
> +#define build_rw_atomic(width, size, reg, imm) \
> +static inline uint ## width ## _t \
> +read_u ## width ## _atomic(const volatile uint ## width ## _t *addr) \
> { \
> - type ret; \
> - asm volatile ( "mov" size " %1,%0" : reg (ret) : "m" (*addr) ); \
> + uint ## width ## _t ret; \
> + asm volatile ( "mov" #size " %1,%0" : "=" #reg (ret) : "m" (*addr) ); \
> return ret; \
> +} \
> +static inline void \
> +write_u ## width ## _atomic(volatile uint ## width ## _t *addr, \
> + uint ## width ## _t val) \
> +{ \
> + asm volatile ( "mov" #size " %1,%0" : "=m" (*addr) : #reg #imm (val) ); \
> }
>
> -#define build_write_atomic(name, size, type, reg) \
> -static inline void name(volatile type *addr, type val) \
> +#define build_arith_sized(op, width, size, input) \
> +static inline void \
> +op ## _u ## width ## _sized(volatile uint ## width ## _t *addr, \
> + uint ## width ## _t val) \
> { \
> - asm volatile ( "mov" size " %1,%0" : "=m" (*addr) : reg (val) ); \
> + asm volatile ( #op #size " %1,%0" : "+m" (*addr) : #input (val) ); \
> }
>
> -#define build_add_sized(name, size, type, reg) \
> - static inline void name(volatile type *addr, type val) \
> - { \
> - asm volatile("add" size " %1,%0" \
> - : "=m" (*addr) \
> - : reg (val)); \
> - }
> -
> -build_read_atomic(read_u8_atomic, "b", uint8_t, "=q")
> -build_read_atomic(read_u16_atomic, "w", uint16_t, "=r")
> -build_read_atomic(read_u32_atomic, "l", uint32_t, "=r")
> -build_read_atomic(read_u64_atomic, "q", uint64_t, "=r")
> -
> -build_write_atomic(write_u8_atomic, "b", uint8_t, "q")
> -build_write_atomic(write_u16_atomic, "w", uint16_t, "r")
> -build_write_atomic(write_u32_atomic, "l", uint32_t, "r")
> -build_write_atomic(write_u64_atomic, "q", uint64_t, "r")
> -
> -build_add_sized(add_u8_sized, "b", uint8_t, "qi")
> -build_add_sized(add_u16_sized, "w", uint16_t, "ri")
> -build_add_sized(add_u32_sized, "l", uint32_t, "ri")
> -build_add_sized(add_u64_sized, "q", uint64_t, "ri")
> -
> -#undef build_read_atomic
> -#undef build_write_atomic
> -#undef build_add_sized
> +build_rw_atomic( 8, b, q, i)
> +build_rw_atomic(16, w, r, i)
> +build_rw_atomic(32, l, r, i)
> +build_rw_atomic(64, q, r, e)
> +
> +build_arith_sized(add, 8, b, qi)
> +build_arith_sized(add, 16, w, ri)
> +build_arith_sized(add, 32, l, ri)
> +build_arith_sized(add, 64, q, re)
> +
> +#undef build_rw_atomic
> +#undef build_arith_sized
>
> void __bad_atomic_size(void);
>
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] x86: consolidate atomic build_*() macros
2017-06-09 18:45 ` Andrew Cooper
@ 2017-06-12 6:31 ` Jan Beulich
0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2017-06-12 6:31 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
>>> On 09.06.17 at 20:45, <andrew.cooper3@citrix.com> wrote:
> On 09/06/17 13:44, Jan Beulich wrote:
>> Use a single macro to define both read and write inline functions.
>> Avoid redundant inputs (including quotes - use stringification
>> instead). Generalize "add" to ease eventual addition of other
>> artihmetic operations.
>>
>> At once correct the artihmetic asm()'s output constraint (needs to be
>> "+" instead of "="), its 64-bit immediate one, and permit suitable
>> immediates for writes.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> I can appreciate what you are trying to do here, but it is a step
> backwards in terms of readability. At least in the past, if you needed
> to find the implementation of read_u16_atomic, you could grep for it.
> Its now completely hidden from any tools.
But these are helper functions needed only in nearby code, so
anyone looking for their definition when (s)he's not already
looking at this code is very likely misguided. IOW hiding them
from any such tools is kind of a nice side effect in this specific
case.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-06-12 6:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-09 12:19 [PATCH 0/2] x86: atomic.h polishing Jan Beulich
2017-06-09 12:43 ` [PATCH 1/2] x86: drop unused barrier parameter from build_{read, write}_atomic() Jan Beulich
2017-06-09 18:28 ` Andrew Cooper
2017-06-09 12:44 ` [PATCH 2/2] x86: consolidate atomic build_*() macros Jan Beulich
2017-06-09 18:45 ` Andrew Cooper
2017-06-12 6:31 ` Jan Beulich
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).