Re: PANIC: blockable slep lock (sx) msi _at_ ....msi.c:374

Attilio Rao wrote:
> Harald Schmalzbauer wrote:
>> Hello,
>>
>> recent changes (during the last 2 days,I guess tha acpi stuff) broke 
>> -current for me:
>>
>> ad6: 476940MB <WDC WD5000KS-07MNB0 07.02E07> at ata3-master SATA300
>> SMP: AP CPU #1 Launched!
>> panic: blockable sleep lock (sx) msi _at_ 
>> /FlashBSD/src/sys/i386/i386/msi.c:374
>> cpuid = 0
>> KDB: enter: panic
>> [thread pid 0 tid 0 ]
>> Stopped at      kdb_enter+0x30: leave
>> db> bt
>> Tracing pid 0 tid 0 td 0xc07c2d60
>> kdb_enter(c07422df,0,c0746e47,c1420bdc,c07c2d60,...) at kdb_enter+0x30
>> panic(c0746e47,c073180d,c0732bb2,c0764c8e,176,...) at panic+0x135
>> witness_checkorder(c082f0fc,1,c0764c8e,176,c55c0980,...) at 
>> witness_checkorder+0xd6
>> _sx_slock(c082f0fc,c0764c8e,176,c1420c64,c06f7e65,...) at _sx_slock+0x5f
>> msi_map(100,c1420d08,c1420d04,c1420c94,c04b5cc5,...) at msi_map+0x22
>> nexus_map_msi(c5552000,c55e4000,100,c1420d08,c1420d04,...) at 
>> nexus_map_msi+0x1f
>> pcib_map_msi(c55d9080,c55e4000,100,c1420d08,c1420d04,...) at 
>> pcib_map_msi+0x86
>> pcib_map_msi(c55e4200,c55e4000,100,c1420d08,c1420d04,...) at 
>> pcib_map_msi+0x86
>> pci_remap_msi_irq(c55e4000,100,c06ecb73,c54fff78,100,...) at 
>> pci_remap_msi_irq+0xeb
>> msi_assign_cpu(c55e6240,0,100,c079d170,c1420d70,...) at 
>> msi_assign_cpu+0x68
>> intr_assign_next_cpu(c55e6240,0,c07631d3,1c7,c54f3a44,...) at 
>> intr_assign_next_cpu+0x23
>> intr_shuffle_irqs(0,141e000,141ec00,141e000,0,...) at 
>> intr_shuffle_irqs+0x5e
>> mi_startup() at mi_startup+0xa0
>> begin() at begin+0x2c
> 
> In this case the culprit is intr_table_lock spinlock I think.
> This can be fixed switching the msi lock to be a spinlock instead than a 
> sx lock.
> 
> However I wonder, it is right to let sleepable lock to arise a WITNESS 
> exception if the lock is acquired in a critical section?
> I can understand this is a simple way to detect if a spinlock has been 
> previously called, but this leads to the 'false positive' case in which 
> we can have something like:
> 
> critical_enter();
> sx_xlock(&lock1);
> etc.etc.
> 

When a CPU enters a critical section, it basically won't be allowed to
run a different thread than the current one, the only exception being
for bottom half interrupt handlers (note that an ithread is not a
bottom half interrupt handler).  So it simply doesn't make sense to do
an action that is meant to reschedule you to another thread when you've
just signaled that you don't want to reschedule.

The real problem is that infrastructure code like MSI should not hold 
locks across calls to consumer code.  That's what needs to be fixed, not
hacks to WITNESS.

Scott