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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.