From owner-svn-src-head@FreeBSD.ORG Mon Dec 1 19:32:34 2008 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B41A21065670; Mon, 1 Dec 2008 19:32:34 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 400AC8FC1E; Mon, 1 Dec 2008 19:32:34 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from localhost.corp.yahoo.com (john@localhost [IPv6:::1]) (authenticated bits=0) by server.baldwin.cx (8.14.3/8.14.3) with ESMTP id mB1JW7SW099801; Mon, 1 Dec 2008 14:32:28 -0500 (EST) (envelope-from jhb@freebsd.org) From: John Baldwin To: Konstantin Belousov Date: Mon, 1 Dec 2008 13:36:36 -0500 User-Agent: KMail/1.9.7 References: <200811221311.mAMDBBU8018510@svn.freebsd.org> In-Reply-To: <200811221311.mAMDBBU8018510@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200812011336.37636.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [IPv6:::1]); Mon, 01 Dec 2008 14:32:28 -0500 (EST) X-Virus-Scanned: ClamAV 0.93.1/8704/Mon Dec 1 11:39:36 2008 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00,NO_RELAYS autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r185170 - head/sys/ufs/ufs X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Dec 2008 19:32:34 -0000 On Saturday 22 November 2008 08:11:11 am Konstantin Belousov wrote: > Author: kib > Date: Sat Nov 22 13:11:11 2008 > New Revision: 185170 > URL: http://svn.freebsd.org/changeset/base/185170 > > Log: > Busy ufs filesystem around block of code that does ".." lookup. Since > mnt_lock is before lock of any vnode on the mp, it uses LK_NOWAIT. Since > MNTK_UNMOUNT may be transient, pdp lock is dropped when vfs_busy() > failed, and operation is retried after some time. This way, ffs_vget() > is not called on the mp that may be in the process of being destroyed by > unmount. > > Check for the VI_DOOMED flag on pdp after its lock is reacquired, to > better detect some situations where directory containing ".." > entry is removed during the lookup. I'm not really sure it matters if the parent directory goes away because it will have deadfs vops so any subsequent operations will already fail, yes? Also, do you really need to grab the VI_LOCK just to check VI_DOOMED? Other places in the kernel check that flag while holding the vnode lock w/o acquiring the interlock. Since you are just doing a single atomic read the interlock doesn't actually close any races anyway. I think it just adds overhead. > Reviewed by: tegge, attilio (previous version) > Tested by: pho > MFC after: 1 month > > Modified: > head/sys/ufs/ufs/ufs_lookup.c > > Modified: head/sys/ufs/ufs/ufs_lookup.c > ============================================================================== > --- head/sys/ufs/ufs/ufs_lookup.c Sat Nov 22 12:36:15 2008 (r185169) > +++ head/sys/ufs/ufs/ufs_lookup.c Sat Nov 22 13:11:11 2008 (r185170) > @@ -157,6 +157,8 @@ ufs_lookup(ap) > int nameiop = cnp->cn_nameiop; > ino_t ino; > int ltype; > + int pdoomed; > + struct mount *mp; > > bp = NULL; > slotoffset = -1; > @@ -578,9 +580,32 @@ found: > pdp = vdp; > if (flags & ISDOTDOT) { > ltype = VOP_ISLOCKED(pdp); > + mp = pdp->v_mount; > + for (;;) { > + error = vfs_busy(mp, MBF_NOWAIT); > + if (error == 0) > + break; > + VOP_UNLOCK(pdp, 0); > + pause("ufs_dd", 1); > + vn_lock(pdp, ltype | LK_RETRY); > + VI_LOCK(pdp); > + pdoomed = pdp->v_iflag & VI_DOOMED; > + VI_UNLOCK(pdp); > + if (pdoomed) > + return (ENOENT); > + } > VOP_UNLOCK(pdp, 0); /* race to get the inode */ > - error = VFS_VGET(pdp->v_mount, ino, cnp->cn_lkflags, &tdp); > + error = VFS_VGET(mp, ino, cnp->cn_lkflags, &tdp); > + vfs_unbusy(mp); > vn_lock(pdp, ltype | LK_RETRY); > + VI_LOCK(pdp); > + pdoomed = pdp->v_iflag & VI_DOOMED; > + VI_UNLOCK(pdp); > + if (pdoomed) { > + if (error == 0) > + vput(tdp); > + error = ENOENT; > + } > if (error) > return (error); > *vpp = tdp; > -- John Baldwin