On Wed, Apr 13, 2016 at 03:31:28PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> The last thing we do before using call_rcu() on an xfs_inode to be
> freed is mark it as invalid. This means there is a window between
> when we know for certain that the inode is going to be freed and
> when we do actually mark it as "freed".
>
> This is important in the context of RCU lookups - we can look up the
> inode, find that it is valid, and then use it as such not realising
> that it is in the final stages of being freed.
>
> As such, mark the inode as being invalid the moment we know it is
> going to be reclaimed. This can be done while we still hold the
> XFS_ILOCK_EXCL and the flush lock in xfs_inode_reclaim, meaning that
> it occurs well before we remove it from the radix tree, and that
> the i_flags_lock, the XFS_ILOCK and the inode flush lock all act as
> synchronisation points for detecting that an inode is about to go
> away.
>
> For defensive purposes, this allows us to add a further check to
> xfs_iflush_cluster to ensure we skip inodes that are being freed
> after we grab the XFS_ILOCK_SHARED and the flush lock - we know that
> if the inode number if valid while we have these locks held we know
> that it has not progressed through reclaim to the point where it is
> clean and is about to be freed.
>
> [bfoster: fixed __xfs_inode_clear_reclaim() using ip->i_ino after it
> had already been zeroed.]
And, of course, in reordering this I dropped this fix because it was
handled by the reworking of tagging code to use pag->pag_agno.
So I've brought that small change forward to this patch (using
pag->pag_agno instead of deriving it from the ip->i_ino in
__xfs_inode_clear_reclaim()).
That means I have to rebase the later cleanup patch too, but the end
result of the patch set is identical...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|