| Message ID | 20250120235721.574973242@goodmis.org (mailing list archive) |
|---|---|
| State | Queued |
| Commit | b71b3c12081866affb87c641019473cdee8a8f8c |
| Headers | show |
| Series | lib/atomic64: ring-buffer: Fix infinite recursion on some 32bit archs | expand |
On Mon, 20 Jan 2025 18:56:57 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > raw_spin_locks can be traced by lockdep or tracing itself. Atomic64 > operations can be used in the tracing infrastructure. When an architecture > does not have true atomic64 operations it can use the generic version that > disables interrupts and uses spin_locks. > > The tracing ring buffer code uses atomic64 operations for the time > keeping. But because some architectures use the default operations, the > locking inside the atomic operations can cause an infinite recursion. > > As atomic64 is an architecture specific operation, it should not be using > raw_spin_locks() but instead arch_spin_locks as that is the purpose of > arch_spin_locks. To be used in architecture specific implementations of > generic infrastructure like atomic64 operations. > Looks good to me. Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thanks, > Cc: stable@vger.kernel.org > Fixes: c84897c0ff592 ("ring-buffer: Remove 32bit timestamp logic") > Closes: https://lore.kernel.org/all/86fb4f86-a0e4-45a2-a2df-3154acc4f086@gaisler.com/ > Reported-by: Ludwig Rydberg <ludwig.rydberg@gaisler.com> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > lib/atomic64.c | 78 +++++++++++++++++++++++++++++++------------------- > 1 file changed, 48 insertions(+), 30 deletions(-) > > diff --git a/lib/atomic64.c b/lib/atomic64.c > index caf895789a1e..1a72bba36d24 100644 > --- a/lib/atomic64.c > +++ b/lib/atomic64.c > @@ -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; > } > -- > 2.45.2 > >
diff --git a/lib/atomic64.c b/lib/atomic64.c index caf895789a1e..1a72bba36d24 100644 --- a/lib/atomic64.c +++ b/lib/atomic64.c @@ -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; }