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/07/2007 11:30:55
--n8g4imXOkfNTN/H1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Jul 07, 2007 at 05:27:21PM +0300, Antti Kantee wrote:
> On Sat Jul 07 2007 at 16:59:09 +0300, Antti Kantee wrote:
> > With what you're suggesting, sounds easier that we simply don't drain t=
he
> > lock at all.  Reference counting of vnodes, VXLOCK, taking an exclusive
> > lock in vclean() etc. should take care of the effects of LK_DRAIN?
> >=20
> > I still don't like it, though, but I think it's the best of the bad
> > options we have.
>=20
> Yup, that appears pretty good per simple testing:
>=20
> Index: vfs_subr.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> RCS file: /cvsroot/src/sys/kern/vfs_subr.c,v
> retrieving revision 1.288
> 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.

Ok. How much testing did you do? I like this idea the best, now.

> @@ -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);

Do we really need this?

Take care,

Bill

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

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

iD8DBQFGj9vfWz+3JHUci9cRAtwCAJ91U5KwjZkwS/pCDrZIlQemrf85BgCbBio8
1/3f10B40e6AoqAMWUNFjSk=
=2SyP
-----END PGP SIGNATURE-----

--n8g4imXOkfNTN/H1--