Date: Mon, 19 Nov 2012 21:53:48 +0100 From: Davide Italiano <davide@freebsd.org> To: Attilio Rao <attilio@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r243307 - head/sys/kern Message-ID: <CACYV=-Hya1-V_RNToWHDD_LFqxEcJYovUjnp0P9b-Q8Hzm3t_w@mail.gmail.com> In-Reply-To: <201211192043.qAJKhJ9i038016@svn.freebsd.org> References: <201211192043.qAJKhJ9i038016@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Nov 19, 2012 at 9:43 PM, Attilio Rao <attilio@freebsd.org> wrote: > Author: attilio > Date: Mon Nov 19 20:43:19 2012 > New Revision: 243307 > URL: http://svnweb.freebsd.org/changeset/base/243307 > > Log: > insmntque() is always called with the lock held in exclusive mode, > then: > - assume the lock is held in exclusive mode and remove a moot check > about the lock acquisition. > - in the destructor remove !MPSAFE specific chunk. > > Reviewed by: kib > MFC after: 2 weeks > > Modified: > head/sys/kern/vfs_subr.c > > Modified: head/sys/kern/vfs_subr.c > ============================================================================== > --- head/sys/kern/vfs_subr.c Mon Nov 19 19:31:55 2012 (r243306) > +++ head/sys/kern/vfs_subr.c Mon Nov 19 20:43:19 2012 (r243307) > @@ -1111,10 +1111,6 @@ insmntque_stddtr(struct vnode *vp, void > > vp->v_data = NULL; > vp->v_op = &dead_vnodeops; > - /* XXX non mp-safe fs may still call insmntque with vnode > - unlocked */ > - if (!VOP_ISLOCKED(vp)) > - vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); > vgone(vp); > vput(vp); > } > @@ -1126,7 +1122,6 @@ int > insmntque1(struct vnode *vp, struct mount *mp, > void (*dtr)(struct vnode *, void *), void *dtr_arg) > { > - int locked; > > KASSERT(vp->v_mount == NULL, > ("insmntque: vnode already on per mount vnode list")); > @@ -1144,18 +1139,15 @@ insmntque1(struct vnode *vp, struct moun > */ > MNT_ILOCK(mp); > VI_LOCK(vp); > - if ((mp->mnt_kern_flag & MNTK_NOINSMNTQ) != 0 && > + if (((mp->mnt_kern_flag & MNTK_NOINSMNTQ) != 0 && > ((mp->mnt_kern_flag & MNTK_UNMOUNTF) != 0 || > - mp->mnt_nvnodelistsize == 0)) { > - locked = VOP_ISLOCKED(vp); > - if (!locked || (locked == LK_EXCLUSIVE && > - (vp->v_vflag & VV_FORCEINSMQ) == 0)) { > - VI_UNLOCK(vp); > - MNT_IUNLOCK(mp); > - if (dtr != NULL) > - dtr(vp, dtr_arg); > - return (EBUSY); > - } > + mp->mnt_nvnodelistsize == 0)) && > + (vp->v_vflag & VV_FORCEINSMQ) == 0) { > + VI_UNLOCK(vp);s > + MNT_IUNLOCK(mp); > + if (dtr != NULL) > + dtr(vp, dtr_arg); > + return (EBUSY); > } > vp->v_mount = mp; > MNT_REF(mp); Thanks for doing this. Attilio, I don't know if this really could help, but what do you think about adding an assertion to check if the vnode is locked? This could help in some cases, e.g. it might be useful to discover the violation of this assumption for a developer which wants to port a new fs into the source tree. Davide
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACYV=-Hya1-V_RNToWHDD_LFqxEcJYovUjnp0P9b-Q8Hzm3t_w>