Skip to content

Commit 44d9b07

Browse files
author
Darrick J. Wong
committed
xfs: only run precommits once per transaction object
Committing a transaction tx0 with a defer ops chain of (A, B, C) creates a chain of transactions that looks like this: tx0 -> txA -> txB -> txC Prior to commit cb04211, __xfs_trans_commit would run precommits on tx0, then call xfs_defer_finish_noroll to convert A-C to tx[A-C]. Unfortunately, after the finish_noroll loop we forgot to run precommits on txC. That was fixed by adding the second precommit call. Unfortunately, none of us remembered that xfs_defer_finish_noroll calls __xfs_trans_commit a second time to commit tx0 before finishing work A in txA and committing that. In other words, we run precommits twice on tx0: xfs_trans_commit(tx0) __xfs_trans_commit(tx0, false) xfs_trans_run_precommits(tx0) xfs_defer_finish_noroll(tx0) xfs_trans_roll(tx0) txA = xfs_trans_dup(tx0) __xfs_trans_commit(tx0, true) xfs_trans_run_precommits(tx0) This currently isn't an issue because the inode item precommit is idempotent; the iunlink item precommit deletes itself so it can't be called again; and the buffer/dquot item precommits only check the incore objects for corruption. However, it doesn't make sense to run precommits twice. Fix this situation by only running precommits after finish_noroll. Cc: <stable@vger.kernel.org> # v6.4 Fixes: cb04211 ("xfs: defered work could create precommits") Signed-off-by: "Darrick J. Wong" <djwong@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de>
1 parent 53b001a commit 44d9b07

1 file changed

Lines changed: 4 additions & 12 deletions

File tree

fs/xfs/xfs_trans.c

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -860,13 +860,6 @@ __xfs_trans_commit(
860860

861861
trace_xfs_trans_commit(tp, _RET_IP_);
862862

863-
error = xfs_trans_run_precommits(tp);
864-
if (error) {
865-
if (tp->t_flags & XFS_TRANS_PERM_LOG_RES)
866-
xfs_defer_cancel(tp);
867-
goto out_unreserve;
868-
}
869-
870863
/*
871864
* Finish deferred items on final commit. Only permanent transactions
872865
* should ever have deferred ops.
@@ -877,13 +870,12 @@ __xfs_trans_commit(
877870
error = xfs_defer_finish_noroll(&tp);
878871
if (error)
879872
goto out_unreserve;
880-
881-
/* Run precommits from final tx in defer chain. */
882-
error = xfs_trans_run_precommits(tp);
883-
if (error)
884-
goto out_unreserve;
885873
}
886874

875+
error = xfs_trans_run_precommits(tp);
876+
if (error)
877+
goto out_unreserve;
878+
887879
/*
888880
* If there is nothing to be logged by the transaction,
889881
* then unlock all of the items associated with the

0 commit comments

Comments
 (0)