Date: Mon, 19 Nov 2012 20:55:26 +0000 From: Attilio Rao <attilio@freebsd.org> To: Davide Italiano <davide@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: <CAJ-FndBUNezKNFPQCqw8j%2B47fOcvqR6nVEy%2BUUxnbqQg7LoY7A@mail.gmail.com> In-Reply-To: <CACYV=-Hya1-V_RNToWHDD_LFqxEcJYovUjnp0P9b-Q8Hzm3t_w@mail.gmail.com> References: <201211192043.qAJKhJ9i038016@svn.freebsd.org> <CACYV=-Hya1-V_RNToWHDD_LFqxEcJYovUjnp0P9b-Q8Hzm3t_w@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Nov 19, 2012 at 8:53 PM, Davide Italiano <davide@freebsd.org> wrote: > 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. Exactly where? insmntque1() already has this. Attilio -- Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndBUNezKNFPQCqw8j%2B47fOcvqR6nVEy%2BUUxnbqQg7LoY7A>