LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Theodore Tso <tytso@mit.edu>
To: Eric Sandeen <sandeen@redhat.com>
Cc: Eric Paris <eparis@redhat.com>,
	linux-kernel@vger.kernel.org, linux-ext4@vger.kernel.org
Subject: Re: general protection fault:  from release_blocks_on_commit
Date: Mon, 27 Oct 2008 21:52:28 -0400	[thread overview]
Message-ID: <20081028015228.GA8320@mit.edu> (raw)
In-Reply-To: <49065805.1080303@redhat.com>

On Mon, Oct 27, 2008 at 07:08:37PM -0500, Eric Sandeen wrote:
> Theodore Tso wrote:
> > On Mon, Oct 27, 2008 at 05:26:47PM -0500, Eric Sandeen wrote:
> >> Ted, you probably need some slab debugging on to hit it.
> > 
> > I had slab debugging enabled, but haven't been able to replicate it
> > yet.  I'll do some more work to try to replicate it.
> > 
> >> I think the problem is that jbd2_journal_commit_transaction may call
> >> __jbd2_journal_drop_transaction(journal, commit_transaction) if the
> >> checkpoint lists are NULL, and this frees the commit_transaction.
> > 
> > I think you're right.  I would probably change the patch around so
> > that after calling __jbd2_jurnal_drop_transaction(), we set
> > commit_transaction to NULL, and then adding an "if
> > (commit_transaction)" to the lines in questions; that way we keep the
> > commit callback outside of the j_list_lock() spinlock.
> 
> Ok, I thought about that; sounds good.  will resend.

I looked at this some more, and at least in theory it could happen
that we could not have any buffers that need to be checkpointed (so
t_checkpoint_list and t_checkpoint_io_list are NULL), but there are
still blocks to be released (or some other users of the jbd2 layer
still wants to have the callback be called).  So I'm currently testing
this patch (see below).

							- Ted

diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index 8b119e1..ebc667b 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -974,6 +974,9 @@ restart_loop:
 	journal->j_committing_transaction = NULL;
 	spin_unlock(&journal->j_state_lock);
 
+	if (journal->j_commit_callback)
+		journal->j_commit_callback(journal, commit_transaction);
+
 	if (commit_transaction->t_checkpoint_list == NULL &&
 	    commit_transaction->t_checkpoint_io_list == NULL) {
 		__jbd2_journal_drop_transaction(journal, commit_transaction);
@@ -995,11 +998,8 @@ restart_loop:
 	}
 	spin_unlock(&journal->j_list_lock);
 
-	if (journal->j_commit_callback)
-		journal->j_commit_callback(journal, commit_transaction);
-
 	trace_mark(jbd2_end_commit, "dev %s transaction %d head %d",
-		   journal->j_devname, commit_transaction->t_tid,
+		   journal->j_devname, journal->j_commit_sequence,
 		   journal->j_tail_sequence);
 	jbd_debug(1, "JBD: commit %d complete, head %d\n",
 		  journal->j_commit_sequence, journal->j_tail_sequence);

  reply	other threads:[~2008-10-28  1:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-21 18:03 Eric Paris
2008-10-27 18:19 ` Theodore Tso
2008-10-27 22:26   ` Eric Sandeen
2008-10-27 23:28     ` Theodore Tso
2008-10-28  0:08       ` Eric Sandeen
2008-10-28  1:52         ` Theodore Tso [this message]
2008-10-28  2:15           ` Eric Sandeen
2008-10-28  2:28           ` [PATCH, RFC] jbd2: Call the commit callback before the transaction could get dropped Theodore Tso
2008-10-28  2:41             ` Eric Sandeen
2008-10-28 13:46               ` Eric Paris

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=20081028015228.GA8320@mit.edu \
    --to=tytso@mit.edu \
    --cc=eparis@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sandeen@redhat.com \
    --subject='Re: general protection fault:  from release_blocks_on_commit' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).