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?

[ ... ]

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

Reply via email to