Re: Call for Testers: Convert watchdog spinlock into a sleepable mutex in if_ndis



On Sun, 2008-06-08 at 03:55 +0200, Paul B. Mahol wrote:
On 6/7/08, Coleman Kane <cokane@xxxxxxxxxxx> wrote:
Hi,

I've got another patch to if_ndis and I'd like to know if any NDIS users
out there can test it for me and give me some feedback. I've converted
the ndis_spinlock in if_ndis into a normal sleepable mutex, and would
like to get some testers. The idea here is to eliminate an unnecessary
"spinning" in the kernel, where it would be preferable to sleep (for CPU
usage, interactivity, etc...).

You can download it here:
*
http://people.freebsd.org/~cokane/patches/if_ndis-spinlock-to-mtx.patch

--
Coleman Kane


I did not found any stability regressions in my environment with linked patch.

But I noticed new LOR (it appears as soon as radio of device is turned on):

lock order reversal:
1st 0xc401726c ndis0 (network driver) @
/usr/local/src/sys/modules/if_ndis/../../dev/if_ndis/if_ndis.c:1648
2nd 0xc09b3e74 HAL preemption lock (HAL lock) @
/usr/local/src/sys/modules/ndis/../../compat/ndis/subr_hal.c:423
KDB: stack backtrace:
db_trace_self_wrapper(c0708534,c3985bcc,c0541a6e,c070adf6,c09b3e74,...)
at db_trace_self_wrapper+0x26
kdb_backtrace(c070adf6,c09b3e74,c09b0a51,c09b0a48,c09b09f8,...) at
kdb_backtrace+0x29
witness_checkorder(c09b3e74,9,c09b09f8,1a7,c04f7364,...) at
witness_checkorder+0x6de
_mtx_lock_flags(c09b3e74,0,c09b09f8,1a7,c457ebd0,...) at _mtx_lock_flags+0xbc
KfRaiseIrql(2,c457ebd0,c3985c48,c09aacdd,c3d45354,...) at KfRaiseIrql+0x63
KfAcquireSpinLock(c3d45354,c094888a,c4017200,c3d6cc40,c4017200,...) at
KfAcquireSpinLock+0x13
IoQueueWorkItem(c457ebd0,c3d6cc40,0,c4017200,c07040ef,...) at
IoQueueWorkItem+0x2d
ndis_tick(c4017200,0,c070699c,166,c077e594,...) at ndis_tick+0x143
softclock(c077e560,0,c0701ca6,4d4,c3cda6ec,...) at softclock+0x24a
ithread_loop(c3cdb2e0,c3985d38,c0701a09,324,c3c99a70,...) at ithread_loop+0x1b5
fork_exit(c04e74f0,c3cdb2e0,c3985d38) at fork_exit+0xb8
fork_trampoline() at fork_trampoline+0x8
--- trap 0, eip = 0, esp = 0xc3985d70, ebp = 0 ---

Try this one:
* http://people.freebsd.org/~cokane/patches/if_ndis-spinlock-to-mtx2.patch

There's the same LOR in the existing code as well, but it is hidden by
WITNESS_SKIPSPIN.

If you can verify that this works, I'll just commit them both as a
single update.

--
Coleman Kane

Attachment: signature.asc
Description: This is a digitally signed message part