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>