From owner-freebsd-stable@FreeBSD.ORG  Fri Apr 16 20:23:19 2010
Return-Path: <owner-freebsd-stable@FreeBSD.ORG>
Delivered-To: freebsd-stable@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 9EA50106566B
	for <freebsd-stable@freebsd.org>; Fri, 16 Apr 2010 20:23:19 +0000 (UTC)
	(envelope-from matthew.fleming@isilon.com)
Received: from seaxch09.isilon.com (seaxch09.isilon.com [74.85.160.25])
	by mx1.freebsd.org (Postfix) with ESMTP id 875708FC1F
	for <freebsd-stable@freebsd.org>; Fri, 16 Apr 2010 20:23:19 +0000 (UTC)
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: Fri, 16 Apr 2010 13:23:17 -0700
Message-ID: <06D5F9F6F655AD4C92E28B662F7F853E039387EF@seaxch09.desktop.isilon.com>
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
Thread-Topic: panic in vget()
Thread-Index: AcrdoqZKbnq8hZ9/RUGbRk6ZTGDcfg==
From: "Matthew Fleming" <matthew.fleming@isilon.com>
To: <freebsd-stable@freebsd.org>
Subject: panic in vget()
X-BeenThere: freebsd-stable@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Production branch of FreeBSD source code <freebsd-stable.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-stable>, 
	<mailto:freebsd-stable-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-stable>
List-Post: <mailto:freebsd-stable@freebsd.org>
List-Help: <mailto:freebsd-stable-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-stable>,
	<mailto:freebsd-stable-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 16 Apr 2010 20:23:19 -0000

I'm looking at this panic in vget() on stable/7:

	if (vp->v_iflag & VI_DOOMED && (flags & LK_RETRY) =3D=3D 0)
		panic("vget: vn_lock failed to return ENOENT\n");

It seems to me that this is not a correct assertion, because if the
caller passed in no lock flags (i.e. just checking the vnode for
validity) then there is a window between the VI_UNLOCK() in _vn_lock(9)
and the subsequent VI_LOCK() in vget() where another thread could have
set VI_DOOMED.

This isn't a problem on CURRENT because the code has been changed to not
allow an empty lock flags.

I believe the following is a potential fix is:

 	vholdl(vp);
 	if ((error =3D vn_lock(vp, flags | LK_INTERLOCK, td)) !=3D 0) {
 		vdrop(vp);
 		return (error);
 	}
 	VI_LOCK(vp);
+	/*
+	 * Deal with a timing window when the interlock is not held
+	 * and VI_DOOMED can be set, since we only have a holdcnt,
+	 * not a usecount.
+	 */
+	if (vp->v_iflag & VI_DOOMED && (flags & LK_RETRY) =3D=3D 0) {
+		KASSERT((flags & LK_TYPE_MASK) =3D=3D 0, ("Unexpected flags
%x", flags));
+		vdropl(vp);
+		return (ENOENT);
+	}
 	/* Upgrade our holdcnt to a usecount. */
 	v_upgrade_usecount(vp);
-	if (vp->v_iflag & VI_DOOMED && (flags & LK_RETRY) =3D=3D 0)
-		panic("vget: vn_lock failed to return ENOENT\n");
 	if (oweinact) {
 		if (vp->v_iflag & VI_OWEINACT)
 			vinactive(vp, td);
 		VI_UNLOCK(vp);
 		if ((oldflags & LK_TYPE_MASK) =3D=3D 0)

Thanks,
matthew