From owner-freebsd-current  Tue Sep 10 23:58:17 2002
Delivered-To: freebsd-current@freebsd.org
Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP
	id 35E2137B401; Tue, 10 Sep 2002 23:58:10 -0700 (PDT)
Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163])
	by mx1.FreeBSD.org (Postfix) with ESMTP
	id A83EA43E42; Tue, 10 Sep 2002 23:58:09 -0700 (PDT)
	(envelope-from dl-freebsd@catspoiler.org)
Received: from mousie.catspoiler.org (mousie.catspoiler.org [192.168.101.2])
	by gw.catspoiler.org (8.12.5/8.12.5) with ESMTP id g8B6w2wr097059;
	Tue, 10 Sep 2002 23:58:06 -0700 (PDT)
	(envelope-from dl-freebsd@catspoiler.org)
Message-Id: <200209110658.g8B6w2wr097059@gw.catspoiler.org>
Date: Tue, 10 Sep 2002 23:58:02 -0700 (PDT)
From: Don Lewis <dl-freebsd@catspoiler.org>
Subject: Re: nfs_inactive() bug? -> panic: lockmgr: locking against myself
To: current@FreeBSD.ORG, jeff@FreeBSD.ORG
In-Reply-To: <200209110644.g8B6inwr097028@gw.catspoiler.org>
MIME-Version: 1.0
Content-Type: TEXT/plain; charset=us-ascii
Sender: owner-freebsd-current@FreeBSD.ORG
Precedence: bulk
List-ID: <freebsd-current.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20freebsd-current>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20freebsd-current>
X-Loop: FreeBSD.ORG

On 10 Sep, Don Lewis wrote:

> It looks like the call to vrele() from vn_close() is executing the
> following code:
> 
>         if (vp->v_usecount == 1) {
>                 vp->v_usecount--;
>                 /*
>                  * We must call VOP_INACTIVE with the node locked.
>                  * If we are doing a vput, the node is already locked,
>                  * but, in the case of vrele, we must explicitly lock
>                  * the vnode before calling VOP_INACTIVE.
>                  */ 
>                 if (vn_lock(vp, LK_EXCLUSIVE | LK_INTERLOCK, td) == 0)
>                         VOP_INACTIVE(vp, td);
>                 VI_LOCK(vp);
>                 if (VSHOULDFREE(vp))
>                         vfree(vp);
>                 else
>                         vlruvp(vp);
>                 VI_UNLOCK(vp);
> 
> It has decremented v_usecount to 0, grabbed an exclusive lock on the
> vnode, and has called VOP_INACTIVE().


> It looks like nfs_inactive() is executing the following code:
> 
>         if (sp) {
>                 /*
>                  * We need a reference to keep the vnode from being
>                  * recycled by getnewvnode while we do the I/O
>                  * associated with discarding the buffers unless we
>                  * are being forcibly unmounted in which case we already
>                  * have our own reference.
>                  */
>                 if (ap->a_vp->v_usecount > 0)
>                         (void) nfs_vinvalbuf(ap->a_vp, 0, sp->s_cred, td, 1);
>                 else if (vget(ap->a_vp, 0, td))
>                         panic("nfs_inactive: lost vnode");
>                 else {
>                         (void) nfs_vinvalbuf(ap->a_vp, 0, sp->s_cred, td, 1);
>                         vrele(ap->a_vp);
>                 }
> 

A potentially better solution just occurred to me.  It looks like it
would be better if vrele() waited to decrement v_usecount until *after*
the call to VOP_INACTIVE() (and after the call to VI_LOCK()).  If that
were done, nfs_inactive() wouldn't need to muck with v_usecount at all.


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message