On Mon, Jun 11, 2012 at 02:15:56PM -0700, Steven Haber wrote: > > On Mon, Jun 11, 2012 at 10:19:07AM -0700, Steven Haber wrote: > >> Hey FreeBSD devs, > >> > >> I hope this is the right forum for this. I haven't used the FreeBSD > >> mailing lists before. I'm a relatively new hire at EMC Isilon. We are > >> using FreeBSD 7.0 as the basis for our scale-out NAS product line. We > >> are frequently hitting the deadlock described here: > >> > >> > http://lists.freebsd.org/pipermail/freebsd-geom/2010-February/003888.htm > >> l > >> > >> The race is that destroy_dev() sleeps indefinitely waiting for thread > >> references to drop from a dev. In the case of geom, we hold the > topology > >> lock the whole time we're in the dev layer. In devfs_open() and > >> devfs_close(), we take a ref on the dev before calling into to geom. > >> > >> The proposed solution of destroy_dev_sched() makes sense to me. I am > >> trying to understand the necessity of Alexander Motin's additional > >> changes. destroy_dev_tq() holds the devmtx during the destroy and > devfs > >> uses this lock to take refs before calling into geom. To me it seems > we > >> should be able to protect the cdev with just the devmtx, provided > >> consumers of d_close(), d_open(), etc. ref and rel the dev > >> appropriately. > > > > From: Konstantin Belousov > > Sent: Monday, June 11, 2012 1:44 PM > > To: Steven Haber > > Cc: freebsd-geom@freebsd.org > > Subject: Re: Geom / destroy_dev() deadlock > > > devmtx is mutex. If taken, then cdevsw methods would be unable to > sleep. > > To clarify, I'm not suggesting additional locking with devmtx. Currently > we use the devmtx to take thread references on the dev. destroy_devl() > sleeps until there are no references. I don't see why it matters if we a > foreground destroy (current code, destroy_dev()) or scheduled destroy > (using destroy_dev_sched()) since the devmtx combined with thread refs > should guarantee we are never in the cdevsw methods during a destroy. I > think that the single routine change is sufficient, and I can't seem to > cause any sort of crash on my system with that change in place.
I do not understand what you are proposing. Could you, please, show the patch ?
pgpTBAdW5oQ9b.pgp
Description: PGP signature