From owner-freebsd-fs@FreeBSD.ORG Mon Dec 31 16:22:03 2012 Return-Path: Delivered-To: fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6AC3D590; Mon, 31 Dec 2012 16:22:03 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wg0-f50.google.com (mail-wg0-f50.google.com [74.125.82.50]) by mx1.freebsd.org (Postfix) with ESMTP id 9E0188FC12; Mon, 31 Dec 2012 16:22:02 +0000 (UTC) Received: by mail-wg0-f50.google.com with SMTP id es5so5744848wgb.17 for ; Mon, 31 Dec 2012 08:22:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=V/IaEb+C9Rh3Ncu44VZZE3Mn4kiZd24HxGGXC24u6SE=; b=KUxMvSEY8AS7gIvDMfJJThg/qEk57PVgM1j09JcmRTT8JpB7tSm6QpYGGo0lnTU9e+ vVyg89kDrYiOixUmm62Ih4C1duMl/eg5z3E2mB5uWh5g2BGf0bwMLE0MCoh34VpJ+GCZ 4A9EgePsx1a5gq1FsP6BUp0WckOhAZgjlYciYYEng1HTSWXrTKCDYdjH5qNdzaRO+7D/ ayBtTrNIaGBfI8Ki0T6xEguFyR8S/pSB3I+Z6yLJ0NxzZDhWDjbDwnWZkIZ4gIAyJila NlTp+rVVfW9LpbRS6gAYsHPg2pQa5swHiaQiiW6Ineuqd90cy0wX+UmxKog3TkZPuD7p CXOA== X-Received: by 10.180.24.133 with SMTP id u5mr55301513wif.17.1356970921656; Mon, 31 Dec 2012 08:22:01 -0800 (PST) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPS id bw9sm49686866wib.5.2012.12.31.08.21.57 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 31 Dec 2012 08:21:58 -0800 (PST) Date: Mon, 31 Dec 2012 17:21:45 +0100 From: Mateusz Guzik To: Ronald Klop Subject: Re: Nandfs use of MNT_VNODE_FOREACH Message-ID: <20121231162145.GA29588@dft-labs.eu> References: <20121227230223.GN82219@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) Cc: gber@freebsd.org, cognet@freebsd.org, fs@freebsd.org X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Dec 2012 16:22:03 -0000 On Mon, Dec 31, 2012 at 11:03:21AM +0100, Ronald Klop wrote: > On Fri, 28 Dec 2012 00:02:23 +0100, Konstantin Belousov > 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