From owner-freebsd-current@FreeBSD.ORG Fri Aug 5 08:18:57 2011 Return-Path: Delivered-To: current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1968F106566B; Fri, 5 Aug 2011 08:18:57 +0000 (UTC) (envelope-from mm@FreeBSD.org) Received: from mail.vx.sk (mail.vx.sk [IPv6:2a01:4f8:100:1043::3]) by mx1.freebsd.org (Postfix) with ESMTP id 52D758FC13; Fri, 5 Aug 2011 08:18:56 +0000 (UTC) Received: from core.vx.sk (localhost [127.0.0.1]) by mail.vx.sk (Postfix) with ESMTP id A1F2E18C9FD; Fri, 5 Aug 2011 10:18:54 +0200 (CEST) X-Virus-Scanned: amavisd-new at mail.vx.sk Received: from mail.vx.sk ([127.0.0.1]) by core.vx.sk (mail.vx.sk [127.0.0.1]) (amavisd-new, port 10024) with LMTP id sO8yCNBB0zq7; Fri, 5 Aug 2011 10:18:49 +0200 (CEST) Received: from [10.0.3.3] (188-167-66-148.dynamic.chello.sk [188.167.66.148]) by mail.vx.sk (Postfix) with ESMTPSA id 9F09D18C9EB; Fri, 5 Aug 2011 10:18:49 +0200 (CEST) Message-ID: <4E3BA769.3090408@FreeBSD.org> Date: Fri, 05 Aug 2011 10:18:49 +0200 From: Martin Matuska User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20110627 Thunderbird/5.0 MIME-Version: 1.0 To: Kostik Belousov References: <20110804125454.GI29364@albert.catwhisker.org> <20110804135932.GG17489@deviant.kiev.zoral.com.ua> In-Reply-To: <20110804135932.GG17489@deviant.kiev.zoral.com.ua> X-Enigmail-Version: 1.2pre Content-Type: multipart/mixed; boundary="------------050805060602090007070309" Cc: Xin Li , current@FreeBSD.org Subject: Re: panic: share -> excl @r224632 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Aug 2011 08:18:57 -0000 This is a multi-part message in MIME format. --------------050805060602090007070309 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit I agree to Kostik's approach, but I suggest implementing it in a separate function and also use for the unmount() part. Please review attached patch. On 04.08.2011 15:59, Kostik Belousov wrote: > On Thu, Aug 04, 2011 at 05:54:54AM -0700, David Wolfskill wrote: >> I've only seen this on my laptop; my build machine doesn't exhibit the >> panic. >> >> r224602 is the most recent point I've built that does not exhibit the >> panic at all. >> >> The first few lines (hand-transcribed; I have no serial console on this >> laptop -- the one shortcoming it has): >> >> shared lock of (lockmgr) devfs @/usr/src/sys/kern/vfs_cache.c:1116 >> while exclusively locked from /usr/src/sys/kern/vfs_subr.c:2134 >> panic: share -> excl >> >> The backtrace (which I wasn't patient enough to trto transcribe twice; >> sorry) appeared to involve "nmount", and the panic occurred directly >> after mounting the file systems during the transition from single-user >> mode to multi-user mode. >> >> The output from "svn update" going from r224602 -> r224632 shows the >> following files being updated: >> >> U usr.sbin/faithd/faithd.8 >> U usr.sbin/jail/jail.8 >> U lib/libproc/proc_create.c >> U sys/arm/arm/irq_dispatch.S >> U sys/arm/sa11x0/sa11x0_irq.S >> U sys/powerpc/booke/locore.S >> U sys/powerpc/booke/platform_bare.c >> U sys/powerpc/booke/pmap.c >> U sys/nfsclient/nfsnode.h >> U sys/nfsclient/nfs_node.c >> U sys/kern/kern_jail.c >> U sys/kern/vfs_mount.c >> U sys/fs/nfsclient/nfsnode.h >> U sys/fs/nfsclient/nfs_clnode.c >> U sys/mips/mips/exception.S >> U sys/dev/ahci/ahci.c >> U sys/dev/ata/chipsets/ata-nvidia.c >> U sys/dev/ath/ath_hal/ah_eeprom_v4k.c >> U sys/i386/ibcs2/imgact_coff.c >> U sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c >> Updated to revision 224632. >> >> Updating to r224648 (this morning) has had no apparent effect on the >> cited panic: it occurs in the same way, at the same time, accompanied by >> the same messages (citing the same files and lines). >> >> I've attached dmesg.boot from r224602. >> >> I will see if I can find a commit that affected at least one of the >> affected files in the above list that I can revert to avoid the panic, >> but I'm a bit slow for a while yet, so I figured I'd finally get around >> to posting this, in the hope that someone cleverer than me will spot >> the problem and suggest a circumvention to try. >> >> And I'm quite willing to try such things. >> >> Note: this is FreeBSD/i386; nothing special: no jails; running on real >> hardware; no attempts to use ZFS.... > I am sure that this is caused by r224614. > I forgot that vn_fullpath cannot operate on the locked vnode. > > Try this. > > diff --git a/sys/kern/vfs_mount.c b/sys/kern/vfs_mount.c > index d601c56..54f123c 100644 > --- a/sys/kern/vfs_mount.c > +++ b/sys/kern/vfs_mount.c > @@ -746,13 +746,15 @@ vfs_domount_first( > struct thread *td, /* Calling thread. */ > struct vfsconf *vfsp, /* File system type. */ > struct vnode *vp, /* Vnode to be covered. */ > + char *ufspath, > int fsflags, /* Flags common to all filesystems. */ > struct vfsoptlist **optlist /* Options local to the filesystem. */ > ) > { > struct vattr va; > + struct nameidata nd; > struct mount *mp; > - struct vnode *newdp; > + struct vnode *newdp, *vp1; > char *fspath, *fbuf; > int error; > > @@ -761,16 +763,45 @@ vfs_domount_first( > KASSERT((fsflags & MNT_UPDATE) == 0, ("MNT_UPDATE shouldn't be here")); > > /* Construct global filesystem path from vp. */ > + VOP_UNLOCK(vp, 0); > error = vn_fullpath_global(td, vp, &fspath, &fbuf); > if (error != 0) { > - vput(vp); > + vrele(vp); > return (error); > } > if (strlen(fspath) >= MNAMELEN) { > - vput(vp); > + vrele(vp); > free(fbuf, M_TEMP); > return (ENAMETOOLONG); > } > + if ((vp->v_iflag & VI_DOOMED) != 0) { > + vrele(vp); > + free(fbuf, M_TEMP); > + return (EBADF); > + } > + > + /* > + * Re-lookup the vnode by path. As a side effect, the vnode is > + * relocked. If vnode was renamed, return ENOENT. > + */ > + NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF | MPSAFE | AUDITVNODE1, > + UIO_SYSSPACE, ufspath, td); > + error = namei(&nd); > + if (error != 0) { > + vrele(vp); > + free(fbuf, M_TEMP); > + return (error); > + } > + if (NDHASGIANT(&nd)) > + mtx_unlock(&Giant); > + NDFREE(&nd, NDF_ONLY_PNBUF); > + vp1 = nd.ni_vp; > + vrele(vp); > + if (vp1 != vp) { > + vput(vp1); > + free(fbuf, M_TEMP); > + return (ENOENT); > + } > > /* > * If the user is not root, ensure that they own the directory > @@ -1084,14 +1115,11 @@ vfs_domount( > NDFREE(&nd, NDF_ONLY_PNBUF); > vp = nd.ni_vp; > if ((fsflags & MNT_UPDATE) == 0) > - error = vfs_domount_first(td, vfsp, vp, fsflags, optlist); > + error = vfs_domount_first(td, vfsp, vp, fspath, fsflags, optlist); > else > error = vfs_domount_update(td, vp, fsflags, optlist); > mtx_unlock(&Giant); > > - ASSERT_VI_UNLOCKED(vp, __func__); > - ASSERT_VOP_UNLOCKED(vp, __func__); > - > return (error); > } > -- Martin Matuska FreeBSD committer http://blog.vx.sk --------------050805060602090007070309 Content-Type: text/x-patch; name="vfs_mount.c.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="vfs_mount.c.patch" Index: sys/kern/vfs_mount.c =================================================================== --- sys/kern/vfs_mount.c (revision 224654) +++ sys/kern/vfs_mount.c (working copy) @@ -362,6 +362,64 @@ vfs_mergeopts(struct vfsoptlist *toopts, struct vf } /* + * Verify vnode's global path + */ +static int +vfs_verify_global_path(struct thread *td, struct vnode *vp, char *fspath) +{ + struct nameidata nd; + struct vnode *vp1; + char *rpath, *fbuf; + int error; + + ASSERT_VOP_ELOCKED(vp, __func__); + + /* Construct global filesystem path from vp. */ + VOP_UNLOCK(vp, 0); + error = vn_fullpath_global(td, vp, &rpath, &fbuf); + if (error != 0) { + vrele(vp); + return (error); + } + if (strlen(rpath) >= MNAMELEN) { + vrele(vp); + free(fbuf, M_TEMP); + return (ENAMETOOLONG); + } + if ((vp->v_iflag & VI_DOOMED) != 0) { + vrele(vp); + free(fbuf, M_TEMP); + return (EBADF); + } + + /* + * Re-lookup the vnode by path. As a side effect, the vnode is + * relocked. If vnode was renamed, return ENOENT. + */ + NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF | MPSAFE | AUDITVNODE1, + UIO_SYSSPACE, fspath, td); + error = namei(&nd); + if (error != 0) { + vrele(vp); + free(fbuf, M_TEMP); + return (error); + } + NDFREE(&nd, NDF_ONLY_PNBUF); + vp1 = nd.ni_vp; + vrele(vp); + if (vp1 != vp) { + vput(vp1); + free(fbuf, M_TEMP); + return (ENOENT); + } + + strlcpy(fspath,rpath,MNAMELEN); + free(fbuf, M_TEMP); + + return (error); +} + +/* * Mount a filesystem. */ int @@ -745,6 +803,7 @@ static int vfs_domount_first( struct thread *td, /* Calling thread. */ struct vfsconf *vfsp, /* File system type. */ + char *fspath, /* Mount path. */ struct vnode *vp, /* Vnode to be covered. */ int fsflags, /* Flags common to all filesystems. */ struct vfsoptlist **optlist /* Options local to the filesystem. */ @@ -753,25 +812,12 @@ vfs_domount_first( struct vattr va; struct mount *mp; struct vnode *newdp; - char *fspath, *fbuf; int error; mtx_assert(&Giant, MA_OWNED); ASSERT_VOP_ELOCKED(vp, __func__); KASSERT((fsflags & MNT_UPDATE) == 0, ("MNT_UPDATE shouldn't be here")); - /* Construct global filesystem path from vp. */ - error = vn_fullpath_global(td, vp, &fspath, &fbuf); - if (error != 0) { - vput(vp); - return (error); - } - if (strlen(fspath) >= MNAMELEN) { - vput(vp); - free(fbuf, M_TEMP); - return (ENAMETOOLONG); - } - /* * If the user is not root, ensure that they own the directory * onto which we are attempting to mount. @@ -793,14 +839,12 @@ vfs_domount_first( } if (error != 0) { vput(vp); - free(fbuf, M_TEMP); return (error); } VOP_UNLOCK(vp, 0); /* Allocate and initialize the filesystem. */ mp = vfs_mount_alloc(vp, vfsp, fspath, td->td_ucred); - free(fbuf, M_TEMP); /* XXXMAC: pass to vfs_mount_alloc? */ mp->mnt_optnew = *optlist; /* Set the mount level flags. */ @@ -1083,15 +1127,15 @@ vfs_domount( mtx_lock(&Giant); NDFREE(&nd, NDF_ONLY_PNBUF); vp = nd.ni_vp; - if ((fsflags & MNT_UPDATE) == 0) - error = vfs_domount_first(td, vfsp, vp, fsflags, optlist); - else + if ((fsflags & MNT_UPDATE) == 0) { + error = vfs_verify_global_path(td, vp, fspath); + if (error == 0) + error = vfs_domount_first(td, vfsp, fspath, vp, + fsflags, optlist); + } else error = vfs_domount_update(td, vp, fsflags, optlist); mtx_unlock(&Giant); - ASSERT_VI_UNLOCKED(vp, __func__); - ASSERT_VOP_UNLOCKED(vp, __func__); - return (error); } @@ -1118,7 +1162,7 @@ unmount(td, uap) { struct mount *mp; struct nameidata nd; - char *pathbuf, *rpathbuf, *fbuf; + char *pathbuf; int error, id0, id1; AUDIT_ARG_VALUE(uap->flags); @@ -1164,15 +1208,10 @@ unmount(td, uap) UIO_SYSSPACE, pathbuf, td); if (namei(&nd) == 0) { NDFREE(&nd, NDF_ONLY_PNBUF); - if (vn_fullpath_global(td, nd.ni_vp, &rpathbuf, - &fbuf) == 0) { - if (strlen(rpathbuf) < MNAMELEN) { - strlcpy(pathbuf, rpathbuf, - MNAMELEN); - } - free(fbuf, M_TEMP); - } - vput(nd.ni_vp); + error = vfs_verify_global_path(td, nd.ni_vp, + pathbuf); + if (error == 0) + vput(nd.ni_vp); } } mtx_lock(&mountlist_mtx); --------------050805060602090007070309--