From owner-freebsd-geom@FreeBSD.ORG Tue Jun 19 18:24:49 2012 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 5E3441065670 for ; Tue, 19 Jun 2012 18:24:49 +0000 (UTC) (envelope-from steven.haber@isilon.com) Received: from mexforwardwc.lss.emc.com (mexforwardwc.lss.emc.com [137.69.117.200]) by mx1.freebsd.org (Postfix) with ESMTP id 341338FC0A for ; Tue, 19 Jun 2012 18:24:49 +0000 (UTC) Received: from scl02-01d02-si01.isus.emc.com (scl02-01d02-si01.isus.emc.com [137.69.225.84]) by mexforwardwc.lss.emc.com (Switch-3.4.3/Switch-3.4.3) with ESMTP id q5JIOgC0014008 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 19 Jun 2012 11:24:42 -0700 Received: from mailhubwc.lss.emc.com (mailhubscprd03.lss.emc.com [137.69.224.145]) by scl02-01d02-si01.isus.emc.com (RSA Interceptor); Tue, 19 Jun 2012 11:24:23 -0700 Received: from seacasht01.desktop.isilon.com (seacasht01.isilon.com [137.69.159.79]) by mailhubwc.lss.emc.com (Switch-3.4.3/Switch-3.4.3) with ESMTP id q5JIOLdv002451; Tue, 19 Jun 2012 11:24:21 -0700 Received: from seaxch01.isilon.com (137.69.158.60) by SEACASHT01.desktop.isilon.com (137.69.159.79) with Microsoft SMTP Server id 14.2.298.4; Tue, 19 Jun 2012 11:24:21 -0700 X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-Class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Date: Tue, 19 Jun 2012 11:24:03 -0700 Message-ID: <56CE86D6660FF84498426FA7A33170B403589417@seaxch01.desktop.isilon.com> In-Reply-To: <20120611225304.GK2337@deviant.kiev.zoral.com.ua> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: Geom / destroy_dev() deadlock Thread-Index: Ac1IJQoi3iwR12mRToa/jZtMD4FdHgGHgpnQ 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> From: Steven Haber To: Konstantin Belousov X-EMM-MHVC: 1 Cc: freebsd-geom@freebsd.org Subject: RE: Geom / destroy_dev() deadlock X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jun 2012 18:24:49 -0000 > 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