From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [patch 3/6] fs: sync_sb_inodes fix Date: Thu, 11 Dec 2008 23:34:50 +0100 Message-ID: <20081211223450.GD8294@wotan.suse.de> References: <20081210072454.GB27096@wotan.suse.de> <20081210072707.GD27096@wotan.suse.de> <20081211135147.59d50e96.akpm@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, mpatocka@redhat.com To: Andrew Morton Return-path: Received: from mx2.suse.de ([195.135.220.15]:53146 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757063AbYLKWev (ORCPT ); Thu, 11 Dec 2008 17:34:51 -0500 Content-Disposition: inline In-Reply-To: <20081211135147.59d50e96.akpm@linux-foundation.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Dec 11, 2008 at 01:51:47PM -0800, Andrew Morton wrote: > On Wed, 10 Dec 2008 08:27:07 +0100 > Nick Piggin wrote: > > > + if (sync) { > > + struct inode *inode, *old_inode = NULL; > > + > > + /* > > + * Data integrity sync. Must wait for all pages under writeback, > > + * because there may have been pages dirtied before our sync > > + * call, but which had writeout started before we write it out. > > + * In which case, the inode may not be on the dirty list, but > > + * we still have to wait for that writeout. > > + */ > > + list_for_each_entry(inode, &sb->s_inodes, i_sb_list) { > > + struct address_space *mapping; > > + > > + if (inode->i_state & (I_FREEING|I_WILL_FREE)) > > + continue; > > + mapping = inode->i_mapping; > > + if (mapping->nrpages == 0) > > + continue; > > + __iget(inode); > > + spin_unlock(&inode_lock); > > + /* > > + * We hold a reference to 'inode' so it couldn't have > > + * been removed from s_inodes list while we dropped the > > + * inode_lock. We cannot iput the inode now as we can > > + * be holding the last reference and we cannot iput it > > + * under inode_lock. So we keep the reference and iput > > + * it later. > > + */ > > hm, tricky. > > Can umount run concurrently with this? What will it say about the busy > inode? AFAIKS umount shouldn't because we've taken a reference on the superblock higher up.