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:13:02PM -0500, Andrew Gallatin wrote:
 > > Luigi Rizzo [luigi at FreeBSD.org] wrote:
 > > 
 > > >   Cleanup and document the implementation of firmware(9) based on
 > > >   a version that i posted earlier on the -current mailing list,
 > > >   and subsequent feedback received.
 > > >   
 > > 
 > > At least for me, firmware(9) has been broken ever since the kld_mtx
 > > was replaced with an sx lock last June.  The problem is that there is
 > > an exclusive lock of kld_sx taken when loading a driver, and then
 > > firmware_get() triggers another xlock of it, leading to a deadlock:
 > ...
 > > 
 > > I've been using a patch
 > > (http://people.freebsd.org/~gallatin/firmware_sx_recurse.diff)
 > > which works around the problem.  Do you think it would be 
 > > possible to commit this?
 > 
 > i suppose it is ok... "iwi" uses a similar technique to avoid

Would you mind committing it for me?  I honestly don't feel
comfortable touching anything in the core kernel, especially locking.
I've tried the patch under WITNESS, and it doesn't complain, but I
fully admit that I don't really understand the locking requirements of
the linker.

BTW, I'm somewhat surprised nobody else has complained about this.
I see that iwi loads its firmware from an ioctl context, but at
least isp seems to load its firmware from attach.  I guess not
many people load scsi drivers after the system is up..

 > recursive locking. I wonder how common is this practice, and whether
 > it makes sense to define some standard macros to implement this.

I'm not sure.. In some cases, recursive locking is OK (just icky), and
in other cases you really want to catch it.  

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.

Drew