Skip to content

Commit 606489d

Browse files
committed
Merge tag 'trace-ringbuffer-v6.14-2' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
Pull trace fing buffer fix from Steven Rostedt: "Fix atomic64 operations on some architectures for the tracing ring buffer: - Have emulating atomic64 use arch_spin_locks instead of raw_spin_locks The tracing ring buffer events have a small timestamp that holds the delta between itself and the event before it. But this can be tricky to update when interrupts come in. It originally just set the deltas to zero for events that interrupted the adding of another event which made all the events in the interrupt have the same timestamp as the event it interrupted. This was not suitable for many tools, so it was eventually fixed. But that fix required adding an atomic64 cmpxchg on the timestamp in cases where an event was added while another event was in the process of being added. Originally, for 32 bit architectures, the manipulation of the 64 bit timestamp was done by a structure that held multiple 32bit words to hold parts of the timestamp and a counter. But as updates to the ring buffer were done, maintaining this became too complex and was replaced by the atomic64 generic operations which are now used by both 64bit and 32bit architectures. Shortly after that, it was reported that riscv32 and other 32 bit architectures that just used the generic atomic64 were locking up. This was because the generic atomic64 operations defined in lib/atomic64.c uses a raw_spin_lock() to emulate an atomic64 operation. The problem here was that raw_spin_lock() can also be traced by the function tracer (which is commonly used for debugging raw spin locks). Since the function tracer uses the tracing ring buffer, which now is being traced internally, this was triggering a recursion and setting off a warning that the spin locks were recusing. There's no reason for the code that emulates atomic64 operations to be using raw_spin_locks which have a lot of debugging infrastructure attached to them (depending on the config options). Instead it should be using the arch_spin_lock() which does not have any infrastructure attached to them and is used by low level infrastructure like RCU locks, lockdep and of course tracing. Using arch_spin_lock()s fixes this issue. - Do not trace in NMI if the architecture uses emulated atomic64 operations Another issue with using the emulated atomic64 operations that uses spin locks to emulate the atomic64 operations is that they cannot be used in NMI context. As an NMI can trigger while holding the atomic64 spin locks it can try to take the same lock and cause a deadlock. Have the ring buffer fail recording events if in NMI context and the architecture uses the emulated atomic64 operations" * tag 'trace-ringbuffer-v6.14-2' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace: atomic64: Use arch_spin_locks instead of raw_spin_locks ring-buffer: Do not allow events in NMI with generic atomic64 cmpxchg()
2 parents 7c1badb + 6c8ad3a commit 606489d

File tree

2 files changed

+55
-32
lines changed

2 files changed

+55
-32
lines changed

kernel/trace/ring_buffer.c

+7-2
Original file line numberDiff line numberDiff line change
@@ -4398,8 +4398,13 @@ rb_reserve_next_event(struct trace_buffer *buffer,
43984398
int nr_loops = 0;
43994399
int add_ts_default;
44004400

4401-
/* ring buffer does cmpxchg, make sure it is safe in NMI context */
4402-
if (!IS_ENABLED(CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG) &&
4401+
/*
4402+
* ring buffer does cmpxchg as well as atomic64 operations
4403+
* (which some archs use locking for atomic64), make sure this
4404+
* is safe in NMI context
4405+
*/
4406+
if ((!IS_ENABLED(CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG) ||
4407+
IS_ENABLED(CONFIG_GENERIC_ATOMIC64)) &&
44034408
(unlikely(in_nmi()))) {
44044409
return NULL;
44054410
}

lib/atomic64.c

+48-30
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,15 @@
2525
* Ensure each lock is in a separate cacheline.
2626
*/
2727
static union {
28-
raw_spinlock_t lock;
28+
arch_spinlock_t lock;
2929
char pad[L1_CACHE_BYTES];
3030
} atomic64_lock[NR_LOCKS] __cacheline_aligned_in_smp = {
3131
[0 ... (NR_LOCKS - 1)] = {
32-
.lock = __RAW_SPIN_LOCK_UNLOCKED(atomic64_lock.lock),
32+
.lock = __ARCH_SPIN_LOCK_UNLOCKED,
3333
},
3434
};
3535

36-
static inline raw_spinlock_t *lock_addr(const atomic64_t *v)
36+
static inline arch_spinlock_t *lock_addr(const atomic64_t *v)
3737
{
3838
unsigned long addr = (unsigned long) v;
3939

@@ -45,49 +45,57 @@ static inline raw_spinlock_t *lock_addr(const atomic64_t *v)
4545
s64 generic_atomic64_read(const atomic64_t *v)
4646
{
4747
unsigned long flags;
48-
raw_spinlock_t *lock = lock_addr(v);
48+
arch_spinlock_t *lock = lock_addr(v);
4949
s64 val;
5050

51-
raw_spin_lock_irqsave(lock, flags);
51+
local_irq_save(flags);
52+
arch_spin_lock(lock);
5253
val = v->counter;
53-
raw_spin_unlock_irqrestore(lock, flags);
54+
arch_spin_unlock(lock);
55+
local_irq_restore(flags);
5456
return val;
5557
}
5658
EXPORT_SYMBOL(generic_atomic64_read);
5759

5860
void generic_atomic64_set(atomic64_t *v, s64 i)
5961
{
6062
unsigned long flags;
61-
raw_spinlock_t *lock = lock_addr(v);
63+
arch_spinlock_t *lock = lock_addr(v);
6264

63-
raw_spin_lock_irqsave(lock, flags);
65+
local_irq_save(flags);
66+
arch_spin_lock(lock);
6467
v->counter = i;
65-
raw_spin_unlock_irqrestore(lock, flags);
68+
arch_spin_unlock(lock);
69+
local_irq_restore(flags);
6670
}
6771
EXPORT_SYMBOL(generic_atomic64_set);
6872

6973
#define ATOMIC64_OP(op, c_op) \
7074
void generic_atomic64_##op(s64 a, atomic64_t *v) \
7175
{ \
7276
unsigned long flags; \
73-
raw_spinlock_t *lock = lock_addr(v); \
77+
arch_spinlock_t *lock = lock_addr(v); \
7478
\
75-
raw_spin_lock_irqsave(lock, flags); \
79+
local_irq_save(flags); \
80+
arch_spin_lock(lock); \
7681
v->counter c_op a; \
77-
raw_spin_unlock_irqrestore(lock, flags); \
82+
arch_spin_unlock(lock); \
83+
local_irq_restore(flags); \
7884
} \
7985
EXPORT_SYMBOL(generic_atomic64_##op);
8086

8187
#define ATOMIC64_OP_RETURN(op, c_op) \
8288
s64 generic_atomic64_##op##_return(s64 a, atomic64_t *v) \
8389
{ \
8490
unsigned long flags; \
85-
raw_spinlock_t *lock = lock_addr(v); \
91+
arch_spinlock_t *lock = lock_addr(v); \
8692
s64 val; \
8793
\
88-
raw_spin_lock_irqsave(lock, flags); \
94+
local_irq_save(flags); \
95+
arch_spin_lock(lock); \
8996
val = (v->counter c_op a); \
90-
raw_spin_unlock_irqrestore(lock, flags); \
97+
arch_spin_unlock(lock); \
98+
local_irq_restore(flags); \
9199
return val; \
92100
} \
93101
EXPORT_SYMBOL(generic_atomic64_##op##_return);
@@ -96,13 +104,15 @@ EXPORT_SYMBOL(generic_atomic64_##op##_return);
96104
s64 generic_atomic64_fetch_##op(s64 a, atomic64_t *v) \
97105
{ \
98106
unsigned long flags; \
99-
raw_spinlock_t *lock = lock_addr(v); \
107+
arch_spinlock_t *lock = lock_addr(v); \
100108
s64 val; \
101109
\
102-
raw_spin_lock_irqsave(lock, flags); \
110+
local_irq_save(flags); \
111+
arch_spin_lock(lock); \
103112
val = v->counter; \
104113
v->counter c_op a; \
105-
raw_spin_unlock_irqrestore(lock, flags); \
114+
arch_spin_unlock(lock); \
115+
local_irq_restore(flags); \
106116
return val; \
107117
} \
108118
EXPORT_SYMBOL(generic_atomic64_fetch_##op);
@@ -131,58 +141,66 @@ ATOMIC64_OPS(xor, ^=)
131141
s64 generic_atomic64_dec_if_positive(atomic64_t *v)
132142
{
133143
unsigned long flags;
134-
raw_spinlock_t *lock = lock_addr(v);
144+
arch_spinlock_t *lock = lock_addr(v);
135145
s64 val;
136146

137-
raw_spin_lock_irqsave(lock, flags);
147+
local_irq_save(flags);
148+
arch_spin_lock(lock);
138149
val = v->counter - 1;
139150
if (val >= 0)
140151
v->counter = val;
141-
raw_spin_unlock_irqrestore(lock, flags);
152+
arch_spin_unlock(lock);
153+
local_irq_restore(flags);
142154
return val;
143155
}
144156
EXPORT_SYMBOL(generic_atomic64_dec_if_positive);
145157

146158
s64 generic_atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n)
147159
{
148160
unsigned long flags;
149-
raw_spinlock_t *lock = lock_addr(v);
161+
arch_spinlock_t *lock = lock_addr(v);
150162
s64 val;
151163

152-
raw_spin_lock_irqsave(lock, flags);
164+
local_irq_save(flags);
165+
arch_spin_lock(lock);
153166
val = v->counter;
154167
if (val == o)
155168
v->counter = n;
156-
raw_spin_unlock_irqrestore(lock, flags);
169+
arch_spin_unlock(lock);
170+
local_irq_restore(flags);
157171
return val;
158172
}
159173
EXPORT_SYMBOL(generic_atomic64_cmpxchg);
160174

161175
s64 generic_atomic64_xchg(atomic64_t *v, s64 new)
162176
{
163177
unsigned long flags;
164-
raw_spinlock_t *lock = lock_addr(v);
178+
arch_spinlock_t *lock = lock_addr(v);
165179
s64 val;
166180

167-
raw_spin_lock_irqsave(lock, flags);
181+
local_irq_save(flags);
182+
arch_spin_lock(lock);
168183
val = v->counter;
169184
v->counter = new;
170-
raw_spin_unlock_irqrestore(lock, flags);
185+
arch_spin_unlock(lock);
186+
local_irq_restore(flags);
171187
return val;
172188
}
173189
EXPORT_SYMBOL(generic_atomic64_xchg);
174190

175191
s64 generic_atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u)
176192
{
177193
unsigned long flags;
178-
raw_spinlock_t *lock = lock_addr(v);
194+
arch_spinlock_t *lock = lock_addr(v);
179195
s64 val;
180196

181-
raw_spin_lock_irqsave(lock, flags);
197+
local_irq_save(flags);
198+
arch_spin_lock(lock);
182199
val = v->counter;
183200
if (val != u)
184201
v->counter += a;
185-
raw_spin_unlock_irqrestore(lock, flags);
202+
arch_spin_unlock(lock);
203+
local_irq_restore(flags);
186204

187205
return val;
188206
}

0 commit comments

Comments
 (0)