linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 1/8] xfs: Lower CIL flush limit for large logs
Date: Wed, 25 Mar 2020 12:41:58 +1100	[thread overview]
Message-ID: <20200325014205.11843-2-david@fromorbit.com> (raw)
In-Reply-To: <20200325014205.11843-1-david@fromorbit.com>

From: Dave Chinner <dchinner@redhat.com>

The current CIL size aggregation limit is 1/8th the log size. This
means for large logs we might be aggregating at least 250MB of dirty objects
in memory before the CIL is flushed to the journal. With CIL shadow
buffers sitting around, this means the CIL is often consuming >500MB
of temporary memory that is all allocated under GFP_NOFS conditions.

Flushing the CIL can take some time to do if there is other IO
ongoing, and can introduce substantial log force latency by itself.
It also pins the memory until the objects are in the AIL and can be
written back and reclaimed by shrinkers. Hence this threshold also
tends to determine the minimum amount of memory XFS can operate in
under heavy modification without triggering the OOM killer.

Modify the CIL space limit to prevent such huge amounts of pinned
metadata from aggregating. We can have 2MB of log IO in flight at
once, so limit aggregation to 16x this size. This threshold was
chosen as it little impact on performance (on 16-way fsmark) or log
traffic but pins a lot less memory on large logs especially under
heavy memory pressure.  An aggregation limit of 8x had 5-10%
performance degradation and a 50% increase in log throughput for
the same workload, so clearly that was too small for highly
concurrent workloads on large logs.

This was found via trace analysis of AIL behaviour. e.g. insertion
from a single CIL flush:

xfs_ail_insert: old lsn 0/0 new lsn 1/3033090 type XFS_LI_INODE flags IN_AIL

$ grep xfs_ail_insert /mnt/scratch/s.t |grep "new lsn 1/3033090" |wc -l
1721823
$

So there were 1.7 million objects inserted into the AIL from this
CIL checkpoint, the first at 2323.392108, the last at 2325.667566 which
was the end of the trace (i.e. it hadn't finished). Clearly a major
problem.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_log_priv.h | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index cfcf3f02e30a3..8c4be91f62d0d 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -316,13 +316,30 @@ struct xfs_cil {
  * tries to keep 25% of the log free, so we need to keep below that limit or we
  * risk running out of free log space to start any new transactions.
  *
- * In order to keep background CIL push efficient, we will set a lower
- * threshold at which background pushing is attempted without blocking current
- * transaction commits.  A separate, higher bound defines when CIL pushes are
- * enforced to ensure we stay within our maximum checkpoint size bounds.
- * threshold, yet give us plenty of space for aggregation on large logs.
+ * In order to keep background CIL push efficient, we only need to ensure the
+ * CIL is large enough to maintain sufficient in-memory relogging to avoid
+ * repeated physical writes of frequently modified metadata. If we allow the CIL
+ * to grow to a substantial fraction of the log, then we may be pinning hundreds
+ * of megabytes of metadata in memory until the CIL flushes. This can cause
+ * issues when we are running low on memory - pinned memory cannot be reclaimed,
+ * and the CIL consumes a lot of memory. Hence we need to set an upper physical
+ * size limit for the CIL that limits the maximum amount of memory pinned by the
+ * CIL but does not limit performance by reducing relogging efficiency
+ * significantly.
+ *
+ * As such, the CIL push threshold ends up being the smaller of two thresholds:
+ * - a threshold large enough that it allows CIL to be pushed and progress to be
+ *   made without excessive blocking of incoming transaction commits. This is
+ *   defined to be 12.5% of the log space - half the 25% push threshold of the
+ *   AIL.
+ * - small enough that it doesn't pin excessive amounts of memory but maintains
+ *   close to peak relogging efficiency. This is defined to be 16x the iclog
+ *   buffer window (32MB) as measurements have shown this to be roughly the
+ *   point of diminishing performance increases under highly concurrent
+ *   modification workloads.
  */
-#define XLOG_CIL_SPACE_LIMIT(log)	(log->l_logsize >> 3)
+#define XLOG_CIL_SPACE_LIMIT(log)	\
+	min_t(int, (log)->l_logsize >> 3, BBTOB(XLOG_TOTAL_REC_SHIFT(log)) << 4)
 
 /*
  * ticket grant locks, queues and accounting have their own cachlines
-- 
2.26.0.rc2


  reply	other threads:[~2020-03-25  1:42 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25  1:41 [PATCH 0/8] xfs: various fixes and cleanups Dave Chinner
2020-03-25  1:41 ` Dave Chinner [this message]
2020-03-25  4:42   ` [PATCH 1/8] xfs: Lower CIL flush limit for large logs Allison Collins
2020-03-25  1:41 ` [PATCH 2/8] xfs: Throttle commits on delayed background CIL push Dave Chinner
2020-03-25  4:42   ` Allison Collins
2020-03-25  5:07   ` Dave Chinner
2020-03-26  5:24   ` Darrick J. Wong
2020-03-26 11:33     ` Brian Foster
2020-03-27  0:40       ` Dave Chinner
2020-03-25  1:42 ` [PATCH 3/8] xfs: don't allow log IO to be throttled Dave Chinner
2020-03-25  4:42   ` Allison Collins
2020-03-25  1:42 ` [PATCH 4/8] xfs: Improve metadata buffer reclaim accountability Dave Chinner
2020-03-25  4:42   ` Allison Collins
2020-03-25 13:30   ` Brian Foster
2020-03-26  5:05   ` Darrick J. Wong
2020-03-25  1:42 ` [PATCH 5/8] xfs: correctly acount for reclaimable slabs Dave Chinner
2020-03-25  4:43   ` Allison Collins
2020-03-25  1:42 ` [PATCH 6/8] xfs: factor common AIL item deletion code Dave Chinner
2020-03-25  4:54   ` Allison Collins
2020-03-25 13:30   ` Brian Foster
2020-03-26  5:10   ` Darrick J. Wong
2020-03-27  0:50     ` Dave Chinner
2020-03-25  1:42 ` [PATCH 7/8] xfs: tail updates only need to occur when LSN changes Dave Chinner
2020-03-25  5:10   ` Allison Collins
2020-03-26  5:14   ` Darrick J. Wong
2020-03-25  1:42 ` [PATCH 8/8] xfs: factor inode lookup from xfs_ifree_cluster Dave Chinner
2020-03-25 13:30   ` Brian Foster
2020-03-25  1:51 ` [PATCH 0/8] xfs: various fixes and cleanups Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200325014205.11843-2-david@fromorbit.com \
    --to=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).