Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 28 Dec 2012 01:02:23 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        fs@freebsd.org
Cc:        gber@freebsd.org, cognet@freebsd.org
Subject:   Nandfs use of MNT_VNODE_FOREACH
Message-ID:  <20121227230223.GN82219@kib.kiev.ua>

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

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

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 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 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, struct =
nandfs_seginfo *seginfo)
 	int error, lockreq, update;
=20
 	td =3D curthread;
-	lockreq =3D LK_EXCLUSIVE | LK_INTERLOCK | LK_RETRY;
+	lockreq =3D LK_EXCLUSIVE | LK_INTERLOCK;
=20
-	MNT_ILOCK(mp);
-
-	MNT_VNODE_FOREACH(vp, mp, mvp) {
+	MNT_VNODE_FOREACH_ACTIVE(vp, mp, mvp) {
 		update =3D 0;
=20
 		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;
 		}
=20
-		if (vp->v_iflag & VI_DOOMED) {
-			vput(vp);
-			MNT_ILOCK(mp);
+		if (vget(vp, lockreq, td) !=3D 0)
 			continue;
-		}
=20
 		nandfs_node =3D VTON(vp);
 		if (nandfs_node->nn_flags & IN_MODIFIED) {
@@ -532,12 +516,8 @@ nandfs_iterate_dirty_vnodes(struct mount *mp, struct n=
andfs_seginfo *seginfo)
=20
 		if (update)
 			nandfs_node_update(nandfs_node);
-
-		MNT_ILOCK(mp);
 	}
=20
-	MNT_IUNLOCK(mp);
-
 	return (0);
 }
=20

--Ll0BBk1HBk/f94B0
Content-Type: application/pgp-signature

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

iQIcBAEBAgAGBQJQ3NN/AAoJEJDCuSvBvK1BDVAP/32k/OjDCGKRKnNgRSMRl2Y7
Kya0raDE68w545/AmsYFMSbiOGNqhsDL6wyG/scbkBzCcsgf5gyNhehqGds2u/it
gV09Eucf0+sUv6Z+yy3414FDpAdDswFVJ8KpK39BQ9Q0zXV+QQzYD+M7ghrX9fQZ
l5jLSFCzPngmWkg70ohdHD4xBRnO3vH+q+jW3dAiQ1uzY6mW9WP28qokrxgBDPMm
4ZaTLxcTAzZ0wdxvcE4OUAbu7Mwg2MiSNGp01vC/sCdShCe4sJ3OJ/Vwcgd5bEiv
sjbIIc6dE+Tehg4LDNZf3W4VtNqLAYY0s2HDNDIxJf0OrFUN2ZDfLgOy1zupuPZi
NpwE4TfRkOXR7DRVPC50eiT+kcqvtYmrVmKvUkn1/xpAkMt+ERZIwVmw5X3r7ZnN
34uZfonn5o8FCVNSJE2rBYRreQUPrb7EDgc64hW5B9aXM0J58xUNvy3PbCaK0Efe
uH3UsVMKn05PGIOP54lMhidin4XEm0Bg4WqIyPvBnGzu06hCMnkSszw/3JV07n4Z
mC9JClN8nP49OV/4rOhdxD6fByDO32vSfdJ7EJZYjIS73BtC39g+4uAoA6Kf6IgI
A10ijTMHA63vP3hHPF9gAb273sYBVI0kQ+S0TtFSbTo75dH58o00R5BTtIq8GqiP
qvjmn55mnJVoV22GKdjs
=zm+D
-----END PGP SIGNATURE-----

--Ll0BBk1HBk/f94B0--



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