Re: [patch 3/6] fs: sync_sb_inodes fix

[Nick Piggin <npiggin@suse.de>]

On Thu, Dec 11, 2008 at 01:51:47PM -0800, Andrew Morton wrote:
> On Wed, 10 Dec 2008 08:27:07 +0100
> Nick Piggin <npiggin@suse.de> 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.