Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 30 Dec 2012 16:58:13 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Ronald Klop <ronald-freebsd8@klop.yi.org>
Cc:        gber@freebsd.org, cognet@freebsd.org, fs@freebsd.org
Subject:   Re: Nandfs use of MNT_VNODE_FOREACH
Message-ID:  <20121230145813.GI82219@kib.kiev.ua>
In-Reply-To: <op.wp4w3fj58527sy@pinky>
References:  <20121227230223.GN82219@kib.kiev.ua> <op.wp4w3fj58527sy@pinky>

next in thread | previous in thread | raw e-mail | index | archive | help

--i6ZTfQE1Row3TVml
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Dec 30, 2012 at 03:55:53PM +0100, Ronald Klop wrote:
> I'm building a patched kernel for my sheevaplug now which reads the kerne=
l =20
>  from nand and has / mounted from nandfs.
> What should be tested specifically? Just booting from it?
I suspect that a writeable mount with some / enough write activity
is good for the test.

Thank you.
>=20
> Ronald.
>=20
> On Fri, 28 Dec 2012 00:02:23 +0100, Konstantin Belousov =20
> <kostikbel@gmail.com> wrote:
>=20
> > I took a look at removing MNT_VNODE_FOREACH interface in the HEAD,
> > and apparently we still have one user of the said interface, probably,
> > due to cross-commits.
> >
> > Namely, nandfs utilizes the interface. First, it is obsoleted and
> > is going to be removed. Second, I believe that MNT_VNODE_FOREACH_ACTIVE
> > would be better choice there, because intent of the loop is to do
> > something with each dirty vnode, and dirty vnode must be active, because
> > there are dirty buffers attached to it.
> >
> > That said, use of vget(LK_RETRY) and immediate check for VI_DOOMED
> > is not useful, I removed the LK_RETRY from the lockflags. Another issue=
 =20
> > is
> > that intent was to avoid sleep for locked vnode, but the check is racy.
> > I used LK_NOWAIT instead.
> >
> > Check for mnt_syncer is also usually done as vp->v_type =3D=3D VNON, but
> > lets postpone this.
> >
> > Could someone who uses the filesystem test the patch below, please ?
> >
> > diff --git a/sys/fs/nandfs/nandfs_segment.c =20
> > b/sys/fs/nandfs/nandfs_segment.c
> > index 836bead..a73b7f2 100644
> > --- a/sys/fs/nandfs/nandfs_segment.c
> > +++ b/sys/fs/nandfs/nandfs_segment.c
> > @@ -481,36 +481,20 @@ nandfs_iterate_dirty_vnodes(struct mount *mp, =20
> > struct nandfs_seginfo *seginfo)
> >  	int error, lockreq, update;
> > 	td =3D curthread;
> > -	lockreq =3D LK_EXCLUSIVE | LK_INTERLOCK | LK_RETRY;
> > +	lockreq =3D LK_EXCLUSIVE | LK_INTERLOCK;
> > -	MNT_ILOCK(mp);
> > -
> > -	MNT_VNODE_FOREACH(vp, mp, mvp) {
> > +	MNT_VNODE_FOREACH_ACTIVE(vp, mp, mvp) {
> >  		update =3D 0;
> > 		if (mp->mnt_syncer =3D=3D vp)
> >  			continue;
> > -		if (VOP_ISLOCKED(vp))
> > -			continue;
> > -
> > -		VI_LOCK(vp);
> > -		MNT_IUNLOCK(mp);
> > -		if (vp->v_iflag & VI_DOOMED) {
> > +		if (VOP_ISLOCKED(vp)) {
> >  			VI_UNLOCK(vp);
> > -			MNT_ILOCK(mp);
> > -			continue;
> > -		}
> > -
> > -		if ((error =3D vget(vp, lockreq, td)) !=3D 0) {
> > -			MNT_ILOCK(mp);
> >  			continue;
> >  		}
> > -		if (vp->v_iflag & VI_DOOMED) {
> > -			vput(vp);
> > -			MNT_ILOCK(mp);
> > +		if (vget(vp, lockreq, td) !=3D 0)
> >  			continue;
> > -		}
> > 		nandfs_node =3D VTON(vp);
> >  		if (nandfs_node->nn_flags & IN_MODIFIED) {
> > @@ -532,12 +516,8 @@ nandfs_iterate_dirty_vnodes(struct mount *mp, =20
> > struct nandfs_seginfo *seginfo)
> > 		if (update)
> >  			nandfs_node_update(nandfs_node);
> > -
> > -		MNT_ILOCK(mp);
> >  	}
> > -	MNT_IUNLOCK(mp);
> > -
> >  	return (0);
> >  }

--i6ZTfQE1Row3TVml
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)

iQIcBAEBAgAGBQJQ4FaFAAoJEJDCuSvBvK1BdzIP/R9pHUDfStuweLrY1grLfGZ3
VjmSNhL7J9rTsslmRJCh1cdVhmBR5fdWOr+Gcm7ZMrxPwb+Qh6MVUv1ng8qMLc7p
REKHX+QbzLBCJtYBd76DkChchBrIzi5rDN9b3XS40XbKBwepSbiRKFNtcz0++oXI
2ldMDEqB1GAq6G2IrNopGDCREkWp/wkIJkh4v/kGNN3+DOhoh2P1C0ENLn0tJ8lg
FwhqEG5ALFc/sKBqdKK3Fpi61jkQ2U7rw8u9v04L2brddUH7bluaIYLMzivr9hfJ
lkg4u2gNCx0sIE+3C2YwkGIblJ8m7dLOOTq/AzHzZNhv0WgGD6CSq8r9fTAmNdVg
Ekwd8Jyi24l48FQOdHCrJ6bXWX8WEo1sXDw4SJaBRJ26pKXhNPdhCyhEsaaNwta3
MVz9y850c4r9W1ljZZbB4OkqGLyCYFEpT3c+Qmeh5DUw3Scd5cFfSmGl6biAJgxk
1TGFE5GObplpDHUhaKRm5fjXl7uOKr+vcSRByqzJODPGaeZb3XIQGjZumzZU6SZs
yffwVVj+1BQqPwaesKnsHrh/JRgmjJNYjiOCScnY9xyZi4g3QMJB/l7qtFPrqGwW
AODtC/GzCdO/1OEDP5IiU1QAbd6t2ulDLKCX1JsLilqChqalt3JYmz9Ft9yoPBaZ
/xPbKqk0hea5UMEiVki4
=Pnie
-----END PGP SIGNATURE-----

--i6ZTfQE1Row3TVml--



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