From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [PATCH 10/11] hfsplus: optimize fsync Date: Tue, 23 Nov 2010 00:18:08 +1100 Message-ID: <20101122131808.GA12831@amd> References: <20101117222117.GA21700@lst.de> <20101117222313.GK21700@lst.de> <20101118135047.GA14669@lst.de> <20101118141657.GA16690@lst.de> <20101122130314.GB12716@amd> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Christoph Hellwig , Nick Piggin , linux-fsdevel@vger.kernel.org To: Nick Piggin Return-path: Received: from ipmail06.adl6.internode.on.net ([150.101.137.145]:37651 "EHLO ipmail06.adl6.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753409Ab0KVNSN (ORCPT ); Mon, 22 Nov 2010 08:18:13 -0500 Content-Disposition: inline In-Reply-To: <20101122130314.GB12716@amd> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Nov 23, 2010 at 12:03:14AM +1100, Nick Piggin wrote: > On Thu, Nov 18, 2010 at 03:16:57PM +0100, Christoph Hellwig wrote: > > On Fri, Nov 19, 2010 at 01:13:55AM +1100, Nick Piggin wrote: > > > >> Anyway, what if writeback noticed pagecache was cleaned at this point, and then > > > >> clears I_DIRTY bits from inode before you test it above? Won't that leave your > > > >> metadata not on disk? > > > > > > I'm not sure if you answered this, though. Can't background writeout go through > > > and clear the I_DIRTY_* bits on your inode, so that a subsequent fsync will skip > > > required writeout because i_state is clean, but your private dirty > > > bits are still set? > > > > > > That was my (poorly worded) concern. > > > > Good point - we should just skip the sync_inode_metadata for that case. > > Which already is a no-op if no dirty bits are set, so just removing the > > I_DIRTY check probably is the best thing we can do here. > > Hmm, it seems that your dirty bit handling is still broken and racy, > along with half the other filesystems. I'll post a couple of patches to > fsdevel for comment. Actually now I look at your updated patch, perhaps it is not. What version does that patch apply against? Do you have a tree or tarball uploaded anywhere? Or can you repost the full series? Thanks, Nick