On Tue, 2024-11-26 at 16:03 +0100, Sebastian Andrzej Siewior wrote: > On 2024-11-26 14:49:40 [+0000], David Woodhouse wrote: > > On Tue, 2024-11-26 at 06:24 -0800, syzbot wrote: > > > syzbot has bisected this issue to: > > > > > > commit 560af5dc839eef08a273908f390cfefefb82aa04 > > > Author: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> > > > Date: Wed Oct 9 15:45:03 2024 +0000 > > > > > > lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING. > > > > That's not it; this has always been broken with PREEMPT_RT I think. > > There was an attempt to fix it in > > https://lore.kernel.org/all/20240227115648.3104-8-dwmw2@xxxxxxxxxxxxx/ > > > > I'll dust that off and try again. > > Oh thank you. The timer has been made to always expire in hardirq due to > HRTIMER_MODE_ABS_HARD, this is why you see the splat. If the hardirq > invocation is needed/ possible then the callback needs to be updated. > > The linked patch has this hunk: > > - read_lock_irqsave(&gpc->lock, flags); > > + local_irq_save(flags); > > + if (!read_trylock(&gpc->lock)) { > … > > + if (in_interrupt()) > > + goto out; > > + > > + read_lock(&gpc->lock); > > This does not work. If interrupts are disabled (due to local_irq_save()) > then read_lock() must not be used. in_interrupt() does not matter. Right. At the end of that discussion, I think I concluded that if we make it use read_trylock() and fall back to the slow path, then it doesn't actually need to disable interrupts at all anyway. > Side note: Using HRTIMER_MODE_ABS would avoid the splat at the cost that > on PREEMPT_RT the timer will be invoked in softirq context (as with > HRTIMER_MODE_ABS_SOFT on !PREEMPT_RT). There is no changed behaviour on > !PREEMPT_RT. Ah, shiny. If that *only* pushes it to softirq context for PREEMPT_RT and leaves it in hardirq context for everything else, I think that's a good choice. I'll have a quick look at eliminating the _irqsave completely though, as it may be beenficial.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature