Date: Sat, 13 Jan 2007 23:34:01 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: hselasky@c2i.net Cc: perforce@freebsd.org Subject: Re: PERFORCE change 112835 for review Message-ID: <20070113.233401.228973006.imp@bsdimp.com> In-Reply-To: <200701140225.05299.hselasky@c2i.net> References: <200701122132.l0CLWuAG096027@repoman.freebsd.org> <20070113.122940.420518338.imp@bsdimp.com> <200701140225.05299.hselasky@c2i.net>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <200701140225.05299.hselasky@c2i.net> Hans Petter Selasky <hselasky@c2i.net> writes: : On Saturday 13 January 2007 20:29, M. Warner Losh wrote: : > In message: <200701122132.l0CLWuAG096027@repoman.freebsd.org> : > : > Hans Petter Selasky <hselasky@freebsd.org> writes: : > : http://perforce.freebsd.org/chv.cgi?CH=112835 : > : : > : Change 112835 by hselasky@hselasky_mini_itx on 2007/01/12 21:32:24 : > : : > : ether_ifattach() must be called without any mutexes held, hence : > : it can sleep. Use generated typedef's for prototypes for exported : > : device methods. This prevents invalid parameter passing and return. : > : > Why do the attach methods even have locked mutexes to drop? That was : > a common error in the early locking efforts that too many network : > drivers inherited. Maybe the right fix is to just remove the : > lock/unlock pairs in attach entirely... : > : : The attach code in question is run from a separate config thread, which by : default locks "sc->sc_mtx" before calling the callback function. Therefore : the code unlocks that lock, to avoid sleeping with mutex locked errors. I understand why mtx can't be held, but there's nothing to contend with... : Probably all of the attach code can run without "sc->sc_mtx" locked, but will : require Giant locked, when changing the device tree, for example when adding : new child devices like MII-bus instances. Giant will be locked when attach is called, but not in the deferred attach. So long as attach returns, this shouldn't be a problem. Adding children does need Giant held at the moment. Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070113.233401.228973006.imp>