Subject: Re: CVS commit: src/sys
To: YAMAMOTO Takashi <>
From: Bill Studenmund <>
List: source-changes
Date: 12/27/2005 15:27:37
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Dec 24, 2005 at 03:28:06PM +0900, YAMAMOTO Takashi wrote:
> > One thing I noted in this change is that vget() will now call=20
> > VOP_INACTIVE() on vnodes that were never activated. Will this cause iss=
> > with other file systems?
> what kind of issues?

Well, I'm not 100% sure. Thus the question.

The old code went out of its way to not call VOP_INACTIVE() on the errored
vnode. Also, it made sure that we did the right thing in face of multiple=
processes each in vget() and failing.

> afaik, vn_lock fails only when the vnode is being reclaimed.
> in that case, calling vrele doesn't hurt.

Yes. My concern, however, was to make sure that we don't end up disturbing=
the vnode in its new life (for the case where we reclaimed the vnode for=20
getnewvnode()). Like call VOP_INACTIVE() on a live vnode.

I've been staring at this for a bit and trying to think of race conditions
that would do this. I think that you're right, and we're ok.  Since we
have spinlocks around reference count updating and we never play with
TAILQ macros unless we were in the transition between zero and one (either
0->1 or 1->0), I think we're fine.

I'm still confused as I don't understand the comment. Since we hold=20
references across these operations, we will not mess up queue membership=20
nor the reference counting. So vrele() will never be a problem; either we=
really are the last reference release on the new life (when VOP_INACTIVE()=
would be correct) or we aren't. Also, if the other user just vclean()ed=20
the vnode, the VOP_INACTIVE() call will be handled by deadfs, which will=20
be fine.

I guess the comment dated to an earlier implementation.

Take care,


Content-Type: application/pgp-signature
Content-Disposition: inline

Version: GnuPG v1.2.3 (NetBSD)