github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-05-31 18:43:33 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

Fix atomic64 operations on some architectures for the tracing ring buffer:

Browse Source 
- 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.
 -----BEGIN PGP SIGNATURE-----
 
 iIoEABYIADIWIQRRSw7ePDh/lE+zeZMp5XQQmuv6qgUCZ5Jr7RQccm9zdGVkdEBn
 b29kbWlzLm9yZwAKCRAp5XQQmuv6qg7cAPoD/H4BRsFa3UUDnxofTlBuj4A7neJd
 rk9ddD9HXH8KywEAhBn1Oujiw81Ayjx7E6s4ednAQX4rldTXBXDyFNuuGgU=
 =b13F
 -----END PGP SIGNATURE-----

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()
This commit is contained in:
Linus Torvalds 2025-01-23 18:02:55 -08:00
commit 606489dbfa
2 changed files with 55 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
kernel/trace/ring_buffer.c
View File

@ -4398,8 +4398,13 @@ rb_reserve_next_event(struct trace_buffer *buffer,
int nr_loops = 0;
int add_ts_default;
/* ring buffer does cmpxchg, make sure it is safe in NMI context */
if (!IS_ENABLED(CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG) &&
/*
* ring buffer does cmpxchg as well as atomic64 operations
* (which some archs use locking for atomic64), make sure this
* is safe in NMI context
*/
if ((!IS_ENABLED(CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG) ||
IS_ENABLED(CONFIG_GENERIC_ATOMIC64)) &&
(unlikely(in_nmi()))) {
return NULL;
}

78
lib/atomic64.c
View File

@ -25,15 +25,15 @@
* Ensure each lock is in a separate cacheline.
*/
static union {
raw_spinlock_t lock;
arch_spinlock_t lock;
char pad[L1_CACHE_BYTES];
} atomic64_lock[NR_LOCKS] __cacheline_aligned_in_smp = {
[0 ... (NR_LOCKS - 1)] = {
.lock = __RAW_SPIN_LOCK_UNLOCKED(atomic64_lock.lock),
.lock = __ARCH_SPIN_LOCK_UNLOCKED,
},
};
static inline raw_spinlock_t *lock_addr(const atomic64_t *v)
static inline arch_spinlock_t *lock_addr(const atomic64_t *v)
{
unsigned long addr = (unsigned long) v;
@ -45,12 +45,14 @@ static inline raw_spinlock_t *lock_addr(const atomic64_t *v)
s64 generic_atomic64_read(const atomic64_t *v)
{
unsigned long flags;
raw_spinlock_t *lock = lock_addr(v);
arch_spinlock_t *lock = lock_addr(v);
s64 val;
raw_spin_lock_irqsave(lock, flags);
local_irq_save(flags);
arch_spin_lock(lock);
val = v->counter;
raw_spin_unlock_irqrestore(lock, flags);
arch_spin_unlock(lock);
local_irq_restore(flags);
return val;
}
EXPORT_SYMBOL(generic_atomic64_read);
@ -58,11 +60,13 @@ EXPORT_SYMBOL(generic_atomic64_read);
void generic_atomic64_set(atomic64_t *v, s64 i)
{
unsigned long flags;
raw_spinlock_t *lock = lock_addr(v);
arch_spinlock_t *lock = lock_addr(v);
raw_spin_lock_irqsave(lock, flags);
local_irq_save(flags);
arch_spin_lock(lock);
v->counter = i;
raw_spin_unlock_irqrestore(lock, flags);
arch_spin_unlock(lock);
local_irq_restore(flags);
}
EXPORT_SYMBOL(generic_atomic64_set);
@ -70,11 +74,13 @@ EXPORT_SYMBOL(generic_atomic64_set);
void generic_atomic64_##op(s64 a, atomic64_t *v) \
{ \
unsigned long flags; \
raw_spinlock_t *lock = lock_addr(v); \
arch_spinlock_t *lock = lock_addr(v); \
\
raw_spin_lock_irqsave(lock, flags); \
local_irq_save(flags); \
arch_spin_lock(lock); \
v->counter c_op a; \
raw_spin_unlock_irqrestore(lock, flags); \
arch_spin_unlock(lock); \
local_irq_restore(flags); \
} \
EXPORT_SYMBOL(generic_atomic64_##op);
@ -82,12 +88,14 @@ EXPORT_SYMBOL(generic_atomic64_##op);
s64 generic_atomic64_##op##_return(s64 a, atomic64_t *v) \
{ \
unsigned long flags; \
raw_spinlock_t *lock = lock_addr(v); \
arch_spinlock_t *lock = lock_addr(v); \
s64 val; \
\
raw_spin_lock_irqsave(lock, flags); \
local_irq_save(flags); \
arch_spin_lock(lock); \
val = (v->counter c_op a); \
raw_spin_unlock_irqrestore(lock, flags); \
arch_spin_unlock(lock); \
local_irq_restore(flags); \
return val; \
} \
EXPORT_SYMBOL(generic_atomic64_##op##_return);
@ -96,13 +104,15 @@ EXPORT_SYMBOL(generic_atomic64_##op##_return);
s64 generic_atomic64_fetch_##op(s64 a, atomic64_t *v) \
{ \
unsigned long flags; \
raw_spinlock_t *lock = lock_addr(v); \
arch_spinlock_t *lock = lock_addr(v); \
s64 val; \
\
raw_spin_lock_irqsave(lock, flags); \
local_irq_save(flags); \
arch_spin_lock(lock); \
val = v->counter; \
v->counter c_op a; \
raw_spin_unlock_irqrestore(lock, flags); \
arch_spin_unlock(lock); \
local_irq_restore(flags); \
return val; \
} \
EXPORT_SYMBOL(generic_atomic64_fetch_##op);
@ -131,14 +141,16 @@ ATOMIC64_OPS(xor, ^=)
s64 generic_atomic64_dec_if_positive(atomic64_t *v)
{
unsigned long flags;
raw_spinlock_t *lock = lock_addr(v);
arch_spinlock_t *lock = lock_addr(v);
s64 val;
raw_spin_lock_irqsave(lock, flags);
local_irq_save(flags);
arch_spin_lock(lock);
val = v->counter - 1;
if (val >= 0)
v->counter = val;
raw_spin_unlock_irqrestore(lock, flags);
arch_spin_unlock(lock);
local_irq_restore(flags);
return val;
}
EXPORT_SYMBOL(generic_atomic64_dec_if_positive);
@ -146,14 +158,16 @@ EXPORT_SYMBOL(generic_atomic64_dec_if_positive);
s64 generic_atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n)
{
unsigned long flags;
raw_spinlock_t *lock = lock_addr(v);
arch_spinlock_t *lock = lock_addr(v);
s64 val;
raw_spin_lock_irqsave(lock, flags);
local_irq_save(flags);
arch_spin_lock(lock);
val = v->counter;
if (val == o)
v->counter = n;
raw_spin_unlock_irqrestore(lock, flags);
arch_spin_unlock(lock);
local_irq_restore(flags);
return val;
}
EXPORT_SYMBOL(generic_atomic64_cmpxchg);
@ -161,13 +175,15 @@ EXPORT_SYMBOL(generic_atomic64_cmpxchg);
s64 generic_atomic64_xchg(atomic64_t *v, s64 new)
{
unsigned long flags;
raw_spinlock_t *lock = lock_addr(v);
arch_spinlock_t *lock = lock_addr(v);
s64 val;
raw_spin_lock_irqsave(lock, flags);
local_irq_save(flags);
arch_spin_lock(lock);
val = v->counter;
v->counter = new;
raw_spin_unlock_irqrestore(lock, flags);
arch_spin_unlock(lock);
local_irq_restore(flags);
return val;
}
EXPORT_SYMBOL(generic_atomic64_xchg);
@ -175,14 +191,16 @@ EXPORT_SYMBOL(generic_atomic64_xchg);
s64 generic_atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u)
{
unsigned long flags;
raw_spinlock_t *lock = lock_addr(v);
arch_spinlock_t *lock = lock_addr(v);
s64 val;
raw_spin_lock_irqsave(lock, flags);
local_irq_save(flags);
arch_spin_lock(lock);
val = v->counter;
if (val != u)
v->counter += a;
raw_spin_unlock_irqrestore(lock, flags);
arch_spin_unlock(lock);
local_irq_restore(flags);
return val;
}