Date: Tue, 19 Jun 2012 11:24:03 -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: <56CE86D6660FF84498426FA7A33170B403589417@seaxch01.desktop.isilon.com> In-Reply-To: <20120611225304.GK2337@deviant.kiev.zoral.com.ua> References: <56CE86D6660FF84498426FA7A33170B4033672EF@seaxch01.desktop.isilon.com> <20120611204334.GH2337@deviant.kiev.zoral.com.ua> <56CE86D6660FF84498426FA7A33170B403367535@seaxch01.desktop.isilon.com> <20120611215610.GJ2337@deviant.kiev.zoral.com.ua> <56CE86D6660FF84498426FA7A33170B403429115@seaxch01.desktop.isilon.com> <20120611225304.GK2337@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Mon, Jun 11, 2012 at 03:27:39PM -0700, Steven Haber wrote: > > I do not understand what you are proposing. Could you, please, show > > > the patch ? > >=20 > > --- > > src/sys/geom/geom_dev.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > >=20 > > diff --git a/src/sys/geom/geom_dev.c b/src/sys/geom/geom_dev.c > > index 38251e1..787235a 100644 > > --- a/src/sys/geom/geom_dev.c > > +++ b/src/sys/geom/geom_dev.c > > @@ -497,7 +497,7 @@ g_dev_orphan(struct g_consumer *cp) > > =20 > > /* Destroy the struct cdev *so we get no more requests */ > > unit =3D dev2unit(dev); > > - destroy_dev(dev); > > + destroy_dev_sched(dev); > > free_unr(unithdr, unit); > > =20 > > /* Wait for the cows to come home */ > > From: Konstantin Belousov > Sent: Monday, June 11, 2012 3:53 PM > To: Steven Haber > Cc: freebsd-geom@freebsd.org > Subject: Re: Geom / destroy_dev() deadlock > > Did you noted the comment above the block you changing ? > The patch would cause races allowing arbitrary kernel memory corruption. > > The moment when the cdev is destroyed is somewhere in future, while > structures that the cdev reference are freed synchronously. > > I tried to put some safety measures into destroy_dev_sched(9), namely > CDP_SCHED_DTR flag that somewhat reduces the possibility of usermode > accessing cdev after destroy_dev_sched(), but this cannot be eliminated > entirely. So destroy_dev_sched() is inherently racey. That explains why there aren't many examples of usage in the kernel. Without doing a scheduled destroy, can you think of any way to prevent the devdrn/geom deadlock? From the original discussion: GEOM calls destroy_dev() while holding the topology lock. Destroy_dev() wants to destroy device, but can't because there are threads that still have it open. The threads can't close it, because to close it they need the topology lock. There may be additional locking that can be done in the dev layer, like a third sleepable lock. We could also set a devdrn flag on the cdev to cause g_dev calls to return with an error prior to taking the topology lock. Steven
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?56CE86D6660FF84498426FA7A33170B403589417>