Re: [PATCH] Move vn_iowait() earlier in the reclaim path

[Lachlan McIlroy <lachlan@sgi.com>]

Dave Chinner wrote:
> On Tue, Aug 05, 2008 at 04:43:29PM +1000, Lachlan McIlroy wrote:
>> Currently by the time we get to vn_iowait() in xfs_reclaim() we have already
>> gone through xfs_inactive()/xfs_free() and recycled the inode.  Any I/O
> 
> xfs_free()? What's that?
Sorry that should have been xfs_ifree() (we set the inode's mode to
zero in there).

> 
>> completions still running (file size updates and unwritten extent conversions)
>> may be working on an inode that is no longer valid.
> 
> The linux inode does not get freed until after ->clear_inode
> completes, hence it is perfectly valid to reference it anywhere
> in the ->clear_inode path.
The problem I see is an assert in xfs_setfilesize() fail:

	ASSERT((ip->i_d.di_mode & S_IFMT) == S_IFREG);

The mode of the XFS inode is zero at this time.

> 
> My bet is that you are seeing I/O completion mark an inode dirty
> that is being freed. ie.  Calling mark_inode_dirty_sync() in the I/O
> completion blindly assumes that the linux inode is still valid, 
> when it may be in the 'being freed' path. e.g. we can put it back on the
> superblock dirty list just before it gets freed for real...
> 
> I came across this about a week ago when tracking down a QA failure
> with a combined linux/XFS inode patch. The fix is to make I/O
> completion call xfs_mark_inode_dirty_sync() so we check that this
> linux inode not in the process of being freed before we try to
> mark it dirty.
> 
> Cheers,
> 
> Dave.