Date: Fri, 9 Jan 2004 00:55:08 -0800 (PST) From: Don Lewis <truckman@FreeBSD.org> To: wpaul@FreeBSD.org Cc: src-committers@FreeBSD.org Subject: Re: cvs commit: src/sys/compat/ndis ndis_var.h src/sys/dev/if_ndis if_ndis.c if_ndisvar.h Message-ID: <200401090855.i098t87E021981@gw.catspoiler.org> In-Reply-To: <20040109074939.28CEC16A4D0@hub.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 8 Jan, Bill Paul wrote: >> * Bill Paul <wpaul@FreeBSD.org> [040108 02:44] wrote: >> > Using these tweaks, I can now get the Broadcom BCM5701 NDIS >> > driver to load and run. Unfortunately, the version I have seems >> > to suffer from the same bug as the SMC 83820 driver, which is >> > that it creates a spinlock during its DriverEntry() routine. >> > I'm still debating the right way to deal with this. >> >> What problem does that present? > > Here are the comments in subr_ndis.c:ndis_lock() which you chose > not to read: > > /* > * Workaround for certain broken NDIS drivers. I have > * encountered one case where a driver creates a spinlock > * within its DriverEntry() routine, which is then destroyed > * in its MiniportHalt() routine. This is a bug, because > * MiniportHalt() is meant to only destroy what MiniportInit() > * creates. This leads to the following problem: > * DriverEntry() <- spinlock created > * MiniportInit() <- NIC initialized > * MiniportHalt() <- NIC halted, spinlock destroyed > * MiniportInit() <- NIC initialized, spinlock not recreated > * NdisAcquireSpinLock(boguslock) <- panic > * To work around this, we poison the spinlock on destroy, and > * if we try to re-acquire the poison pill^Wspinlock, we init > * it again so subsequent calls will work. > * > * Drivers that behave in this way are likely not officially > * certified by Microsoft, since their I would expect the > * Microsoft NDIS test tool to catch mistakes like this. As jhb suggested the other day, why not use mtx_pool_alloc()? Since you are using mutexes with different options that the mtxpool_sleep mutex pool, you'll have to create your own pool with mtx_pool_create(). Just call mtx_pool_create() with the appropriate arguments in ndis_libinit(), and call mtx_pool_destroy() in ndis_libfini(). Instead of explicitly allocating a mutex in ndis_create_lock(), just call mtx_pool_alloc() and save the pointer that it returns. You don't have to do anything special when you are done with the mutex other than to make sure that it is unlocked before you abandon it. The only restriction on on mutex pools is that should should not grab any other mutexes while holding lock on a mutex from a pool.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200401090855.i098t87E021981>