From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/2] xfs: Lower CIL flush limit for large logs
Date: Mon, 16 Sep 2019 09:33:25 -0700 [thread overview]
Message-ID: <20190916163325.GZ2229799@magnolia> (raw)
In-Reply-To: <20190909015159.19662-2-david@fromorbit.com>
On Mon, Sep 09, 2019 at 11:51:58AM +1000, Dave Chinner wrote:
> 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.
It would be nice to capture at least some of this reasoning in the
already lengthy comment preceeding the #define....
> 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>
> ---
> fs/xfs/xfs_log_priv.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index b880c23cb6e4..187a43ffeaf7 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -329,7 +329,8 @@ struct xfs_cil {
> * enforced to ensure we stay within our maximum checkpoint size bounds.
> * threshold, yet give us plenty of space for aggregation on large logs.
...also, does XLOG_CIL_SPACE_LIMIT correspond to "a lower threshold at
which background pushing is attempted", or "a separate, higher bound"?
I think it's the first (????) but ... I don't know. The name made me
think it was the second, but the single use of the symbol suggests the
first. :)
--D
> */
> -#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.23.0.rc1
>
next prev parent reply other threads:[~2019-09-16 16:33 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-09 1:51 [RFC PATCH 0/2] xfs: hard limit background CIL push size Dave Chinner
2019-09-09 1:51 ` [PATCH 1/2] xfs: Lower CIL flush limit for large logs Dave Chinner
2019-09-16 16:33 ` Darrick J. Wong [this message]
2019-09-24 22:29 ` Dave Chinner
2019-09-25 12:08 ` Brian Foster
2019-09-27 22:47 ` Dave Chinner
2019-09-30 12:24 ` Brian Foster
2019-09-09 1:51 ` [PATCH 2/2] xfs: hard limit the background CIL push Dave Chinner
2019-09-16 16:42 ` Darrick J. Wong
2019-09-24 22:36 ` Dave Chinner
2019-09-24 22:41 ` Darrick J. Wong
2019-09-30 6:03 [PATCH v2 0/2] xfs: limit CIL push sizes Dave Chinner
2019-09-30 6:03 ` [PATCH 1/2] xfs: Lower CIL flush limit for large logs Dave Chinner
2019-09-30 16:55 ` Brian Foster
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=20190916163325.GZ2229799@magnolia \
--to=darrick.wong@oracle.com \
--cc=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).