Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Sep 2002 23:58:02 -0700 (PDT)
From:      Don Lewis <dl-freebsd@catspoiler.org>
To:        current@FreeBSD.ORG, jeff@FreeBSD.ORG
Subject:   Re: nfs_inactive() bug? -> panic: lockmgr: locking against myself
Message-ID:  <200209110658.g8B6w2wr097059@gw.catspoiler.org>
In-Reply-To: <200209110644.g8B6inwr097028@gw.catspoiler.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200209110658.g8B6w2wr097059>