cvs commit: src/sys/arm/xscale/ixp425 ixp425_npe.c src/sys/dev/ipw if_ipw.c if_ipwvar.h src/sys/dev/isp isp_freebsd.h src/sys/dev/iwi if_iwi.c if_iwivar.h src/sys/dev/mxge if_mxge.c src/sys/kern subr_firmware.c src/sys/sys firmware.h src/sys/tools fw_stub.awk

[Andrew Gallatin]

Luigi Rizzo writes:
 > On Wed, Feb 21, 2007 at 12:50:44PM -0500, Andrew Gallatin wrote:
 > ...
 > > Speaking of icky, I'm really worried about the pedantic nature of
 > > FreeBSD's WITNESS and "never, ever sleep while holding a mutex" dogma
 > > causing as many problems as it solves.  For example, iwi seems to
 > > simply drop its lock when loading firmware (presumably to avoid
 > > WITNESS squawking about holding a mutex while grabbing an sx).  This
 > > certainly shuts up WITNESS, but it may also introduce a race,
 > > especially if the linker needs to sleep and wait for the disk.
 > 
 > i am not sure i follow you here...
 > Of course when you drop the lock you risk that the underlying
 > data structure is manipulated (or in the worst case freed),
 > but usually you can avoid this with something like
 > 
 > 	<while locked>
 > 	sc->flags |= LEAVE_ME_ALONE
 > 	UNLOCK

Sorry, I hadn't noticed that iwi set a flag like that.  I was
searching for IWI_UNLOCK(), and it was off the screen :). I've used
hacks like that myself in the past.  But still, it looks like it only
protects the fw loading, and if I knew the driver well enough, I could
probably find a scenario where something else protected by the lock
races with a firmware load.

I just think it would be safer, and less hacky to be allowed to hold
a driver mutex while potentially sleeping in the firmware code (and in
similar scenarios).  I tried using an sx lock in one of my drivers for
this reason, and eventually got burned when I added multicast support,
because something called my if_ioctl with a mutex held.

Drew