On Wed, Jul 14, 2021 at 01:36:56PM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@xxxxxxxxxx> > > 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 <dchinner@xxxxxxxxxx> 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.
