Date: Wed, 21 Feb 2007 12:50:44 -0500 (EST) From: Andrew Gallatin <gallatin@cs.duke.edu> To: Luigi Rizzo <rizzo@icir.org> Cc: cvs-src@FreeBSD.org, Luigi Rizzo <luigi@FreeBSD.org>, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: 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 Message-ID: <17884.34420.308021.423716@grasshopper.cs.duke.edu> In-Reply-To: <20070221092332.A90766@xorpc.icir.org> References: <200702151721.l1FHLWno019525@repoman.freebsd.org> <20070221121302.A20229@grasshopper.cs.duke.edu> <20070221092332.A90766@xorpc.icir.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Luigi Rizzo writes: > On Wed, Feb 21, 2007 at 12:13:02PM -0500, Andrew Gallatin wrote: > > Luigi Rizzo [luigi@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?17884.34420.308021.423716>