From owner-cvs-all@FreeBSD.ORG Fri Jan 9 00:55:21 2004 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 4876116A4CE; Fri, 9 Jan 2004 00:55:21 -0800 (PST) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7FF4D43D5A; Fri, 9 Jan 2004 00:55:19 -0800 (PST) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.12.9p2/8.12.9) with ESMTP id i098t87E021981; Fri, 9 Jan 2004 00:55:12 -0800 (PST) (envelope-from truckman@FreeBSD.org) Message-Id: <200401090855.i098t87E021981@gw.catspoiler.org> Date: Fri, 9 Jan 2004 00:55:08 -0800 (PST) From: Don Lewis To: wpaul@FreeBSD.org In-Reply-To: <20040109074939.28CEC16A4D0@hub.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii cc: cvs-src@FreeBSD.org cc: alfred@FreeBSD.org cc: cvs-all@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 X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Jan 2004 08:55:21 -0000 On 8 Jan, Bill Paul wrote: >> * Bill Paul [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.