Skip to content

Commit 47a6df7

Browse files
author
Darrick J. Wong
committed
xfs: shut down filesystem if we xfs_trans_cancel with deferred work items
While debugging some very strange rmap corruption reports in connection with the online directory repair code. I root-caused the error to the following incorrect sequence: <start repair transaction> <expand directory, causing a deferred rmap to be queued> <roll transaction> <cancel transaction> Obviously, we should have committed the transaction instead of cancelling it. Thinking more broadly, however, xfs_trans_cancel should have warned us that we were throwing away work item that we already committed to performing. This is not correct, and we need to shut down the filesystem. Change xfs_trans_cancel to complain in the loudest manner if we're cancelling any transaction with deferred work items attached. Signed-off-by: Darrick J. Wong <[email protected]> Reviewed-by: Dave Chinner <[email protected]>
1 parent 2585cf9 commit 47a6df7

File tree

1 file changed

+10
-1
lines changed

1 file changed

+10
-1
lines changed

fs/xfs/xfs_trans.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -942,8 +942,17 @@ xfs_trans_cancel(
942942

943943
trace_xfs_trans_cancel(tp, _RET_IP_);
944944

945-
if (tp->t_flags & XFS_TRANS_PERM_LOG_RES)
945+
/*
946+
* It's never valid to cancel a transaction with deferred ops attached,
947+
* because the transaction is effectively dirty. Complain about this
948+
* loudly before freeing the in-memory defer items.
949+
*/
950+
if (!list_empty(&tp->t_dfops)) {
951+
ASSERT(xfs_is_shutdown(mp) || list_empty(&tp->t_dfops));
952+
ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
953+
dirty = true;
946954
xfs_defer_cancel(tp);
955+
}
947956

948957
/*
949958
* See if the caller is relying on us to shut down the

0 commit comments

Comments
 (0)