From owner-svn-src-all@FreeBSD.ORG Mon Nov 19 20:53:51 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 47CB9595; Mon, 19 Nov 2012 20:53:51 +0000 (UTC) (envelope-from davide.italiano@gmail.com) Received: from mail-vc0-f182.google.com (mail-vc0-f182.google.com [209.85.220.182]) by mx1.freebsd.org (Postfix) with ESMTP id 9A5938FC0C; Mon, 19 Nov 2012 20:53:50 +0000 (UTC) Received: by mail-vc0-f182.google.com with SMTP id fo13so7267763vcb.13 for ; Mon, 19 Nov 2012 12:53:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=bDlVD9BgmaiA82aiOzB473Vy3WkPE/Rdi265g9HODKA=; b=lhsxbAGU1GnhrgzV72rttP17e9pFTyjX8Z+wh3Vgx8owzAlVhn8qYR5sl25bzInHUY QQd4OcJiKPge93Vm+/JUeCPjP47knVJ3Y5TtK9QxAU7X5ArYmaO67SDe6eBycZDMpoXS Gu4qVQlosX0lStLjajX+hank6aLTj7gs1ZpXcd0M/mGJa12Hn1K46/JV0bsoLn3UfeNi EvXl261BACEIcuXh0xPTTydyk5ylO/jg04te0mjfj8JM7W8ctQsHvom+RKtVWkrJW5e0 eieGlt1bSrdkLTnbBf3RDoc4fSkA24skpWa2ROxBElqHVv5tCNxMwebrtd+crkBXauvj JnEw== MIME-Version: 1.0 Received: by 10.52.89.235 with SMTP id br11mr18243382vdb.77.1353358428522; Mon, 19 Nov 2012 12:53:48 -0800 (PST) Sender: davide.italiano@gmail.com Received: by 10.58.247.132 with HTTP; Mon, 19 Nov 2012 12:53:48 -0800 (PST) In-Reply-To: <201211192043.qAJKhJ9i038016@svn.freebsd.org> References: <201211192043.qAJKhJ9i038016@svn.freebsd.org> Date: Mon, 19 Nov 2012 21:53:48 +0100 X-Google-Sender-Auth: JXLaSMw0A4ijoZrKYZ0VV9Hgq68 Message-ID: Subject: Re: svn commit: r243307 - head/sys/kern From: Davide Italiano To: Attilio Rao Content-Type: text/plain; charset=ISO-8859-1 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Nov 2012 20:53:51 -0000 On Mon, Nov 19, 2012 at 9:43 PM, Attilio Rao 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