From owner-freebsd-fs@FreeBSD.ORG Sun Dec 30 14:55:57 2012 Return-Path: Delivered-To: fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id EA11C1D0; Sun, 30 Dec 2012 14:55:57 +0000 (UTC) (envelope-from ronald-freebsd8@klop.yi.org) Received: from smarthost1.greenhost.nl (smarthost1.greenhost.nl [195.190.28.78]) by mx1.freebsd.org (Postfix) with ESMTP id 6DCDE8FC12; Sun, 30 Dec 2012 14:55:57 +0000 (UTC) Received: from smtp.greenhost.nl ([213.108.104.138]) by smarthost1.greenhost.nl with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1TpKIs-00049A-OS; Sun, 30 Dec 2012 15:55:55 +0100 Received: from h253044.upc-h.chello.nl ([62.194.253.44] helo=pinky) by smtp.greenhost.nl with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.72) (envelope-from ) id 1TpKIs-0001M9-Qu; Sun, 30 Dec 2012 15:55:54 +0100 Content-Type: text/plain; charset=us-ascii; format=flowed; delsp=yes To: fs@freebsd.org, "Konstantin Belousov" Subject: Re: Nandfs use of MNT_VNODE_FOREACH References: <20121227230223.GN82219@kib.kiev.ua> Date: Sun, 30 Dec 2012 15:55:53 +0100 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit From: "Ronald Klop" Message-ID: In-Reply-To: <20121227230223.GN82219@kib.kiev.ua> User-Agent: Opera Mail/12.12 (Win32) X-Virus-Scanned: by clamav at smarthost1.samage.net X-Spam-Level: / X-Spam-Score: -0.0 X-Spam-Status: No, score=-0.0 required=5.0 tests=BAYES_40 autolearn=disabled version=3.3.1 X-Scan-Signature: d3d6c6694e059b137bd8e4e2c0542d46 Cc: gber@freebsd.org, cognet@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: Sun, 30 Dec 2012 14:55:58 -0000 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 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); > }