Nandfs use of MNT_VNODE_FOREACH

[Mateusz Guzik]

On Mon, Dec 31, 2012 at 11:03:21AM +0100, Ronald Klop wrote:
> On Fri, 28 Dec 2012 00:02:23 +0100, Konstantin Belousov
> <kostikbel at gmail.com> wrote:
> 
> >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);
> > }
> >
> 
> It looks like it hangs. No DHCP yet, so no ping also. But I have
> remote debugging.
> 

Cursory look suggests that with this patch we leak interlock for
syncer vnode and this is possibly the problem.

Can you adjust this function so that 'if (mp->mnt_syncer == vp)'
performs VI_UNLOCK as well?

Something like:
if (mp->mnt_syncer == vp || VOP_ISLOCKED(vp)) {
	VI_UNLOCK(mp);
	continue;
}

I will have time to dig into this next week.
-- 
Mateusz Guzik <mjguzik gmail.com>