svn commit: r243307 - head/sys/kern

[Davide Italiano]

On Mon, Nov 19, 2012 at 9:43 PM, Attilio Rao <attilio at 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