Nandfs use of MNT_VNODE_FOREACH

[Konstantin Belousov]

On Sun, Dec 30, 2012 at 03:55:53PM +0100, Ronald Klop wrote:
> 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?
I suspect that a writeable mount with some / enough write activity
is good for the test.

Thank you.
> 
> Ronald.
> 
> On Fri, 28 Dec 2012 00:02:23 +0100, Konstantin Belousov  
> <kostikbel at 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);
> >  }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20121230/176c1ec6/attachment.sig>