From: Ben Myers <bpm@sgi.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 14/14] xfs: only update the last_sync_lsn when a transaction completes
Date: Wed, 10 Oct 2012 09:28:41 -0500 [thread overview]
Message-ID: <20121010142841.GV13214@sgi.com> (raw)
In-Reply-To: <1349693772-8064-15-git-send-email-david@fromorbit.com>
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?
-Ben
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2012-10-10 14:27 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 [this message]
2012-10-10 22:57 ` Dave Chinner
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=20121010142841.GV13214@sgi.com \
--to=bpm@sgi.com \
--cc=david@fromorbit.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