From: Dave Chinner <david@fromorbit.com>
To: Ben Myers <bpm@sgi.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 14/14] xfs: only update the last_sync_lsn when a transaction completes
Date: Thu, 11 Oct 2012 09:57:00 +1100 [thread overview]
Message-ID: <20121010225700.GY23644@dastard> (raw)
In-Reply-To: <20121010142841.GV13214@sgi.com>
On Wed, Oct 10, 2012 at 09:28:41AM -0500, Ben Myers wrote:
> Hey Dave,
>
> On Mon, Oct 08, 2012 at 09:56:12PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > The log write code stamps each iclog with the current tail LSN in
> > the iclog header so that recovery knows where to find the tail of
> > thelog once it has found the head. Normally this is taken from the
> > first item on the AIL - the log item that corresponds to the oldest
> > active item in the log.
> >
> > The problem is that when the AIL is empty, the tail lsn is dervied
> > from the the l_last_sync_lsn, which is the LSN of the last iclog to
> > be written to the log. In most cases this doesn't happen, because
> > the AIL is rarely empty on an active filesystem. However, when it
> > does, it opens up an interesting case when the transaction being
> > committed to the iclog spans multiple iclogs.
> >
> > That is, the first iclog is stamped with the l_last_sync_lsn, and IO
> > is issued. Then the next iclog is setup, the changes copied into the
> > iclog (takes some time), and then the l_last_sync_lsn is stamped
> > into the header and IO is issued. This is still the same
> > transaction, so the tail lsn of both iclogs must be the same for log
> > recovery to find the entire transaction to be able to replay it.
> >
> > The problem arises in that the iclog buffer IO completion updates
> > the l_last_sync_lsn with it's own LSN. Therefore, If the first iclog
> > completes it's IO before the second iclog is filled and has the tail
> > lsn stamped in it, it will stamp the LSN of the first iclog into
> > it's tail lsn field. If the system fails at this point, log recovery
> > will not see a complete transaction, so the transaction will no be
> > replayed.
> >
> > The fix is simple - the l_last_sync_lsn is updated when a iclog
> > buffer IO completes, and this is incorrect. The l_last_sync_lsn
> > shoul dbe updated when a transaction is completed by a iclog buffer
> > IO. That is, only iclog buffers that have transaction commit
> > callbacks attached to them should update the l_last_sync_lsn. This
> > means that the last_sync_lsn will only move forward when a commit
> > record it written, not in the middle of a large transaction that is
> > rolling through multiple iclog buffers.
> >
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > Reviewed-by: Mark Tinguely <tinguely@sgi.com>
>
> I think this one is appropriate for the 3.7 release. What say you?
Yes, if you want. It's not a common or high risk problem without the
preceeding changes, though, so I'm not concerned if it stays with
this patchset....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2012-10-10 22:55 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-08 10:55 [PATCH 00/14 V5]: xfs: remove the xfssyncd mess Dave Chinner
2012-10-08 10:55 ` [PATCH 01/14] xfs: xfs_syncd_stop must die Dave Chinner
2012-10-08 10:56 ` [PATCH 02/14] xfs: rationalise xfs_mount_wq users Dave Chinner
2012-10-08 10:56 ` [PATCH 03/14] xfs: don't run the sync work if the filesystem is read-only Dave Chinner
2012-10-08 10:56 ` [PATCH 04/14] xfs: sync work is now only periodic log work Dave Chinner
2012-10-08 10:56 ` [PATCH 05/14] xfs: Bring some sanity to log unmounting Dave Chinner
2012-10-08 10:56 ` [PATCH 06/14] xfs: xfs_sync_data is redundant Dave Chinner
2012-10-09 21:01 ` Ben Myers
2012-10-09 21:28 ` Dave Chinner
2012-10-09 21:41 ` Ben Myers
2012-10-09 22:23 ` Brian Foster
2012-10-09 22:54 ` Dave Chinner
2012-10-08 10:56 ` [PATCH 07/14] xfs: syncd workqueue is no more Dave Chinner
2012-10-08 10:56 ` [PATCH 08/14] xfs: xfs_sync_fsdata is redundant Dave Chinner
2012-10-08 10:56 ` [PATCH 09/14] xfs: move xfs_quiesce_attr() into xfs_super.c Dave Chinner
2012-10-08 10:56 ` [PATCH 10/14] xfs: xfs_quiesce_attr() should quiesce the log like unmount Dave Chinner
2012-10-08 10:56 ` [PATCH 11/14] xfs: rename xfs_sync.[ch] to xfs_icache.[ch] Dave Chinner
2012-10-08 10:56 ` [PATCH 12/14] xfs: move inode locking functions to xfs_inode.c Dave Chinner
2012-10-08 10:56 ` [PATCH 13/14] xfs: remove xfs_iget.c Dave Chinner
2012-10-08 10:56 ` [PATCH 14/14] xfs: only update the last_sync_lsn when a transaction completes Dave Chinner
2012-10-10 14:28 ` Ben Myers
2012-10-10 22:57 ` Dave Chinner [this message]
2012-10-08 22:39 ` [PATCH 00/14 V5]: xfs: remove the xfssyncd mess Ben Myers
2012-10-11 2:31 ` Ben Myers
2012-10-11 5:03 ` Dave Chinner
2012-10-16 18:19 ` Ben Myers
2012-10-10 21:33 ` Christoph Hellwig
2012-10-11 1:51 ` Ben Myers
2012-10-16 19:19 ` Ben Myers
2012-10-17 19:24 ` Ben Myers
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=20121010225700.GY23644@dastard \
--to=david@fromorbit.com \
--cc=bpm@sgi.com \
--cc=xfs@oss.sgi.com \
/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