Thread (37 messages) flat view 37 messages, 2 authors, 6d ago

Re: [PATCH v5 10/18] ring-buffer: Use panic-friendly locking in ring_buffer_iter interface

From: sashiko-bot@kernel.org
Date: 2026-08-12 16:54:12

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] rb_reader_lock() lacks an oops_in_progress check, leading to deadlocks during non-NMI panic dumps.
--
--- Patch [10]: [PATCH v5 10/18] ring-buffer: Use panic-friendly locking in ring_buffer_iter interface ---
commit 15fbd37892d8549b59334542a9c228e531aa61b5
Author: Vincent Donnefort [off-list ref]

ring-buffer: Use panic-friendly locking in ring_buffer_iter interface

This patch replaces raw_spin_lock_irqsave() with rb_reader_lock() in the
iterator functions. The goal is to make the iterators panic-friendly so
trace_remote can dump the buffer on panic without deadlocking.
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index a7d9936b4c835..a6f2293f409b8 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -5616,15 +5619,18 @@ void ring_buffer_iter_reset(struct ring_buffer_iter *iter)
 {
 	struct ring_buffer_per_cpu *cpu_buffer;
 	unsigned long flags;
+	bool dolock;
 
 	if (!iter)
 		return;
 
 	cpu_buffer = iter->cpu_buffer;
 
-	raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
+	local_irq_save(flags);
+	dolock = rb_reader_lock(cpu_buffer);
[Severity: High]
This is a pre-existing issue, but does rb_reader_lock() lack an
oops_in_progress check?

If a panic occurs in a normal (non-NMI) context, in_nmi() evaluates to false
in rb_reader_lock():

static inline bool rb_reader_lock(struct ring_buffer_per_cpu *cpu_buffer)
{
	if (likely(!in_nmi())) {
		raw_spin_lock(&cpu_buffer->reader_lock);
		return true;
	}

During a panic, smp_send_stop() will forcefully halt other CPUs. If one of
those stopped CPUs was holding cpu_buffer->reader_lock, the panicking CPU
will spin forever in raw_spin_lock().

Since the goal here is to make these iterators panic-friendly, does
rb_reader_lock() also need to bypass the lock (e.g. check oops_in_progress) 
to avoid deadlocks during non-NMI panics?
 	rb_iter_reset(iter);
-	raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
+	rb_reader_unlock(cpu_buffer, dolock);
+	local_irq_restore(flags);
 }
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812161340.2417322-1-vdonnefort@google.com?part=10
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help