From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cuda.sgi.com (cuda2.sgi.com [192.48.176.25]) by oss.sgi.com (8.14.3/8.14.3/SuSE Linux 0.8) with ESMTP id n4AHpID4194700 for ; Sun, 10 May 2009 12:51:19 -0500 Received: from mail.sandeen.net (localhost [127.0.0.1]) by cuda.sgi.com (Spam Firewall) with ESMTP id 642822765D1 for ; Sun, 10 May 2009 10:51:23 -0700 (PDT) Received: from mail.sandeen.net (sandeen.net [209.173.210.139]) by cuda.sgi.com with ESMTP id Gze0bEbrlF02mdG3 for ; Sun, 10 May 2009 10:51:23 -0700 (PDT) Message-ID: <4A07141A.5060303@sandeen.net> Date: Sun, 10 May 2009 12:51:22 -0500 From: Eric Sandeen MIME-Version: 1.0 Subject: Re: [PATCH 2/5] xfs: cleanup ->sync_fs References: <20090426140305.113371000@bombadil.infradead.org> <20090426140707.713299000@bombadil.infradead.org> In-Reply-To: <20090426140707.713299000@bombadil.infradead.org> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: xfs-bounces@oss.sgi.com Errors-To: xfs-bounces@oss.sgi.com To: Christoph Hellwig Cc: xfs@oss.sgi.com Christoph Hellwig wrote: > Sort out ->sync_fs to not perform a superblock writeback for the wait = 0 case > as that is just an optional first pass and the superblock will be written back > properly in the next call with wait = 1. Instead perform an opportunistic > quota writeback to have less work later. Also remove the freeze special case > as we do a proper wait = 1 call in the freeze code anyway. > > Also rename the function to xfs_fs_sync_fs to match the normal naming > convention, update comments and avoid calling into the laptop_mode logic on > an error. > > Signed-off-by: Christoph Hellwig > > > Index: linux-2.6/fs/xfs/linux-2.6/xfs_super.c > =================================================================== > --- linux-2.6.orig/fs/xfs/linux-2.6/xfs_super.c 2009-04-26 10:39:20.433949442 +0200 > +++ linux-2.6/fs/xfs/linux-2.6/xfs_super.c 2009-04-26 10:43:31.297949640 +0200 > @@ -1105,7 +1105,7 @@ xfs_fs_put_super( > } > > STATIC int > -xfs_fs_sync_super( > +xfs_fs_sync_fs( > struct super_block *sb, > int wait) > { > @@ -1113,23 +1113,23 @@ xfs_fs_sync_super( > int error; > > /* > - * Treat a sync operation like a freeze. This is to work > - * around a race in sync_inodes() which works in two phases > - * - an asynchronous flush, which can write out an inode > - * without waiting for file size updates to complete, and a > - * synchronous flush, which wont do anything because the > - * async flush removed the inode's dirty flag. Also > - * sync_inodes() will not see any files that just have > - * outstanding transactions to be flushed because we don't > - * dirty the Linux inode until after the transaction I/O > - * completes. > + * Not much we can do fort the first async pass. Writing out the ^ typo > + * superblock would be contra-productive as we are going to redirty ^ "counter-productive" is more common > + * when writing out other data and metadata (and writing out a single > + * block is quite fast anyway. ^ add a ")" > + * > + * Try to asynchronously kick of quota syncing at least. ^ "off?" > */ > - if (wait || unlikely(sb->s_frozen == SB_FREEZE_WRITE)) > - error = xfs_quiesce_data(mp); > - else > - error = xfs_sync_fsdata(mp, 0); > + if (!wait) { > + XFS_QM_DQSYNC(mp, SYNC_BDFLUSH); > + return 0; > + } > + Is it worth keeping a comment about this still being similar to the freeze path? (xfs_quiesce_data) > + error = xfs_quiesce_data(mp); > + if (error) > + return -error; > > - if (unlikely(laptop_mode)) { > + if (laptop_mode) { > int prev_sync_seq = mp->m_sync_seq; > > /* > @@ -1148,7 +1148,7 @@ xfs_fs_sync_super( > mp->m_sync_seq != prev_sync_seq); > } > > - return -error; > + return 0; > } > > STATIC int > @@ -1522,7 +1522,7 @@ static struct super_operations xfs_super > .write_inode = xfs_fs_write_inode, > .clear_inode = xfs_fs_clear_inode, > .put_super = xfs_fs_put_super, > - .sync_fs = xfs_fs_sync_super, > + .sync_fs = xfs_fs_sync_fs, > .freeze_fs = xfs_fs_freeze, > .statfs = xfs_fs_statfs, > .remount_fs = xfs_fs_remount, > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs