From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: Re: [PATCH] vfs: Merge sync_supers(), sync_filesystems() and sync_blockdevs() Date: Thu, 23 Apr 2009 08:31:27 -0400 Message-ID: <20090423123126.GA25313@infradead.org> References: <1240415781-17834-1-git-send-email-jack@suse.cz> <1240415781-17834-3-git-send-email-jack@suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: LKML , linux-fsdevel@vger.kernel.org, Andrew Morton , Jens Axboe To: Jan Kara Return-path: Content-Disposition: inline In-Reply-To: <1240415781-17834-3-git-send-email-jack@suse.cz> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Wed, Apr 22, 2009 at 05:56:21PM +0200, Jan Kara wrote: > These three functions are quite similar so merge them to save superblock list > traversal code. As a bonus we get livelock avoidance for all these superblock > traversals. Also remove the condition that if wait == 0 and sb->s_dirt is > not set, then ->sync_fs() is not called. This does not really make much sence > since s_dirt is generally used by filesystem to mean that ->write_super() needs > to be called. But ->sync_fs() does different things. I even suspect that some > filesystems (btrfs?) sets s_dirt just to fool this logic. Some more comments after looking at it in more details: - the FSSYNC_SUPER case really needs to do a trylock on the mutex, otherwise any in-progress sync would block pdflush for a long time. And as any real sync should write out the superblock it's not needed anyway during that time. (Need to double-check the filesystems, though) - sync_filesystems really should move to fs/sync.c - I get more and more inclined to make sync just case of looping over the superblocks and do an fsync_super. A plain sync fsync_super might be too slow so we can try to do an async one first and then a sync one as a second pass - that wakeup_pdflush in do_sync looks extremly fishy, we need to do all page writeback via sync_inodes_(sb) anyway, and doing this in parallel from pdflush just introduced tons of potential race opportunities - now if sync_filesystems just ends up calling __fsync_super for the normal sync path I wonder if there really is a point unifying it with the periodic write_super case.