Commit 3d4b4a3e authored by Brian Foster's avatar Brian Foster Committed by Darrick J. Wong

xfs: remove bli from AIL before release on transaction abort

When a buffer is modified, logged and committed, it ultimately ends
up sitting on the AIL with a dirty bli waiting for metadata
writeback. If another transaction locks and invalidates the buffer
(freeing an inode chunk, for example) in the meantime, the bli is
flagged as stale, the dirty state is cleared and the bli remains in
the AIL.

If a shutdown occurs before the transaction that has invalidated the
buffer is committed, the transaction is ultimately aborted. The log
items are flagged as such and ->iop_unlock() handles the aborted
items. Because the bli is clean (due to the invalidation),
->iop_unlock() unconditionally releases it. The log item may still
reside in the AIL, however, which means the I/O completion handler
may still run and attempt to access it. This results in assert
failure due to the release of the bli while still present in the AIL
and a subsequent NULL dereference and panic in the buffer I/O
completion handling. This can be reproduced by running generic/388
in repetition.

To avoid this problem, update xfs_buf_item_unlock() to first check
whether the bli is aborted and if so, remove it from the AIL before
it is released. This ensures that the bli is no longer accessed
during the shutdown sequence after it has been freed.
Signed-off-by: default avatarBrian Foster <bfoster@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarCarlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent 79e641ce
...@@ -636,20 +636,23 @@ xfs_buf_item_unlock( ...@@ -636,20 +636,23 @@ xfs_buf_item_unlock(
/* /*
* Clean buffers, by definition, cannot be in the AIL. However, aborted * Clean buffers, by definition, cannot be in the AIL. However, aborted
* buffers may be dirty and hence in the AIL. Therefore if we are * buffers may be in the AIL regardless of dirty state. An aborted
* aborting a buffer and we've just taken the last refernce away, we * transaction that invalidates a buffer already in the AIL may have
* have to check if it is in the AIL before freeing it. We need to free * marked it stale and cleared the dirty state, for example.
* it in this case, because an aborted transaction has already shut the *
* filesystem down and this is the last chance we will have to do so. * Therefore if we are aborting a buffer and we've just taken the last
* reference away, we have to check if it is in the AIL before freeing
* it. We need to free it in this case, because an aborted transaction
* has already shut the filesystem down and this is the last chance we
* will have to do so.
*/ */
if (atomic_dec_and_test(&bip->bli_refcount)) { if (atomic_dec_and_test(&bip->bli_refcount)) {
if (clean) if (aborted) {
xfs_buf_item_relse(bp);
else if (aborted) {
ASSERT(XFS_FORCED_SHUTDOWN(lip->li_mountp)); ASSERT(XFS_FORCED_SHUTDOWN(lip->li_mountp));
xfs_trans_ail_remove(lip, SHUTDOWN_LOG_IO_ERROR); xfs_trans_ail_remove(lip, SHUTDOWN_LOG_IO_ERROR);
xfs_buf_item_relse(bp); xfs_buf_item_relse(bp);
} } else if (clean)
xfs_buf_item_relse(bp);
} }
if (!(flags & XFS_BLI_HOLD)) if (!(flags & XFS_BLI_HOLD))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment