Subject: Re: LK_DRAIN vs the interlock vs VOP_INACTIVE, was Re: reboot problems unmounting root
To: Antti Kantee <pooka@cs.hut.fi>
From: Bill Stouder-Studenmund <wrstuden@netbsd.org>
List: tech-kern
Date: 07/09/2007 10:36:40
--7qSK/uQB79J36Y4o
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Jul 07, 2007 at 11:02:20PM +0300, Antti Kantee wrote:
> On Sat Jul 07 2007 at 11:30:55 -0700, Bill Stouder-Studenmund wrote:
> > > diff -p -u -r1.288 vfs_subr.c
> > > --- vfs_subr.c	5 Jun 2007 12:31:31 -0000	1.288
> > > +++ vfs_subr.c	7 Jul 2007 14:27:01 -0000
> > > @@ -1522,12 +1522,11 @@ vclean(struct vnode *vp, int flags, stru
> > > =20
> > >  	/*
> > >  	 * Even if the count is zero, the VOP_INACTIVE routine may still
> > > -	 * have the object locked while it cleans it out. The VOP_LOCK
> > > -	 * ensures that the VOP_INACTIVE routine is done with its work.
> > > -	 * For active vnodes, it ensures that no other activity can
> > > +	 * have the object locked while it cleans it out.  For
> > > +	 * active vnodes, it ensures that no other activity can
> > >  	 * occur while the underlying object is being cleaned out.
> > >  	 */
> > > -	VOP_LOCK(vp, LK_DRAIN | LK_INTERLOCK);
> > > +	VOP_LOCK(vp, LK_EXCLUSIVE | LK_INTERLOCK);
> > > =20
> > >  	/*
> > >  	 * Clean out any cached data associated with the vnode.
> >=20
> > Ok. How much testing did you do? I like this idea the best, now.
>=20
> Tested all the things that were wrong, like revoke upper/foo, revoke
> lower/foo while holding upper/foo, unmount -f /lower while having open
> files in upper, forced reclamation of vnodes through ls -lR etc stuff.
> But in qemu, I'm not running it on my desktop yet.
>=20
> > > @@ -1649,6 +1648,7 @@ vclean(struct vnode *vp, int flags, stru
> > >  	 */
> > >  	vp->v_op =3D dead_vnodeop_p;
> > >  	vp->v_tag =3D VT_NON;
> > > +	vp->v_vnlock =3D NULL;
> > >  	simple_lock(&vp->v_interlock);
> > >  	VN_KNOTE(vp, NOTE_REVOKE);	/* FreeBSD has this in vn_pollgone() */
> > >  	vp->v_flag &=3D ~(VXLOCK|VLOCKSWORK);
> >=20
> > Do we really need this?
>=20
> No, but it was hanging around from a previous patch and it's the right
> thing to do even though not necessary: the lock is dead from an exporting
> point-of-view.

Ok, please check it in.

Take care,

Bill

--7qSK/uQB79J36Y4o
Content-Type: application/pgp-signature
Content-Disposition: inline

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

iD8DBQFGknIoWz+3JHUci9cRAlxfAJ4jqUjLI6m1QRmqfQToteV01U90wQCfTRqs
nWR0p5SiaIUozVFOQ7UkEVE=
=deer
-----END PGP SIGNATURE-----

--7qSK/uQB79J36Y4o--