Subject: Re: CVS commit: src/sys
To: Chuck Silvers <chuq@chuq.com>
From: Bill Studenmund <wrstuden@netbsd.org>
List: source-changes
Date: 12/27/2005 18:46:21
--AptwxgnoZDC4KQWS
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Dec 27, 2005 at 05:20:26PM -0800, Bill Studenmund wrote:
> On Tue, Dec 27, 2005 at 03:54:15PM -0800, Chuck Silvers wrote:
> > On Tue, Dec 27, 2005 at 03:37:58PM -0800, Bill Studenmund wrote:
> > > On Tue, Dec 27, 2005 at 02:49:24PM -0800, Chuck Silvers wrote:
> > > > I'm also suspicious of this change.  this change doesn't seem relat=
ed
> > > > to the purpose given in the check-in comment, is it?  if you really=
 want
> > > > to reduce the amount of duplicate code, why not just split it into
> > > > multiple functions?
> > >=20
> > > A separate checkin might have been good. However at this point, a "cv=
s=20
> > > admin -m" will probably be enough to fix things up. I agree we should=
 note=20
> > > this change, but I don't think it's so bad to include it.
> >=20
> > if the purpose is to reduce the amount of duplicate code, why not just =
split
> > the function into multiple functions rather than changing the requireme=
nts
> > of VOP_INACTIVE() in subtle ways?
>=20
> I'm sorry, I don't think it is changing the semantics of VOP_INACTIVE().=
=20
> It took me a while to figure that out, but the only times one of these=20
> vrele()s would call VOP_INACTIVE() is: 1) if the vnode were revoked, in=
=20
> which case we send the call is handed by deadfs. 2) if the vnode has been=
=20

Sorry, that should have been "in which case we send the call to deadfs".

> reused and the reusing thread has already called vrele(). In the latter=
=20
> case, we _should_ call VOP_INACTIVE(), as the new user has let the vnode=
=20
> go back onto the free list. In the former case, I think we will do the=20
> same call to deadfs we would do otherwise; right now if you VOP_REVOKE() =
a=20
> vnode, there will be a vrele() call that will VOP_INACTIVE() on deadfs.
>=20
> So I think the semantics are the same.
>=20
> Actually, the change closes a small window of error. In the reuse case, i=
f=20
> the vget() call happens after the new user has already vrele()'d the node=
,=20
> the old code would skip a VOP_INACTIVE() call that would of happened had=
=20
> the vget() code not held a reference.

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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (NetBSD)

iD8DBQFDsfx8Wz+3JHUci9cRAst1AKCJ9vpzgoGVKZCRYNUATM4p7q7ITQCglZnN
wUMmr9LBK9YDaMdtiefJ0Gw=
=z3zZ
-----END PGP SIGNATURE-----

--AptwxgnoZDC4KQWS--