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

[Dave Chinner <david@fromorbit.com>]

On Tue, Aug 05, 2008 at 05:52:34PM +1000, Lachlan McIlroy wrote:
> 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.

Ok, so the question has to be why is there I/O still in progress
after the truncate is supposed to have already occurred and the
vn_iowait() in xfs_itruncate_start() been executed.

Something doesn't add up here - you can't be doing I/O on a file
with no extents or delalloc blocks, hence that means we should be
passing through the truncate path in xfs_inactive() before we
call xfs_ifree() and therefore doing the vn_iowait()..

Hmmmm - the vn_iowait() is conditional based on:

        /* wait for the completion of any pending DIOs */
        if (new_size < ip->i_size)
                vn_iowait(ip);

We are truncating to zero (new_size == 0), so the only case where
this would not wait is if ip->i_size == 0. Still - I can't see
how we'd be doing I/O on an inode with a zero i_size. I suspect
ensuring we call vn_iowait() if newsize == 0 as well would fix
the problem. If not, there's something much more subtle going
on here that we should understand....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com