Date: Mon, 11 Jun 2012 14:15:56 -0700 From: Steven Haber <steven.haber@isilon.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-geom@freebsd.org Subject: RE: Geom / destroy_dev() deadlock Message-ID: <56CE86D6660FF84498426FA7A33170B403367535@seaxch01.desktop.isilon.com> In-Reply-To: <20120611204334.GH2337@deviant.kiev.zoral.com.ua> References: <56CE86D6660FF84498426FA7A33170B4033672EF@seaxch01.desktop.isilon.com> <20120611204334.GH2337@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Mon, Jun 11, 2012 at 10:19:07AM -0700, Steven Haber wrote: >> Hey FreeBSD devs, >> =20 >> 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: >>=20 >> http://lists.freebsd.org/pipermail/freebsd-geom/2010-February/003888.htm >> l >>=20 >> 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. >>=20 >> 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. Steven
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?56CE86D6660FF84498426FA7A33170B403367535>