On Tue, Aug 05, 2008 at 05:37:11PM +1000, 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?
>
> > 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.
>
> 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.
Oh, I should point out that that xfs_mark_inode_dirty sync looked
like this prior to the patch I posted:
/*
* If the linux inode is valid, mark it dirty.
* Used when commiting a dirty inode into a transaction so that
* the inode will get written back by the linux code
*/
void
xfs_mark_inode_dirty_sync(
xfs_inode_t *ip)
{
struct inode *inode = VFS_I(ip);
if (!(inode->i_state & (I_WILL_FREE|I_FREEING|I_CLEAR)))
mark_inode_dirty_sync(inode);
}
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|