xfs: avoid moving stale inodes in the AIL

When an inode has been marked stale because the cluster is being
freed, we don't want to (re-)insert this inode into the AIL. There
is a race condition where the cluster buffer may be unpinned before
the inode is inserted into the AIL during transaction committed
processing. If the buffer is unpinned before the inode item has been
committed and inserted, then it is possible for the buffer to be
released and hence processthe stale inode callbacks before the inode
is inserted into the AIL.

In this case, we then insert a clean, stale inode into the AIL which
will never get removed by an IO completion. It will, however, get
reclaimed and that triggers an assert in xfs_inode_free()
complaining about freeing an inode still in the AIL.

This race can be avoided by not moving stale inodes forward in the AIL
during transaction commit completion processing. This closes the
race condition by ensuring we never insert clean stale inodes into
the AIL. It is safe to do this because a dirty stale inode, by
definition, must already be in the AIL.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
This commit is contained in:
Dave Chinner 2010-11-30 15:15:46 +11:00 committed by Alex Elder
parent 309c848002
commit de25c1818c

View File

@ -657,18 +657,37 @@ xfs_inode_item_unlock(
} }
/* /*
* This is called to find out where the oldest active copy of the * This is called to find out where the oldest active copy of the inode log
* inode log item in the on disk log resides now that the last log * item in the on disk log resides now that the last log write of it completed
* write of it completed at the given lsn. Since we always re-log * at the given lsn. Since we always re-log all dirty data in an inode, the
* all dirty data in an inode, the latest copy in the on disk log * latest copy in the on disk log is the only one that matters. Therefore,
* is the only one that matters. Therefore, simply return the * simply return the given lsn.
* given lsn. *
* If the inode has been marked stale because the cluster is being freed, we
* don't want to (re-)insert this inode into the AIL. There is a race condition
* where the cluster buffer may be unpinned before the inode is inserted into
* the AIL during transaction committed processing. If the buffer is unpinned
* before the inode item has been committed and inserted, then it is possible
* for the buffer to be written and IO completions before the inode is inserted
* into the AIL. In that case, we'd be inserting a clean, stale inode into the
* AIL which will never get removed. It will, however, get reclaimed which
* triggers an assert in xfs_inode_free() complaining about freein an inode
* still in the AIL.
*
* To avoid this, return a lower LSN than the one passed in so that the
* transaction committed code will not move the inode forward in the AIL but
* will still unpin it properly.
*/ */
STATIC xfs_lsn_t STATIC xfs_lsn_t
xfs_inode_item_committed( xfs_inode_item_committed(
struct xfs_log_item *lip, struct xfs_log_item *lip,
xfs_lsn_t lsn) xfs_lsn_t lsn)
{ {
struct xfs_inode_log_item *iip = INODE_ITEM(lip);
struct xfs_inode *ip = iip->ili_inode;
if (xfs_iflags_test(ip, XFS_ISTALE))
return lsn - 1;
return lsn; return lsn;
} }