All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joel Becker <Joel.Becker@oracle.com>
To: Theodore Tso <tytso@mit.edu>,
	ocfs2-devel@oss.oracle.com, linux-ext4@vger.kernel.org,
	mfasheh@suse.com
Subject: Re: [Ocfs2-devel] [PATCH] [RFC] jbd2: Add buffer triggers
Date: Wed, 8 Oct 2008 16:17:52 -0700	[thread overview]
Message-ID: <20081008231752.GA5310@mail.oracle.com> (raw)
In-Reply-To: <20081007010154.GE26632@mail.oracle.com>

On Mon, Oct 06, 2008 at 06:01:54PM -0700, Joel Becker wrote:
> On Mon, Oct 06, 2008 at 07:32:48PM -0400, Theodore Tso wrote:
> > On Mon, Oct 06, 2008 at 02:42:52PM -0700, Joel Becker wrote:
> > I'm not 100% sure.....  The other area that we should check very
> > closely is jbd2_journal_commit_transaction(); in some cases, if
> > jh->b_committed_data is NULL, the frozen data is thrown away (around
> > line 850 in transaction.c).  I *think* this happens if b_frozen_data
> > was only copied to escape the buffer, but I'm not certain; in any
> > case, there's a potential that in that case you might lose the
> > calculated checksum and the correct value wouldn't get written to the
> > final location on disk.  
<snip>
> 	Looking at the checkpoint part, though, I think we're not safe.
> The buffer is attached to the original transaction's checkpoint list
> after the commit.  This buffer has the un-checksummed b_data.  If the
> later transaction commits before the checkpoint happens, all is good.
> But if the buffer lazily writes to disk while the later transaction is
> still running, the original transaction could be considered "done",
> updating the journal superblock.  If we crash at that moment, we have a
> bad checksum on disk.

	I chatted with Mark some about this today.  He pointed out that,
logically, b_data can't be checkpointed until its data isn't in the
journal - it might be newer than the most recent transaction.  So I
looked in the checkpoint code to see where this is handled, and the
checkpoint code simply forces a commit new enough to encompass b_data
when it wants to put that block to disk.
	In other words, I think that the commit trigger is safe in all
circumstances once moved up in journal_commit_transaction().  I'll be
cooking that up shortly.

Joel

-- 

To spot the expert, pick the one who predicts the job will take the
longest and cost the most.

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

WARNING: multiple messages have this Message-ID (diff)
From: Joel Becker <Joel.Becker@oracle.com>
To: Theodore Tso <tytso@mit.edu>,
	ocfs2-devel@oss.oracle.com, linux-ext4@vger.kernel.org,
	mfasheh@suse.com
Subject: [Ocfs2-devel] [PATCH] [RFC] jbd2: Add buffer triggers
Date: Wed, 8 Oct 2008 16:17:52 -0700	[thread overview]
Message-ID: <20081008231752.GA5310@mail.oracle.com> (raw)
In-Reply-To: <20081007010154.GE26632@mail.oracle.com>

On Mon, Oct 06, 2008 at 06:01:54PM -0700, Joel Becker wrote:
> On Mon, Oct 06, 2008 at 07:32:48PM -0400, Theodore Tso wrote:
> > On Mon, Oct 06, 2008 at 02:42:52PM -0700, Joel Becker wrote:
> > I'm not 100% sure.....  The other area that we should check very
> > closely is jbd2_journal_commit_transaction(); in some cases, if
> > jh->b_committed_data is NULL, the frozen data is thrown away (around
> > line 850 in transaction.c).  I *think* this happens if b_frozen_data
> > was only copied to escape the buffer, but I'm not certain; in any
> > case, there's a potential that in that case you might lose the
> > calculated checksum and the correct value wouldn't get written to the
> > final location on disk.  
<snip>
> 	Looking at the checkpoint part, though, I think we're not safe.
> The buffer is attached to the original transaction's checkpoint list
> after the commit.  This buffer has the un-checksummed b_data.  If the
> later transaction commits before the checkpoint happens, all is good.
> But if the buffer lazily writes to disk while the later transaction is
> still running, the original transaction could be considered "done",
> updating the journal superblock.  If we crash at that moment, we have a
> bad checksum on disk.

	I chatted with Mark some about this today.  He pointed out that,
logically, b_data can't be checkpointed until its data isn't in the
journal - it might be newer than the most recent transaction.  So I
looked in the checkpoint code to see where this is handled, and the
checkpoint code simply forces a commit new enough to encompass b_data
when it wants to put that block to disk.
	In other words, I think that the commit trigger is safe in all
circumstances once moved up in journal_commit_transaction().  I'll be
cooking that up shortly.

Joel

-- 

To spot the expert, pick the one who predicts the job will take the
longest and cost the most.

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

  reply	other threads:[~2008-10-08 23:18 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-17 23:26 [PATCH] [RFC] jbd2: Add buffer triggers Joel Becker
2008-09-17 23:26 ` [Ocfs2-devel] " Joel Becker
2008-09-17 23:27 ` [PATCH] ocfs2: Use the new jbd_journal_set_triggers() to printk Joel Becker
2008-09-17 23:27   ` [Ocfs2-devel] " Joel Becker
2008-09-19 21:46 ` [PATCH] [RFC] jbd2: Add buffer triggers Andreas Dilger
2008-09-19 21:46   ` [Ocfs2-devel] " Andreas Dilger
2008-09-20  3:36   ` Joel Becker
2008-09-20  3:36     ` Joel Becker
2008-09-29  1:25 ` Theodore Tso
2008-09-29  1:25   ` [Ocfs2-devel] " Theodore Tso
2008-10-04  0:03   ` Theodore Tso
2008-10-04  0:03     ` [Ocfs2-devel] " Theodore Tso
2008-10-06 21:37     ` Joel Becker
2008-10-06 21:37       ` [Ocfs2-devel] " Joel Becker
2008-10-06 21:42       ` Joel Becker
2008-10-06 21:42         ` [Ocfs2-devel] " Joel Becker
2008-10-06 23:32         ` Theodore Tso
2008-10-06 23:32           ` [Ocfs2-devel] " Theodore Tso
2008-10-07  1:01           ` Joel Becker
2008-10-07  1:01             ` Joel Becker
2008-10-08 23:17             ` Joel Becker [this message]
2008-10-08 23:17               ` Joel Becker
2008-10-16 17:42               ` Theodore Tso
2008-10-16 17:42                 ` Theodore Tso
2008-10-16 19:40                 ` Joel Becker
2008-10-16 19:40                   ` Joel Becker
2008-10-16 19:44                   ` Joel Becker
2008-10-16 19:44                     ` [Ocfs2-devel] " Joel Becker
2008-10-17 12:28                   ` Theodore Tso
2008-10-17 12:28                     ` Theodore Tso
2008-10-17 17:11                     ` Mark Fasheh
2008-10-17 17:11                       ` Mark Fasheh
2008-10-07  1:04           ` Joel Becker
2008-10-07  1:04             ` Joel Becker

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=20081008231752.GA5310@mail.oracle.com \
    --to=joel.becker@oracle.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=mfasheh@suse.com \
    --cc=ocfs2-devel@oss.oracle.com \
    --cc=tytso@mit.edu \
    /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.