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>