Skip to content

Commit f8182f6

Browse files
palmer-dabbeltpalmer-dabbelt
committed
RISC-V Atomic Cleanups
This patch set is the result of some feedback that filtered through after our original patch set was reviewed, some of which was the result of me missing some email. It contains: * A new READ_ONCE in arch_spin_is_locked() * __test_and_op_bit_ord() is now actually ordered * Improvements to various comments * Removal of some dead code
2 parents 4fbd8d1 + bf73055 commit f8182f6

File tree

5 files changed

+60
-84
lines changed

5 files changed

+60
-84
lines changed

‎arch/riscv/include/asm/atomic.h

Lines changed: 54 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -50,30 +50,30 @@ static __always_inline void atomic64_set(atomic64_t *v, long i)
5050
* have the AQ or RL bits set. These don't return anything, so there's only
5151
* one version to worry about.
5252
*/
53-
#define ATOMIC_OP(op, asm_op, c_op, I, asm_type, c_type, prefix) \
54-
static __always_inline void atomic##prefix##_##op(c_type i, atomic##prefix##_t *v) \
55-
{ \
56-
__asm__ __volatile__ ( \
57-
"amo" #asm_op "." #asm_type " zero, %1, %0" \
58-
: "+A" (v->counter) \
59-
: "r" (I) \
60-
: "memory"); \
53+
#define ATOMIC_OP(op, asm_op, I, asm_type, c_type, prefix) \
54+
static __always_inline void atomic##prefix##_##op(c_type i, atomic##prefix##_t *v) \
55+
{ \
56+
__asm__ __volatile__ ( \
57+
"amo" #asm_op "." #asm_type " zero, %1, %0" \
58+
: "+A" (v->counter) \
59+
: "r" (I) \
60+
: "memory"); \
6161
}
6262

6363
#ifdef CONFIG_GENERIC_ATOMIC64
64-
#define ATOMIC_OPS(op, asm_op, c_op, I) \
65-
ATOMIC_OP (op, asm_op, c_op, I, w, int, )
64+
#define ATOMIC_OPS(op, asm_op, I) \
65+
ATOMIC_OP (op, asm_op, I, w, int, )
6666
#else
67-
#define ATOMIC_OPS(op, asm_op, c_op, I) \
68-
ATOMIC_OP (op, asm_op, c_op, I, w, int, ) \
69-
ATOMIC_OP (op, asm_op, c_op, I, d, long, 64)
67+
#define ATOMIC_OPS(op, asm_op, I) \
68+
ATOMIC_OP (op, asm_op, I, w, int, ) \
69+
ATOMIC_OP (op, asm_op, I, d, long, 64)
7070
#endif
7171

72-
ATOMIC_OPS(add, add, +, i)
73-
ATOMIC_OPS(sub, add, +, -i)
74-
ATOMIC_OPS(and, and, &, i)
75-
ATOMIC_OPS( or, or, |, i)
76-
ATOMIC_OPS(xor, xor, ^, i)
72+
ATOMIC_OPS(add, add, i)
73+
ATOMIC_OPS(sub, add, -i)
74+
ATOMIC_OPS(and, and, i)
75+
ATOMIC_OPS( or, or, i)
76+
ATOMIC_OPS(xor, xor, i)
7777

7878
#undef ATOMIC_OP
7979
#undef ATOMIC_OPS
@@ -83,7 +83,7 @@ ATOMIC_OPS(xor, xor, ^, i)
8383
* There's two flavors of these: the arithmatic ops have both fetch and return
8484
* versions, while the logical ops only have fetch versions.
8585
*/
86-
#define ATOMIC_FETCH_OP(op, asm_op, c_op, I, asm_or, c_or, asm_type, c_type, prefix) \
86+
#define ATOMIC_FETCH_OP(op, asm_op, I, asm_or, c_or, asm_type, c_type, prefix) \
8787
static __always_inline c_type atomic##prefix##_fetch_##op##c_or(c_type i, atomic##prefix##_t *v) \
8888
{ \
8989
register c_type ret; \
@@ -103,13 +103,13 @@ static __always_inline c_type atomic##prefix##_##op##_return##c_or(c_type i, ato
103103

104104
#ifdef CONFIG_GENERIC_ATOMIC64
105105
#define ATOMIC_OPS(op, asm_op, c_op, I, asm_or, c_or) \
106-
ATOMIC_FETCH_OP (op, asm_op, c_op, I, asm_or, c_or, w, int, ) \
106+
ATOMIC_FETCH_OP (op, asm_op, I, asm_or, c_or, w, int, ) \
107107
ATOMIC_OP_RETURN(op, asm_op, c_op, I, asm_or, c_or, w, int, )
108108
#else
109109
#define ATOMIC_OPS(op, asm_op, c_op, I, asm_or, c_or) \
110-
ATOMIC_FETCH_OP (op, asm_op, c_op, I, asm_or, c_or, w, int, ) \
110+
ATOMIC_FETCH_OP (op, asm_op, I, asm_or, c_or, w, int, ) \
111111
ATOMIC_OP_RETURN(op, asm_op, c_op, I, asm_or, c_or, w, int, ) \
112-
ATOMIC_FETCH_OP (op, asm_op, c_op, I, asm_or, c_or, d, long, 64) \
112+
ATOMIC_FETCH_OP (op, asm_op, I, asm_or, c_or, d, long, 64) \
113113
ATOMIC_OP_RETURN(op, asm_op, c_op, I, asm_or, c_or, d, long, 64)
114114
#endif
115115

@@ -126,28 +126,28 @@ ATOMIC_OPS(sub, add, +, -i, .aqrl, )
126126
#undef ATOMIC_OPS
127127

128128
#ifdef CONFIG_GENERIC_ATOMIC64
129-
#define ATOMIC_OPS(op, asm_op, c_op, I, asm_or, c_or) \
130-
ATOMIC_FETCH_OP(op, asm_op, c_op, I, asm_or, c_or, w, int, )
129+
#define ATOMIC_OPS(op, asm_op, I, asm_or, c_or) \
130+
ATOMIC_FETCH_OP(op, asm_op, I, asm_or, c_or, w, int, )
131131
#else
132-
#define ATOMIC_OPS(op, asm_op, c_op, I, asm_or, c_or) \
133-
ATOMIC_FETCH_OP(op, asm_op, c_op, I, asm_or, c_or, w, int, ) \
134-
ATOMIC_FETCH_OP(op, asm_op, c_op, I, asm_or, c_or, d, long, 64)
132+
#define ATOMIC_OPS(op, asm_op, I, asm_or, c_or) \
133+
ATOMIC_FETCH_OP(op, asm_op, I, asm_or, c_or, w, int, ) \
134+
ATOMIC_FETCH_OP(op, asm_op, I, asm_or, c_or, d, long, 64)
135135
#endif
136136

137-
ATOMIC_OPS(and, and, &, i, , _relaxed)
138-
ATOMIC_OPS(and, and, &, i, .aq , _acquire)
139-
ATOMIC_OPS(and, and, &, i, .rl , _release)
140-
ATOMIC_OPS(and, and, &, i, .aqrl, )
137+
ATOMIC_OPS(and, and, i, , _relaxed)
138+
ATOMIC_OPS(and, and, i, .aq , _acquire)
139+
ATOMIC_OPS(and, and, i, .rl , _release)
140+
ATOMIC_OPS(and, and, i, .aqrl, )
141141

142-
ATOMIC_OPS( or, or, |, i, , _relaxed)
143-
ATOMIC_OPS( or, or, |, i, .aq , _acquire)
144-
ATOMIC_OPS( or, or, |, i, .rl , _release)
145-
ATOMIC_OPS( or, or, |, i, .aqrl, )
142+
ATOMIC_OPS( or, or, i, , _relaxed)
143+
ATOMIC_OPS( or, or, i, .aq , _acquire)
144+
ATOMIC_OPS( or, or, i, .rl , _release)
145+
ATOMIC_OPS( or, or, i, .aqrl, )
146146

147-
ATOMIC_OPS(xor, xor, ^, i, , _relaxed)
148-
ATOMIC_OPS(xor, xor, ^, i, .aq , _acquire)
149-
ATOMIC_OPS(xor, xor, ^, i, .rl , _release)
150-
ATOMIC_OPS(xor, xor, ^, i, .aqrl, )
147+
ATOMIC_OPS(xor, xor, i, , _relaxed)
148+
ATOMIC_OPS(xor, xor, i, .aq , _acquire)
149+
ATOMIC_OPS(xor, xor, i, .rl , _release)
150+
ATOMIC_OPS(xor, xor, i, .aqrl, )
151151

152152
#undef ATOMIC_OPS
153153

@@ -182,13 +182,13 @@ ATOMIC_OPS(add_negative, add, <, 0)
182182
#undef ATOMIC_OP
183183
#undef ATOMIC_OPS
184184

185-
#define ATOMIC_OP(op, func_op, c_op, I, c_type, prefix) \
185+
#define ATOMIC_OP(op, func_op, I, c_type, prefix) \
186186
static __always_inline void atomic##prefix##_##op(atomic##prefix##_t *v) \
187187
{ \
188188
atomic##prefix##_##func_op(I, v); \
189189
}
190190

191-
#define ATOMIC_FETCH_OP(op, func_op, c_op, I, c_type, prefix) \
191+
#define ATOMIC_FETCH_OP(op, func_op, I, c_type, prefix) \
192192
static __always_inline c_type atomic##prefix##_fetch_##op(atomic##prefix##_t *v) \
193193
{ \
194194
return atomic##prefix##_fetch_##func_op(I, v); \
@@ -202,16 +202,16 @@ static __always_inline c_type atomic##prefix##_##op##_return(atomic##prefix##_t
202202

203203
#ifdef CONFIG_GENERIC_ATOMIC64
204204
#define ATOMIC_OPS(op, asm_op, c_op, I) \
205-
ATOMIC_OP (op, asm_op, c_op, I, int, ) \
206-
ATOMIC_FETCH_OP (op, asm_op, c_op, I, int, ) \
205+
ATOMIC_OP (op, asm_op, I, int, ) \
206+
ATOMIC_FETCH_OP (op, asm_op, I, int, ) \
207207
ATOMIC_OP_RETURN(op, asm_op, c_op, I, int, )
208208
#else
209209
#define ATOMIC_OPS(op, asm_op, c_op, I) \
210-
ATOMIC_OP (op, asm_op, c_op, I, int, ) \
211-
ATOMIC_FETCH_OP (op, asm_op, c_op, I, int, ) \
210+
ATOMIC_OP (op, asm_op, I, int, ) \
211+
ATOMIC_FETCH_OP (op, asm_op, I, int, ) \
212212
ATOMIC_OP_RETURN(op, asm_op, c_op, I, int, ) \
213-
ATOMIC_OP (op, asm_op, c_op, I, long, 64) \
214-
ATOMIC_FETCH_OP (op, asm_op, c_op, I, long, 64) \
213+
ATOMIC_OP (op, asm_op, I, long, 64) \
214+
ATOMIC_FETCH_OP (op, asm_op, I, long, 64) \
215215
ATOMIC_OP_RETURN(op, asm_op, c_op, I, long, 64)
216216
#endif
217217

@@ -300,8 +300,13 @@ static __always_inline long atomic64_inc_not_zero(atomic64_t *v)
300300

301301
/*
302302
* atomic_{cmp,}xchg is required to have exactly the same ordering semantics as
303-
* {cmp,}xchg and the operations that return, so they need a barrier. We just
304-
* use the other implementations directly.
303+
* {cmp,}xchg and the operations that return, so they need a barrier.
304+
*/
305+
/*
306+
* FIXME: atomic_cmpxchg_{acquire,release,relaxed} are all implemented by
307+
* assigning the same barrier to both the LR and SC operations, but that might
308+
* not make any sense. We're waiting on a memory model specification to
309+
* determine exactly what the right thing to do is here.
305310
*/
306311
#define ATOMIC_OP(c_t, prefix, c_or, size, asm_or) \
307312
static __always_inline c_t atomic##prefix##_cmpxchg##c_or(atomic##prefix##_t *v, c_t o, c_t n) \

‎arch/riscv/include/asm/barrier.h

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,29 +38,6 @@
3838
#define smp_rmb() RISCV_FENCE(r,r)
3939
#define smp_wmb() RISCV_FENCE(w,w)
4040

41-
/*
42-
* These fences exist to enforce ordering around the relaxed AMOs. The
43-
* documentation defines that
44-
* "
45-
* atomic_fetch_add();
46-
* is equivalent to:
47-
* smp_mb__before_atomic();
48-
* atomic_fetch_add_relaxed();
49-
* smp_mb__after_atomic();
50-
* "
51-
* So we emit full fences on both sides.
52-
*/
53-
#define __smb_mb__before_atomic() smp_mb()
54-
#define __smb_mb__after_atomic() smp_mb()
55-
56-
/*
57-
* These barriers prevent accesses performed outside a spinlock from being moved
58-
* inside a spinlock. Since RISC-V sets the aq/rl bits on our spinlock only
59-
* enforce release consistency, we need full fences here.
60-
*/
61-
#define smb_mb__before_spinlock() smp_mb()
62-
#define smb_mb__after_spinlock() smp_mb()
63-
6441
#include <asm-generic/barrier.h>
6542

6643
#endif /* __ASSEMBLY__ */

‎arch/riscv/include/asm/bitops.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
: "memory");
6868

6969
#define __test_and_op_bit(op, mod, nr, addr) \
70-
__test_and_op_bit_ord(op, mod, nr, addr, )
70+
__test_and_op_bit_ord(op, mod, nr, addr, .aqrl)
7171
#define __op_bit(op, mod, nr, addr) \
7272
__op_bit_ord(op, mod, nr, addr, )
7373

‎arch/riscv/include/asm/spinlock.h

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424

2525
/* FIXME: Replace this with a ticket lock, like MIPS. */
2626

27-
#define arch_spin_is_locked(x) ((x)->lock != 0)
27+
#define arch_spin_is_locked(x) (READ_ONCE((x)->lock) != 0)
2828

2929
static inline void arch_spin_unlock(arch_spinlock_t *lock)
3030
{
@@ -58,15 +58,6 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
5858
}
5959
}
6060

61-
static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
62-
{
63-
smp_rmb();
64-
do {
65-
cpu_relax();
66-
} while (arch_spin_is_locked(lock));
67-
smp_acquire__after_ctrl_dep();
68-
}
69-
7061
/***********************************************************/
7162

7263
static inline void arch_read_lock(arch_rwlock_t *lock)

‎arch/riscv/include/asm/tlbflush.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@
1717

1818
#ifdef CONFIG_MMU
1919

20-
/* Flush entire local TLB */
20+
/*
21+
* Flush entire local TLB. 'sfence.vma' implicitly fences with the instruction
22+
* cache as well, so a 'fence.i' is not necessary.
23+
*/
2124
static inline void local_flush_tlb_all(void)
2225
{
2326
__asm__ __volatile__ ("sfence.vma" : : : "memory");

0 commit comments

Comments
 (0)