x86, kasan: add KASAN checks to atomic operations

PATCH Superseded Dmitry Vyukov 3 participants 15 messages May 26, 2017 – Jun 6, 2017

Subsystems (7)

Memory management x86 s390 Generic include/asm header files +3 more subsystems

lore

Share & tools

lore

Dmitry Vyukov

author reviewer

2017-05-26 19:11 UTC #0 lore raw

v2 cover x86, kasan: add KASAN checks to atomic operations

KASAN uses compiler instrumentation to intercept all memory accesses. But it does not see memory accesses done in assembly code. One notable user of assembly code is atomic operations. Frequently, for example, an atomic reference decrement is the last access to an object and a good candidate for a racy use-after-free.

Atomic operations are defined in arch files, but KASAN instrumentation is required for several archs that support KASAN. Later we will need similar hooks for KMSAN (uninit use detector) and KTSAN (data race detector).

This change introduces wrappers around atomic operations that can be used to add KASAN/KMSAN/KTSAN instrumentation across several archs, and adds KASAN checks to them.

This patch uses the wrappers only for x86 arch. Arm64 will be switched later. And we also plan to instrument bitops in a similar way.

Within a day it has found its first bug:

BUG: KASAN: use-after-free in atomic_dec_and_test
arch/x86/include/asm/atomic.h:123 [inline] at addr ffff880079c30158
Write of size 4 by task syz-executor6/25698
CPU: 2 PID: 25698 Comm: syz-executor6 Not tainted 4.10.0+ #302
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 kasan_check_write+0x14/0x20 mm/kasan/kasan.c:344
 atomic_dec_and_test arch/x86/include/asm/atomic.h:123 [inline]
 put_task_struct include/linux/sched/task.h:93 [inline]
 put_ctx+0xcf/0x110 kernel/events/core.c:1131
 perf_event_release_kernel+0x3ad/0xc90 kernel/events/core.c:4322
 perf_release+0x37/0x50 kernel/events/core.c:4338
 __fput+0x332/0x800 fs/file_table.c:209
 ____fput+0x15/0x20 fs/file_table.c:245
 task_work_run+0x197/0x260 kernel/task_work.c:116
 exit_task_work include/linux/task_work.h:21 [inline]
 do_exit+0xb38/0x29c0 kernel/exit.c:880
 do_group_exit+0x149/0x420 kernel/exit.c:984
 get_signal+0x7e0/0x1820 kernel/signal.c:2318
 do_signal+0xd2/0x2190 arch/x86/kernel/signal.c:808
 exit_to_usermode_loop+0x200/0x2a0 arch/x86/entry/common.c:157
 syscall_return_slowpath arch/x86/entry/common.c:191 [inline]
 do_syscall_64+0x6fc/0x930 arch/x86/entry/common.c:286
 entry_SYSCALL64_slow_path+0x25/0x25
RIP: 0033:0x4458d9
RSP: 002b:00007f3f07187cf8 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
RAX: fffffffffffffe00 RBX: 00000000007080c8 RCX: 00000000004458d9
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000007080c8
RBP: 00000000007080a8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000000 R14: 00007f3f071889c0 R15: 00007f3f07188700
Object at ffff880079c30140, in cache task_struct size: 5376
Allocated:
PID = 25681
 kmem_cache_alloc_node+0x122/0x6f0 mm/slab.c:3662
 alloc_task_struct_node kernel/fork.c:153 [inline]
 dup_task_struct kernel/fork.c:495 [inline]
 copy_process.part.38+0x19c8/0x4aa0 kernel/fork.c:1560
 copy_process kernel/fork.c:1531 [inline]
 _do_fork+0x200/0x1010 kernel/fork.c:1994
 SYSC_clone kernel/fork.c:2104 [inline]
 SyS_clone+0x37/0x50 kernel/fork.c:2098
 do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:281
 return_from_SYSCALL_64+0x0/0x7a
Freed:
PID = 25681
 __cache_free mm/slab.c:3514 [inline]
 kmem_cache_free+0x71/0x240 mm/slab.c:3774
 free_task_struct kernel/fork.c:158 [inline]
 free_task+0x151/0x1d0 kernel/fork.c:370
 copy_process.part.38+0x18e5/0x4aa0 kernel/fork.c:1931
 copy_process kernel/fork.c:1531 [inline]
 _do_fork+0x200/0x1010 kernel/fork.c:1994
 SYSC_clone kernel/fork.c:2104 [inline]
 SyS_clone+0x37/0x50 kernel/fork.c:2098
 do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:281
 return_from_SYSCALL_64+0x0/0x7a

Changes since v1:
- dropped "x86: remove unused atomic_inc_short()" patch it is mailed separately
- rebased on top of tip/locking/core head
- other changes noted within individual patches

Dmitry Vyukov (7):
x86: un-macro-ify atomic ops implementation
x86: use long long for 64-bit atomic ops
asm-generic: add atomic-instrumented.h
x86: switch atomic.h to use atomic-instrumented.h
kasan: allow kasan_check_read/write() to accept pointers to volatiles
asm-generic: add KASAN instrumentation to atomic operations asm-generic, x86: add comments for atomic instrumentation

 arch/s390/include/asm/atomic_ops.h        |  14 +-
 arch/s390/include/asm/bitops.h            |  12 +-
 arch/x86/include/asm/atomic.h             | 159 +++++++-----
 arch/x86/include/asm/atomic64_32.h        | 153 ++++++-----
 arch/x86/include/asm/atomic64_64.h        | 151 ++++++-----
 arch/x86/include/asm/cmpxchg.h            |  12 +-
 arch/x86/include/asm/cmpxchg_32.h         |   8 +-
 arch/x86/include/asm/cmpxchg_64.h         |   4 +-
 include/asm-generic/atomic-instrumented.h | 417 ++++++++++++++++++++++++++++++
 include/linux/kasan-checks.h              |  10 +-
 include/linux/types.h                     |   2 +-
 mm/kasan/kasan.c                          |   4 +-
 12 files changed, 727 insertions(+), 219 deletions(-)
 create mode 100644 include/asm-generic/atomic-instrumented.h

Signature · 1 line

2.13.0.219.gdb65acc882-goog

Dmitry Vyukov

author reviewer

2017-05-26 19:11 UTC #1 lore raw

PATCH v2 1/7 x86: un-macro-ify atomic ops implementation

CPP turns perfectly readable code into an unreadable, unmaintainable mess. Ingo suggested to write them out as-is. Do this.

Signed-off-by Dmitry Vyukov <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Andrey Ryabinin <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Andrew Morton <[email protected]>

Cc [email protected]

---
 arch/x86/include/asm/atomic.h      | 68 +++++++++++++++++++++------------
 arch/x86/include/asm/atomic64_32.h | 77 ++++++++++++++++++++++++++------------
 arch/x86/include/asm/atomic64_64.h | 67 +++++++++++++++++++++------------
 3 files changed, 142 insertions(+), 70 deletions(-)

arch/x86/include/asm/atomic.h +45 ~~-23~~

index caa5798c92f4..1a4a6a99dcd8 100644
@@ -197,35 +197,57 @@ static inline int atomic_xchg(atomic_t *v, int new)
 	return xchg(&v->counter, new);
 }
 
-#define ATOMIC_OP(op)							\
-static inline void atomic_##op(int i, atomic_t *v)			\
-{									\
-	asm volatile(LOCK_PREFIX #op"l %1,%0"				\
-			: "+m" (v->counter)				\
-			: "ir" (i)					\
-			: "memory");					\
+static inline void atomic_and(int i, atomic_t *v)
+{
+	asm volatile(LOCK_PREFIX "andl %1,%0"
+			: "+m" (v->counter)
+			: "ir" (i)
+			: "memory");
+}
+
+static inline int atomic_fetch_and(int i, atomic_t *v)
+{
+	int val = atomic_read(v);
+
+	do {
+	} while (!atomic_try_cmpxchg(v, &val, val & i));
+	return val;
+}
+
+static inline void atomic_or(int i, atomic_t *v)
+{
+	asm volatile(LOCK_PREFIX "orl %1,%0"
+			: "+m" (v->counter)
+			: "ir" (i)
+			: "memory");
 }
 
-#define ATOMIC_FETCH_OP(op, c_op)					\
-static inline int atomic_fetch_##op(int i, atomic_t *v)			\
-{									\
-	int val = atomic_read(v);					\
-	do {								\
-	} while (!atomic_try_cmpxchg(v, &val, val c_op i));		\
-	return val;							\
+static inline int atomic_fetch_or(int i, atomic_t *v)
+{
+	int val = atomic_read(v);
+
+	do {
+	} while (!atomic_try_cmpxchg(v, &val, val | i));
+	return val;
 }
 
-#define ATOMIC_OPS(op, c_op)						\
-	ATOMIC_OP(op)							\
-	ATOMIC_FETCH_OP(op, c_op)
 
-ATOMIC_OPS(and, &)
-ATOMIC_OPS(or , |)
-ATOMIC_OPS(xor, ^)
+static inline void atomic_xor(int i, atomic_t *v)
+{
+	asm volatile(LOCK_PREFIX "xorl %1,%0"
+			: "+m" (v->counter)
+			: "ir" (i)
+			: "memory");
+}
 
-#undef ATOMIC_OPS
-#undef ATOMIC_FETCH_OP
-#undef ATOMIC_OP
+static inline int atomic_fetch_xor(int i, atomic_t *v)
+{
+	int val = atomic_read(v);
+
+	do {
+	} while (!atomic_try_cmpxchg(v, &val, val ^ i));
+	return val;
+}
 
 /**
  * __atomic_add_unless - add unless the number is already a given value

arch/x86/include/asm/atomic64_32.h +53 ~~-24~~

90 lines not on this page · raw message

arch/x86/include/asm/atomic64_64.h +44 ~~-23~~

78 lines not on this page · raw message

Signature · 1 line

2.13.0.219.gdb65acc882-goog

Dmitry Vyukov

author reviewer

2017-05-26 19:10 UTC #2 lore raw

PATCH v2 2/7 x86: use long long for 64-bit atomic ops

In reply to Dmitry Vyukov

Some 64-bit atomic operations use 'long long' as operand/return type (e.g. asm-generic/atomic64.h, arch/x86/include/asm/atomic64_32.h); while others use 'long' (e.g. arch/x86/include/asm/atomic64_64.h). This makes it impossible to write portable code.
For example, there is no format specifier that prints result of atomic64_read() without warnings. atomic64_try_cmpxchg() is almost impossible to use in portable fashion because it requires either 'long *' or 'long long *' as argument depending on arch.

Switch arch/x86/include/asm/atomic64_64.h to 'long long'.

Signed-off-by Dmitry Vyukov <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Will Deacon <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Andrey Ryabinin <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc [email protected]

---
Changes since v1:
- reverted stray s/long/long long/ replace in comment
- added arch/s390 changes to fix build errors/warnings

---
 arch/s390/include/asm/atomic_ops.h | 14 +++++-----
 arch/s390/include/asm/bitops.h     | 12 ++++-----
 arch/x86/include/asm/atomic64_64.h | 52 +++++++++++++++++++-------------------
 include/linux/types.h              |  2 +-
 4 files changed, 40 insertions(+), 40 deletions(-)

arch/s390/include/asm/atomic_ops.h +7 -7

index ac9e2b939d04..055a9083e52d 100644
@@ -31,10 +31,10 @@ __ATOMIC_OPS(__atomic_and, int, "lan")
 __ATOMIC_OPS(__atomic_or,  int, "lao")
 __ATOMIC_OPS(__atomic_xor, int, "lax")
 
-__ATOMIC_OPS(__atomic64_add, long, "laag")
-__ATOMIC_OPS(__atomic64_and, long, "lang")
-__ATOMIC_OPS(__atomic64_or,  long, "laog")
-__ATOMIC_OPS(__atomic64_xor, long, "laxg")
+__ATOMIC_OPS(__atomic64_add, long long, "laag")
+__ATOMIC_OPS(__atomic64_and, long long, "lang")
+__ATOMIC_OPS(__atomic64_or,  long long, "laog")
+__ATOMIC_OPS(__atomic64_xor, long long, "laxg")
 
 #undef __ATOMIC_OPS
 #undef __ATOMIC_OP

@@ -46,7 +46,7 @@ static inline void __atomic_add_const(int val, int *ptr)
 		: [ptr] "+Q" (*ptr) : [val] "i" (val) : "cc");
 }
 
-static inline void __atomic64_add_const(long val, long *ptr)
+static inline void __atomic64_add_const(long val, long long *ptr)
 {
 	asm volatile(
 		"	agsi	%[ptr],%[val]\n"

@@ -82,7 +82,7 @@ __ATOMIC_OPS(__atomic_xor, "xr")
 #undef __ATOMIC_OPS
 
 #define __ATOMIC64_OP(op_name, op_string)				\
-static inline long op_name(long val, long *ptr)				\
+static inline long op_name(long val, long long *ptr)			\
 {									\
 	long old, new;							\
 									\

@@ -118,7 +118,7 @@ static inline int __atomic_cmpxchg(int *ptr, int old, int new)
 	return old;
 }
 
-static inline long __atomic64_cmpxchg(long *ptr, long old, long new)
+static inline long __atomic64_cmpxchg(long long *ptr, long old, long new)
 {
 	asm volatile(
 		"	csg	%[old],%[new],%[ptr]"

arch/s390/include/asm/bitops.h +6 -6

index d92047da5ccb..8912f52bca5d 100644
@@ -80,7 +80,7 @@ static inline void set_bit(unsigned long nr, volatile unsigned long *ptr)
 	}
 #endif
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	__atomic64_or(mask, addr);
+	__atomic64_or(mask, (long long *)addr);
 }
 
 static inline void clear_bit(unsigned long nr, volatile unsigned long *ptr)

@@ -101,7 +101,7 @@ static inline void clear_bit(unsigned long nr, volatile unsigned long *ptr)
 	}
 #endif
 	mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
-	__atomic64_and(mask, addr);
+	__atomic64_and(mask, (long long *)addr);
 }
 
 static inline void change_bit(unsigned long nr, volatile unsigned long *ptr)

@@ -122,7 +122,7 @@ static inline void change_bit(unsigned long nr, volatile unsigned long *ptr)
 	}
 #endif
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	__atomic64_xor(mask, addr);
+	__atomic64_xor(mask, (long long *)addr);
 }
 
 static inline int

@@ -132,7 +132,7 @@ test_and_set_bit(unsigned long nr, volatile unsigned long *ptr)
 	unsigned long old, mask;
 
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	old = __atomic64_or_barrier(mask, addr);
+	old = __atomic64_or_barrier(mask, (long long *)addr);
 	return (old & mask) != 0;
 }

@@ -143,7 +143,7 @@ test_and_clear_bit(unsigned long nr, volatile unsigned long *ptr)
 	unsigned long old, mask;
 
 	mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
-	old = __atomic64_and_barrier(mask, addr);
+	old = __atomic64_and_barrier(mask, (long long *)addr);
 	return (old & ~mask) != 0;
 }

@@ -154,7 +154,7 @@ test_and_change_bit(unsigned long nr, volatile unsigned long *ptr)
 	unsigned long old, mask;
 
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	old = __atomic64_xor_barrier(mask, addr);
+	old = __atomic64_xor_barrier(mask, (long long *)addr);
 	return (old & mask) != 0;
 }

arch/x86/include/asm/atomic64_64.h +26 ~~-26~~

174 lines not on this page · raw message

include/linux/types.h +1 -1

index 1e7bd24848fc..569fc6db1bd5 100644
@@ -177,7 +177,7 @@ typedef struct {
 
 #ifdef CONFIG_64BIT
 typedef struct {
-	long counter;
+	long long counter;
 } atomic64_t;
 #endif

Signature · 7 lines

2.13.0.219.gdb65acc882-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

[email protected]

reviewer NAK

2017-05-27 23:15 UTC #3 lore raw

Re: [PATCH v2 2/7] x86: use long long for 64-bit atomic ops

Quoted 368 lines · Dmitry Vyukov

Switch arch/x86/include/asm/atomic64_64.h to 'long long'.

Signed-off-by Dmitry Vyukov <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Will Deacon <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Andrey Ryabinin <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc [email protected]

---
Changes since v1:
- reverted stray s/long/long long/ replace in comment
- added arch/s390 changes to fix build errors/warnings
---

arch/s390/include/asm/atomic_ops.h | 14 +++++-----
arch/s390/include/asm/bitops.h     | 12 ++++-----
arch/x86/include/asm/atomic64_64.h | 52
+++++++++++++++++++-------------------
include/linux/types.h              |  2 +-
4 files changed, 40 insertions(+), 40 deletions(-)

arch/s390/include/asm/atomic_ops.h +5 -5

index ac9e2b939d04..055a9083e52d 100644
@@ -31,10 +31,10 @@ __ATOMIC_OPS(__atomic_and, int, "lan")
 __ATOMIC_OPS(__atomic_or,  int, "lao")
 __ATOMIC_OPS(__atomic_xor, int, "lax")
 
-__ATOMIC_OPS(__atomic64_add, long, "laag")
-__ATOMIC_OPS(__atomic64_and, long, "lang")
-__ATOMIC_OPS(__atomic64_or,  long, "laog")
-__ATOMIC_OPS(__atomic64_xor, long, "laxg")
+__ATOMIC_OPS(__atomic64_add, long long, "laag")
+__ATOMIC_OPS(__atomic64_and, long long, "lang")
+__ATOMIC_OPS(__atomic64_or,  long long, "laog")
+__ATOMIC_OPS(__atomic64_xor, long long, "laxg")
 
 #undef __ATOMIC_OPS
 #undef __ATOMIC_OP

@@ -46,7 +46,7 @@ static inline void __atomic_add_const(int val, int
 *ptr)
 		: [ptr] "+Q" (*ptr) : [val] "i" (val) : "cc");
 }
 
-static inline void __atomic64_add_const(long val, long *ptr)
+static inline void __atomic64_add_const(long val, long long *ptr)
 {
 	asm volatile(

		"	agsi	%[ptr],%[val]\n"

arch/s390/include/asm/atomic_ops.h +2 -2

@@ -82,7 +82,7 @@ __ATOMIC_OPS(__atomic_xor, "xr")
 #undef __ATOMIC_OPS
 
 #define __ATOMIC64_OP(op_name, op_string)				\
-static inline long op_name(long val, long *ptr)				\
+static inline long op_name(long val, long long *ptr)			\
 {									\
 	long old, new;							\
 									\

@@ -118,7 +118,7 @@ static inline int __atomic_cmpxchg(int *ptr, int
 old, int new)
 	return old;
 }
 
-static inline long __atomic64_cmpxchg(long *ptr, long old, long new)
+static inline long __atomic64_cmpxchg(long long *ptr, long old, long
 new)
 {

	asm volatile(
		"	csg	%[old],%[new],%[ptr]"

arch/s390/include/asm/bitops.h +1 -1

index d92047da5ccb..8912f52bca5d 100644
@@ -80,7 +80,7 @@ static inline void set_bit(unsigned long nr, volatile
 unsigned long *ptr)
 	}
 #endif
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	__atomic64_or(mask, addr);
+	__atomic64_or(mask, (long long *)addr);
 }

static inline void clear_bit(unsigned long nr, volatile unsigned long *ptr)

arch/s390/include/asm/bitops.h +1 -1

@@ -101,7 +101,7 @@ static inline void clear_bit(unsigned long nr,
 volatile unsigned long *ptr)
 	}
 #endif
 	mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
-	__atomic64_and(mask, addr);
+	__atomic64_and(mask, (long long *)addr);
 }

static inline void change_bit(unsigned long nr, volatile unsigned long *ptr)

arch/s390/include/asm/bitops.h +1 -1

@@ -122,7 +122,7 @@ static inline void change_bit(unsigned long nr,
 volatile unsigned long *ptr)
 	}
 #endif
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	__atomic64_xor(mask, addr);
+	__atomic64_xor(mask, (long long *)addr);
 }

static inline int

arch/s390/include/asm/bitops.h +3 -3

@@ -132,7 +132,7 @@ test_and_set_bit(unsigned long nr, volatile
 unsigned long *ptr)
 	unsigned long old, mask;
 
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	old = __atomic64_or_barrier(mask, addr);
+	old = __atomic64_or_barrier(mask, (long long *)addr);
 	return (old & mask) != 0;
 }

@@ -143,7 +143,7 @@ test_and_clear_bit(unsigned long nr, volatile
 unsigned long *ptr)
 	unsigned long old, mask;
 
 	mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
-	old = __atomic64_and_barrier(mask, addr);
+	old = __atomic64_and_barrier(mask, (long long *)addr);
 	return (old & ~mask) != 0;
 }

@@ -154,7 +154,7 @@ test_and_change_bit(unsigned long nr, volatile
 unsigned long *ptr)
 	unsigned long old, mask;
 
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	old = __atomic64_xor_barrier(mask, addr);
+	old = __atomic64_xor_barrier(mask, (long long *)addr);
 	return (old & mask) != 0;
 }

arch/x86/include/asm/atomic64_64.h +3 -3

index 8db8879a6d8c..8555cd19a916 100644
@@ -16,7 +16,7 @@
 * Atomically reads the value of @v.
 * Doesn't imply a read memory barrier.
 */
-static inline long atomic64_read(const atomic64_t *v)
+static inline long long atomic64_read(const atomic64_t *v)
 {
 	return READ_ONCE((v)->counter);
 }

@@ -28,7 +28,7 @@ static inline long atomic64_read(const atomic64_t *v)
 *
 * Atomically sets the value of @v to @i.
 */
-static inline void atomic64_set(atomic64_t *v, long i)
+static inline void atomic64_set(atomic64_t *v, long long i)
 {
 	WRITE_ONCE(v->counter, i);
 }

@@ -40,7 +40,7 @@ static inline void atomic64_set(atomic64_t *v, long
 i)
 *
 * Atomically adds @i to @v.
 */
-static __always_inline void atomic64_add(long i, atomic64_t *v)
+static __always_inline void atomic64_add(long long i, atomic64_t *v)
 {
 	asm volatile(LOCK_PREFIX "addq %1,%0"

		     : "=m" (v->counter)

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -54,7 +54,7 @@ static __always_inline void atomic64_add(long i,
 atomic64_t *v)
 *
 * Atomically subtracts @i from @v.
 */
-static inline void atomic64_sub(long i, atomic64_t *v)
+static inline void atomic64_sub(long long i, atomic64_t *v)
 {
 	asm volatile(LOCK_PREFIX "subq %1,%0"

		     : "=m" (v->counter)

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -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 bool atomic64_sub_and_test(long i, atomic64_t *v)
+static inline bool atomic64_sub_and_test(long long i, atomic64_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "subq", v->counter, "er", i, "%0", e);

}

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -136,7 +136,7 @@ static inline bool 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 bool atomic64_add_negative(long i, atomic64_t *v)
+static inline bool atomic64_add_negative(long long i, atomic64_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "addq", v->counter, "er", i, "%0", s);

}

arch/x86/include/asm/atomic64_64.h +4 -4

@@ -148,22 +148,22 @@ static inline bool atomic64_add_negative(long i,
 atomic64_t *v)
 *
 * Atomically adds @i to @v and returns @i + @v
 */
-static __always_inline long atomic64_add_return(long i, atomic64_t *v)
+static __always_inline long long atomic64_add_return(long long i,
 atomic64_t *v)
 {
 	return i + xadd(&v->counter, i);
 }
 
-static inline long atomic64_sub_return(long i, atomic64_t *v)
+static inline long long atomic64_sub_return(long long i, atomic64_t
 *v)
 {
 	return atomic64_add_return(-i, v);
 }
 
-static inline long atomic64_fetch_add(long i, atomic64_t *v)
+static inline long long atomic64_fetch_add(long long i, atomic64_t *v)
 {
 	return xadd(&v->counter, i);
 }
 
-static inline long atomic64_fetch_sub(long i, atomic64_t *v)
+static inline long long atomic64_fetch_sub(long long i, atomic64_t *v)

{
	return xadd(&v->counter, -i);
}

arch/x86/include/asm/atomic64_64.h +3 -3

@@ -171,18 +171,18 @@ static inline long atomic64_fetch_sub(long i,
 atomic64_t *v)
 #define atomic64_inc_return(v)  (atomic64_add_return(1, (v)))
 #define atomic64_dec_return(v)  (atomic64_sub_return(1, (v)))
 
-static inline long atomic64_cmpxchg(atomic64_t *v, long old, long new)
+static inline long long atomic64_cmpxchg(atomic64_t *v, long long old,
 long long new)
 {
 	return cmpxchg(&v->counter, old, new);
 }
 
 #define atomic64_try_cmpxchg atomic64_try_cmpxchg
-static __always_inline bool atomic64_try_cmpxchg(atomic64_t *v, long
 *old, long new)
+static __always_inline bool atomic64_try_cmpxchg(atomic64_t *v, long
 long *old, long long new)
 {
 	return try_cmpxchg(&v->counter, old, new);
 }
 
-static inline long atomic64_xchg(atomic64_t *v, long new)
+static inline long long atomic64_xchg(atomic64_t *v, long long new)

{
	return xchg(&v->counter, new);
}

arch/x86/include/asm/atomic64_64.h +2 -2

@@ -196,9 +196,9 @@ 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 bool atomic64_add_unless(atomic64_t *v, long a, long u)
+static inline bool atomic64_add_unless(atomic64_t *v, long long a,
 long long u)
 {
-	long c = atomic64_read(v);
+	long long c = atomic64_read(v);
 	do {

		if (unlikely(c == u))
			return false;

arch/x86/include/asm/atomic64_64.h +2 -2

@@ -215,9 +215,9 @@ static inline bool atomic64_add_unless(atomic64_t
 *v, long a, long u)
 * The function returns the old value of *v minus 1, even if
 * the atomic variable, v, was not decremented.
 */
-static inline long atomic64_dec_if_positive(atomic64_t *v)
+static inline long long atomic64_dec_if_positive(atomic64_t *v)
 {
-	long dec, c = atomic64_read(v);
+	long long dec, c = atomic64_read(v);
 	do {
 		dec = c - 1;

		if (unlikely(dec < 0))

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -226,7 +226,7 @@ static inline long
 atomic64_dec_if_positive(atomic64_t *v)
 	return dec;
 }
 
-static inline void atomic64_and(long i, atomic64_t *v)
+static inline void atomic64_and(long long i, atomic64_t *v)
 {
 	asm volatile(LOCK_PREFIX "andq %1,%0"

			: "+m" (v->counter)

arch/x86/include/asm/atomic64_64.h +3 -3

@@ -234,16 +234,16 @@ static inline void atomic64_and(long i,
 atomic64_t *v)
 			: "memory");
 }
 
-static inline long atomic64_fetch_and(long i, atomic64_t *v)
+static inline long long atomic64_fetch_and(long long i, atomic64_t *v)
 {
-	long val = atomic64_read(v);
+	long long val = atomic64_read(v);
 
 	do {
 	} while (!atomic64_try_cmpxchg(v, &val, val & i));
 	return val;
 }
 
-static inline void atomic64_or(long i, atomic64_t *v)
+static inline void atomic64_or(long long i, atomic64_t *v)
 {
 	asm volatile(LOCK_PREFIX "orq %1,%0"

			: "+m" (v->counter)

arch/x86/include/asm/atomic64_64.h +3 -3

@@ -251,16 +251,16 @@ static inline void atomic64_or(long i, atomic64_t
 *v)
 			: "memory");
 }
 
-static inline long atomic64_fetch_or(long i, atomic64_t *v)
+static inline long long atomic64_fetch_or(long long i, atomic64_t *v)
 {
-	long val = atomic64_read(v);
+	long long val = atomic64_read(v);
 
 	do {
 	} while (!atomic64_try_cmpxchg(v, &val, val | i));
 	return val;
 }
 
-static inline void atomic64_xor(long i, atomic64_t *v)
+static inline void atomic64_xor(long long i, atomic64_t *v)
 {
 	asm volatile(LOCK_PREFIX "xorq %1,%0"

			: "+m" (v->counter)

arch/x86/include/asm/atomic64_64.h +2 -2

@@ -268,9 +268,9 @@ static inline void atomic64_xor(long i, atomic64_t
 *v)
 			: "memory");
 }
 
-static inline long atomic64_fetch_xor(long i, atomic64_t *v)
+static inline long long atomic64_fetch_xor(long long i, atomic64_t *v)
 {
-	long val = atomic64_read(v);
+	long long val = atomic64_read(v);
 
 	do {

	} while (!atomic64_try_cmpxchg(v, &val, val ^ i));

include/linux/types.h +0 -0

index 1e7bd24848fc..569fc6db1bd5 100644

include/linux/types.h typedef struct { · line 183

Quoted 8 lines

include/linux/types.h +1 -1

@@ -177,7 +177,7 @@ typedef struct {
 
 #ifdef CONFIG_64BIT
 typedef struct {
-	long counter;
+	long long counter;
 } atomic64_t;
 #endif

NAK - this is what u64/s64 is for.

Signature · 7 lines

Sent from my Android device with K-9 Mail. Please excuse my brevity.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Dmitry Vyukov

author reviewer

2017-05-28 09:29 UTC #4 lore raw

Re: [PATCH v2 2/7] x86: use long long for 64-bit atomic ops

Quoted 370 lines · On Sun, May 28, 2017 at 1:02 AM, <[email protected]> wrote:

Quoted 369 lines · Dmitry Vyukov

Switch arch/x86/include/asm/atomic64_64.h to 'long long'.

Signed-off-by Dmitry Vyukov <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Will Deacon <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Andrey Ryabinin <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc [email protected]

---
Changes since v1:
- reverted stray s/long/long long/ replace in comment
- added arch/s390 changes to fix build errors/warnings
---

arch/s390/include/asm/atomic_ops.h | 14 +++++-----
arch/s390/include/asm/bitops.h     | 12 ++++-----
arch/x86/include/asm/atomic64_64.h | 52
+++++++++++++++++++-------------------
include/linux/types.h              |  2 +-
4 files changed, 40 insertions(+), 40 deletions(-)

arch/s390/include/asm/atomic_ops.h +5 -5

index ac9e2b939d04..055a9083e52d 100644
@@ -31,10 +31,10 @@ __ATOMIC_OPS(__atomic_and, int, "lan")
 __ATOMIC_OPS(__atomic_or,  int, "lao")
 __ATOMIC_OPS(__atomic_xor, int, "lax")
 
-__ATOMIC_OPS(__atomic64_add, long, "laag")
-__ATOMIC_OPS(__atomic64_and, long, "lang")
-__ATOMIC_OPS(__atomic64_or,  long, "laog")
-__ATOMIC_OPS(__atomic64_xor, long, "laxg")
+__ATOMIC_OPS(__atomic64_add, long long, "laag")
+__ATOMIC_OPS(__atomic64_and, long long, "lang")
+__ATOMIC_OPS(__atomic64_or,  long long, "laog")
+__ATOMIC_OPS(__atomic64_xor, long long, "laxg")
 
 #undef __ATOMIC_OPS
 #undef __ATOMIC_OP

@@ -46,7 +46,7 @@ static inline void __atomic_add_const(int val, int
 *ptr)
              : [ptr] "+Q" (*ptr) : [val] "i" (val) : "cc");
 }
 
-static inline void __atomic64_add_const(long val, long *ptr)
+static inline void __atomic64_add_const(long val, long long *ptr)
 {
      asm volatile(

              "       agsi    %[ptr],%[val]\n"

arch/s390/include/asm/atomic_ops.h +2 -2

@@ -82,7 +82,7 @@ __ATOMIC_OPS(__atomic_xor, "xr")
 #undef __ATOMIC_OPS
 
 #define __ATOMIC64_OP(op_name, op_string)                             \
-static inline long op_name(long val, long *ptr)                               \
+static inline long op_name(long val, long long *ptr)                  \
 {                                                                     \
      long old, new;                                                  \
                                                                      \

@@ -118,7 +118,7 @@ static inline int __atomic_cmpxchg(int *ptr, int
 old, int new)
      return old;
 }
 
-static inline long __atomic64_cmpxchg(long *ptr, long old, long new)
+static inline long __atomic64_cmpxchg(long long *ptr, long old, long
 new)
 {

      asm volatile(
              "       csg     %[old],%[new],%[ptr]"

arch/s390/include/asm/bitops.h +1 -1

index d92047da5ccb..8912f52bca5d 100644
@@ -80,7 +80,7 @@ static inline void set_bit(unsigned long nr, volatile
 unsigned long *ptr)
      }
 #endif
      mask = 1UL << (nr & (BITS_PER_LONG - 1));
-      __atomic64_or(mask, addr);
+      __atomic64_or(mask, (long long *)addr);
 }

static inline void clear_bit(unsigned long nr, volatile unsigned long *ptr)

arch/s390/include/asm/bitops.h +1 -1

@@ -101,7 +101,7 @@ static inline void clear_bit(unsigned long nr,
 volatile unsigned long *ptr)
      }
 #endif
      mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
-      __atomic64_and(mask, addr);
+      __atomic64_and(mask, (long long *)addr);
 }

static inline void change_bit(unsigned long nr, volatile unsigned long *ptr)

arch/s390/include/asm/bitops.h +1 -1

@@ -122,7 +122,7 @@ static inline void change_bit(unsigned long nr,
 volatile unsigned long *ptr)
      }
 #endif
      mask = 1UL << (nr & (BITS_PER_LONG - 1));
-      __atomic64_xor(mask, addr);
+      __atomic64_xor(mask, (long long *)addr);
 }

static inline int

arch/s390/include/asm/bitops.h +3 -3

@@ -132,7 +132,7 @@ test_and_set_bit(unsigned long nr, volatile
 unsigned long *ptr)
      unsigned long old, mask;
 
      mask = 1UL << (nr & (BITS_PER_LONG - 1));
-      old = __atomic64_or_barrier(mask, addr);
+      old = __atomic64_or_barrier(mask, (long long *)addr);
      return (old & mask) != 0;
 }

@@ -143,7 +143,7 @@ test_and_clear_bit(unsigned long nr, volatile
 unsigned long *ptr)
      unsigned long old, mask;
 
      mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
-      old = __atomic64_and_barrier(mask, addr);
+      old = __atomic64_and_barrier(mask, (long long *)addr);
      return (old & ~mask) != 0;
 }

@@ -154,7 +154,7 @@ test_and_change_bit(unsigned long nr, volatile
 unsigned long *ptr)
      unsigned long old, mask;
 
      mask = 1UL << (nr & (BITS_PER_LONG - 1));
-      old = __atomic64_xor_barrier(mask, addr);
+      old = __atomic64_xor_barrier(mask, (long long *)addr);
      return (old & mask) != 0;
 }

arch/x86/include/asm/atomic64_64.h +3 -3

index 8db8879a6d8c..8555cd19a916 100644
@@ -16,7 +16,7 @@
 * Atomically reads the value of @v.
 * Doesn't imply a read memory barrier.
 */
-static inline long atomic64_read(const atomic64_t *v)
+static inline long long atomic64_read(const atomic64_t *v)
 {
      return READ_ONCE((v)->counter);
 }

@@ -28,7 +28,7 @@ static inline long atomic64_read(const atomic64_t *v)
 *
 * Atomically sets the value of @v to @i.
 */
-static inline void atomic64_set(atomic64_t *v, long i)
+static inline void atomic64_set(atomic64_t *v, long long i)
 {
      WRITE_ONCE(v->counter, i);
 }

@@ -40,7 +40,7 @@ static inline void atomic64_set(atomic64_t *v, long
 i)
 *
 * Atomically adds @i to @v.
 */
-static __always_inline void atomic64_add(long i, atomic64_t *v)
+static __always_inline void atomic64_add(long long i, atomic64_t *v)
 {
      asm volatile(LOCK_PREFIX "addq %1,%0"

                   : "=m" (v->counter)

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -54,7 +54,7 @@ static __always_inline void atomic64_add(long i,
 atomic64_t *v)
 *
 * Atomically subtracts @i from @v.
 */
-static inline void atomic64_sub(long i, atomic64_t *v)
+static inline void atomic64_sub(long long i, atomic64_t *v)
 {
      asm volatile(LOCK_PREFIX "subq %1,%0"

                   : "=m" (v->counter)

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -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 bool atomic64_sub_and_test(long i, atomic64_t *v)
+static inline bool atomic64_sub_and_test(long long i, atomic64_t *v)
 {
      GEN_BINARY_RMWcc(LOCK_PREFIX "subq", v->counter, "er", i, "%0", e);

}

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -136,7 +136,7 @@ static inline bool 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 bool atomic64_add_negative(long i, atomic64_t *v)
+static inline bool atomic64_add_negative(long long i, atomic64_t *v)
 {
      GEN_BINARY_RMWcc(LOCK_PREFIX "addq", v->counter, "er", i, "%0", s);

}

arch/x86/include/asm/atomic64_64.h +4 -4

@@ -148,22 +148,22 @@ static inline bool atomic64_add_negative(long i,
 atomic64_t *v)
 *
 * Atomically adds @i to @v and returns @i + @v
 */
-static __always_inline long atomic64_add_return(long i, atomic64_t *v)
+static __always_inline long long atomic64_add_return(long long i,
 atomic64_t *v)
 {
      return i + xadd(&v->counter, i);
 }
 
-static inline long atomic64_sub_return(long i, atomic64_t *v)
+static inline long long atomic64_sub_return(long long i, atomic64_t
 *v)
 {
      return atomic64_add_return(-i, v);
 }
 
-static inline long atomic64_fetch_add(long i, atomic64_t *v)
+static inline long long atomic64_fetch_add(long long i, atomic64_t *v)
 {
      return xadd(&v->counter, i);
 }
 
-static inline long atomic64_fetch_sub(long i, atomic64_t *v)
+static inline long long atomic64_fetch_sub(long long i, atomic64_t *v)

{
      return xadd(&v->counter, -i);
}

arch/x86/include/asm/atomic64_64.h +3 -3

@@ -171,18 +171,18 @@ static inline long atomic64_fetch_sub(long i,
 atomic64_t *v)
 #define atomic64_inc_return(v)  (atomic64_add_return(1, (v)))
 #define atomic64_dec_return(v)  (atomic64_sub_return(1, (v)))
 
-static inline long atomic64_cmpxchg(atomic64_t *v, long old, long new)
+static inline long long atomic64_cmpxchg(atomic64_t *v, long long old,
 long long new)
 {
      return cmpxchg(&v->counter, old, new);
 }
 
 #define atomic64_try_cmpxchg atomic64_try_cmpxchg
-static __always_inline bool atomic64_try_cmpxchg(atomic64_t *v, long
 *old, long new)
+static __always_inline bool atomic64_try_cmpxchg(atomic64_t *v, long
 long *old, long long new)
 {
      return try_cmpxchg(&v->counter, old, new);
 }
 
-static inline long atomic64_xchg(atomic64_t *v, long new)
+static inline long long atomic64_xchg(atomic64_t *v, long long new)

{
      return xchg(&v->counter, new);
}

arch/x86/include/asm/atomic64_64.h +2 -2

@@ -196,9 +196,9 @@ 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 bool atomic64_add_unless(atomic64_t *v, long a, long u)
+static inline bool atomic64_add_unless(atomic64_t *v, long long a,
 long long u)
 {
-      long c = atomic64_read(v);
+      long long c = atomic64_read(v);
      do {

              if (unlikely(c == u))
                      return false;

arch/x86/include/asm/atomic64_64.h +2 -2

@@ -215,9 +215,9 @@ static inline bool atomic64_add_unless(atomic64_t
 *v, long a, long u)
 * The function returns the old value of *v minus 1, even if
 * the atomic variable, v, was not decremented.
 */
-static inline long atomic64_dec_if_positive(atomic64_t *v)
+static inline long long atomic64_dec_if_positive(atomic64_t *v)
 {
-      long dec, c = atomic64_read(v);
+      long long dec, c = atomic64_read(v);
      do {
              dec = c - 1;

              if (unlikely(dec < 0))

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -226,7 +226,7 @@ static inline long
 atomic64_dec_if_positive(atomic64_t *v)
      return dec;
 }
 
-static inline void atomic64_and(long i, atomic64_t *v)
+static inline void atomic64_and(long long i, atomic64_t *v)
 {
      asm volatile(LOCK_PREFIX "andq %1,%0"

                      : "+m" (v->counter)

arch/x86/include/asm/atomic64_64.h +3 -3

@@ -234,16 +234,16 @@ static inline void atomic64_and(long i,
 atomic64_t *v)
                      : "memory");
 }
 
-static inline long atomic64_fetch_and(long i, atomic64_t *v)
+static inline long long atomic64_fetch_and(long long i, atomic64_t *v)
 {
-      long val = atomic64_read(v);
+      long long val = atomic64_read(v);
 
      do {
      } while (!atomic64_try_cmpxchg(v, &val, val & i));
      return val;
 }
 
-static inline void atomic64_or(long i, atomic64_t *v)
+static inline void atomic64_or(long long i, atomic64_t *v)
 {
      asm volatile(LOCK_PREFIX "orq %1,%0"

                      : "+m" (v->counter)

arch/x86/include/asm/atomic64_64.h +3 -3

@@ -251,16 +251,16 @@ static inline void atomic64_or(long i, atomic64_t
 *v)
                      : "memory");
 }
 
-static inline long atomic64_fetch_or(long i, atomic64_t *v)
+static inline long long atomic64_fetch_or(long long i, atomic64_t *v)
 {
-      long val = atomic64_read(v);
+      long long val = atomic64_read(v);
 
      do {
      } while (!atomic64_try_cmpxchg(v, &val, val | i));
      return val;
 }
 
-static inline void atomic64_xor(long i, atomic64_t *v)
+static inline void atomic64_xor(long long i, atomic64_t *v)
 {
      asm volatile(LOCK_PREFIX "xorq %1,%0"

                      : "+m" (v->counter)

arch/x86/include/asm/atomic64_64.h +2 -2

@@ -268,9 +268,9 @@ static inline void atomic64_xor(long i, atomic64_t
 *v)
                      : "memory");
 }
 
-static inline long atomic64_fetch_xor(long i, atomic64_t *v)
+static inline long long atomic64_fetch_xor(long long i, atomic64_t *v)
 {
-      long val = atomic64_read(v);
+      long long val = atomic64_read(v);
 
      do {

      } while (!atomic64_try_cmpxchg(v, &val, val ^ i));

include/linux/types.h +0 -0

index 1e7bd24848fc..569fc6db1bd5 100644
@@ -177,7 +177,7 @@ typedef struct {

Quoted 9 lines

include/linux/types.h typedef struct { · line 183
Quoted 7 lines
include/linux/types.h +1 -1
 #ifdef CONFIG_64BIT
 typedef struct {
-      long counter;
+      long long counter;
 } atomic64_t;
 #endif
 
NAK - this is what u64/s64 is for.

Hi,

Patch 3 adds atomic-instrumented.h which now contains:

+static __always_inline long long atomic64_read(const atomic64_t *v) +{
+ return arch_atomic64_read(v);
+}

without this patch that will become

+static __always_inline s64 atomic64_read(const atomic64_t *v)

Right?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

[email protected]

reviewer

2017-05-28 09:44 UTC #5 lore raw

Re: [PATCH v2 2/7] x86: use long long for 64-bit atomic ops

Quoted 416 lines · Dmitry Vyukov

Quoted 1 line · On Sun, May 28, 2017 at 1:02 AM, <[email protected]> wrote:
On May 26, 2017 12:09:04 PM PDT, Dmitry Vyukov <[email protected]>
Quoted 68 lines · wrote:
Quoted 68 lines
Some 64-bit atomic operations use 'long long' as operand/return type (e.g. asm-generic/atomic64.h, arch/x86/include/asm/atomic64_32.h); while others use 'long' (e.g. arch/x86/include/asm/atomic64_64.h). This makes it impossible to write portable code.
For example, there is no format specifier that prints result of atomic64_read() without warnings. atomic64_try_cmpxchg() is almost impossible to use in portable fashion because it requires either 'long *' or 'long long *' as argument depending on arch.
Switch arch/x86/include/asm/atomic64_64.h to 'long long'.
Signed-off-by Dmitry Vyukov <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Will Deacon <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Andrey Ryabinin <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Cc [email protected]
---
Changes since v1:
- reverted stray s/long/long long/ replace in comment
- added arch/s390 changes to fix build errors/warnings
---
arch/s390/include/asm/atomic_ops.h | 14 +++++-----
arch/s390/include/asm/bitops.h     | 12 ++++-----
arch/x86/include/asm/atomic64_64.h | 52
+++++++++++++++++++-------------------
include/linux/types.h              |  2 +-
4 files changed, 40 insertions(+), 40 deletions(-)
arch/s390/include/asm/atomic_ops.h +5 -5
index ac9e2b939d04..055a9083e52d 100644
@@ -31,10 +31,10 @@ __ATOMIC_OPS(__atomic_and, int, "lan")
 __ATOMIC_OPS(__atomic_or,  int, "lao")
 __ATOMIC_OPS(__atomic_xor, int, "lax")
 
-__ATOMIC_OPS(__atomic64_add, long, "laag")
-__ATOMIC_OPS(__atomic64_and, long, "lang")
-__ATOMIC_OPS(__atomic64_or,  long, "laog")
-__ATOMIC_OPS(__atomic64_xor, long, "laxg")
+__ATOMIC_OPS(__atomic64_add, long long, "laag")
+__ATOMIC_OPS(__atomic64_and, long long, "lang")
+__ATOMIC_OPS(__atomic64_or,  long long, "laog")
+__ATOMIC_OPS(__atomic64_xor, long long, "laxg")
 
 #undef __ATOMIC_OPS
 #undef __ATOMIC_OP
@@ -46,7 +46,7 @@ static inline void __atomic_add_const(int val, int
 *ptr)
              : [ptr] "+Q" (*ptr) : [val] "i" (val) : "cc");
 }
 
-static inline void __atomic64_add_const(long val, long *ptr)
+static inline void __atomic64_add_const(long val, long long *ptr)
 {
      asm volatile(
              "       agsi    %[ptr],%[val]\n"
arch/s390/include/asm/atomic_ops.h +0 -0
@@ -82,7 +82,7 @@ __ATOMIC_OPS(__atomic_xor, "xr")
 #undef __ATOMIC_OPS
 
Quoted 1 line
arch/s390/include/asm/atomic_ops.h __ATOMIC_OPS(__atomic_xor, "xr") · line 84
Quoted 1 line
arch/s390/include/asm/atomic_ops.h +0 -0
 #define __ATOMIC64_OP(op_name, op_string)                           
\
Quoted 1 line
arch/s390/include/asm/atomic_ops.h __ATOMIC_OPS(__atomic_xor, "xr") · line 85
Quoted 1 line
arch/s390/include/asm/atomic_ops.h +0 -1
-static inline long op_name(long val, long *ptr)                     
        \
Quoted 1 line
arch/s390/include/asm/atomic_ops.h __ATOMIC_OPS(__atomic_xor, "xr") · line 85
Quoted 1 line
arch/s390/include/asm/atomic_ops.h +1 -0
+static inline long op_name(long val, long long *ptr)                
\
Quoted 1 line
arch/s390/include/asm/atomic_ops.h __ATOMIC_OPS(__atomic_xor, "xr") · line 86
Quoted 1 line
arch/s390/include/asm/atomic_ops.h +0 -0
 {                                                                   
\
Quoted 1 line
arch/s390/include/asm/atomic_ops.h __ATOMIC_OPS(__atomic_xor, "xr") · line 87
Quoted 1 line
arch/s390/include/asm/atomic_ops.h +0 -0
      long old, new;                                                
\
Quoted 1 line
Quoted 1 line
\
Quoted 5 lines
Quoted 5 lines
arch/s390/include/asm/atomic_ops.h +0 -0
@@ -118,7 +118,7 @@ static inline int __atomic_cmpxchg(int *ptr, int
 old, int new)
      return old;
 }
 
Quoted 12 lines
arch/s390/include/asm/bitops.h static inline void set_bit(unsigned long nr,
Quoted 12 lines
arch/s390/include/asm/atomic_ops.h +1 -1
-static inline long __atomic64_cmpxchg(long *ptr, long old, long new)
+static inline long __atomic64_cmpxchg(long long *ptr, long old, long
 new)
 {
      asm volatile(
              "       csg     %[old],%[new],%[ptr]"
arch/s390/include/asm/bitops.h +0 -0
index d92047da5ccb..8912f52bca5d 100644
@@ -80,7 +80,7 @@ static inline void set_bit(unsigned long nr,
volatile
Quoted 19 lines
Quoted 19 lines
arch/s390/include/asm/bitops.h +1 -1
 unsigned long *ptr)
      }
 #endif
      mask = 1UL << (nr & (BITS_PER_LONG - 1));
-      __atomic64_or(mask, addr);
+      __atomic64_or(mask, (long long *)addr);
 }
 
static inline void clear_bit(unsigned long nr, volatile unsigned long *ptr)
arch/s390/include/asm/bitops.h +1 -1
@@ -101,7 +101,7 @@ static inline void clear_bit(unsigned long nr,
 volatile unsigned long *ptr)
      }
 #endif
      mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
-      __atomic64_and(mask, addr);
+      __atomic64_and(mask, (long long *)addr);
 }
 
Quoted 1 line
Quoted 1 line
static inline void change_bit(unsigned long nr, volatile unsigned
long
Quoted 41 lines
Quoted 41 lines
*ptr)
arch/s390/include/asm/bitops.h +1 -1
@@ -122,7 +122,7 @@ static inline void change_bit(unsigned long nr,
 volatile unsigned long *ptr)
      }
 #endif
      mask = 1UL << (nr & (BITS_PER_LONG - 1));
-      __atomic64_xor(mask, addr);
+      __atomic64_xor(mask, (long long *)addr);
 }
 
static inline int
arch/s390/include/asm/bitops.h +3 -3
@@ -132,7 +132,7 @@ test_and_set_bit(unsigned long nr, volatile
 unsigned long *ptr)
      unsigned long old, mask;
 
      mask = 1UL << (nr & (BITS_PER_LONG - 1));
-      old = __atomic64_or_barrier(mask, addr);
+      old = __atomic64_or_barrier(mask, (long long *)addr);
      return (old & mask) != 0;
 }
@@ -143,7 +143,7 @@ test_and_clear_bit(unsigned long nr, volatile
 unsigned long *ptr)
      unsigned long old, mask;
 
      mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
-      old = __atomic64_and_barrier(mask, addr);
+      old = __atomic64_and_barrier(mask, (long long *)addr);
      return (old & ~mask) != 0;
 }
@@ -154,7 +154,7 @@ test_and_change_bit(unsigned long nr, volatile
 unsigned long *ptr)
      unsigned long old, mask;
 
      mask = 1UL << (nr & (BITS_PER_LONG - 1));
-      old = __atomic64_xor_barrier(mask, addr);
+      old = __atomic64_xor_barrier(mask, (long long *)addr);
      return (old & mask) != 0;
 }
Quoted 15 lines
arch/x86/include/asm/atomic64_64.h static inline long atomic64_read(const atomic64_t
Quoted 15 lines
arch/x86/include/asm/atomic64_64.h +1 -1
index 8db8879a6d8c..8555cd19a916 100644
@@ -16,7 +16,7 @@
 * Atomically reads the value of @v.
 * Doesn't imply a read memory barrier.
 */
-static inline long atomic64_read(const atomic64_t *v)
+static inline long long atomic64_read(const atomic64_t *v)
 {
      return READ_ONCE((v)->counter);
 }
@@ -28,7 +28,7 @@ static inline long atomic64_read(const atomic64_t
*v)
Quoted 29 lines
Quoted 29 lines
arch/x86/include/asm/atomic64_64.h +2 -2
 *
 * Atomically sets the value of @v to @i.
 */
-static inline void atomic64_set(atomic64_t *v, long i)
+static inline void atomic64_set(atomic64_t *v, long long i)
 {
      WRITE_ONCE(v->counter, i);
 }
@@ -40,7 +40,7 @@ static inline void atomic64_set(atomic64_t *v, long
 i)
 *
 * Atomically adds @i to @v.
 */
-static __always_inline void atomic64_add(long i, atomic64_t *v)
+static __always_inline void atomic64_add(long long i, atomic64_t *v)
 {
      asm volatile(LOCK_PREFIX "addq %1,%0"
                   : "=m" (v->counter)
arch/x86/include/asm/atomic64_64.h +1 -1
@@ -54,7 +54,7 @@ static __always_inline void atomic64_add(long i,
 atomic64_t *v)
 *
 * Atomically subtracts @i from @v.
 */
-static inline void atomic64_sub(long i, atomic64_t *v)
+static inline void atomic64_sub(long long i, atomic64_t *v)
 {
      asm volatile(LOCK_PREFIX "subq %1,%0"
                   : "=m" (v->counter)
arch/x86/include/asm/atomic64_64.h +0 -0
@@ -70,7 +70,7 @@ static inline void atomic64_sub(long i, atomic64_t
Quoted 8 lines
arch/x86/include/asm/atomic64_64.h static inline void atomic64_sub(long i, atomic64_t *v)
Quoted 8 lines
arch/x86/include/asm/atomic64_64.h +1 -1
 *v)
 * true if the result is zero, or false for all
 * other cases.
 */
-static inline bool atomic64_sub_and_test(long i, atomic64_t *v)
+static inline bool atomic64_sub_and_test(long long i, atomic64_t *v)
 {
      GEN_BINARY_RMWcc(LOCK_PREFIX "subq", v->counter, "er", i,
"%0", e);
Quoted 2 lines
arch/x86/include/asm/atomic64_64.h static inline bool
Quoted 2 lines
}
arch/x86/include/asm/atomic64_64.h +0 -0
@@ -136,7 +136,7 @@ static inline bool
atomic64_inc_and_test(atomic64_t
Quoted 8 lines
arch/x86/include/asm/atomic64_64.h static inline bool atomic64_inc_and_test(atomic64_t *v)
Quoted 8 lines
arch/x86/include/asm/atomic64_64.h +1 -1
 *v)
 * if the result is negative, or false when
 * result is greater than or equal to zero.
 */
-static inline bool atomic64_add_negative(long i, atomic64_t *v)
+static inline bool atomic64_add_negative(long long i, atomic64_t *v)
 {
      GEN_BINARY_RMWcc(LOCK_PREFIX "addq", v->counter, "er", i,
"%0", s);
Quoted 2 lines
arch/x86/include/asm/atomic64_64.h static inline bool atomic64_add_negative(long
Quoted 2 lines
}
arch/x86/include/asm/atomic64_64.h +0 -0
@@ -148,22 +148,22 @@ static inline bool atomic64_add_negative(long
i,
Quoted 5 lines
arch/x86/include/asm/atomic64_64.h static inline bool atomic64_add_negative(long i, atomic64_t *v)
Quoted 5 lines
arch/x86/include/asm/atomic64_64.h +0 -1
 atomic64_t *v)
 *
 * Atomically adds @i to @v and returns @i + @v
 */
-static __always_inline long atomic64_add_return(long i, atomic64_t
*v)
Quoted 13 lines
Quoted 13 lines
arch/x86/include/asm/atomic64_64.h +2 -1
+static __always_inline long long atomic64_add_return(long long i,
 atomic64_t *v)
 {
      return i + xadd(&v->counter, i);
 }
 
-static inline long atomic64_sub_return(long i, atomic64_t *v)
+static inline long long atomic64_sub_return(long long i, atomic64_t
 *v)
 {
      return atomic64_add_return(-i, v);
 }
 
Quoted 2 lines
arch/x86/include/asm/atomic64_64.h static inline bool atomic64_add_negative(long · line 164
Quoted 2 lines
arch/x86/include/asm/atomic64_64.h +1 -1
-static inline long atomic64_fetch_add(long i, atomic64_t *v)
+static inline long long atomic64_fetch_add(long long i, atomic64_t
*v)
Quoted 6 lines
arch/x86/include/asm/atomic64_64.h static inline bool atomic64_add_negative(long i, atomic64_t *v)
Quoted 6 lines
arch/x86/include/asm/atomic64_64.h +1 -1
 {
      return xadd(&v->counter, i);
 }
 
-static inline long atomic64_fetch_sub(long i, atomic64_t *v)
+static inline long long atomic64_fetch_sub(long long i, atomic64_t
*v)
Quoted 9 lines
arch/x86/include/asm/atomic64_64.h static inline long atomic64_fetch_sub(long i, · line 175
Quoted 9 lines
{
      return xadd(&v->counter, -i);
}
arch/x86/include/asm/atomic64_64.h +0 -1
@@ -171,18 +171,18 @@ static inline long atomic64_fetch_sub(long i,
 atomic64_t *v)
 #define atomic64_inc_return(v)  (atomic64_add_return(1, (v)))
 #define atomic64_dec_return(v)  (atomic64_sub_return(1, (v)))
 
-static inline long atomic64_cmpxchg(atomic64_t *v, long old, long
new)
Quoted 1 line
arch/x86/include/asm/atomic64_64.h static inline long atomic64_fetch_sub(long i, · line 175
Quoted 1 line
arch/x86/include/asm/atomic64_64.h +1 -0
+static inline long long atomic64_cmpxchg(atomic64_t *v, long long
old,
Quoted 14 lines
Quoted 14 lines
arch/x86/include/asm/atomic64_64.h +1 -1
 long long new)
 {
      return cmpxchg(&v->counter, old, new);
 }
 
 #define atomic64_try_cmpxchg atomic64_try_cmpxchg
-static __always_inline bool atomic64_try_cmpxchg(atomic64_t *v, long
 *old, long new)
+static __always_inline bool atomic64_try_cmpxchg(atomic64_t *v, long
 long *old, long long new)
 {
      return try_cmpxchg(&v->counter, old, new);
 }
 
Quoted 11 lines
arch/x86/include/asm/atomic64_64.h static inline long atomic64_xchg(atomic64_t *v, · line 200
Quoted 11 lines
arch/x86/include/asm/atomic64_64.h +1 -1
-static inline long atomic64_xchg(atomic64_t *v, long new)
+static inline long long atomic64_xchg(atomic64_t *v, long long new)
{
      return xchg(&v->counter, new);
}
arch/x86/include/asm/atomic64_64.h +0 -1
@@ -196,9 +196,9 @@ 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 bool atomic64_add_unless(atomic64_t *v, long a, long
u)
Quoted 36 lines
Quoted 36 lines
arch/x86/include/asm/atomic64_64.h +2 -1
+static inline bool atomic64_add_unless(atomic64_t *v, long long a,
 long long u)
 {
-      long c = atomic64_read(v);
+      long long c = atomic64_read(v);
      do {
              if (unlikely(c == u))
                      return false;
arch/x86/include/asm/atomic64_64.h +2 -2
@@ -215,9 +215,9 @@ static inline bool atomic64_add_unless(atomic64_t
 *v, long a, long u)
 * The function returns the old value of *v minus 1, even if
 * the atomic variable, v, was not decremented.
 */
-static inline long atomic64_dec_if_positive(atomic64_t *v)
+static inline long long atomic64_dec_if_positive(atomic64_t *v)
 {
-      long dec, c = atomic64_read(v);
+      long long dec, c = atomic64_read(v);
      do {
              dec = c - 1;
              if (unlikely(dec < 0))
arch/x86/include/asm/atomic64_64.h +1 -1
@@ -226,7 +226,7 @@ static inline long
 atomic64_dec_if_positive(atomic64_t *v)
      return dec;
 }
 
-static inline void atomic64_and(long i, atomic64_t *v)
+static inline void atomic64_and(long long i, atomic64_t *v)
 {
      asm volatile(LOCK_PREFIX "andq %1,%0"
                      : "+m" (v->counter)
arch/x86/include/asm/atomic64_64.h +0 -0
@@ -234,16 +234,16 @@ static inline void atomic64_and(long i,
 atomic64_t *v)
                      : "memory");
 }
 
Quoted 2 lines
arch/x86/include/asm/atomic64_64.h static inline void atomic64_and(long i, · line 238
Quoted 2 lines
arch/x86/include/asm/atomic64_64.h +1 -1
-static inline long atomic64_fetch_and(long i, atomic64_t *v)
+static inline long long atomic64_fetch_and(long long i, atomic64_t
*v)
Quoted 9 lines
Quoted 9 lines
arch/x86/include/asm/atomic64_64.h +1 -1
 {
-      long val = atomic64_read(v);
+      long long val = atomic64_read(v);
 
      do {
      } while (!atomic64_try_cmpxchg(v, &val, val & i));
      return val;
 }
 
Quoted 6 lines
arch/x86/include/asm/atomic64_64.h static inline void atomic64_or(long i,
Quoted 6 lines
arch/x86/include/asm/atomic64_64.h +1 -1
-static inline void atomic64_or(long i, atomic64_t *v)
+static inline void atomic64_or(long long i, atomic64_t *v)
 {
      asm volatile(LOCK_PREFIX "orq %1,%0"
                      : "+m" (v->counter)
arch/x86/include/asm/atomic64_64.h +0 -0
@@ -251,16 +251,16 @@ static inline void atomic64_or(long i,
atomic64_t
Quoted 6 lines
arch/x86/include/asm/atomic64_64.h static inline void atomic64_or(long i, atomic64_t *v)
Quoted 6 lines
arch/x86/include/asm/atomic64_64.h +1 -1
 *v)
                      : "memory");
 }
 
-static inline long atomic64_fetch_or(long i, atomic64_t *v)
+static inline long long atomic64_fetch_or(long long i, atomic64_t
*v)
Quoted 9 lines
Quoted 9 lines
arch/x86/include/asm/atomic64_64.h +1 -1
 {
-      long val = atomic64_read(v);
+      long long val = atomic64_read(v);
 
      do {
      } while (!atomic64_try_cmpxchg(v, &val, val | i));
      return val;
 }
 
Quoted 6 lines
arch/x86/include/asm/atomic64_64.h static inline void atomic64_xor(long i,
Quoted 6 lines
arch/x86/include/asm/atomic64_64.h +1 -1
-static inline void atomic64_xor(long i, atomic64_t *v)
+static inline void atomic64_xor(long long i, atomic64_t *v)
 {
      asm volatile(LOCK_PREFIX "xorq %1,%0"
                      : "+m" (v->counter)
arch/x86/include/asm/atomic64_64.h +0 -0
@@ -268,9 +268,9 @@ static inline void atomic64_xor(long i,
atomic64_t
Quoted 6 lines
arch/x86/include/asm/atomic64_64.h static inline void atomic64_xor(long i, atomic64_t *v)
Quoted 6 lines
arch/x86/include/asm/atomic64_64.h +1 -1
 *v)
                      : "memory");
 }
 
-static inline long atomic64_fetch_xor(long i, atomic64_t *v)
+static inline long long atomic64_fetch_xor(long long i, atomic64_t
*v)
Quoted 12 lines
Quoted 12 lines
arch/x86/include/asm/atomic64_64.h +1 -1
 {
-      long val = atomic64_read(v);
+      long long val = atomic64_read(v);
 
      do {
      } while (!atomic64_try_cmpxchg(v, &val, val ^ i));
include/linux/types.h +0 -0
index 1e7bd24848fc..569fc6db1bd5 100644
@@ -177,7 +177,7 @@ typedef struct {
 
Quoted 9 lines
include/linux/types.h typedef struct { · line 183
Quoted 7 lines
include/linux/types.h +1 -1
 #ifdef CONFIG_64BIT
 typedef struct {
-      long counter;
+      long long counter;
 } atomic64_t;
 #endif
 
NAK - this is what u64/s64 is for.
Hi,
Patch 3 adds atomic-instrumented.h which now contains:
(quoted diff) +4 -0
+static __always_inline long long atomic64_read(const atomic64_t *v)
+{
+       return arch_atomic64_read(v);
+}

Quoted 5 lines

without this patch that will become
+static __always_inline s64 atomic64_read(const atomic64_t *v)
Right?

Yes.

Signature · 7 lines

Sent from my Android device with K-9 Mail. Please excuse my brevity.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Dmitry Vyukov

author reviewer

2017-05-29 14:45 UTC #6 lore raw

Re: [PATCH v2 2/7] x86: use long long for 64-bit atomic ops

Quoted 32 lines · On Sun, May 28, 2017 at 11:34 AM, <[email protected]> wrote:

Quoted 31 lines · Dmitry Vyukov
Quoted 1 line · On Sun, May 28, 2017 at 1:02 AM, <[email protected]> wrote:
On May 26, 2017 12:09:04 PM PDT, Dmitry Vyukov <[email protected]>
Quoted 28 lines · wrote:
Quoted 28 lines
Some 64-bit atomic operations use 'long long' as operand/return type (e.g. asm-generic/atomic64.h, arch/x86/include/asm/atomic64_32.h); while others use 'long' (e.g. arch/x86/include/asm/atomic64_64.h). This makes it impossible to write portable code.
For example, there is no format specifier that prints result of atomic64_read() without warnings. atomic64_try_cmpxchg() is almost impossible to use in portable fashion because it requires either 'long *' or 'long long *' as argument depending on arch.
Switch arch/x86/include/asm/atomic64_64.h to 'long long'.
Signed-off-by Dmitry Vyukov <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Will Deacon <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Andrey Ryabinin <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Cc [email protected]
---
Changes since v1:
- reverted stray s/long/long long/ replace in comment
- added arch/s390 changes to fix build errors/warnings
---

[snip]

Quoted 12 lines

Quoted 12 lines
Quoted 1 line
NAK - this is what u64/s64 is for.
Hi,
Patch 3 adds atomic-instrumented.h which now contains:
(quoted diff) +4 -0
+static __always_inline long long atomic64_read(const atomic64_t *v)
+{
+       return arch_atomic64_read(v);
+}

Quoted 7 lines

Quoted 5 lines
without this patch that will become
+static __always_inline s64 atomic64_read(const atomic64_t *v)
Right?
Yes.

I see that s64 is not the same as long on x86_64 (long long vs long), so it's still not possible to e.g. portably print a result of atomic_read(). But it's a separate issue that we don't need to solve now.

Also all wrappers like:

void atomic64_set(atomic64_t *v, s64 i)

{
    arch_atomic64_set(v, i);
}

lead to type conversions, but at least my compiler does not bark on it.

The only remaining problem is with atomic64_try_cmpxchg, which is simply not possible to use now (not possible to declare the *old type).
I will need something along the following lines to fix it (then callers can use s64* for old).
Sounds good?

arch/x86/include/asm/atomic64_64.h +0 -0

@@ -177,7 +177,7 @@ static inline long

arch_atomic64_cmpxchg(atomic64_t *v, long old, long new)
}

#define arch_atomic64_try_cmpxchg arch_atomic64_try_cmpxchg
-static __always_inline bool arch_atomic64_try_cmpxchg(atomic64_t *v, long *old, long new)
+static __always_inline bool arch_atomic64_try_cmpxchg(atomic64_t *v, s64 *old, long new)

 {
        return try_cmpxchg(&v->counter, old, new);

}
@@ -198,7 +198,7 @@ static inline long arch_atomic64_xchg(atomic64_t *v, long new)
*/
static inline bool arch_atomic64_add_unless(atomic64_t *v, long a, long u) {
- long c = arch_atomic64_read(v);

+       s64 c = arch_atomic64_read(v);
        do {
                if (unlikely(c == u))
                        return false;

@@ -217,7 +217,7 @@ static inline bool arch_atomic64_add_unless(atomic64_t *v, long a, long u)
*/
static inline long arch_atomic64_dec_if_positive(atomic64_t *v) {
- long dec, c = arch_atomic64_read(v);

+       s64 dec, c = arch_atomic64_read(v);
        do {
                dec = c - 1;
                if (unlikely(dec < 0))

@@ -236,7 +236,7 @@ static inline void arch_atomic64_and(long i, atomic64_t *v)

static inline long arch_atomic64_fetch_and(long i, atomic64_t *v) {
- long val = arch_atomic64_read(v);
+ s64 val = arch_atomic64_read(v);

        do {
        } while (!arch_atomic64_try_cmpxchg(v, &val, val & i));

@@ -253,7 +253,7 @@ static inline void arch_atomic64_or(long i, atomic64_t *v)

static inline long arch_atomic64_fetch_or(long i, atomic64_t *v) {
- long val = arch_atomic64_read(v);
+ s64 val = arch_atomic64_read(v);

        do {
        } while (!arch_atomic64_try_cmpxchg(v, &val, val | i));

@@ -270,7 +270,7 @@ static inline void arch_atomic64_xor(long i, atomic64_t *v)

static inline long arch_atomic64_fetch_xor(long i, atomic64_t *v) {
- long val = arch_atomic64_read(v);
+ s64 val = arch_atomic64_read(v);

        do {
        } while (!arch_atomic64_try_cmpxchg(v, &val, val ^ i));

arch/x86/include/asm/cmpxchg.h +1 -1

index e8cf95908fe5..9e2faa85eb02 100644
@@ -157,7 +157,7 @@ extern void __add_wrong_size(void)
 #define __raw_try_cmpxchg(_ptr, _pold, _new, size, lock)               \
 ({                                                                     \
        bool success;                                                   \
-       __typeof__(_ptr) _old = (_pold);                                \
+       __typeof__(_ptr) _old = (__typeof__(_ptr))(_pold);              \
        __typeof__(*(_ptr)) __old = *_old;                              \
        __typeof__(*(_ptr)) __new = (_new);                             \
        switch (size) {                                                 \

Dmitry Vyukov

author reviewer

2017-06-06 10:12 UTC #7 lore raw

Re: [PATCH v2 2/7] x86: use long long for 64-bit atomic ops

Quoted 34 lines · Dmitry Vyukov

Quoted 27 lines · On Sun, May 28, 2017 at 11:34 AM, <[email protected]> wrote:
Quoted 26 lines · Dmitry Vyukov
Quoted 1 line · On Sun, May 28, 2017 at 1:02 AM, <[email protected]> wrote:
On May 26, 2017 12:09:04 PM PDT, Dmitry Vyukov <[email protected]>
Quoted 23 lines · wrote:
Quoted 23 lines
Some 64-bit atomic operations use 'long long' as operand/return type (e.g. asm-generic/atomic64.h, arch/x86/include/asm/atomic64_32.h); while others use 'long' (e.g. arch/x86/include/asm/atomic64_64.h). This makes it impossible to write portable code.
For example, there is no format specifier that prints result of atomic64_read() without warnings. atomic64_try_cmpxchg() is almost impossible to use in portable fashion because it requires either 'long *' or 'long long *' as argument depending on arch.
Switch arch/x86/include/asm/atomic64_64.h to 'long long'.
Signed-off-by Dmitry Vyukov <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Will Deacon <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Andrey Ryabinin <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Cc [email protected]
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
---
Changes since v1:
- reverted stray s/long/long long/ replace in comment
- added arch/s390 changes to fix build errors/warnings
---

Quoted 3 lines

[snip]
Quoted 1 line
Quoted 1 line
Quoted 1 line
NAK - this is what u64/s64 is for.

Mailed v3 with all requested changes.

Quoted 106 lines

Quoted 9 lines

Quoted 9 lines
Hi,
Patch 3 adds atomic-instrumented.h which now contains:
(quoted diff) +4 -0
+static __always_inline long long atomic64_read(const atomic64_t *v)
+{
+       return arch_atomic64_read(v);
+}

Quoted 7 lines

Quoted 5 lines
without this patch that will become
+static __always_inline s64 atomic64_read(const atomic64_t *v)
Right?
Yes.

Also all wrappers like:

void atomic64_set(atomic64_t *v, s64 i)

{
    arch_atomic64_set(v, i);
}

lead to type conversions, but at least my compiler does not bark on it.

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -177,7 +177,7 @@ static inline long
 arch_atomic64_cmpxchg(atomic64_t *v, long old, long new)
 }
 
 #define arch_atomic64_try_cmpxchg arch_atomic64_try_cmpxchg
-static __always_inline bool arch_atomic64_try_cmpxchg(atomic64_t *v,
 long *old, long new)
+static __always_inline bool arch_atomic64_try_cmpxchg(atomic64_t *v,
 s64 *old, long new)

 {
        return try_cmpxchg(&v->counter, old, new);

}

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -198,7 +198,7 @@ static inline long arch_atomic64_xchg(atomic64_t
 *v, long new)
  */
 static inline bool arch_atomic64_add_unless(atomic64_t *v, long a, long u)
 {
-       long c = arch_atomic64_read(v);
+       s64 c = arch_atomic64_read(v);
        do {
                if (unlikely(c == u))

                        return false;

arch/x86/include/asm/atomic64_64.h +1 -1

@@ -217,7 +217,7 @@ static inline bool
 arch_atomic64_add_unless(atomic64_t *v, long a, long u)
  */
 static inline long arch_atomic64_dec_if_positive(atomic64_t *v)
 {
-       long dec, c = arch_atomic64_read(v);
+       s64 dec, c = arch_atomic64_read(v);
        do {
                dec = c - 1;

                if (unlikely(dec < 0))

arch/x86/include/asm/atomic64_64.h +3 -3

@@ -236,7 +236,7 @@ static inline void arch_atomic64_and(long i, atomic64_t *v)
 
 static inline long arch_atomic64_fetch_and(long i, atomic64_t *v)
 {
-       long val = arch_atomic64_read(v);
+       s64 val = arch_atomic64_read(v);
 
        do {
        } while (!arch_atomic64_try_cmpxchg(v, &val, val & i));

@@ -253,7 +253,7 @@ static inline void arch_atomic64_or(long i, atomic64_t *v)
 
 static inline long arch_atomic64_fetch_or(long i, atomic64_t *v)
 {
-       long val = arch_atomic64_read(v);
+       s64 val = arch_atomic64_read(v);
 
        do {
        } while (!arch_atomic64_try_cmpxchg(v, &val, val | i));

@@ -270,7 +270,7 @@ static inline void arch_atomic64_xor(long i, atomic64_t *v)
 
 static inline long arch_atomic64_fetch_xor(long i, atomic64_t *v)
 {
-       long val = arch_atomic64_read(v);
+       s64 val = arch_atomic64_read(v);
 
        do {
        } while (!arch_atomic64_try_cmpxchg(v, &val, val ^ i));

arch/x86/include/asm/cmpxchg.h +0 -0

index e8cf95908fe5..9e2faa85eb02 100644

arch/x86/include/asm/cmpxchg.h extern void __add_wrong_size(void) · line 163

Quoted 8 lines

arch/x86/include/asm/cmpxchg.h +1 -1

@@ -157,7 +157,7 @@ extern void __add_wrong_size(void)
 #define __raw_try_cmpxchg(_ptr, _pold, _new, size, lock)               \
 ({                                                                     \
        bool success;                                                   \
-       __typeof__(_ptr) _old = (_pold);                                \
+       __typeof__(_ptr) _old = (__typeof__(_ptr))(_pold);              \
        __typeof__(*(_ptr)) __old = *_old;                              \
        __typeof__(*(_ptr)) __new = (_new);                             \
        switch (size) {                                                 \

Heiko Carstens

2017-05-29 10:49 UTC #8 lore raw

Re: [PATCH v2 2/7] x86: use long long for 64-bit atomic ops

In reply to Dmitry Vyukov

Quoted 9 lines · Dmitry Vyukov

Some 64-bit atomic operations use 'long long' as operand/return type (e.g. asm-generic/atomic64.h, arch/x86/include/asm/atomic64_32.h); while others use 'long' (e.g. arch/x86/include/asm/atomic64_64.h). This makes it impossible to write portable code.
For example, there is no format specifier that prints result of atomic64_read() without warnings. atomic64_try_cmpxchg() is almost impossible to use in portable fashion because it requires either 'long *' or 'long long *' as argument depending on arch.

Quoted 8 lines

Switch arch/x86/include/asm/atomic64_64.h to 'long long'.
Signed-off-by Dmitry Vyukov <[email protected]>
---
Changes since v1:
- reverted stray s/long/long long/ replace in comment
- added arch/s390 changes to fix build errors/warnings

If you change s390 code, please add the relevant mailing list and/or maintainers please.

Quoted 20 lines

arch/s390/include/asm/atomic_ops.h +4 -4

index ac9e2b939d04..055a9083e52d 100644
@@ -31,10 +31,10 @@ __ATOMIC_OPS(__atomic_and, int, "lan")
 __ATOMIC_OPS(__atomic_or,  int, "lao")
 __ATOMIC_OPS(__atomic_xor, int, "lax")
 
-__ATOMIC_OPS(__atomic64_add, long, "laag")
-__ATOMIC_OPS(__atomic64_and, long, "lang")
-__ATOMIC_OPS(__atomic64_or,  long, "laog")
-__ATOMIC_OPS(__atomic64_xor, long, "laxg")
+__ATOMIC_OPS(__atomic64_add, long long, "laag")
+__ATOMIC_OPS(__atomic64_and, long long, "lang")
+__ATOMIC_OPS(__atomic64_or,  long long, "laog")
+__ATOMIC_OPS(__atomic64_xor, long long, "laxg")
 
 #undef __ATOMIC_OPS
 #undef __ATOMIC_OP

arch/s390/include/asm/atomic_ops.h static inline void __atomic_add_const(int val, int *ptr) · line 49

Quoted 5 lines

arch/s390/include/asm/atomic_ops.h +1 -1

@@ -46,7 +46,7 @@ static inline void __atomic_add_const(int val, int *ptr)
 		: [ptr] "+Q" (*ptr) : [val] "i" (val) : "cc");
 }
 
-static inline void __atomic64_add_const(long val, long *ptr)
+static inline void __atomic64_add_const(long val, long long *ptr)

If you change this then val should be long long (or s64) too.

arch/s390/include/asm/atomic_ops.h __ATOMIC_OPS(__atomic_xor, "xr") · line 87

Quoted 4 lines

arch/s390/include/asm/atomic_ops.h +1 -1

-static inline long op_name(long val, long *ptr)				\
+static inline long op_name(long val, long long *ptr)			\
 {									\
 	long old, new;							\

Same here. You only changed the type of *ptr, but left the rest alone. Everything should have the same type.

Dmitry Vyukov

author reviewer

2017-05-29 11:03 UTC #9 lore raw

Re: [PATCH v2 2/7] x86: use long long for 64-bit atomic ops

Quoted 50 lines · Heiko Carstens

Quoted 9 lines · Dmitry Vyukov
Some 64-bit atomic operations use 'long long' as operand/return type (e.g. asm-generic/atomic64.h, arch/x86/include/asm/atomic64_32.h); while others use 'long' (e.g. arch/x86/include/asm/atomic64_64.h). This makes it impossible to write portable code.
For example, there is no format specifier that prints result of atomic64_read() without warnings. atomic64_try_cmpxchg() is almost impossible to use in portable fashion because it requires either 'long *' or 'long long *' as argument depending on arch.
Quoted 8 lines
Switch arch/x86/include/asm/atomic64_64.h to 'long long'.
Signed-off-by Dmitry Vyukov <[email protected]>
---
Changes since v1:
- reverted stray s/long/long long/ replace in comment
- added arch/s390 changes to fix build errors/warnings
If you change s390 code, please add the relevant mailing list and/or maintainers please.
Quoted 20 lines
arch/s390/include/asm/atomic_ops.h +4 -4
index ac9e2b939d04..055a9083e52d 100644
@@ -31,10 +31,10 @@ __ATOMIC_OPS(__atomic_and, int, "lan")
 __ATOMIC_OPS(__atomic_or,  int, "lao")
 __ATOMIC_OPS(__atomic_xor, int, "lax")
 
-__ATOMIC_OPS(__atomic64_add, long, "laag")
-__ATOMIC_OPS(__atomic64_and, long, "lang")
-__ATOMIC_OPS(__atomic64_or,  long, "laog")
-__ATOMIC_OPS(__atomic64_xor, long, "laxg")
+__ATOMIC_OPS(__atomic64_add, long long, "laag")
+__ATOMIC_OPS(__atomic64_and, long long, "lang")
+__ATOMIC_OPS(__atomic64_or,  long long, "laog")
+__ATOMIC_OPS(__atomic64_xor, long long, "laxg")
 
 #undef __ATOMIC_OPS
 #undef __ATOMIC_OP
arch/s390/include/asm/atomic_ops.h static inline void __atomic_add_const(int val, int *ptr) · line 49
Quoted 5 lines
arch/s390/include/asm/atomic_ops.h +1 -1
@@ -46,7 +46,7 @@ static inline void __atomic_add_const(int val, int *ptr)
              : [ptr] "+Q" (*ptr) : [val] "i" (val) : "cc");
 }
 
-static inline void __atomic64_add_const(long val, long *ptr)
+static inline void __atomic64_add_const(long val, long long *ptr)
If you change this then val should be long long (or s64) too.

Quoted 7 lines

arch/s390/include/asm/atomic_ops.h __ATOMIC_OPS(__atomic_xor, "xr") · line 87
Quoted 4 lines
arch/s390/include/asm/atomic_ops.h +1 -1
-static inline long op_name(long val, long *ptr)                              \
+static inline long op_name(long val, long long *ptr)                 \
 {                                                                    \
      long old, new;                                                  \
Same here. You only changed the type of *ptr, but left the rest alone. Everything should have the same type.

I will try to follow hpa's suggestion in the next version of the patch. If it work out, I will not need to touch s390 code.

Still thanks for the review.

Dmitry Vyukov

author reviewer

2017-05-26 19:10 UTC #10 lore raw

PATCH v2 3/7 asm-generic: add atomic-instrumented.h

In reply to Dmitry Vyukov

The new header allows to wrap per-arch atomic operations and add common functionality to all of them.

Signed-off-by Dmitry Vyukov <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Will Deacon <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Andrey Ryabinin <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc [email protected]

---
 include/asm-generic/atomic-instrumented.h | 319 ++++++++++++++++++++++++++++++
 1 file changed, 319 insertions(+)

include/asm-generic/atomic-instrumented.h new file +319 -0

new file mode 100644
index 000000000000..fd483115d4c6
@@ -0,0 +1,319 @@
+#ifndef _LINUX_ATOMIC_INSTRUMENTED_H
+#define _LINUX_ATOMIC_INSTRUMENTED_H
+
+static __always_inline int atomic_read(const atomic_t *v)
+{
+	return arch_atomic_read(v);
+}
+
+static __always_inline long long atomic64_read(const atomic64_t *v)
+{
+	return arch_atomic64_read(v);
+}
+
+static __always_inline void atomic_set(atomic_t *v, int i)
+{
+	arch_atomic_set(v, i);
+}
+
+static __always_inline void atomic64_set(atomic64_t *v, long long i)
+{
+	arch_atomic64_set(v, i);
+}
+
+static __always_inline int atomic_xchg(atomic_t *v, int i)
+{
+	return arch_atomic_xchg(v, i);
+}
+
+static __always_inline long long atomic64_xchg(atomic64_t *v, long long i)
+{
+	return arch_atomic64_xchg(v, i);
+}
+
+static __always_inline int atomic_cmpxchg(atomic_t *v, int old, int new)
+{
+	return arch_atomic_cmpxchg(v, old, new);
+}
+
+static __always_inline long long atomic64_cmpxchg(atomic64_t *v, long long old,
+						  long long new)
+{
+	return arch_atomic64_cmpxchg(v, old, new);
+}
+
+#ifdef arch_atomic_try_cmpxchg
+#define atomic_try_cmpxchg atomic_try_cmpxchg
+static __always_inline bool atomic_try_cmpxchg(atomic_t *v, int *old, int new)
+{
+	return arch_atomic_try_cmpxchg(v, old, new);
+}
+#endif
+
+#ifdef arch_atomic64_try_cmpxchg
+#define atomic64_try_cmpxchg atomic64_try_cmpxchg
+static __always_inline bool atomic64_try_cmpxchg(atomic64_t *v, long long *old,
+						 long long new)
+{
+	return arch_atomic64_try_cmpxchg(v, old, new);
+}
+#endif
+
+static __always_inline int __atomic_add_unless(atomic_t *v, int a, int u)
+{
+	return __arch_atomic_add_unless(v, a, u);
+}
+
+
+static __always_inline bool atomic64_add_unless(atomic64_t *v, long long a,
+						long long u)
+{
+	return arch_atomic64_add_unless(v, a, u);
+}
+
+static __always_inline void atomic_inc(atomic_t *v)
+{
+	arch_atomic_inc(v);
+}
+
+static __always_inline void atomic64_inc(atomic64_t *v)
+{
+	arch_atomic64_inc(v);
+}
+
+static __always_inline void atomic_dec(atomic_t *v)
+{
+	arch_atomic_dec(v);
+}
+
+static __always_inline void atomic64_dec(atomic64_t *v)
+{
+	arch_atomic64_dec(v);
+}
+
+static __always_inline void atomic_add(int i, atomic_t *v)
+{
+	arch_atomic_add(i, v);
+}
+
+static __always_inline void atomic64_add(long long i, atomic64_t *v)
+{
+	arch_atomic64_add(i, v);
+}
+
+static __always_inline void atomic_sub(int i, atomic_t *v)
+{
+	arch_atomic_sub(i, v);
+}
+
+static __always_inline void atomic64_sub(long long i, atomic64_t *v)
+{
+	arch_atomic64_sub(i, v);
+}
+
+static __always_inline void atomic_and(int i, atomic_t *v)
+{
+	arch_atomic_and(i, v);
+}
+
+static __always_inline void atomic64_and(long long i, atomic64_t *v)
+{
+	arch_atomic64_and(i, v);
+}
+
+static __always_inline void atomic_or(int i, atomic_t *v)
+{
+	arch_atomic_or(i, v);
+}
+
+static __always_inline void atomic64_or(long long i, atomic64_t *v)
+{
+	arch_atomic64_or(i, v);
+}
+
+static __always_inline void atomic_xor(int i, atomic_t *v)
+{
+	arch_atomic_xor(i, v);
+}
+
+static __always_inline void atomic64_xor(long long i, atomic64_t *v)
+{
+	arch_atomic64_xor(i, v);
+}
+
+static __always_inline int atomic_inc_return(atomic_t *v)
+{
+	return arch_atomic_inc_return(v);
+}
+
+static __always_inline long long atomic64_inc_return(atomic64_t *v)
+{
+	return arch_atomic64_inc_return(v);
+}
+
+static __always_inline int atomic_dec_return(atomic_t *v)
+{
+	return arch_atomic_dec_return(v);
+}
+
+static __always_inline long long atomic64_dec_return(atomic64_t *v)
+{
+	return arch_atomic64_dec_return(v);
+}
+
+static __always_inline long long atomic64_inc_not_zero(atomic64_t *v)
+{
+	return arch_atomic64_inc_not_zero(v);
+}
+
+static __always_inline long long atomic64_dec_if_positive(atomic64_t *v)
+{
+	return arch_atomic64_dec_if_positive(v);
+}
+
+static __always_inline bool atomic_dec_and_test(atomic_t *v)
+{
+	return arch_atomic_dec_and_test(v);
+}
+
+static __always_inline bool atomic64_dec_and_test(atomic64_t *v)
+{
+	return arch_atomic64_dec_and_test(v);
+}
+
+static __always_inline bool atomic_inc_and_test(atomic_t *v)
+{
+	return arch_atomic_inc_and_test(v);
+}
+
+static __always_inline bool atomic64_inc_and_test(atomic64_t *v)
+{
+	return arch_atomic64_inc_and_test(v);
+}
+
+static __always_inline int atomic_add_return(int i, atomic_t *v)
+{
+	return arch_atomic_add_return(i, v);
+}
+
+static __always_inline long long atomic64_add_return(long long i, atomic64_t *v)
+{
+	return arch_atomic64_add_return(i, v);
+}
+
+static __always_inline int atomic_sub_return(int i, atomic_t *v)
+{
+	return arch_atomic_sub_return(i, v);
+}
+
+static __always_inline long long atomic64_sub_return(long long i, atomic64_t *v)
+{
+	return arch_atomic64_sub_return(i, v);
+}
+
+static __always_inline int atomic_fetch_add(int i, atomic_t *v)
+{
+	return arch_atomic_fetch_add(i, v);
+}
+
+static __always_inline long long atomic64_fetch_add(long long i, atomic64_t *v)
+{
+	return arch_atomic64_fetch_add(i, v);
+}
+
+static __always_inline int atomic_fetch_sub(int i, atomic_t *v)
+{
+	return arch_atomic_fetch_sub(i, v);
+}
+
+static __always_inline long long atomic64_fetch_sub(long long i, atomic64_t *v)
+{
+	return arch_atomic64_fetch_sub(i, v);
+}
+
+static __always_inline int atomic_fetch_and(int i, atomic_t *v)
+{
+	return arch_atomic_fetch_and(i, v);
+}
+
+static __always_inline long long atomic64_fetch_and(long long i, atomic64_t *v)
+{
+	return arch_atomic64_fetch_and(i, v);
+}
+
+static __always_inline int atomic_fetch_or(int i, atomic_t *v)
+{
+	return arch_atomic_fetch_or(i, v);
+}
+
+static __always_inline long long atomic64_fetch_or(long long i, atomic64_t *v)
+{
+	return arch_atomic64_fetch_or(i, v);
+}
+
+static __always_inline int atomic_fetch_xor(int i, atomic_t *v)
+{
+	return arch_atomic_fetch_xor(i, v);
+}
+
+static __always_inline long long atomic64_fetch_xor(long long i, atomic64_t *v)
+{
+	return arch_atomic64_fetch_xor(i, v);
+}
+
+static __always_inline bool atomic_sub_and_test(int i, atomic_t *v)
+{
+	return arch_atomic_sub_and_test(i, v);
+}
+
+static __always_inline bool atomic64_sub_and_test(long long i, atomic64_t *v)
+{
+	return arch_atomic64_sub_and_test(i, v);
+}
+
+static __always_inline bool atomic_add_negative(int i, atomic_t *v)
+{
+	return arch_atomic_add_negative(i, v);
+}
+
+static __always_inline bool atomic64_add_negative(long long i, atomic64_t *v)
+{
+	return arch_atomic64_add_negative(i, v);
+}
+
+#define cmpxchg(ptr, old, new)				\
+({							\
+	arch_cmpxchg((ptr), (old), (new));		\
+})
+
+#define sync_cmpxchg(ptr, old, new)			\
+({							\
+	arch_sync_cmpxchg((ptr), (old), (new));		\
+})
+
+#define cmpxchg_local(ptr, old, new)			\
+({							\
+	arch_cmpxchg_local((ptr), (old), (new));	\
+})
+
+#define cmpxchg64(ptr, old, new)			\
+({							\
+	arch_cmpxchg64((ptr), (old), (new));		\
+})
+
+#define cmpxchg64_local(ptr, old, new)			\
+({							\
+	arch_cmpxchg64_local((ptr), (old), (new));	\
+})
+
+#define cmpxchg_double(p1, p2, o1, o2, n1, n2)				\
+({									\
+	arch_cmpxchg_double((p1), (p2), (o1), (o2), (n1), (n2));	\
+})
+
+#define cmpxchg_double_local(p1, p2, o1, o2, n1, n2)			\
+({									\
+	arch_cmpxchg_double_local((p1), (p2), (o1), (o2), (n1), (n2));	\
+})
+
+#endif /* _LINUX_ATOMIC_INSTRUMENTED_H */

Signature · 7 lines

2.13.0.219.gdb65acc882-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Dmitry Vyukov

author reviewer

2017-05-26 19:10 UTC #11 lore raw

PATCH v2 4/7 x86: switch atomic.h to use atomic-instrumented.h

In reply to Dmitry Vyukov

Add arch_ prefix to all atomic operations and include <asm-generic/atomic-instrumented.h>. This will allow to add KASAN instrumentation to all atomic ops.

Signed-off-by Dmitry Vyukov <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Will Deacon <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Andrey Ryabinin <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc [email protected]

---
Changes since v1:
- reverted unnecessary change in __raw_try_cmpxchg()
- reverted s/try_cmpxchg/arch_try_cmpxchg/ change try_cmpxchg is x86 implementation detail

---
 arch/x86/include/asm/atomic.h      | 108 ++++++++++++++++++++-----------------
 arch/x86/include/asm/atomic64_32.h | 106 ++++++++++++++++++------------------
 arch/x86/include/asm/atomic64_64.h | 108 ++++++++++++++++++-------------------
 arch/x86/include/asm/cmpxchg.h     |  12 ++---
 arch/x86/include/asm/cmpxchg_32.h  |   8 +--
 arch/x86/include/asm/cmpxchg_64.h  |   4 +-
 6 files changed, 178 insertions(+), 168 deletions(-)

arch/x86/include/asm/atomic.h +58 ~~-50~~

index 1a4a6a99dcd8..b7900346c77e 100644
@@ -16,36 +16,42 @@
 #define ATOMIC_INIT(i)	{ (i) }
 
 /**
- * atomic_read - read atomic variable
+ * arch_atomic_read - read atomic variable
  * @v: pointer of type atomic_t
  *
  * Atomically reads the value of @v.
  */
-static __always_inline int atomic_read(const atomic_t *v)
+static __always_inline int arch_atomic_read(const atomic_t *v)
 {
 	return READ_ONCE((v)->counter);
 }
 
 /**
- * atomic_set - set atomic variable
+ * arch_atomic_set - set atomic variable
  * @v: pointer of type atomic_t
  * @i: required value
  *
  * Atomically sets the value of @v to @i.
  */
-static __always_inline void atomic_set(atomic_t *v, int i)
+static __always_inline void arch_atomic_set(atomic_t *v, int i)
 {
+	/*
+	 * We could use WRITE_ONCE_NOCHECK() if it exists, similar to
+	 * READ_ONCE_NOCHECK() in arch_atomic_read(). But there is no such
+	 * thing at the moment, and introducing it for this case does not
+	 * worth it.
+	 */
 	WRITE_ONCE(v->counter, i);
 }
 
 /**
- * atomic_add - add integer to atomic variable
+ * arch_atomic_add - add integer to atomic variable
  * @i: integer value to add
  * @v: pointer of type atomic_t
  *
  * Atomically adds @i to @v.
  */
-static __always_inline void atomic_add(int i, atomic_t *v)
+static __always_inline void arch_atomic_add(int i, atomic_t *v)
 {
 	asm volatile(LOCK_PREFIX "addl %1,%0"
 		     : "+m" (v->counter)

@@ -53,13 +59,13 @@ static __always_inline void atomic_add(int i, atomic_t *v)
 }
 
 /**
- * atomic_sub - subtract integer from atomic variable
+ * arch_atomic_sub - subtract integer from atomic variable
  * @i: integer value to subtract
  * @v: pointer of type atomic_t
  *
  * Atomically subtracts @i from @v.
  */
-static __always_inline void atomic_sub(int i, atomic_t *v)
+static __always_inline void arch_atomic_sub(int i, atomic_t *v)
 {
 	asm volatile(LOCK_PREFIX "subl %1,%0"
 		     : "+m" (v->counter)

@@ -67,7 +73,7 @@ static __always_inline void atomic_sub(int i, atomic_t *v)
 }
 
 /**
- * atomic_sub_and_test - subtract value from variable and test result
+ * arch_atomic_sub_and_test - subtract value from variable and test result
  * @i: integer value to subtract
  * @v: pointer of type atomic_t
  *

@@ -75,63 +81,63 @@ 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 bool atomic_sub_and_test(int i, atomic_t *v)
+static __always_inline bool arch_atomic_sub_and_test(int i, atomic_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "subl", v->counter, "er", i, "%0", e);
 }
 
 /**
- * atomic_inc - increment atomic variable
+ * arch_atomic_inc - increment atomic variable
  * @v: pointer of type atomic_t
  *
  * Atomically increments @v by 1.
  */
-static __always_inline void atomic_inc(atomic_t *v)
+static __always_inline void arch_atomic_inc(atomic_t *v)
 {
 	asm volatile(LOCK_PREFIX "incl %0"
 		     : "+m" (v->counter));
 }
 
 /**
- * atomic_dec - decrement atomic variable
+ * arch_atomic_dec - decrement atomic variable
  * @v: pointer of type atomic_t
  *
  * Atomically decrements @v by 1.
  */
-static __always_inline void atomic_dec(atomic_t *v)
+static __always_inline void arch_atomic_dec(atomic_t *v)
 {
 	asm volatile(LOCK_PREFIX "decl %0"
 		     : "+m" (v->counter));
 }
 
 /**
- * atomic_dec_and_test - decrement and test
+ * arch_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 __always_inline bool atomic_dec_and_test(atomic_t *v)
+static __always_inline bool arch_atomic_dec_and_test(atomic_t *v)
 {
 	GEN_UNARY_RMWcc(LOCK_PREFIX "decl", v->counter, "%0", e);
 }
 
 /**
- * atomic_inc_and_test - increment and test
+ * arch_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 __always_inline bool atomic_inc_and_test(atomic_t *v)
+static __always_inline bool arch_atomic_inc_and_test(atomic_t *v)
 {
 	GEN_UNARY_RMWcc(LOCK_PREFIX "incl", v->counter, "%0", e);
 }
 
 /**
- * atomic_add_negative - add and test if negative
+ * arch_atomic_add_negative - add and test if negative
  * @i: integer value to add
  * @v: pointer of type atomic_t
  *

@@ -139,65 +145,65 @@ static __always_inline bool 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 bool atomic_add_negative(int i, atomic_t *v)
+static __always_inline bool arch_atomic_add_negative(int i, atomic_t *v)
 {
 	GEN_BINARY_RMWcc(LOCK_PREFIX "addl", v->counter, "er", i, "%0", s);
 }
 
 /**
- * atomic_add_return - add integer and return
+ * arch_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 __always_inline int atomic_add_return(int i, atomic_t *v)
+static __always_inline int arch_atomic_add_return(int i, atomic_t *v)
 {
 	return i + xadd(&v->counter, i);
 }
 
 /**
- * atomic_sub_return - subtract integer and return
+ * arch_atomic_sub_return - subtract integer and return
  * @v: pointer of type atomic_t
  * @i: integer value to subtract
  *
  * Atomically subtracts @i from @v and returns @v - @i
  */
-static __always_inline int atomic_sub_return(int i, atomic_t *v)
+static __always_inline int arch_atomic_sub_return(int i, atomic_t *v)
 {
-	return atomic_add_return(-i, v);
+	return arch_atomic_add_return(-i, v);
 }
 
-#define atomic_inc_return(v)  (atomic_add_return(1, v))
-#define atomic_dec_return(v)  (atomic_sub_return(1, v))
+#define arch_atomic_inc_return(v)  (arch_atomic_add_return(1, v))
+#define arch_atomic_dec_return(v)  (arch_atomic_sub_return(1, v))
 
-static __always_inline int atomic_fetch_add(int i, atomic_t *v)
+static __always_inline int arch_atomic_fetch_add(int i, atomic_t *v)
 {
 	return xadd(&v->counter, i);
 }
 
-static __always_inline int atomic_fetch_sub(int i, atomic_t *v)
+static __always_inline int arch_atomic_fetch_sub(int i, atomic_t *v)
 {
 	return xadd(&v->counter, -i);
 }
 
-static __always_inline int atomic_cmpxchg(atomic_t *v, int old, int new)
+static __always_inline int arch_atomic_cmpxchg(atomic_t *v, int old, int new)
 {
-	return cmpxchg(&v->counter, old, new);
+	return arch_cmpxchg(&v->counter, old, new);
 }
 
-#define atomic_try_cmpxchg atomic_try_cmpxchg
-static __always_inline bool atomic_try_cmpxchg(atomic_t *v, int *old, int new)
+#define arch_atomic_try_cmpxchg arch_atomic_try_cmpxchg
+static __always_inline bool arch_atomic_try_cmpxchg(atomic_t *v, int *old, int new)
 {
 	return try_cmpxchg(&v->counter, old, new);
 }
 
-static inline int atomic_xchg(atomic_t *v, int new)
+static inline int arch_atomic_xchg(atomic_t *v, int new)
 {
 	return xchg(&v->counter, new);
 }
 
-static inline void atomic_and(int i, atomic_t *v)
+static inline void arch_atomic_and(int i, atomic_t *v)
 {
 	asm volatile(LOCK_PREFIX "andl %1,%0"
 			: "+m" (v->counter)

@@ -205,16 +211,16 @@ static inline void atomic_and(int i, atomic_t *v)
 			: "memory");
 }
 
-static inline int atomic_fetch_and(int i, atomic_t *v)
+static inline int arch_atomic_fetch_and(int i, atomic_t *v)
 {
-	int val = atomic_read(v);
+	int val = arch_atomic_read(v);
 
 	do {
-	} while (!atomic_try_cmpxchg(v, &val, val & i));
+	} while (!arch_atomic_try_cmpxchg(v, &val, val & i));
 	return val;
 }
 
-static inline void atomic_or(int i, atomic_t *v)
+static inline void arch_atomic_or(int i, atomic_t *v)
 {
 	asm volatile(LOCK_PREFIX "orl %1,%0"
 			: "+m" (v->counter)

@@ -222,17 +228,17 @@ static inline void atomic_or(int i, atomic_t *v)
 			: "memory");
 }
 
-static inline int atomic_fetch_or(int i, atomic_t *v)
+static inline int arch_atomic_fetch_or(int i, atomic_t *v)
 {
-	int val = atomic_read(v);
+	int val = arch_atomic_read(v);
 
 	do {
-	} while (!atomic_try_cmpxchg(v, &val, val | i));
+	} while (!arch_atomic_try_cmpxchg(v, &val, val | i));
 	return val;
 }
 
 
-static inline void atomic_xor(int i, atomic_t *v)
+static inline void arch_atomic_xor(int i, atomic_t *v)
 {
 	asm volatile(LOCK_PREFIX "xorl %1,%0"
 			: "+m" (v->counter)

@@ -240,17 +246,17 @@ static inline void atomic_xor(int i, atomic_t *v)
 			: "memory");
 }
 
-static inline int atomic_fetch_xor(int i, atomic_t *v)
+static inline int arch_atomic_fetch_xor(int i, atomic_t *v)
 {
-	int val = atomic_read(v);
+	int val = arch_atomic_read(v);
 
 	do {
-	} while (!atomic_try_cmpxchg(v, &val, val ^ i));
+	} while (!arch_atomic_try_cmpxchg(v, &val, val ^ i));
 	return val;
 }
 
 /**
- * __atomic_add_unless - add unless the number is already a given value
+ * __arch_atomic_add_unless - add unless the number is already a given value
  * @v: pointer of type atomic_t
  * @a: the amount to add to v...
  * @u: ...unless v is equal to u.

@@ -258,13 +264,13 @@ static inline int atomic_fetch_xor(int i, atomic_t *v)
  * Atomically adds @a to @v, so long as @v was not already @u.
  * Returns the old value of @v.
  */
-static __always_inline int __atomic_add_unless(atomic_t *v, int a, int u)
+static __always_inline int __arch_atomic_add_unless(atomic_t *v, int a, int u)
 {
-	int c = atomic_read(v);
+	int c = arch_atomic_read(v);
 	do {
 		if (unlikely(c == u))
 			break;
-	} while (!atomic_try_cmpxchg(v, &c, c + a));
+	} while (!arch_atomic_try_cmpxchg(v, &c, c + a));
 	return c;
 }

@@ -287,4 +293,6 @@ static __always_inline short int atomic_inc_short(short int *v)
 # include <asm/atomic64_64.h>
 #endif
 
+#include <asm-generic/atomic-instrumented.h>
+
 #endif /* _ASM_X86_ATOMIC_H */

arch/x86/include/asm/atomic64_32.h +54 ~~-52~~

332 lines not on this page · raw message

arch/x86/include/asm/atomic64_64.h +54 ~~-54~~

314 lines not on this page · raw message

arch/x86/include/asm/cmpxchg.h +6 -6

index d90296d061e8..e8cf95908fe5 100644
@@ -144,13 +144,13 @@ extern void __add_wrong_size(void)
 # include <asm/cmpxchg_64.h>
 #endif
 
-#define cmpxchg(ptr, old, new)						\
+#define arch_cmpxchg(ptr, old, new)					\
 	__cmpxchg(ptr, old, new, sizeof(*(ptr)))
 
-#define sync_cmpxchg(ptr, old, new)					\
+#define arch_sync_cmpxchg(ptr, old, new)				\
 	__sync_cmpxchg(ptr, old, new, sizeof(*(ptr)))
 
-#define cmpxchg_local(ptr, old, new)					\
+#define arch_cmpxchg_local(ptr, old, new)				\
 	__cmpxchg_local(ptr, old, new, sizeof(*(ptr)))

@@ -220,7 +220,7 @@ extern void __add_wrong_size(void)
 #define __try_cmpxchg(ptr, pold, new, size)				\
 	__raw_try_cmpxchg((ptr), (pold), (new), (size), LOCK_PREFIX)
 
-#define try_cmpxchg(ptr, pold, new)					\
+#define try_cmpxchg(ptr, pold, new) 					\
 	__try_cmpxchg((ptr), (pold), (new), sizeof(*(ptr)))
 
 /*

@@ -249,10 +249,10 @@ extern void __add_wrong_size(void)
 	__ret;								\
 })
 
-#define cmpxchg_double(p1, p2, o1, o2, n1, n2) \
+#define arch_cmpxchg_double(p1, p2, o1, o2, n1, n2) \
 	__cmpxchg_double(LOCK_PREFIX, p1, p2, o1, o2, n1, n2)
 
-#define cmpxchg_double_local(p1, p2, o1, o2, n1, n2) \
+#define arch_cmpxchg_double_local(p1, p2, o1, o2, n1, n2) \
 	__cmpxchg_double(, p1, p2, o1, o2, n1, n2)
 
 #endif	/* ASM_X86_CMPXCHG_H */

arch/x86/include/asm/cmpxchg_32.h +4 -4

index e4959d023af8..d897291d2bf9 100644
@@ -35,10 +35,10 @@ static inline void set_64bit(volatile u64 *ptr, u64 value)
 }
 
 #ifdef CONFIG_X86_CMPXCHG64
-#define cmpxchg64(ptr, o, n)						\
+#define arch_cmpxchg64(ptr, o, n)					\
 	((__typeof__(*(ptr)))__cmpxchg64((ptr), (unsigned long long)(o), \
 					 (unsigned long long)(n)))
-#define cmpxchg64_local(ptr, o, n)					\
+#define arch_cmpxchg64_local(ptr, o, n)					\
 	((__typeof__(*(ptr)))__cmpxchg64_local((ptr), (unsigned long long)(o), \
 					       (unsigned long long)(n)))
 #endif

@@ -75,7 +75,7 @@ static inline u64 __cmpxchg64_local(volatile u64 *ptr, u64 old, u64 new)
  * to simulate the cmpxchg8b on the 80386 and 80486 CPU.
  */
 
-#define cmpxchg64(ptr, o, n)					\
+#define arch_cmpxchg64(ptr, o, n)				\
 ({								\
 	__typeof__(*(ptr)) __ret;				\
 	__typeof__(*(ptr)) __old = (o);				\

@@ -92,7 +92,7 @@ static inline u64 __cmpxchg64_local(volatile u64 *ptr, u64 old, u64 new)
 	__ret; })
 
 
-#define cmpxchg64_local(ptr, o, n)				\
+#define arch_cmpxchg64_local(ptr, o, n)				\
 ({								\
 	__typeof__(*(ptr)) __ret;				\
 	__typeof__(*(ptr)) __old = (o);				\

arch/x86/include/asm/cmpxchg_64.h +2 -2

index caa23a34c963..fafaebacca2d 100644
@@ -6,13 +6,13 @@ static inline void set_64bit(volatile u64 *ptr, u64 val)
 	*ptr = val;
 }
 
-#define cmpxchg64(ptr, o, n)						\
+#define arch_cmpxchg64(ptr, o, n)					\
 ({									\
 	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
 	cmpxchg((ptr), (o), (n));					\
 })
 
-#define cmpxchg64_local(ptr, o, n)					\
+#define arch_cmpxchg64_local(ptr, o, n)					\
 ({									\
 	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
 	cmpxchg_local((ptr), (o), (n));					\

Signature · 7 lines

2.13.0.219.gdb65acc882-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Dmitry Vyukov

author reviewer

2017-05-26 19:10 UTC #12 lore raw

PATCH v2 5/7 kasan: allow kasan_check_read/write() to accept pointers to volatiles

In reply to Dmitry Vyukov

Currently kasan_check_read/write() accept 'const void*', make them accept 'const volatile void*'. This is required for instrumentation of atomic operations and there is just no reason to not allow that.

Signed-off-by Dmitry Vyukov <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Andrey Ryabinin <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Andrew Morton <[email protected]>

Cc [email protected]

---
 include/linux/kasan-checks.h | 10 ++++++----
 mm/kasan/kasan.c             |  4 ++--
 2 files changed, 8 insertions(+), 6 deletions(-)

include/linux/kasan-checks.h +6 -4

index b7f8aced7870..41960fecf783 100644
@@ -2,11 +2,13 @@
 #define _LINUX_KASAN_CHECKS_H
 
 #ifdef CONFIG_KASAN
-void kasan_check_read(const void *p, unsigned int size);
-void kasan_check_write(const void *p, unsigned int size);
+void kasan_check_read(const volatile void *p, unsigned int size);
+void kasan_check_write(const volatile void *p, unsigned int size);
 #else
-static inline void kasan_check_read(const void *p, unsigned int size) { }
-static inline void kasan_check_write(const void *p, unsigned int size) { }
+static inline void kasan_check_read(const volatile void *p, unsigned int size)
+{ }
+static inline void kasan_check_write(const volatile void *p, unsigned int size)
+{ }
 #endif
 
 #endif

mm/kasan/kasan.c +2 -2

index 98b27195e38b..db46e66eb1d4 100644
@@ -333,13 +333,13 @@ static void check_memory_region(unsigned long addr,
 	check_memory_region_inline(addr, size, write, ret_ip);
 }
 
-void kasan_check_read(const void *p, unsigned int size)
+void kasan_check_read(const volatile void *p, unsigned int size)
 {
 	check_memory_region((unsigned long)p, size, false, _RET_IP_);
 }
 EXPORT_SYMBOL(kasan_check_read);
 
-void kasan_check_write(const void *p, unsigned int size)
+void kasan_check_write(const volatile void *p, unsigned int size)
 {
 	check_memory_region((unsigned long)p, size, true, _RET_IP_);
 }

Signature · 7 lines

2.13.0.219.gdb65acc882-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Dmitry Vyukov

author reviewer

2017-05-26 19:10 UTC #13 lore raw

PATCH v2 6/7 asm-generic: add KASAN instrumentation to atomic operations

In reply to Dmitry Vyukov

Add manual KASAN checks to atomic operations.

Signed-off-by Dmitry Vyukov <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Will Deacon <[email protected]>,

Cc Andrew Morton <[email protected]>,

Cc Andrey Ryabinin <[email protected]>,

Cc Ingo Molnar <[email protected]>,

Cc [email protected]

---
 include/asm-generic/atomic-instrumented.h | 76 +++++++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 4 deletions(-)

include/asm-generic/atomic-instrumented.h +72 -4

index fd483115d4c6..7f8eb761f896 100644
@@ -1,44 +1,54 @@
 #ifndef _LINUX_ATOMIC_INSTRUMENTED_H
 #define _LINUX_ATOMIC_INSTRUMENTED_H
 
+#include <linux/kasan-checks.h>
+
 static __always_inline int atomic_read(const atomic_t *v)
 {
+	kasan_check_read(v, sizeof(*v));
 	return arch_atomic_read(v);
 }
 
 static __always_inline long long atomic64_read(const atomic64_t *v)
 {
+	kasan_check_read(v, sizeof(*v));
 	return arch_atomic64_read(v);
 }
 
 static __always_inline void atomic_set(atomic_t *v, int i)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic_set(v, i);
 }
 
 static __always_inline void atomic64_set(atomic64_t *v, long long i)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic64_set(v, i);
 }
 
 static __always_inline int atomic_xchg(atomic_t *v, int i)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_xchg(v, i);
 }
 
 static __always_inline long long atomic64_xchg(atomic64_t *v, long long i)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_xchg(v, i);
 }
 
 static __always_inline int atomic_cmpxchg(atomic_t *v, int old, int new)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_cmpxchg(v, old, new);
 }
 
 static __always_inline long long atomic64_cmpxchg(atomic64_t *v, long long old,
 						  long long new)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_cmpxchg(v, old, new);
 }

@@ -46,6 +56,8 @@ static __always_inline long long atomic64_cmpxchg(atomic64_t *v, long long old,
 #define atomic_try_cmpxchg atomic_try_cmpxchg
 static __always_inline bool atomic_try_cmpxchg(atomic_t *v, int *old, int new)
 {
+	kasan_check_write(v, sizeof(*v));
+	kasan_check_read(old, sizeof(*old));
 	return arch_atomic_try_cmpxchg(v, old, new);
 }
 #endif

@@ -55,12 +67,15 @@ static __always_inline bool atomic_try_cmpxchg(atomic_t *v, int *old, int new)
 static __always_inline bool atomic64_try_cmpxchg(atomic64_t *v, long long *old,
 						 long long new)
 {
+	kasan_check_write(v, sizeof(*v));
+	kasan_check_read(old, sizeof(*old));
 	return arch_atomic64_try_cmpxchg(v, old, new);
 }
 #endif
 
 static __always_inline int __atomic_add_unless(atomic_t *v, int a, int u)
 {
+	kasan_check_write(v, sizeof(*v));
 	return __arch_atomic_add_unless(v, a, u);
 }

@@ -68,242 +83,295 @@ static __always_inline int __atomic_add_unless(atomic_t *v, int a, int u)
 static __always_inline bool atomic64_add_unless(atomic64_t *v, long long a,
 						long long u)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_add_unless(v, a, u);
 }
 
 static __always_inline void atomic_inc(atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic_inc(v);
 }
 
 static __always_inline void atomic64_inc(atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic64_inc(v);
 }
 
 static __always_inline void atomic_dec(atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic_dec(v);
 }
 
 static __always_inline void atomic64_dec(atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic64_dec(v);
 }
 
 static __always_inline void atomic_add(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic_add(i, v);
 }
 
 static __always_inline void atomic64_add(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic64_add(i, v);
 }
 
 static __always_inline void atomic_sub(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic_sub(i, v);
 }
 
 static __always_inline void atomic64_sub(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic64_sub(i, v);
 }
 
 static __always_inline void atomic_and(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic_and(i, v);
 }
 
 static __always_inline void atomic64_and(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic64_and(i, v);
 }
 
 static __always_inline void atomic_or(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic_or(i, v);
 }
 
 static __always_inline void atomic64_or(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic64_or(i, v);
 }
 
 static __always_inline void atomic_xor(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic_xor(i, v);
 }
 
 static __always_inline void atomic64_xor(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	arch_atomic64_xor(i, v);
 }
 
 static __always_inline int atomic_inc_return(atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_inc_return(v);
 }
 
 static __always_inline long long atomic64_inc_return(atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_inc_return(v);
 }
 
 static __always_inline int atomic_dec_return(atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_dec_return(v);
 }
 
 static __always_inline long long atomic64_dec_return(atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_dec_return(v);
 }
 
 static __always_inline long long atomic64_inc_not_zero(atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_inc_not_zero(v);
 }
 
 static __always_inline long long atomic64_dec_if_positive(atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_dec_if_positive(v);
 }
 
 static __always_inline bool atomic_dec_and_test(atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_dec_and_test(v);
 }
 
 static __always_inline bool atomic64_dec_and_test(atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_dec_and_test(v);
 }
 
 static __always_inline bool atomic_inc_and_test(atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_inc_and_test(v);
 }
 
 static __always_inline bool atomic64_inc_and_test(atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_inc_and_test(v);
 }
 
 static __always_inline int atomic_add_return(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_add_return(i, v);
 }
 
 static __always_inline long long atomic64_add_return(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_add_return(i, v);
 }
 
 static __always_inline int atomic_sub_return(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_sub_return(i, v);
 }
 
 static __always_inline long long atomic64_sub_return(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_sub_return(i, v);
 }
 
 static __always_inline int atomic_fetch_add(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_fetch_add(i, v);
 }
 
 static __always_inline long long atomic64_fetch_add(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_fetch_add(i, v);
 }
 
 static __always_inline int atomic_fetch_sub(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_fetch_sub(i, v);
 }
 
 static __always_inline long long atomic64_fetch_sub(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_fetch_sub(i, v);
 }
 
 static __always_inline int atomic_fetch_and(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_fetch_and(i, v);
 }
 
 static __always_inline long long atomic64_fetch_and(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_fetch_and(i, v);
 }
 
 static __always_inline int atomic_fetch_or(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_fetch_or(i, v);
 }
 
 static __always_inline long long atomic64_fetch_or(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_fetch_or(i, v);
 }
 
 static __always_inline int atomic_fetch_xor(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_fetch_xor(i, v);
 }
 
 static __always_inline long long atomic64_fetch_xor(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_fetch_xor(i, v);
 }
 
 static __always_inline bool atomic_sub_and_test(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_sub_and_test(i, v);
 }
 
 static __always_inline bool atomic64_sub_and_test(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_sub_and_test(i, v);
 }
 
 static __always_inline bool atomic_add_negative(int i, atomic_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic_add_negative(i, v);
 }
 
 static __always_inline bool atomic64_add_negative(long long i, atomic64_t *v)
 {
+	kasan_check_write(v, sizeof(*v));
 	return arch_atomic64_add_negative(i, v);
 }
 
 #define cmpxchg(ptr, old, new)				\
 ({							\
+	__typeof__(ptr) ___ptr = (ptr);			\
+	kasan_check_write(___ptr, sizeof(*___ptr));	\
 	arch_cmpxchg((ptr), (old), (new));		\
 })
 
 #define sync_cmpxchg(ptr, old, new)			\
 ({							\
-	arch_sync_cmpxchg((ptr), (old), (new));		\
+	__typeof__(ptr) ___ptr = (ptr);			\
+	kasan_check_write(___ptr, sizeof(*___ptr));	\
+	arch_sync_cmpxchg(___ptr, (old), (new));	\
 })
 
 #define cmpxchg_local(ptr, old, new)			\
 ({							\
-	arch_cmpxchg_local((ptr), (old), (new));	\
+	__typeof__(ptr) ____ptr = (ptr);		\
+	kasan_check_write(____ptr, sizeof(*____ptr));	\
+	arch_cmpxchg_local(____ptr, (old), (new));	\
 })
 
 #define cmpxchg64(ptr, old, new)			\
 ({							\
-	arch_cmpxchg64((ptr), (old), (new));		\
+	__typeof__(ptr) ____ptr = (ptr);		\
+	kasan_check_write(____ptr, sizeof(*____ptr));	\
+	arch_cmpxchg64(____ptr, (old), (new));		\
 })
 
 #define cmpxchg64_local(ptr, old, new)			\
 ({							\
-	arch_cmpxchg64_local((ptr), (old), (new));	\
+	__typeof__(ptr) ____ptr = (ptr);		\
+	kasan_check_write(____ptr, sizeof(*____ptr));	\
+	arch_cmpxchg64_local(____ptr, (old), (new));	\
 })
 
 #define cmpxchg_double(p1, p2, o1, o2, n1, n2)				\

Signature · 7 lines

2.13.0.219.gdb65acc882-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Dmitry Vyukov

author reviewer

2017-05-26 19:10 UTC #14 lore raw

PATCH v2 7/7 asm-generic, x86: add comments for atomic instrumentation

In reply to Dmitry Vyukov

The comments are factored out from the code changes to make them easier to read. Add them separately to explain some non-obvious aspects.

Signed-off-by Dmitry Vyukov <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Will Deacon <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Andrey Ryabinin <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc [email protected]

---
 arch/x86/include/asm/atomic.h             |  7 +++++++
 include/asm-generic/atomic-instrumented.h | 30 ++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

arch/x86/include/asm/atomic.h +7 -0

index b7900346c77e..8a9e65e585db 100644
@@ -23,6 +23,13 @@
  */
 static __always_inline int arch_atomic_read(const atomic_t *v)
 {
+	/*
+	 * Note: READ_ONCE() here leads to double instrumentation as
+	 * both READ_ONCE() and atomic_read() contain instrumentation.
+	 * This is a deliberate choice. READ_ONCE_NOCHECK() is compiled to a
+	 * non-inlined function call that considerably increases binary size
+	 * and stack usage under KASAN.
+	 */
 	return READ_ONCE((v)->counter);
 }

include/asm-generic/atomic-instrumented.h +30 -0

index 7f8eb761f896..1134af090976 100644
@@ -1,3 +1,15 @@
+/*
+ * This file provides wrappers with KASAN instrumentation for atomic operations.
+ * To use this functionality an arch's atomic.h file needs to define all
+ * atomic operations with arch_ prefix (e.g. arch_atomic_read()) and include
+ * this file at the end. This file provides atomic_read() that forwards to
+ * arch_atomic_read() for actual atomic operation.
+ * Note: if an arch atomic operation is implemented by means of other atomic
+ * operations (e.g. atomic_read()/atomic_cmpxchg() loop), then it needs to use
+ * arch_ variants (i.e. arch_atomic_read()/arch_atomic_cmpxchg()) to avoid
+ * double instrumentation.
+ */
+
 #ifndef _LINUX_ATOMIC_INSTRUMENTED_H
 #define _LINUX_ATOMIC_INSTRUMENTED_H

@@ -339,6 +351,15 @@ static __always_inline bool atomic64_add_negative(long long i, atomic64_t *v)
 	return arch_atomic64_add_negative(i, v);
 }
 
+/*
+ * In the following macros we need to be careful to not clash with arch_ macros.
+ * arch_xchg() can be defined as an extended statement expression as well,
+ * if we define a __ptr variable, and arch_xchg() also defines __ptr variable,
+ * and we pass __ptr as an argument to arch_xchg(), it will use own __ptr
+ * instead of ours. This leads to unpleasant crashes. To avoid the problem
+ * the following macros declare variables with lots of underscores.
+ */
+
 #define cmpxchg(ptr, old, new)				\
 ({							\
 	__typeof__(ptr) ___ptr = (ptr);			\

@@ -374,6 +395,15 @@ static __always_inline bool atomic64_add_negative(long long i, atomic64_t *v)
 	arch_cmpxchg64_local(____ptr, (old), (new));	\
 })
 
+/*
+ * Originally we had the following code here:
+ *     __typeof__(p1) ____p1 = (p1);
+ *     kasan_check_write(____p1, 2 * sizeof(*____p1));
+ *     arch_cmpxchg_double(____p1, (p2), (o1), (o2), (n1), (n2));
+ * But it leads to compilation failures (see gcc issue 72873).
+ * So for now it's left non-instrumented.
+ * There are few callers of cmpxchg_double(), so it's not critical.
+ */
 #define cmpxchg_double(p1, p2, o1, o2, n1, n2)				\
 ({									\
 	arch_cmpxchg_double((p1), (p2), (o1), (o2), (n1), (n2));	\

Signature · 7 lines

2.13.0.219.gdb65acc882-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>