Date: Sun, 30 Dec 2012 15:55:53 +0100 From: "Ronald Klop" <ronald-freebsd8@klop.yi.org> To: fs@freebsd.org, "Konstantin Belousov" <kostikbel@gmail.com> Cc: gber@freebsd.org, cognet@freebsd.org Subject: Re: Nandfs use of MNT_VNODE_FOREACH Message-ID: <op.wp4w3fj58527sy@pinky> In-Reply-To: <20121227230223.GN82219@kib.kiev.ua> References: <20121227230223.GN82219@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
I'm building a patched kernel for my sheevaplug now which reads the kernel from nand and has / mounted from nandfs. What should be tested specifically? Just booting from it? Ronald. On Fri, 28 Dec 2012 00:02:23 +0100, Konstantin Belousov <kostikbel@gmail.com> wrote: > 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 == 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; > td = curthread; > - lockreq = LK_EXCLUSIVE | LK_INTERLOCK | LK_RETRY; > + lockreq = LK_EXCLUSIVE | LK_INTERLOCK; > - MNT_ILOCK(mp); > - > - MNT_VNODE_FOREACH(vp, mp, mvp) { > + MNT_VNODE_FOREACH_ACTIVE(vp, mp, mvp) { > update = 0; > if (mp->mnt_syncer == 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 = vget(vp, lockreq, td)) != 0) { > - MNT_ILOCK(mp); > continue; > } > - if (vp->v_iflag & VI_DOOMED) { > - vput(vp); > - MNT_ILOCK(mp); > + if (vget(vp, lockreq, td) != 0) > continue; > - } > nandfs_node = VTON(vp); > if (nandfs_node->nn_flags & IN_MODIFIED) { > @@ -532,12 +516,8 @@ nandfs_iterate_dirty_vnodes(struct mount *mp, > struct nandfs_seginfo *seginfo) > if (update) > nandfs_node_update(nandfs_node); > - > - MNT_ILOCK(mp); > } > - MNT_IUNLOCK(mp); > - > return (0); > }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?op.wp4w3fj58527sy>