Skip to content

Commit b41b46c

Browse files
Dave Chinnerdjwong
authored andcommitted
xfs: remove the m_active_trans counter
It's a global atomic counter, and we are hitting it at a rate of half a million transactions a second, so it's bouncing the counter cacheline all over the place on large machines. We don't actually need it anymore - it used to be required because the VFS freeze code could not track/prevent filesystem transactions that were running, but that problem no longer exists. Hence to remove the counter, we simply have to ensure that nothing calls xfs_sync_sb() while we are trying to quiesce the filesytem. That only happens if the log worker is still running when we call xfs_quiesce_attr(). The log worker is cancelled at the end of xfs_quiesce_attr() by calling xfs_log_quiesce(), so just call it early here and then we can remove the counter altogether. Concurrent create, 50 million inodes, identical 16p/16GB virtual machines on different physical hosts. Machine A has twice the CPU cores per socket of machine B: unpatched patched machine A: 3m16s 2m00s machine B: 4m04s 4m05s Create rates: unpatched patched machine A: 282k+/-31k 468k+/-21k machine B: 231k+/-8k 233k+/-11k Concurrent rm of same 50 million inodes: unpatched patched machine A: 6m42s 2m33s machine B: 4m47s 4m47s The transaction rate on the fast machine went from just under 300k/sec to 700k/sec, which indicates just how much of a bottleneck this atomic counter was. Signed-off-by: Dave Chinner <[email protected]> Reviewed-by: Darrick J. Wong <[email protected]> Signed-off-by: Darrick J. Wong <[email protected]>
1 parent b0dff46 commit b41b46c

File tree

3 files changed

+16
-29
lines changed

3 files changed

+16
-29
lines changed

fs/xfs/xfs_mount.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,6 @@ typedef struct xfs_mount {
176176
uint64_t m_resblks; /* total reserved blocks */
177177
uint64_t m_resblks_avail;/* available reserved blocks */
178178
uint64_t m_resblks_save; /* reserved blks @ remount,ro */
179-
atomic_t m_active_trans; /* number trans frozen */
180179
struct delayed_work m_reclaim_work; /* background inode reclaim */
181180
struct delayed_work m_eofblocks_work; /* background eof blocks
182181
trimming */

fs/xfs/xfs_super.c

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -874,18 +874,18 @@ xfs_restore_resvblks(struct xfs_mount *mp)
874874
* there is no log replay required to write the inodes to disk - this is the
875875
* primary difference between a sync and a quiesce.
876876
*
877-
* Note: xfs_log_quiesce() stops background log work - the callers must ensure
878-
* it is started again when appropriate.
877+
* We cancel log work early here to ensure all transactions the log worker may
878+
* run have finished before we clean up and log the superblock and write an
879+
* unmount record. The unfreeze process is responsible for restarting the log
880+
* worker correctly.
879881
*/
880882
void
881883
xfs_quiesce_attr(
882884
struct xfs_mount *mp)
883885
{
884886
int error = 0;
885887

886-
/* wait for all modifications to complete */
887-
while (atomic_read(&mp->m_active_trans) > 0)
888-
delay(100);
888+
cancel_delayed_work_sync(&mp->m_log->l_work);
889889

890890
/* force the log to unpin objects from the now complete transactions */
891891
xfs_log_force(mp, XFS_LOG_SYNC);
@@ -899,12 +899,6 @@ xfs_quiesce_attr(
899899
if (error)
900900
xfs_warn(mp, "xfs_attr_quiesce: failed to log sb changes. "
901901
"Frozen image may not be consistent.");
902-
/*
903-
* Just warn here till VFS can correctly support
904-
* read-only remount without racing.
905-
*/
906-
WARN_ON(atomic_read(&mp->m_active_trans) != 0);
907-
908902
xfs_log_quiesce(mp);
909903
}
910904

@@ -1793,7 +1787,6 @@ static int xfs_init_fs_context(
17931787
INIT_RADIX_TREE(&mp->m_perag_tree, GFP_ATOMIC);
17941788
spin_lock_init(&mp->m_perag_lock);
17951789
mutex_init(&mp->m_growlock);
1796-
atomic_set(&mp->m_active_trans, 0);
17971790
INIT_WORK(&mp->m_flush_inodes_work, xfs_flush_inodes_worker);
17981791
INIT_DELAYED_WORK(&mp->m_reclaim_work, xfs_reclaim_worker);
17991792
INIT_DELAYED_WORK(&mp->m_eofblocks_work, xfs_eofblocks_worker);

fs/xfs/xfs_trans.c

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ xfs_trans_free(
6868
xfs_extent_busy_clear(tp->t_mountp, &tp->t_busy, false);
6969

7070
trace_xfs_trans_free(tp, _RET_IP_);
71-
atomic_dec(&tp->t_mountp->m_active_trans);
7271
if (!(tp->t_flags & XFS_TRANS_NO_WRITECOUNT))
7372
sb_end_intwrite(tp->t_mountp->m_super);
7473
xfs_trans_free_dqinfo(tp);
@@ -125,8 +124,6 @@ xfs_trans_dup(
125124
xfs_defer_move(ntp, tp);
126125

127126
xfs_trans_dup_dqinfo(tp, ntp);
128-
129-
atomic_inc(&tp->t_mountp->m_active_trans);
130127
return ntp;
131128
}
132129

@@ -275,7 +272,6 @@ xfs_trans_alloc(
275272
*/
276273
WARN_ON(resp->tr_logres > 0 &&
277274
mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
278-
atomic_inc(&mp->m_active_trans);
279275

280276
tp->t_magic = XFS_TRANS_HEADER_MAGIC;
281277
tp->t_flags = flags;
@@ -299,20 +295,19 @@ xfs_trans_alloc(
299295

300296
/*
301297
* Create an empty transaction with no reservation. This is a defensive
302-
* mechanism for routines that query metadata without actually modifying
303-
* them -- if the metadata being queried is somehow cross-linked (think a
304-
* btree block pointer that points higher in the tree), we risk deadlock.
305-
* However, blocks grabbed as part of a transaction can be re-grabbed.
306-
* The verifiers will notice the corrupt block and the operation will fail
307-
* back to userspace without deadlocking.
298+
* mechanism for routines that query metadata without actually modifying them --
299+
* if the metadata being queried is somehow cross-linked (think a btree block
300+
* pointer that points higher in the tree), we risk deadlock. However, blocks
301+
* grabbed as part of a transaction can be re-grabbed. The verifiers will
302+
* notice the corrupt block and the operation will fail back to userspace
303+
* without deadlocking.
308304
*
309-
* Note the zero-length reservation; this transaction MUST be cancelled
310-
* without any dirty data.
305+
* Note the zero-length reservation; this transaction MUST be cancelled without
306+
* any dirty data.
311307
*
312-
* Callers should obtain freeze protection to avoid two conflicts with fs
313-
* freezing: (1) having active transactions trip the m_active_trans ASSERTs;
314-
* and (2) grabbing buffers at the same time that freeze is trying to drain
315-
* the buffer LRU list.
308+
* Callers should obtain freeze protection to avoid a conflict with fs freezing
309+
* where we can be grabbing buffers at the same time that freeze is trying to
310+
* drain the buffer LRU list.
316311
*/
317312
int
318313
xfs_trans_alloc_empty(

0 commit comments

Comments
 (0)