archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <>
To: Dave Chinner <>
Subject: Re: [PATCH 5/5] xfs: order CIL checkpoint start records
Date: Wed, 14 Jul 2021 07:34:00 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Wed, Jul 14, 2021 at 01:36:56PM +1000, Dave Chinner wrote:
> From: Dave Chinner <>
> Because log recovery depends on strictly ordered start records as
> well as strictly ordered commit records.
> This is a zero day bug in the way XFS writes pipelined transactions
> to the journal which is exposed by fixing the zero day bug that
> prevents the CIL from pipelining checkpoints. This re-introduces
> explicit concurrent commits back into the on-disk journal and hence
> out of order start records.
> The XFS journal commit code has never ordered start records and we
> have relied on strict commit record ordering for correct recovery
> ordering of concurrently written transactions. Unfortunately, root
> cause analysis uncovered the fact that log recovery uses the LSN of
> the start record for transaction commit processing. Hence, whilst
> the commits are processed in strict order by recovery, the LSNs
> associated with the commits can be out of order and so recovery may
> stamp incorrect LSNs into objects and/or misorder intents in the AIL
> for later processing. This can result in log recovery failures
> and/or on disk corruption, sometimes silent.
> Because this is a long standing log recovery issue, we can't just
> fix log recovery and call it good. This still leaves older kernels
> susceptible to recovery failures and corruption when replaying a log
> from a kernel that pipelines checkpoints. There is also the issue
> that in-memory ordering for AIL pushing and data integrity
> operations are based on checkpoint start LSNs, and if the start LSN
> is incorrect in the journal, it is also incorrect in memory.
> Hence there's really only one choice for fixing this zero-day bug:
> we need to strictly order checkpoint start records in ascending
> sequence order in the log, the same way we already strictly order
> commit records.
> Signed-off-by: Dave Chinner <>

I can't say I like the overloading of a mostly trivial function
with the record enum.  I think just two separate helpers would
be much more obvious.

But technically this looks fine.

  reply	other threads:[~2021-07-14  6:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-14  3:36 [PATCH 0/5 v2] xfs: strictly order log start records Dave Chinner
2021-07-14  3:36 ` [PATCH 1/5] xfs: move xlog_commit_record to xfs_log_cil.c Dave Chinner
2021-07-14  3:36 ` [PATCH 2/5] xfs: pass a CIL context to xlog_write() Dave Chinner
2021-07-14  3:36 ` [PATCH 3/5] xfs: factor out log write ordering from xlog_cil_push_work() Dave Chinner
2021-07-14  3:36 ` [PATCH 4/5] xfs: attached iclog callbacks in xlog_cil_set_ctx_write_state() Dave Chinner
2021-07-14  6:21   ` Christoph Hellwig
2021-07-14 22:36   ` Darrick J. Wong
2021-07-14  3:36 ` [PATCH 5/5] xfs: order CIL checkpoint start records Dave Chinner
2021-07-14  6:34   ` Christoph Hellwig [this message]
2021-07-14 22:39   ` Darrick J. Wong
2021-08-09 18:39 ` [PATCH 0/5 v2] xfs: strictly order log " Darrick J. Wong
  -- strict thread matches above, loose matches on Subject: below --
2021-08-10  5:21 [PATCH 0/5 v3] " Dave Chinner
2021-08-10  5:21 ` [PATCH 5/5] xfs: order CIL checkpoint " Dave Chinner
2021-08-12  7:49   ` Christoph Hellwig
2021-06-30  7:21 [PATCH 0/5] xfs: strictly order log " Dave Chinner
2021-06-30  7:21 ` [PATCH 5/5] xfs: order CIL checkpoint " 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:

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

  git send-email \ \ \ \ \

* 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).