Re: [PATCH] Change ll_rw_block() calls in JBD

[Zoltan Menyhart <Zoltan.Menyhart@bull.net>]

I have got some crashes due to:

Assertion failure in __journal_file_buffer():
      "jh->b_transaction == transaction || jh->b_transaction == 0"

[<a0000002053b44e0>] __journal_file_buffer+0x420/0x7c0 [jbd]
      r32 : e000000161a1f3e0  jh
      r33 : e00000010396a380  transaction
      r34 : 0000000000000008  jlist == BJ_Locked

*(struct journal_head *) 0xe000000161a1f3e0: // jh
{
 b_bh = 0xe00000048bb36930,
 b_jcount = 0x0,
 b_jlist = 0x1,
 b_modified = 0x0,
 b_frozen_data = 0x0,
 b_committed_data = 0x0,
 b_transaction = 0xe0000020014adb80,	// ->j_running_transaction
 b_next_transaction = 0x0,
 b_tnext = 0xe0000001c17306e0,
 b_tprev = 0xe00000204757e540,
 b_cp_transaction = 0x0,
 b_cpnext = 0x0,
 b_cpprev = 0x0
}

*(struct buffer_head *) 0xe00000048bb36930: // jh->b_bh
{
b_state = 0x8201d,
b_this_page = 0xe00000048bb33d88,
b_page = 0xa07ffffff9201300,
b_count = {
  counter = 0x2
},
b_size = 0x1000,
b_blocknr = 0xadc001,
b_data = 0xe000000492a0e000,
b_bdev = 0xe0000023fe1ca300,
b_end_io = 0xa000000100630be0,
b_private = 0xe000000161a1f3e0,
b_assoc_buffers = {
  next = 0xe00000048bb36978,
  prev = 0xe00000048bb36978
}

--- Called from --- :

journal_submit_data_buffers+0x200/0x660 [jbd]
      r32 : e0000001035ec100  journal
      r33 : e00000010396a380  commit_transaction

As you can see, the current "jh" has been stolen for the new
"->j_running_transaction" while we released temporarily "->j_list_lock"
in the middle of "journal_submit_data_buffers()".

Therefore the test "jh->b_jlist != BJ_SyncData", i.e. if it is still
on a (_any_) sync. list is not enough.

--- linux-2.6.16.20-orig/fs/jbd/commit.c	2006-06-20 17:19:47.000000000 +0200
+++ linux-2.6.16.20/fs/jbd/commit.c	2006-06-20 17:35:54.000000000 +0200
@@ -219,15 +219,26 @@
 				bufs = 0;
 				lock_buffer(bh);
 				spin_lock(&journal->j_list_lock);
+				/* Stolen (e.g. for a new transaction) ? */
+				if (jh != commit_transaction->t_sync_datalist) {
+					unlock_buffer(bh);
+					JBUFFER_TRACE(jh, "stolen sync. data");
+					put_bh(bh);
+					continue;
+				}
 				/* Someone already cleaned up the buffer? */
-				if (!buffer_jbd(bh)
-					|| jh->b_jlist != BJ_SyncData) {
+
+				// Can this happen???
+
+				if (!buffer_jbd(bh)) {
 					unlock_buffer(bh);
 					BUFFER_TRACE(bh, "already cleaned up");
 					put_bh(bh);
 					continue;
 				}
 				put_bh(bh);
+				J_ASSERT_JH(jh, jh->b_transaction ==
+							commit_transaction);
 			}
 			if (test_clear_buffer_dirty(bh)) {
 				BUFFER_TRACE(bh, "needs writeout, submitting");

I am not really sure that the test "!buffer_jbd(bh)" is really useful.
I left it alone for not introducing a new bug.
If you can confirm that it is not necessary, I can take it away.

Thanks,

Zoltan