Date: Mon, 31 Dec 2012 17:21:45 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Ronald Klop <ronald-freebsd8@klop.yi.org> Cc: gber@freebsd.org, cognet@freebsd.org, fs@freebsd.org Subject: Re: Nandfs use of MNT_VNODE_FOREACH Message-ID: <20121231162145.GA29588@dft-labs.eu> In-Reply-To: <op.wp6d7vdo8527sy@212-182-167-131.ip.telfort.nl> References: <20121227230223.GN82219@kib.kiev.ua> <op.wp6d7vdo8527sy@212-182-167-131.ip.telfort.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121231162145.GA29588>