The xfs_trans context should be active after it is allocated, and deactive when it is freed. The rolling transaction should be specially considered, because in the case when we clear the old transaction the thread's NOFS state shouldn't be changed, as a result we have to set NOFS in the old transaction's t_pflags in xfs_trans_context_swap(). So these helpers are refactored as, - xfs_trans_context_set() Used in xfs_trans_alloc() - xfs_trans_context_clear() Used in xfs_trans_free() And a new helper is instroduced to handle the rolling transaction, - xfs_trans_context_swap() Used in rolling transaction This patch is based on Darrick's work to fix the issue in xfs/141 in the earlier version. [1] 1. https://lore.kernel.org/linux-xfs/20201104001649.GN7123@magnolia Cc: Darrick J. Wong <darrick.wong@xxxxxxxxxx> Cc: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx> Cc: Christoph Hellwig <hch@xxxxxx> Cc: Dave Chinner <david@xxxxxxxxxxxxx> Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx> --- fs/xfs/xfs_trans.c | 25 +++++++++++-------------- fs/xfs/xfs_trans.h | 13 +++++++++++++ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 11d390f0d3f2..aa213c3e2408 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -67,6 +67,10 @@ xfs_trans_free( xfs_extent_busy_sort(&tp->t_busy); xfs_extent_busy_clear(tp->t_mountp, &tp->t_busy, false); + + /* Detach the transaction from this thread. */ + xfs_trans_context_clear(tp); + trace_xfs_trans_free(tp, _RET_IP_); if (!(tp->t_flags & XFS_TRANS_NO_WRITECOUNT)) sb_end_intwrite(tp->t_mountp->m_super); @@ -119,7 +123,9 @@ xfs_trans_dup( ntp->t_rtx_res = tp->t_rtx_res - tp->t_rtx_res_used; tp->t_rtx_res = tp->t_rtx_res_used; - ntp->t_pflags = tp->t_pflags; + + /* Associate the new transaction with this thread. */ + xfs_trans_context_swap(tp, ntp); /* move deferred ops over to the new tp */ xfs_defer_move(ntp, tp); @@ -153,9 +159,6 @@ xfs_trans_reserve( int error = 0; bool rsvd = (tp->t_flags & XFS_TRANS_RESERVE) != 0; - /* Mark this thread as being in a transaction */ - xfs_trans_context_set(tp); - /* * Attempt to reserve the needed disk blocks by decrementing * the number needed from the number available. This will @@ -163,10 +166,9 @@ xfs_trans_reserve( */ if (blocks > 0) { error = xfs_mod_fdblocks(mp, -((int64_t)blocks), rsvd); - if (error != 0) { - xfs_trans_context_clear(tp); + if (error != 0) return -ENOSPC; - } + tp->t_blk_res += blocks; } @@ -241,8 +243,6 @@ xfs_trans_reserve( tp->t_blk_res = 0; } - xfs_trans_context_clear(tp); - return error; } @@ -284,6 +284,8 @@ xfs_trans_alloc( INIT_LIST_HEAD(&tp->t_dfops); tp->t_firstblock = NULLFSBLOCK; + /* Mark this thread as being in a transaction */ + xfs_trans_context_set(tp); error = xfs_trans_reserve(tp, resp, blocks, rtextents); if (error) { xfs_trans_cancel(tp); @@ -878,7 +880,6 @@ __xfs_trans_commit( xfs_log_commit_cil(mp, tp, &commit_lsn, regrant); - xfs_trans_context_clear(tp); xfs_trans_free(tp); /* @@ -911,7 +912,6 @@ __xfs_trans_commit( tp->t_ticket = NULL; } - xfs_trans_context_clear(tp); xfs_trans_free_items(tp, !!error); xfs_trans_free(tp); @@ -971,9 +971,6 @@ xfs_trans_cancel( tp->t_ticket = NULL; } - /* mark this thread as no longer being in a transaction */ - xfs_trans_context_clear(tp); - xfs_trans_free_items(tp, dirty); xfs_trans_free(tp); } diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h index 44b11c64a15e..12380eaaf7ce 100644 --- a/fs/xfs/xfs_trans.h +++ b/fs/xfs/xfs_trans.h @@ -280,4 +280,17 @@ xfs_trans_context_clear(struct xfs_trans *tp) memalloc_nofs_restore(tp->t_pflags); } +static inline void +xfs_trans_context_swap(struct xfs_trans *tp, struct xfs_trans *ntp) +{ + ntp->t_pflags = tp->t_pflags; + /* + * For the rolling transaction, we have to set NOFS in the old + * transaction's t_pflags so that when we clear the context on + * the old transaction we don't actually change the thread's NOFS + * state. + */ + tp->t_pflags = current->flags | PF_MEMALLOC_NOFS; +} + #endif /* __XFS_TRANS_H__ */ -- 2.18.4
