On Fri, Nov 19, 2010 at 12:50 AM, Christoph Hellwig <hch@xxxxxx> wrote: > On Thu, Nov 18, 2010 at 05:40:44PM +1100, Nick Piggin wrote: >> Does this guy ever get cleared in the synchronous path? Or only in writeback? >> I couldn't see where it gets cleared from direct writeout, I wonder if we should >> import the pagecache tagged dirty check from writeback to keep this in sync >> better? (anyway, separate issue) [cut] > > What do you mean with direct writeout? Bad terminology, I meant actually fsync driven writeout. > For hfsplus we always write > the inode through writeback_single_inode, either from the writeback > threads, or through sync_inode_metadata (or previously write_inode_now) > in fsync. Both of these clear I_DIRTY. Ah OK, I missed that one, thanks. > But one thing I noticed when > reading through the above is that I_DIRTY is overkill here - we only > care about metadata, as the pagecache is handled by vfs_fsync. Just > checking I_DIRTY_SYNC (and eventually I_DIRTY_DATASYNC when adding > support for an optimized fdatasync) would be enough. Yes that's true. > The code is > take from generic_file_fsync, which is used or copied by a lot of > filesystems, so it looks like no one really cared about the possibly > superflous I_DIRTY_PAGES checking so far. [paste] >> >> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html