On Mon, Aug 17, 2026 at 02:04:53PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
> to consider:
> 
> Pre-existing issues:
> - [High] Incomplete panic-friendly locking leaves the system vulnerable to 
> deadlocks during non-NMI panics.
> --
> 
> --- Patch [10]: [PATCH v6 10/18] ring-buffer: Use panic-friendly locking in 
> ring_buffer_iter interface ---
> commit ecd8a71adf986114125493b0d7821b40d48b7d2e
> Author: Vincent Donnefort <[email protected]>
> 
> ring-buffer: Use panic-friendly locking in ring_buffer_iter interface
> 
> This commit replaces raw_spin_lock_irqsave with rb_reader_lock in the
> ring_buffer_iter functions. This prepares the iterator interface for dumping
> the buffer on panic without deadlocking on the reader_lock.
> 
> > 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
> [ ... ]
> > @@ -6302,11 +6308,14 @@ ring_buffer_iter_peek(struct ring_buffer_iter 
> > *iter, u64 *ts)
> >     struct ring_buffer_per_cpu *cpu_buffer = iter->cpu_buffer;
> >     struct ring_buffer_event *event;
> >     unsigned long flags;
> > +   bool dolock;
> >  
> >   again:
> > -   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() actually prevent
> deadlocks during non-NMI panics?
> 
> Looking at rb_reader_lock(), it only falls back to raw_spin_trylock() when
> in_nmi() is true:
> 
> 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;
>       }
> ...
> 
> If a standard oops or sysrq-c triggers a panic in a normal process or softirq
> context where in_nmi() is false, won't this unconditionally call
> raw_spin_lock()?
> 
> If smp_send_stop() halted another CPU that was currently holding the
> reader_lock, or if the panicking CPU interrupted a context holding the lock,
> this would spin forever and prevent kdump or panic_timeout from working.
> 
> Should rb_reader_lock() also check oops_in_progress to fully realize the
> panic-friendly locking behavior intended by this patch?

I do not see how this can happen. All rb_reader_lock users are also disabling
with either local_irq_save() or raw_spin_lock_irqsave().

This is also what ftrace_dump_one relies on.


> 
> [ ... ]
> 
> -- 
> Sashiko AI review ยท 
> https://sashiko.dev/#/patchset/[email protected]?part=10
> 

-- 
Vincent

Reply via email to