Re: [syzbot] [kvm?] WARNING: locking bug in kvm_xen_set_evtchn_fast

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux