Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1416737 > unrolled thread

[PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs "H. Peter Anvin" <hpa@linux.intel.com> - 2016-06-08 01:40 +0200
    [tip:x86/asm] x86, asm: use bool for bitops and other assembly  outputs "tip-bot for H. Peter Anvin" <tipbot@zytor.com> - 2016-06-08 01:50 +0200
    Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs Peter Zijlstra <peterz@infradead.org> - 2016-06-08 10:00 +0200
      Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs Ingo Molnar <mingo@kernel.org> - 2016-06-08 10:30 +0200
        Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs Ingo Molnar <mingo@kernel.org> - 2016-06-08 10:40 +0200
          Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs "H. Peter Anvin" <hpa@zytor.com> - 2016-06-08 11:00 +0200
            Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs Ingo Molnar <mingo@kernel.org> - 2016-06-08 11:10 +0200
              Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs "H. Peter Anvin" <hpa@zytor.com> - 2016-06-08 11:20 +0200
                Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs Ingo Molnar <mingo@kernel.org> - 2016-06-08 11:30 +0200
                  Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs "H. Peter Anvin" <hpa@zytor.com> - 2016-06-08 11:40 +0200
                    Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs Peter Zijlstra <peterz@infradead.org> - 2016-06-08 11:50 +0200
                      Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs "H. Peter Anvin" <hpa@zytor.com> - 2016-06-08 11:50 +0200
                  Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs "H. Peter Anvin" <hpa@zytor.com> - 2016-06-08 11:50 +0200
        Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs "H. Peter Anvin" <hpa@zytor.com> - 2016-06-08 11:00 +0200
          Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly  outputs Ingo Molnar <mingo@kernel.org> - 2016-06-08 11:10 +0200

#1416737 — [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

From: "H. Peter Anvin" <hpa@zytor.com>

The gcc people have confirmed that using "bool" when combined with
inline assembly always is treated as a byte-sized operand that can be
assumed to be 0 or 1, which is exactly what the SET instruction
emits.  Change the output types and intermediate variables of as many
operations as practical to "bool".

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/boot/bitops.h             |  8 +++++---
 arch/x86/boot/boot.h               |  8 ++++----
 arch/x86/boot/string.c             |  2 +-
 arch/x86/include/asm/apm.h         |  6 +++---
 arch/x86/include/asm/archrandom.h  | 16 ++++++++--------
 arch/x86/include/asm/atomic.h      |  8 ++++----
 arch/x86/include/asm/atomic64_64.h | 10 +++++-----
 arch/x86/include/asm/bitops.h      | 28 ++++++++++++++--------------
 arch/x86/include/asm/local.h       |  8 ++++----
 arch/x86/include/asm/percpu.h      |  8 ++++----
 arch/x86/include/asm/rmwcc.h       |  4 ++--
 arch/x86/include/asm/rwsem.h       | 17 +++++++++--------
 include/linux/random.h             | 12 ++++++------
 13 files changed, 69 insertions(+), 66 deletions(-)

diff --git a/arch/x86/boot/bitops.h b/arch/x86/boot/bitops.h
index 878e4b9..0d41d68 100644
--- a/arch/x86/boot/bitops.h
+++ b/arch/x86/boot/bitops.h
@@ -16,14 +16,16 @@
 #define BOOT_BITOPS_H
 #define _LINUX_BITOPS_H		/* Inhibit inclusion of <linux/bitops.h> */
 
-static inline int constant_test_bit(int nr, const void *addr)
+#include <linux/types.h>
+
+static inline bool constant_test_bit(int nr, const void *addr)
 {
 	const u32 *p = (const u32 *)addr;
 	return ((1UL << (nr & 31)) & (p[nr >> 5])) != 0;
 }
-static inline int variable_test_bit(int nr, const void *addr)
+static inline bool variable_test_bit(int nr, const void *addr)
 {
-	u8 v;
+	bool v;
 	const u32 *p = (const u32 *)addr;
 
 	asm("btl %2,%1; setc %0" : "=qm" (v) : "m" (*p), "Ir" (nr));
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 9011a88..2edb2d5 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -176,16 +176,16 @@ static inline void wrgs32(u32 v, addr_t addr)
 }
 
 /* Note: these only return true/false, not a signed return value! */
-static inline int memcmp_fs(const void *s1, addr_t s2, size_t len)
+static inline bool memcmp_fs(const void *s1, addr_t s2, size_t len)
 {
-	u8 diff;
+	bool diff;
 	asm volatile("fs; repe; cmpsb; setnz %0"
 		     : "=qm" (diff), "+D" (s1), "+S" (s2), "+c" (len));
 	return diff;
 }
-static inline int memcmp_gs(const void *s1, addr_t s2, size_t len)
+static inline bool memcmp_gs(const void *s1, addr_t s2, size_t len)
 {
-	u8 diff;
+	bool diff;
 	asm volatile("gs; repe; cmpsb; setnz %0"
 		     : "=qm" (diff), "+D" (s1), "+S" (s2), "+c" (len));
 	return diff;
diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
index 318b846..cc3bd58 100644
--- a/arch/x86/boot/string.c
+++ b/arch/x86/boot/string.c
@@ -17,7 +17,7 @@
 
 int memcmp(const void *s1, const void *s2, size_t len)
 {
-	u8 diff;
+	bool diff;
 	asm("repe; cmpsb; setnz %0"
 	    : "=qm" (diff), "+D" (s1), "+S" (s2), "+c" (len));
 	return diff;
diff --git a/arch/x86/include/asm/apm.h b/arch/x86/include/asm/apm.h
index 20370c6..93eebc63 100644
--- a/arch/x86/include/asm/apm.h
+++ b/arch/x86/include/asm/apm.h
@@ -45,11 +45,11 @@ static inline void apm_bios_call_asm(u32 func, u32 ebx_in, u32 ecx_in,
 		: "memory", "cc");
 }
 
-static inline u8 apm_bios_call_simple_asm(u32 func, u32 ebx_in,
-						u32 ecx_in, u32 *eax)
+static inline bool apm_bios_call_simple_asm(u32 func, u32 ebx_in,
+					    u32 ecx_in, u32 *eax)
 {
 	int	cx, dx, si;
-	u8	error;
+	bool	error;
 
 	/*
 	 * N.B. We do NOT need a cld after the BIOS call
diff --git a/arch/x86/include/asm/archrandom.h b/arch/x86/include/asm/archrandom.h
index 69f1366..ab6f599 100644
--- a/arch/x86/include/asm/archrandom.h
+++ b/arch/x86/include/asm/archrandom.h
@@ -43,7 +43,7 @@
 #ifdef CONFIG_ARCH_RANDOM
 
 /* Instead of arch_get_random_long() when alternatives haven't run. */
-static inline int rdrand_long(unsigned long *v)
+static inline bool rdrand_long(unsigned long *v)
 {
 	int ok;
 	asm volatile("1: " RDRAND_LONG "\n\t"
@@ -53,13 +53,13 @@ static inline int rdrand_long(unsigned long *v)
 		     "2:"
 		     : "=r" (ok), "=a" (*v)
 		     : "0" (RDRAND_RETRY_LOOPS));
-	return ok;
+	return !!ok;
 }
 
 /* A single attempt at RDSEED */
 static inline bool rdseed_long(unsigned long *v)
 {
-	unsigned char ok;
+	bool ok;
 	asm volatile(RDSEED_LONG "\n\t"
 		     "setc %0"
 		     : "=qm" (ok), "=a" (*v));
@@ -67,7 +67,7 @@ static inline bool rdseed_long(unsigned long *v)
 }
 
 #define GET_RANDOM(name, type, rdrand, nop)			\
-static inline int name(type *v)					\
+static inline bool name(type *v)				\
 {								\
 	int ok;							\
 	alternative_io("movl $0, %0\n\t"			\
@@ -80,13 +80,13 @@ static inline int name(type *v)					\
 		       X86_FEATURE_RDRAND,                      \
 		       ASM_OUTPUT2("=r" (ok), "=a" (*v)),       \
 		       "0" (RDRAND_RETRY_LOOPS));		\
-	return ok;						\
+	return !!ok;						\
 }
 
 #define GET_SEED(name, type, rdseed, nop)			\
-static inline int name(type *v)					\
+static inline bool name(type *v)				\
 {								\
-	unsigned char ok;					\
+	bool ok;						\
 	alternative_io("movb $0, %0\n\t"			\
 		       nop,					\
 		       rdseed "\n\t"				\
@@ -119,7 +119,7 @@ GET_SEED(arch_get_random_seed_int, unsigned int, RDSEED_INT, ASM_NOP4);
 
 #else
 
-static inline int rdrand_long(unsigned long *v)
+static inline bool rdrand_long(unsigned long *v)
 {
 	return 0;
 }
diff --git a/arch/x86/include/asm/atomic.h b/arch/x86/include/asm/atomic.h
index 3e86742..17d8812 100644
--- a/arch/x86/include/asm/atomic.h
+++ b/arch/x86/include/asm/atomic.h
@@ -75,7 +75,7 @@ static __always_inline void atomic_sub(int i, atomic_t *v)
  * true if the result is zero, or false for all
  * other cases.
  */
-static __always_inline int atomic_sub_and_test(int i, atomic_t *v)
+static __always_inline bool atomic_sub_and_test(int i, atomic_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "subl", v->counter, "er", i, "%0", "e");
 }
@@ -112,7 +112,7 @@ static __always_inline void atomic_dec(atomic_t *v)
  * returns true if the result is 0, or false for all other
  * cases.
  */
-static __always_inline int atomic_dec_and_test(atomic_t *v)
+static __always_inline bool atomic_dec_and_test(atomic_t *v)
 {
 	GEN_UNARY_RMWcc(LOCK_PREFIX "decl", v->counter, "%0", "e");
 }
@@ -125,7 +125,7 @@ static __always_inline int atomic_dec_and_test(atomic_t *v)
  * and returns true if the result is zero, or false for all
  * other cases.
  */
-static __always_inline int atomic_inc_and_test(atomic_t *v)
+static __always_inline bool atomic_inc_and_test(atomic_t *v)
 {
 	GEN_UNARY_RMWcc(LOCK_PREFIX "incl", v->counter, "%0", "e");
 }
@@ -139,7 +139,7 @@ static __always_inline int atomic_inc_and_test(atomic_t *v)
  * if the result is negative, or false when
  * result is greater than or equal to zero.
  */
-static __always_inline int atomic_add_negative(int i, atomic_t *v)
+static __always_inline bool atomic_add_negative(int i, atomic_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "addl", v->counter, "er", i, "%0", "s");
 }
diff --git a/arch/x86/include/asm/atomic64_64.h b/arch/x86/include/asm/atomic64_64.h
index 0373510..4f881d7 100644
--- a/arch/x86/include/asm/atomic64_64.h
+++ b/arch/x86/include/asm/atomic64_64.h
@@ -70,7 +70,7 @@ static inline void atomic64_sub(long i, atomic64_t *v)
  * true if the result is zero, or false for all
  * other cases.
  */
-static inline int atomic64_sub_and_test(long i, atomic64_t *v)
+static inline bool atomic64_sub_and_test(long i, atomic64_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "subq", v->counter, "er", i, "%0", "e");
 }
@@ -109,7 +109,7 @@ static __always_inline void atomic64_dec(atomic64_t *v)
  * returns true if the result is 0, or false for all other
  * cases.
  */
-static inline int atomic64_dec_and_test(atomic64_t *v)
+static inline bool atomic64_dec_and_test(atomic64_t *v)
 {
 	GEN_UNARY_RMWcc(LOCK_PREFIX "decq", v->counter, "%0", "e");
 }
@@ -122,7 +122,7 @@ static inline int atomic64_dec_and_test(atomic64_t *v)
  * and returns true if the result is zero, or false for all
  * other cases.
  */
-static inline int atomic64_inc_and_test(atomic64_t *v)
+static inline bool atomic64_inc_and_test(atomic64_t *v)
 {
 	GEN_UNARY_RMWcc(LOCK_PREFIX "incq", v->counter, "%0", "e");
 }
@@ -136,7 +136,7 @@ static inline int atomic64_inc_and_test(atomic64_t *v)
  * if the result is negative, or false when
  * result is greater than or equal to zero.
  */
-static inline int atomic64_add_negative(long i, atomic64_t *v)
+static inline bool atomic64_add_negative(long i, atomic64_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "addq", v->counter, "er", i, "%0", "s");
 }
@@ -180,7 +180,7 @@ static inline long atomic64_xchg(atomic64_t *v, long new)
  * Atomically adds @a to @v, so long as it was not @u.
  * Returns the old value of @v.
  */
-static inline int atomic64_add_unless(atomic64_t *v, long a, long u)
+static inline bool atomic64_add_unless(atomic64_t *v, long a, long u)
 {
 	long c, old;
 	c = atomic64_read(v);
diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index b2b797d..8cbb7f4 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -201,7 +201,7 @@ static __always_inline void change_bit(long nr, volatile unsigned long *addr)
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static __always_inline int test_and_set_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool test_and_set_bit(long nr, volatile unsigned long *addr)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "bts", *addr, "Ir", nr, "%0", "c");
 }
@@ -213,7 +213,7 @@ static __always_inline int test_and_set_bit(long nr, volatile unsigned long *add
  *
  * This is the same as test_and_set_bit on x86.
  */
-static __always_inline int
+static __always_inline bool
 test_and_set_bit_lock(long nr, volatile unsigned long *addr)
 {
 	return test_and_set_bit(nr, addr);
@@ -228,9 +228,9 @@ test_and_set_bit_lock(long nr, volatile unsigned long *addr)
  * If two examples of this operation race, one can appear to succeed
  * but actually fail.  You must protect multiple accesses with a lock.
  */
-static __always_inline int __test_and_set_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool __test_and_set_bit(long nr, volatile unsigned long *addr)
 {
-	unsigned char oldbit;
+	bool oldbit;
 
 	asm("bts %2,%1\n\t"
 	    "setc %0"
@@ -247,7 +247,7 @@ static __always_inline int __test_and_set_bit(long nr, volatile unsigned long *a
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static __always_inline int test_and_clear_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool test_and_clear_bit(long nr, volatile unsigned long *addr)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "btr", *addr, "Ir", nr, "%0", "c");
 }
@@ -268,9 +268,9 @@ static __always_inline int test_and_clear_bit(long nr, volatile unsigned long *a
  * accessed from a hypervisor on the same CPU if running in a VM: don't change
  * this without also updating arch/x86/kernel/kvm.c
  */
-static __always_inline int __test_and_clear_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool __test_and_clear_bit(long nr, volatile unsigned long *addr)
 {
-	unsigned char oldbit;
+	bool oldbit;
 
 	asm volatile("btr %2,%1\n\t"
 		     "setc %0"
@@ -280,9 +280,9 @@ static __always_inline int __test_and_clear_bit(long nr, volatile unsigned long
 }
 
 /* WARNING: non atomic and it can be reordered! */
-static __always_inline int __test_and_change_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool __test_and_change_bit(long nr, volatile unsigned long *addr)
 {
-	unsigned char oldbit;
+	bool oldbit;
 
 	asm volatile("btc %2,%1\n\t"
 		     "setc %0"
@@ -300,20 +300,20 @@ static __always_inline int __test_and_change_bit(long nr, volatile unsigned long
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static __always_inline int test_and_change_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool test_and_change_bit(long nr, volatile unsigned long *addr)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "btc", *addr, "Ir", nr, "%0", "c");
 }
 
-static __always_inline int constant_test_bit(long nr, const volatile unsigned long *addr)
+static __always_inline bool constant_test_bit(long nr, const volatile unsigned long *addr)
 {
 	return ((1UL << (nr & (BITS_PER_LONG-1))) &
 		(addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
 }
 
-static __always_inline int variable_test_bit(long nr, volatile const unsigned long *addr)
+static __always_inline bool variable_test_bit(long nr, volatile const unsigned long *addr)
 {
-	unsigned char oldbit;
+	bool oldbit;
 
 	asm volatile("bt %2,%1\n\t"
 		     "setc %0"
@@ -329,7 +329,7 @@ static __always_inline int variable_test_bit(long nr, volatile const unsigned lo
  * @nr: bit number to test
  * @addr: Address to start counting from
  */
-static int test_bit(int nr, const volatile unsigned long *addr);
+static bool test_bit(int nr, const volatile unsigned long *addr);
 #endif
 
 #define test_bit(nr, addr)			\
diff --git a/arch/x86/include/asm/local.h b/arch/x86/include/asm/local.h
index 4ad6560..0cdc65b 100644
--- a/arch/x86/include/asm/local.h
+++ b/arch/x86/include/asm/local.h
@@ -50,7 +50,7 @@ static inline void local_sub(long i, local_t *l)
  * true if the result is zero, or false for all
  * other cases.
  */
-static inline int local_sub_and_test(long i, local_t *l)
+static inline bool local_sub_and_test(long i, local_t *l)
 {
 	GEN_BINARY_RMWcc(_ASM_SUB, l->a.counter, "er", i, "%0", "e");
 }
@@ -63,7 +63,7 @@ static inline int local_sub_and_test(long i, local_t *l)
  * returns true if the result is 0, or false for all other
  * cases.
  */
-static inline int local_dec_and_test(local_t *l)
+static inline bool local_dec_and_test(local_t *l)
 {
 	GEN_UNARY_RMWcc(_ASM_DEC, l->a.counter, "%0", "e");
 }
@@ -76,7 +76,7 @@ static inline int local_dec_and_test(local_t *l)
  * and returns true if the result is zero, or false for all
  * other cases.
  */
-static inline int local_inc_and_test(local_t *l)
+static inline bool local_inc_and_test(local_t *l)
 {
 	GEN_UNARY_RMWcc(_ASM_INC, l->a.counter, "%0", "e");
 }
@@ -90,7 +90,7 @@ static inline int local_inc_and_test(local_t *l)
  * if the result is negative, or false when
  * result is greater than or equal to zero.
  */
-static inline int local_add_negative(long i, local_t *l)
+static inline bool local_add_negative(long i, local_t *l)
 {
 	GEN_BINARY_RMWcc(_ASM_ADD, l->a.counter, "er", i, "%0", "s");
 }
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 65039e9..184d7f3 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -510,14 +510,14 @@ do {									\
 /* This is not atomic against other CPUs -- CPU preemption needs to be off */
 #define x86_test_and_clear_bit_percpu(bit, var)				\
 ({									\
-	unsigned char old__;						\
+	bool old__;							\
 	asm volatile("btr %2,"__percpu_arg(1)"\n\tsetc %0"		\
 		     : "=qm" (old__), "+m" (var)			\
 		     : "dIr" (bit));					\
 	old__;								\
 })
 
-static __always_inline int x86_this_cpu_constant_test_bit(unsigned int nr,
+static __always_inline bool x86_this_cpu_constant_test_bit(unsigned int nr,
                         const unsigned long __percpu *addr)
 {
 	unsigned long __percpu *a = (unsigned long *)addr + nr / BITS_PER_LONG;
@@ -529,10 +529,10 @@ static __always_inline int x86_this_cpu_constant_test_bit(unsigned int nr,
 #endif
 }
 
-static inline int x86_this_cpu_variable_test_bit(int nr,
+static inline bool x86_this_cpu_variable_test_bit(int nr,
                         const unsigned long __percpu *addr)
 {
-	unsigned char oldbit;
+	bool oldbit;
 
 	asm volatile("bt "__percpu_arg(2)",%1\n\t"
 			"setc %0"
diff --git a/arch/x86/include/asm/rmwcc.h b/arch/x86/include/asm/rmwcc.h
index 8f7866a..a15b73d 100644
--- a/arch/x86/include/asm/rmwcc.h
+++ b/arch/x86/include/asm/rmwcc.h
@@ -23,11 +23,11 @@ cc_label:								\
 
 #define __GEN_RMWcc(fullop, var, cc, ...)				\
 do {									\
-	char c;								\
+	bool c;								\
 	asm volatile (fullop "; set" cc " %1"				\
 			: "+m" (var), "=qm" (c)				\
 			: __VA_ARGS__ : "memory");			\
-	return c != 0;							\
+	return c;							\
 } while (0)
 
 #define GEN_UNARY_RMWcc(op, var, arg0, cc)				\
diff --git a/arch/x86/include/asm/rwsem.h b/arch/x86/include/asm/rwsem.h
index 453744c..c508770 100644
--- a/arch/x86/include/asm/rwsem.h
+++ b/arch/x86/include/asm/rwsem.h
@@ -77,7 +77,7 @@ static inline void __down_read(struct rw_semaphore *sem)
 /*
  * trylock for reading -- returns 1 if successful, 0 if contention
  */
-static inline int __down_read_trylock(struct rw_semaphore *sem)
+static inline bool __down_read_trylock(struct rw_semaphore *sem)
 {
 	long result, tmp;
 	asm volatile("# beginning __down_read_trylock\n\t"
@@ -93,7 +93,7 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
 		     : "+m" (sem->count), "=&a" (result), "=&r" (tmp)
 		     : "i" (RWSEM_ACTIVE_READ_BIAS)
 		     : "memory", "cc");
-	return result >= 0 ? 1 : 0;
+	return result >= 0;
 }
 
 /*
@@ -134,9 +134,10 @@ static inline int __down_write_killable(struct rw_semaphore *sem)
 /*
  * trylock for writing -- returns 1 if successful, 0 if contention
  */
-static inline int __down_write_trylock(struct rw_semaphore *sem)
+static inline bool __down_write_trylock(struct rw_semaphore *sem)
 {
-	long result, tmp;
+	bool result;
+	long tmp0, tmp1;
 	asm volatile("# beginning __down_write_trylock\n\t"
 		     "  mov          %0,%1\n\t"
 		     "1:\n\t"
@@ -144,14 +145,14 @@ static inline int __down_write_trylock(struct rw_semaphore *sem)
 		     /* was the active mask 0 before? */
 		     "  jnz          2f\n\t"
 		     "  mov          %1,%2\n\t"
-		     "  add          %3,%2\n\t"
+		     "  add          %4,%2\n\t"
 		     LOCK_PREFIX "  cmpxchg  %2,%0\n\t"
 		     "  jnz	     1b\n\t"
 		     "2:\n\t"
-		     "  sete         %b1\n\t"
-		     "  movzbl       %b1, %k1\n\t"
+		     "  sete         %3\n\t"
 		     "# ending __down_write_trylock\n\t"
-		     : "+m" (sem->count), "=&a" (result), "=&r" (tmp)
+		     : "+m" (sem->count), "=&a" (tmp0), "=&r" (tmp1),
+		       "=qm" (result)
 		     : "er" (RWSEM_ACTIVE_WRITE_BIAS)
 		     : "memory", "cc");
 	return result;
diff --git a/include/linux/random.h b/include/linux/random.h
index e47e533..3d6e981 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -95,27 +95,27 @@ static inline void prandom_seed_state(struct rnd_state *state, u64 seed)
 #ifdef CONFIG_ARCH_RANDOM
 # include <asm/archrandom.h>
 #else
-static inline int arch_get_random_long(unsigned long *v)
+static inline bool arch_get_random_long(unsigned long *v)
 {
 	return 0;
 }
-static inline int arch_get_random_int(unsigned int *v)
+static inline bool arch_get_random_int(unsigned int *v)
 {
 	return 0;
 }
-static inline int arch_has_random(void)
+static inline bool arch_has_random(void)
 {
 	return 0;
 }
-static inline int arch_get_random_seed_long(unsigned long *v)
+static inline bool arch_get_random_seed_long(unsigned long *v)
 {
 	return 0;
 }
-static inline int arch_get_random_seed_int(unsigned int *v)
+static inline bool arch_get_random_seed_int(unsigned int *v)
 {
 	return 0;
 }
-static inline int arch_has_random_seed(void)
+static inline bool arch_has_random_seed(void)
 {
 	return 0;
 }
-- 
2.7.3.0.11.gd79db92

[toc] | [next] | [standalone]

#1416751 — [tip:x86/asm] x86, asm: use bool for bitops and other assembly outputs

Commit-ID:  d3f78b979e4e060c1b36402fa7096a36a9c266da
Gitweb:     http://git.kernel.org/tip/d3f78b979e4e060c1b36402fa7096a36a9c266da
Author:     H. Peter Anvin <hpa@zytor.com>
AuthorDate: Tue, 7 Jun 2016 16:31:01 -0700
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Tue, 7 Jun 2016 16:36:42 -0700

x86, asm: use bool for bitops and other assembly outputs

The gcc people have confirmed that using "bool" when combined with
inline assembly always is treated as a byte-sized operand that can be
assumed to be 0 or 1, which is exactly what the SET instruction
emits.  Change the output types and intermediate variables of as many
operations as practical to "bool".

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Link: http://lkml.kernel.org/r/1465342269-492350-3-git-send-email-hpa@linux.intel.com
---
 arch/x86/boot/bitops.h             |  8 +++++---
 arch/x86/boot/boot.h               |  8 ++++----
 arch/x86/boot/string.c             |  2 +-
 arch/x86/include/asm/apm.h         |  6 +++---
 arch/x86/include/asm/archrandom.h  | 16 ++++++++--------
 arch/x86/include/asm/atomic.h      |  8 ++++----
 arch/x86/include/asm/atomic64_64.h | 10 +++++-----
 arch/x86/include/asm/bitops.h      | 28 ++++++++++++++--------------
 arch/x86/include/asm/local.h       |  8 ++++----
 arch/x86/include/asm/percpu.h      |  8 ++++----
 arch/x86/include/asm/rmwcc.h       |  4 ++--
 arch/x86/include/asm/rwsem.h       | 17 +++++++++--------
 include/linux/random.h             | 12 ++++++------
 13 files changed, 69 insertions(+), 66 deletions(-)

diff --git a/arch/x86/boot/bitops.h b/arch/x86/boot/bitops.h
index 878e4b9..0d41d68 100644
--- a/arch/x86/boot/bitops.h
+++ b/arch/x86/boot/bitops.h
@@ -16,14 +16,16 @@
 #define BOOT_BITOPS_H
 #define _LINUX_BITOPS_H		/* Inhibit inclusion of <linux/bitops.h> */
 
-static inline int constant_test_bit(int nr, const void *addr)
+#include <linux/types.h>
+
+static inline bool constant_test_bit(int nr, const void *addr)
 {
 	const u32 *p = (const u32 *)addr;
 	return ((1UL << (nr & 31)) & (p[nr >> 5])) != 0;
 }
-static inline int variable_test_bit(int nr, const void *addr)
+static inline bool variable_test_bit(int nr, const void *addr)
 {
-	u8 v;
+	bool v;
 	const u32 *p = (const u32 *)addr;
 
 	asm("btl %2,%1; setc %0" : "=qm" (v) : "m" (*p), "Ir" (nr));
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 9011a88..2edb2d5 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -176,16 +176,16 @@ static inline void wrgs32(u32 v, addr_t addr)
 }
 
 /* Note: these only return true/false, not a signed return value! */
-static inline int memcmp_fs(const void *s1, addr_t s2, size_t len)
+static inline bool memcmp_fs(const void *s1, addr_t s2, size_t len)
 {
-	u8 diff;
+	bool diff;
 	asm volatile("fs; repe; cmpsb; setnz %0"
 		     : "=qm" (diff), "+D" (s1), "+S" (s2), "+c" (len));
 	return diff;
 }
-static inline int memcmp_gs(const void *s1, addr_t s2, size_t len)
+static inline bool memcmp_gs(const void *s1, addr_t s2, size_t len)
 {
-	u8 diff;
+	bool diff;
 	asm volatile("gs; repe; cmpsb; setnz %0"
 		     : "=qm" (diff), "+D" (s1), "+S" (s2), "+c" (len));
 	return diff;
diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
index 318b846..cc3bd58 100644
--- a/arch/x86/boot/string.c
+++ b/arch/x86/boot/string.c
@@ -17,7 +17,7 @@
 
 int memcmp(const void *s1, const void *s2, size_t len)
 {
-	u8 diff;
+	bool diff;
 	asm("repe; cmpsb; setnz %0"
 	    : "=qm" (diff), "+D" (s1), "+S" (s2), "+c" (len));
 	return diff;
diff --git a/arch/x86/include/asm/apm.h b/arch/x86/include/asm/apm.h
index 20370c6..93eebc63 100644
--- a/arch/x86/include/asm/apm.h
+++ b/arch/x86/include/asm/apm.h
@@ -45,11 +45,11 @@ static inline void apm_bios_call_asm(u32 func, u32 ebx_in, u32 ecx_in,
 		: "memory", "cc");
 }
 
-static inline u8 apm_bios_call_simple_asm(u32 func, u32 ebx_in,
-						u32 ecx_in, u32 *eax)
+static inline bool apm_bios_call_simple_asm(u32 func, u32 ebx_in,
+					    u32 ecx_in, u32 *eax)
 {
 	int	cx, dx, si;
-	u8	error;
+	bool	error;
 
 	/*
 	 * N.B. We do NOT need a cld after the BIOS call
diff --git a/arch/x86/include/asm/archrandom.h b/arch/x86/include/asm/archrandom.h
index 69f1366..ab6f599 100644
--- a/arch/x86/include/asm/archrandom.h
+++ b/arch/x86/include/asm/archrandom.h
@@ -43,7 +43,7 @@
 #ifdef CONFIG_ARCH_RANDOM
 
 /* Instead of arch_get_random_long() when alternatives haven't run. */
-static inline int rdrand_long(unsigned long *v)
+static inline bool rdrand_long(unsigned long *v)
 {
 	int ok;
 	asm volatile("1: " RDRAND_LONG "\n\t"
@@ -53,13 +53,13 @@ static inline int rdrand_long(unsigned long *v)
 		     "2:"
 		     : "=r" (ok), "=a" (*v)
 		     : "0" (RDRAND_RETRY_LOOPS));
-	return ok;
+	return !!ok;
 }
 
 /* A single attempt at RDSEED */
 static inline bool rdseed_long(unsigned long *v)
 {
-	unsigned char ok;
+	bool ok;
 	asm volatile(RDSEED_LONG "\n\t"
 		     "setc %0"
 		     : "=qm" (ok), "=a" (*v));
@@ -67,7 +67,7 @@ static inline bool rdseed_long(unsigned long *v)
 }
 
 #define GET_RANDOM(name, type, rdrand, nop)			\
-static inline int name(type *v)					\
+static inline bool name(type *v)				\
 {								\
 	int ok;							\
 	alternative_io("movl $0, %0\n\t"			\
@@ -80,13 +80,13 @@ static inline int name(type *v)					\
 		       X86_FEATURE_RDRAND,                      \
 		       ASM_OUTPUT2("=r" (ok), "=a" (*v)),       \
 		       "0" (RDRAND_RETRY_LOOPS));		\
-	return ok;						\
+	return !!ok;						\
 }
 
 #define GET_SEED(name, type, rdseed, nop)			\
-static inline int name(type *v)					\
+static inline bool name(type *v)				\
 {								\
-	unsigned char ok;					\
+	bool ok;						\
 	alternative_io("movb $0, %0\n\t"			\
 		       nop,					\
 		       rdseed "\n\t"				\
@@ -119,7 +119,7 @@ GET_SEED(arch_get_random_seed_int, unsigned int, RDSEED_INT, ASM_NOP4);
 
 #else
 
-static inline int rdrand_long(unsigned long *v)
+static inline bool rdrand_long(unsigned long *v)
 {
 	return 0;
 }
diff --git a/arch/x86/include/asm/atomic.h b/arch/x86/include/asm/atomic.h
index 3e86742..17d8812 100644
--- a/arch/x86/include/asm/atomic.h
+++ b/arch/x86/include/asm/atomic.h
@@ -75,7 +75,7 @@ static __always_inline void atomic_sub(int i, atomic_t *v)
  * true if the result is zero, or false for all
  * other cases.
  */
-static __always_inline int atomic_sub_and_test(int i, atomic_t *v)
+static __always_inline bool atomic_sub_and_test(int i, atomic_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "subl", v->counter, "er", i, "%0", "e");
 }
@@ -112,7 +112,7 @@ static __always_inline void atomic_dec(atomic_t *v)
  * returns true if the result is 0, or false for all other
  * cases.
  */
-static __always_inline int atomic_dec_and_test(atomic_t *v)
+static __always_inline bool atomic_dec_and_test(atomic_t *v)
 {
 	GEN_UNARY_RMWcc(LOCK_PREFIX "decl", v->counter, "%0", "e");
 }
@@ -125,7 +125,7 @@ static __always_inline int atomic_dec_and_test(atomic_t *v)
  * and returns true if the result is zero, or false for all
  * other cases.
  */
-static __always_inline int atomic_inc_and_test(atomic_t *v)
+static __always_inline bool atomic_inc_and_test(atomic_t *v)
 {
 	GEN_UNARY_RMWcc(LOCK_PREFIX "incl", v->counter, "%0", "e");
 }
@@ -139,7 +139,7 @@ static __always_inline int atomic_inc_and_test(atomic_t *v)
  * if the result is negative, or false when
  * result is greater than or equal to zero.
  */
-static __always_inline int atomic_add_negative(int i, atomic_t *v)
+static __always_inline bool atomic_add_negative(int i, atomic_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "addl", v->counter, "er", i, "%0", "s");
 }
diff --git a/arch/x86/include/asm/atomic64_64.h b/arch/x86/include/asm/atomic64_64.h
index 0373510..4f881d7 100644
--- a/arch/x86/include/asm/atomic64_64.h
+++ b/arch/x86/include/asm/atomic64_64.h
@@ -70,7 +70,7 @@ static inline void atomic64_sub(long i, atomic64_t *v)
  * true if the result is zero, or false for all
  * other cases.
  */
-static inline int atomic64_sub_and_test(long i, atomic64_t *v)
+static inline bool atomic64_sub_and_test(long i, atomic64_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "subq", v->counter, "er", i, "%0", "e");
 }
@@ -109,7 +109,7 @@ static __always_inline void atomic64_dec(atomic64_t *v)
  * returns true if the result is 0, or false for all other
  * cases.
  */
-static inline int atomic64_dec_and_test(atomic64_t *v)
+static inline bool atomic64_dec_and_test(atomic64_t *v)
 {
 	GEN_UNARY_RMWcc(LOCK_PREFIX "decq", v->counter, "%0", "e");
 }
@@ -122,7 +122,7 @@ static inline int atomic64_dec_and_test(atomic64_t *v)
  * and returns true if the result is zero, or false for all
  * other cases.
  */
-static inline int atomic64_inc_and_test(atomic64_t *v)
+static inline bool atomic64_inc_and_test(atomic64_t *v)
 {
 	GEN_UNARY_RMWcc(LOCK_PREFIX "incq", v->counter, "%0", "e");
 }
@@ -136,7 +136,7 @@ static inline int atomic64_inc_and_test(atomic64_t *v)
  * if the result is negative, or false when
  * result is greater than or equal to zero.
  */
-static inline int atomic64_add_negative(long i, atomic64_t *v)
+static inline bool atomic64_add_negative(long i, atomic64_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "addq", v->counter, "er", i, "%0", "s");
 }
@@ -180,7 +180,7 @@ static inline long atomic64_xchg(atomic64_t *v, long new)
  * Atomically adds @a to @v, so long as it was not @u.
  * Returns the old value of @v.
  */
-static inline int atomic64_add_unless(atomic64_t *v, long a, long u)
+static inline bool atomic64_add_unless(atomic64_t *v, long a, long u)
 {
 	long c, old;
 	c = atomic64_read(v);
diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index b2b797d..8cbb7f4 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -201,7 +201,7 @@ static __always_inline void change_bit(long nr, volatile unsigned long *addr)
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static __always_inline int test_and_set_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool test_and_set_bit(long nr, volatile unsigned long *addr)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "bts", *addr, "Ir", nr, "%0", "c");
 }
@@ -213,7 +213,7 @@ static __always_inline int test_and_set_bit(long nr, volatile unsigned long *add
  *
  * This is the same as test_and_set_bit on x86.
  */
-static __always_inline int
+static __always_inline bool
 test_and_set_bit_lock(long nr, volatile unsigned long *addr)
 {
 	return test_and_set_bit(nr, addr);
@@ -228,9 +228,9 @@ test_and_set_bit_lock(long nr, volatile unsigned long *addr)
  * If two examples of this operation race, one can appear to succeed
  * but actually fail.  You must protect multiple accesses with a lock.
  */
-static __always_inline int __test_and_set_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool __test_and_set_bit(long nr, volatile unsigned long *addr)
 {
-	unsigned char oldbit;
+	bool oldbit;
 
 	asm("bts %2,%1\n\t"
 	    "setc %0"
@@ -247,7 +247,7 @@ static __always_inline int __test_and_set_bit(long nr, volatile unsigned long *a
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static __always_inline int test_and_clear_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool test_and_clear_bit(long nr, volatile unsigned long *addr)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "btr", *addr, "Ir", nr, "%0", "c");
 }
@@ -268,9 +268,9 @@ static __always_inline int test_and_clear_bit(long nr, volatile unsigned long *a
  * accessed from a hypervisor on the same CPU if running in a VM: don't change
  * this without also updating arch/x86/kernel/kvm.c
  */
-static __always_inline int __test_and_clear_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool __test_and_clear_bit(long nr, volatile unsigned long *addr)
 {
-	unsigned char oldbit;
+	bool oldbit;
 
 	asm volatile("btr %2,%1\n\t"
 		     "setc %0"
@@ -280,9 +280,9 @@ static __always_inline int __test_and_clear_bit(long nr, volatile unsigned long
 }
 
 /* WARNING: non atomic and it can be reordered! */
-static __always_inline int __test_and_change_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool __test_and_change_bit(long nr, volatile unsigned long *addr)
 {
-	unsigned char oldbit;
+	bool oldbit;
 
 	asm volatile("btc %2,%1\n\t"
 		     "setc %0"
@@ -300,20 +300,20 @@ static __always_inline int __test_and_change_bit(long nr, volatile unsigned long
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static __always_inline int test_and_change_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool test_and_change_bit(long nr, volatile unsigned long *addr)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "btc", *addr, "Ir", nr, "%0", "c");
 }
 
-static __always_inline int constant_test_bit(long nr, const volatile unsigned long *addr)
+static __always_inline bool constant_test_bit(long nr, const volatile unsigned long *addr)
 {
 	return ((1UL << (nr & (BITS_PER_LONG-1))) &
 		(addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
 }
 
-static __always_inline int variable_test_bit(long nr, volatile const unsigned long *addr)
+static __always_inline bool variable_test_bit(long nr, volatile const unsigned long *addr)
 {
-	unsigned char oldbit;
+	bool oldbit;
 
 	asm volatile("bt %2,%1\n\t"
 		     "setc %0"
@@ -329,7 +329,7 @@ static __always_inline int variable_test_bit(long nr, volatile const unsigned lo
  * @nr: bit number to test
  * @addr: Address to start counting from
  */
-static int test_bit(int nr, const volatile unsigned long *addr);
+static bool test_bit(int nr, const volatile unsigned long *addr);
 #endif
 
 #define test_bit(nr, addr)			\
diff --git a/arch/x86/include/asm/local.h b/arch/x86/include/asm/local.h
index 4ad6560..0cdc65b 100644
--- a/arch/x86/include/asm/local.h
+++ b/arch/x86/include/asm/local.h
@@ -50,7 +50,7 @@ static inline void local_sub(long i, local_t *l)
  * true if the result is zero, or false for all
  * other cases.
  */
-static inline int local_sub_and_test(long i, local_t *l)
+static inline bool local_sub_and_test(long i, local_t *l)
 {
 	GEN_BINARY_RMWcc(_ASM_SUB, l->a.counter, "er", i, "%0", "e");
 }
@@ -63,7 +63,7 @@ static inline int local_sub_and_test(long i, local_t *l)
  * returns true if the result is 0, or false for all other
  * cases.
  */
-static inline int local_dec_and_test(local_t *l)
+static inline bool local_dec_and_test(local_t *l)
 {
 	GEN_UNARY_RMWcc(_ASM_DEC, l->a.counter, "%0", "e");
 }
@@ -76,7 +76,7 @@ static inline int local_dec_and_test(local_t *l)
  * and returns true if the result is zero, or false for all
  * other cases.
  */
-static inline int local_inc_and_test(local_t *l)
+static inline bool local_inc_and_test(local_t *l)
 {
 	GEN_UNARY_RMWcc(_ASM_INC, l->a.counter, "%0", "e");
 }
@@ -90,7 +90,7 @@ static inline int local_inc_and_test(local_t *l)
  * if the result is negative, or false when
  * result is greater than or equal to zero.
  */
-static inline int local_add_negative(long i, local_t *l)
+static inline bool local_add_negative(long i, local_t *l)
 {
 	GEN_BINARY_RMWcc(_ASM_ADD, l->a.counter, "er", i, "%0", "s");
 }
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 65039e9..184d7f3 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -510,14 +510,14 @@ do {									\
 /* This is not atomic against other CPUs -- CPU preemption needs to be off */
 #define x86_test_and_clear_bit_percpu(bit, var)				\
 ({									\
-	unsigned char old__;						\
+	bool old__;							\
 	asm volatile("btr %2,"__percpu_arg(1)"\n\tsetc %0"		\
 		     : "=qm" (old__), "+m" (var)			\
 		     : "dIr" (bit));					\
 	old__;								\
 })
 
-static __always_inline int x86_this_cpu_constant_test_bit(unsigned int nr,
+static __always_inline bool x86_this_cpu_constant_test_bit(unsigned int nr,
                         const unsigned long __percpu *addr)
 {
 	unsigned long __percpu *a = (unsigned long *)addr + nr / BITS_PER_LONG;
@@ -529,10 +529,10 @@ static __always_inline int x86_this_cpu_constant_test_bit(unsigned int nr,
 #endif
 }
 
-static inline int x86_this_cpu_variable_test_bit(int nr,
+static inline bool x86_this_cpu_variable_test_bit(int nr,
                         const unsigned long __percpu *addr)
 {
-	unsigned char oldbit;
+	bool oldbit;
 
 	asm volatile("bt "__percpu_arg(2)",%1\n\t"
 			"setc %0"
diff --git a/arch/x86/include/asm/rmwcc.h b/arch/x86/include/asm/rmwcc.h
index 8f7866a..a15b73d 100644
--- a/arch/x86/include/asm/rmwcc.h
+++ b/arch/x86/include/asm/rmwcc.h
@@ -23,11 +23,11 @@ cc_label:								\
 
 #define __GEN_RMWcc(fullop, var, cc, ...)				\
 do {									\
-	char c;								\
+	bool c;								\
 	asm volatile (fullop "; set" cc " %1"				\
 			: "+m" (var), "=qm" (c)				\
 			: __VA_ARGS__ : "memory");			\
-	return c != 0;							\
+	return c;							\
 } while (0)
 
 #define GEN_UNARY_RMWcc(op, var, arg0, cc)				\
diff --git a/arch/x86/include/asm/rwsem.h b/arch/x86/include/asm/rwsem.h
index 453744c..c508770 100644
--- a/arch/x86/include/asm/rwsem.h
+++ b/arch/x86/include/asm/rwsem.h
@@ -77,7 +77,7 @@ static inline void __down_read(struct rw_semaphore *sem)
 /*
  * trylock for reading -- returns 1 if successful, 0 if contention
  */
-static inline int __down_read_trylock(struct rw_semaphore *sem)
+static inline bool __down_read_trylock(struct rw_semaphore *sem)
 {
 	long result, tmp;
 	asm volatile("# beginning __down_read_trylock\n\t"
@@ -93,7 +93,7 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
 		     : "+m" (sem->count), "=&a" (result), "=&r" (tmp)
 		     : "i" (RWSEM_ACTIVE_READ_BIAS)
 		     : "memory", "cc");
-	return result >= 0 ? 1 : 0;
+	return result >= 0;
 }
 
 /*
@@ -134,9 +134,10 @@ static inline int __down_write_killable(struct rw_semaphore *sem)
 /*
  * trylock for writing -- returns 1 if successful, 0 if contention
  */
-static inline int __down_write_trylock(struct rw_semaphore *sem)
+static inline bool __down_write_trylock(struct rw_semaphore *sem)
 {
-	long result, tmp;
+	bool result;
+	long tmp0, tmp1;
 	asm volatile("# beginning __down_write_trylock\n\t"
 		     "  mov          %0,%1\n\t"
 		     "1:\n\t"
@@ -144,14 +145,14 @@ static inline int __down_write_trylock(struct rw_semaphore *sem)
 		     /* was the active mask 0 before? */
 		     "  jnz          2f\n\t"
 		     "  mov          %1,%2\n\t"
-		     "  add          %3,%2\n\t"
+		     "  add          %4,%2\n\t"
 		     LOCK_PREFIX "  cmpxchg  %2,%0\n\t"
 		     "  jnz	     1b\n\t"
 		     "2:\n\t"
-		     "  sete         %b1\n\t"
-		     "  movzbl       %b1, %k1\n\t"
+		     "  sete         %3\n\t"
 		     "# ending __down_write_trylock\n\t"
-		     : "+m" (sem->count), "=&a" (result), "=&r" (tmp)
+		     : "+m" (sem->count), "=&a" (tmp0), "=&r" (tmp1),
+		       "=qm" (result)
 		     : "er" (RWSEM_ACTIVE_WRITE_BIAS)
 		     : "memory", "cc");
 	return result;
diff --git a/include/linux/random.h b/include/linux/random.h
index e47e533..3d6e981 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -95,27 +95,27 @@ static inline void prandom_seed_state(struct rnd_state *state, u64 seed)
 #ifdef CONFIG_ARCH_RANDOM
 # include <asm/archrandom.h>
 #else
-static inline int arch_get_random_long(unsigned long *v)
+static inline bool arch_get_random_long(unsigned long *v)
 {
 	return 0;
 }
-static inline int arch_get_random_int(unsigned int *v)
+static inline bool arch_get_random_int(unsigned int *v)
 {
 	return 0;
 }
-static inline int arch_has_random(void)
+static inline bool arch_has_random(void)
 {
 	return 0;
 }
-static inline int arch_get_random_seed_long(unsigned long *v)
+static inline bool arch_get_random_seed_long(unsigned long *v)
 {
 	return 0;
 }
-static inline int arch_get_random_seed_int(unsigned int *v)
+static inline bool arch_get_random_seed_int(unsigned int *v)
 {
 	return 0;
 }
-static inline int arch_has_random_seed(void)
+static inline bool arch_has_random_seed(void)
 {
 	return 0;
 }

[toc] | [prev] | [next] | [standalone]

#1417003 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

On Tue, Jun 07, 2016 at 04:31:01PM -0700, H. Peter Anvin wrote:
> From: "H. Peter Anvin" <hpa@zytor.com>
> 
> The gcc people have confirmed that using "bool" when combined with
> inline assembly always is treated as a byte-sized operand that can be
> assumed to be 0 or 1, which is exactly what the SET instruction
> emits.  Change the output types and intermediate variables of as many
> operations as practical to "bool".
> 
> Signed-off-by: H. Peter Anvin <hpa@zytor.com>
> ---
>  arch/x86/boot/bitops.h             |  8 +++++---
>  arch/x86/boot/boot.h               |  8 ++++----
>  arch/x86/boot/string.c             |  2 +-
>  arch/x86/include/asm/apm.h         |  6 +++---
>  arch/x86/include/asm/archrandom.h  | 16 ++++++++--------
>  arch/x86/include/asm/atomic.h      |  8 ++++----
>  arch/x86/include/asm/atomic64_64.h | 10 +++++-----
>  arch/x86/include/asm/bitops.h      | 28 ++++++++++++++--------------
>  arch/x86/include/asm/local.h       |  8 ++++----
>  arch/x86/include/asm/percpu.h      |  8 ++++----
>  arch/x86/include/asm/rmwcc.h       |  4 ++--
>  arch/x86/include/asm/rwsem.h       | 17 +++++++++--------
>  include/linux/random.h             | 12 ++++++------
>  13 files changed, 69 insertions(+), 66 deletions(-)

So the only concern I have with this is that the x86 function signatures
are now different from the other architectures.

Not sure how much if anything that matters..

[toc] | [prev] | [next] | [standalone]

#1417042 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Jun 07, 2016 at 04:31:01PM -0700, H. Peter Anvin wrote:
> > From: "H. Peter Anvin" <hpa@zytor.com>
> > 
> > The gcc people have confirmed that using "bool" when combined with
> > inline assembly always is treated as a byte-sized operand that can be
> > assumed to be 0 or 1, which is exactly what the SET instruction
> > emits.  Change the output types and intermediate variables of as many
> > operations as practical to "bool".
> > 
> > Signed-off-by: H. Peter Anvin <hpa@zytor.com>
> > ---
> >  arch/x86/boot/bitops.h             |  8 +++++---
> >  arch/x86/boot/boot.h               |  8 ++++----
> >  arch/x86/boot/string.c             |  2 +-
> >  arch/x86/include/asm/apm.h         |  6 +++---
> >  arch/x86/include/asm/archrandom.h  | 16 ++++++++--------
> >  arch/x86/include/asm/atomic.h      |  8 ++++----
> >  arch/x86/include/asm/atomic64_64.h | 10 +++++-----
> >  arch/x86/include/asm/bitops.h      | 28 ++++++++++++++--------------
> >  arch/x86/include/asm/local.h       |  8 ++++----
> >  arch/x86/include/asm/percpu.h      |  8 ++++----
> >  arch/x86/include/asm/rmwcc.h       |  4 ++--
> >  arch/x86/include/asm/rwsem.h       | 17 +++++++++--------
> >  include/linux/random.h             | 12 ++++++------
> >  13 files changed, 69 insertions(+), 66 deletions(-)
> 
> So the only concern I have with this is that the x86 function signatures
> are now different from the other architectures.
> 
> Not sure how much if anything that matters..

It does matter:

 In file included from arch/x86/kernel/cpu/common.c:21:0:
 ./arch/x86/include/asm/archrandom.h:95:20: error: redefinition of ‘arch_get_random_long’
 static inline bool arch_get_random_long(unsigned long *v)
 In file included from ./arch/x86/include/asm/stackprotector.h:43:0,
 include/linux/random.h:98:20: note: previous definition of ‘arch_get_random_long’ was here

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1417053 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Tue, Jun 07, 2016 at 04:31:01PM -0700, H. Peter Anvin wrote:
> > > From: "H. Peter Anvin" <hpa@zytor.com>
> > > 
> > > The gcc people have confirmed that using "bool" when combined with
> > > inline assembly always is treated as a byte-sized operand that can be
> > > assumed to be 0 or 1, which is exactly what the SET instruction
> > > emits.  Change the output types and intermediate variables of as many
> > > operations as practical to "bool".
> > > 
> > > Signed-off-by: H. Peter Anvin <hpa@zytor.com>
> > > ---
> > >  arch/x86/boot/bitops.h             |  8 +++++---
> > >  arch/x86/boot/boot.h               |  8 ++++----
> > >  arch/x86/boot/string.c             |  2 +-
> > >  arch/x86/include/asm/apm.h         |  6 +++---
> > >  arch/x86/include/asm/archrandom.h  | 16 ++++++++--------
> > >  arch/x86/include/asm/atomic.h      |  8 ++++----
> > >  arch/x86/include/asm/atomic64_64.h | 10 +++++-----
> > >  arch/x86/include/asm/bitops.h      | 28 ++++++++++++++--------------
> > >  arch/x86/include/asm/local.h       |  8 ++++----
> > >  arch/x86/include/asm/percpu.h      |  8 ++++----
> > >  arch/x86/include/asm/rmwcc.h       |  4 ++--
> > >  arch/x86/include/asm/rwsem.h       | 17 +++++++++--------
> > >  include/linux/random.h             | 12 ++++++------
> > >  13 files changed, 69 insertions(+), 66 deletions(-)
> > 
> > So the only concern I have with this is that the x86 function signatures
> > are now different from the other architectures.
> > 
> > Not sure how much if anything that matters..
> 
> It does matter:
> 
>  In file included from arch/x86/kernel/cpu/common.c:21:0:
>  ./arch/x86/include/asm/archrandom.h:95:20: error: redefinition of ‘arch_get_random_long’
>  static inline bool arch_get_random_long(unsigned long *v)
>  In file included from ./arch/x86/include/asm/stackprotector.h:43:0,
>  include/linux/random.h:98:20: note: previous definition of ‘arch_get_random_long’ was here

Note that this particular build error was introduced by b0bdba9825fe, a later 
patch in this series - but in generaly I'm uneasy about allowing function 
signatures diverge between architectures.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1417097 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

On 06/08/16 01:33, Ingo Molnar wrote:
> 
> Note that this particular build error was introduced by b0bdba9825fe, a later 
> patch in this series - but in generaly I'm uneasy about allowing function 
> signatures diverge between architectures.
> 

For the bitops, they already do: PowerPC, for example, have "unsigned
long" in places where x86 has "int".  This is obviously undesirable, but
apparently we have not found it enough of a problem to deal with.  One
could easily argue the ppc definition is the better one; I was myself
considering promoting the x86 side to "long" to handle enormous bitmaps.
 At the same time, it is hard to avoid the fact that ppc has unsigned
bitops operations and x86 has signed ones when they are both native
instructions.

	-hpa

[toc] | [prev] | [next] | [standalone]

#1417108 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

* H. Peter Anvin <hpa@zytor.com> wrote:

> On 06/08/16 01:33, Ingo Molnar wrote:
> > 
> > Note that this particular build error was introduced by b0bdba9825fe, a later 
> > patch in this series - but in generaly I'm uneasy about allowing function 
> > signatures diverge between architectures.
> > 
> 
> For the bitops, they already do: PowerPC, for example, have "unsigned
> long" in places where x86 has "int".  This is obviously undesirable, but
> apparently we have not found it enough of a problem to deal with.
>
> One could easily argue the ppc definition is the better one; I was myself 
> considering promoting the x86 side to "long" to handle enormous bitmaps. At the 
> same time, it is hard to avoid the fact that ppc has unsigned bitops operations 
> and x86 has signed ones when they are both native instructions.

That's a divergence with an underlying reason - but not harmonizing the return 
code is an unforced error AFAICS and can be fixed.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1417134 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

On 06/08/16 02:01, Ingo Molnar wrote:
> 
> That's a divergence with an underlying reason - but not harmonizing the return 
> code is an unforced error AFAICS and can be fixed.
>

Perhaps.  It is also no real question that "bool" is the right return
type for a single bit.  Changing that in all architectures at one time
is a major undertaking, however, and it seems to me that it would be
better to leave that to the respective architecture maintainers.

Perhaps I'm wrong, but I'd really like to avoid the upcasting to "int"
which isn't needed, because in my testing I find that it definitely
encourages gcc to generate poor code.

	-hpa

[toc] | [prev] | [next] | [standalone]

#1417161 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

* H. Peter Anvin <hpa@zytor.com> wrote:

> On 06/08/16 02:01, Ingo Molnar wrote:
> > 
> > That's a divergence with an underlying reason - but not harmonizing the return 
> > code is an unforced error AFAICS and can be fixed.
> >
> 
> Perhaps.  It is also no real question that "bool" is the right return
> type for a single bit.  Changing that in all architectures at one time
> is a major undertaking, however, and it seems to me that it would be
> better to leave that to the respective architecture maintainers.

Yeah, so extrapolating from past performance in most cases that is really a 
shorthand for 'it will never happen' :-/

Also, unless I'm missing something it's not really 'hard' or dangerous per se to 
do that change for every architecture, just incredibly boring! ;-)

I'm not sure how much it matters though, given other asymmetries in the bitops API 
signatures - does anyone have any preferences?

> Perhaps I'm wrong, but I'd really like to avoid the upcasting to "int"
> which isn't needed, because in my testing I find that it definitely
> encourages gcc to generate poor code.

Yeah, absolutely. I hate 'bool' with a vengence but if 'int' generates worse code 
with modern compilers then I'm not going to argue for worse code. Would a 'char' 
return type be very weird?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1417182 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

On 06/08/16 02:20, Ingo Molnar wrote:
> 
> Yeah, absolutely. I hate 'bool' with a vengence but if 'int' generates worse code 
> with modern compilers then I'm not going to argue for worse code. Would a 'char' 
> return type be very weird?
> 

Yes.  I have to admit I don't share your hatred for "bool" -- it gives
the compiler a fairly crucial bit of information about what the possible
values are for a certain piece of data.

Upcasting to char loses that, and may case gcc to manifest the value as
an integer instead of retaining it in the flags.  It is, however, less
likely to cause gcc to then try to widen the value to word size (which
is an extra instruction on x86), but moving the value out of and back
into the flags register is the big cost.

	-hpa

[toc] | [prev] | [next] | [standalone]

#1417201 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

On Wed, Jun 08, 2016 at 02:31:31AM -0700, H. Peter Anvin wrote:
> On 06/08/16 02:20, Ingo Molnar wrote:
> > 
> > Yeah, absolutely. I hate 'bool' with a vengence but if 'int' generates worse code 
> > with modern compilers then I'm not going to argue for worse code. Would a 'char' 
> > return type be very weird?
> > 
> 
> Yes.  I have to admit I don't share your hatred for "bool" -- it gives
> the compiler a fairly crucial bit of information about what the possible
> values are for a certain piece of data.
> 
> Upcasting to char loses that, and may case gcc to manifest the value as
> an integer instead of retaining it in the flags.  It is, however, less
> likely to cause gcc to then try to widen the value to word size (which
> is an extra instruction on x86), but moving the value out of and back
> into the flags register is the big cost.

So I think using bool as return type or argument is fine, using it in
structures is 'insane'.

[toc] | [prev] | [next] | [standalone]

#1417205 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

On 06/08/16 02:39, Peter Zijlstra wrote:
> 
> So I think using bool as return type or argument is fine, using it in
> structures is 'insane'.
> 

Yes, scalar use only, and not across the kernel boundary, please.

	-hpa

[toc] | [prev] | [next] | [standalone]

#1417206 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

On 06/08/16 02:20, Ingo Molnar wrote:
> 
> Also, unless I'm missing something it's not really 'hard' or dangerous per se to 
> do that change for every architecture, just incredibly boring! ;-)
> 
> I'm not sure how much it matters though, given other asymmetries in the bitops API 
> signatures - does anyone have any preferences?
> 

My big concern is doing a change to another architectures which I can't
test... and I will have to make assumptions about the properties of
asm() in that architecture.

Furthermore, if/when other architectures get support for =@cc or an
equivalent, they probably want to change their stuff.

>> Perhaps I'm wrong, but I'd really like to avoid the upcasting to "int"
>> which isn't needed, because in my testing I find that it definitely
>> encourages gcc to generate poor code.
> 
> Yeah, absolutely. I hate 'bool' with a vengence but if 'int' generates worse code 
> with modern compilers then I'm not going to argue for worse code. Would a 'char' 
> return type be very weird?

Another reason to not hate on "bool" so much: I have personally gotten
bitten a few to many times by programmers who thought returning -1 or 2
for what was normally a 0/1 flag in some kind of exceptional case.  If
the variable is bool you know that that can't happen.

One thing, though: we should NOT use bool as input to a system call,
because coming from userspace we cannot enforce the required invariant
that bits[7:1] == 0.

[toc] | [prev] | [next] | [standalone]

#1417100 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

On 06/08/16 01:28, Ingo Molnar wrote:
> 
> It does matter:
> 
>  In file included from arch/x86/kernel/cpu/common.c:21:0:
>  ./arch/x86/include/asm/archrandom.h:95:20: error: redefinition of ‘arch_get_random_long’
>  static inline bool arch_get_random_long(unsigned long *v)
>  In file included from ./arch/x86/include/asm/stackprotector.h:43:0,
>  include/linux/random.h:98:20: note: previous definition of ‘arch_get_random_long’ was here
> 

Actually this is an indication of a major bug, quite possibly
pre-existing; we somehow are trying to include both the stub and the
non-stub version of arch_get_random_long(), which is an obvious no-no.
I will look at it in the morning (02:00 here.)

	-hpa

[toc] | [prev] | [next] | [standalone]

#1417122 — Re: [PATCH 02/10] x86, asm: use bool for bitops and other assembly outputs

* H. Peter Anvin <hpa@zytor.com> wrote:

> On 06/08/16 01:28, Ingo Molnar wrote:
> > 
> > It does matter:
> > 
> >  In file included from arch/x86/kernel/cpu/common.c:21:0:
> >  ./arch/x86/include/asm/archrandom.h:95:20: error: redefinition of ‘arch_get_random_long’
> >  static inline bool arch_get_random_long(unsigned long *v)
> >  In file included from ./arch/x86/include/asm/stackprotector.h:43:0,
> >  include/linux/random.h:98:20: note: previous definition of ‘arch_get_random_long’ was here
> > 
> 
> Actually this is an indication of a major bug, quite possibly
> pre-existing; we somehow are trying to include both the stub and the
> non-stub version of arch_get_random_long(), which is an obvious no-no.

Yeah.

> I will look at it in the morning (02:00 here.)

Sure, take your time! I've rewinded x86/asm to 08dd8cd06ed9 for the time being.

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web