From: "Amit K. Arora" <aarora@linux.vnet.ibm.com>
To: linux-fsdevel@vger.kernel.org
Subject: Protect update of b_transaction by j_list_lock
Date: Thu, 13 Sep 2007 17:58:07 +0530 [thread overview]
Message-ID: <20070913122807.GA21154@amitarora.in.ibm.com> (raw)
Hello,
At all other places, j_list_lock is being held when b_transaction is
being modified. But at one place in transaction.c, this lock is not
held. Also, the journal-head.h file which defines journal_head
structure says that b_transaction should be protected by two locks -
j_list_lock and jbd_lock_bh_state() :
/*
* Pointer to the compound transaction which owns this buffer's
* metadata: either the running transaction or the committing
* transaction (if there is one). Only applies to buffers on a
* transaction's data or metadata journaling list.
* [j_list_lock] [jbd_lock_bh_state()]
*/
transaction_t *b_transaction;
This was observed while debugging a problem of 'b_transaction' getting
corrupted. Here is the trace :
-----------------------------------------------------------------
[c00000000267bc50] d0000000000e0654 .__journal_refile_buffer+0x100/0x16c
[jbd]
[c00000000267bcf0] d0000000000e48bc
.journal_commit_transaction+0x136c/0x16e0
[jbd]
[c00000000267be00] d0000000000e9968 .kjournald+0xf0/0x2e8 [jbd]
[c00000000267bee0] c000000000080fb8 .kthread+0x128/0x178
[c00000000267bf90] c0000000000264bc .kernel_thread+0x4c/0x68
3:mon> e
cpu 0x3: Vector: 700 (Program Check) at [c00000000267b930]
pc: d0000000000e02b0: .__journal_file_buffer+0x74/0x2e0 [jbd]
lr: d0000000000e0654: .__journal_refile_buffer+0x100/0x16c [jbd]
sp: c00000000267bbb0
msr: 8000000000029032
current = 0xc00000000241d3d0
paca = 0xc000000000464900
pid = 16224, comm = kjournald
kernel BUG in __journal_file_buffer at fs/jbd/transaction.c:1951!
3:mon> r
R00 = 0000000000000001 R16 = 4000000001c00000
R01 = c00000000267bbb0 R17 = c000000000371060
R02 = d000000000102d20 R18 = 0000000000000000
R03 = c000000038b25208 R19 = 00000000002bf000
R04 = c0000001e6dfee80 R20 = c0000000027e4680
R05 = 0000000000000002 R21 = 0000000000000000
R06 = 0000000000000283 R22 = 0000000000000fdc
R07 = 0000000000000000 R23 = 0000000000000000
R08 = c000000000464900 R24 = c0000000e8ad5024
R09 = c0000001e6dfee80 R25 = c0000001b6b726e8
R10 = c000000038b25208 R26 = c000000001f5c780
R11 = 0000000000080000 R27 = 0000000000000002
R12 = 0000000024000048 R28 = c0000001e6dfee80
R13 = c000000000464900 R29 = c000000038b25208
R14 = 0000000000000000 R30 = d000000000101f80
R15 = c000000000372620 R31 = c000000163ce7908
pc = d0000000000e02b0 .__journal_file_buffer+0x74/0x2e0 [jbd]
lr = d0000000000e0654 .__journal_refile_buffer+0x100/0x16c [jbd]
msr = 8000000000029032 cr = 44000042
ctr = d0000000000e0170 xer = 0000000020000000 trap = 700
-----------------------------------------------------------------
The assertion is coming from following code (fs/jbd/transaction.c):
/*
* File a buffer on the given transaction list.
*/
void __journal_file_buffer(struct journal_head *jh,
transaction_t *transaction, int jlist)
{
struct journal_head **list = NULL;
int was_dirty = 0;
struct buffer_head *bh = jh2bh(jh);
J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh));
assert_spin_locked(&transaction->t_journal->j_list_lock);<==HERE
J_ASSERT_JH(jh, jh->b_jlist < BJ_Types);
J_ASSERT_JH(jh, jh->b_transaction == transaction ||
jh->b_transaction == 0);
:
:
It looks like the transaction pointer got corrupted. On code inspection,
I could find a place where b_transaction is being updated without
holding the j_list_lock. Tried to fix this in the following patch and
the bug is no longer being discovered. Please see if this change is
okay.
Following patch has been tested successfully for more than two weeks:
Signed-off-by: Amit Arora <aarora@in.ibm.com>
diff -Nuarp a/fs/jbd/transaction.c b/fs/jbd/transaction.c
--- a/fs/jbd/transaction.c 2007-08-10 15:34:08.000000000 +0530
+++ b/fs/jbd/transaction.c 2007-08-10 15:56:02.000000000 +0530
@@ -693,15 +693,15 @@ repeat:
* sure it doesn't get written to disk before the caller actually
* commits the new data
*/
+ spin_lock(&journal->j_list_lock);
if (!jh->b_transaction) {
JBUFFER_TRACE(jh, "no transaction");
J_ASSERT_JH(jh, !jh->b_next_transaction);
jh->b_transaction = transaction;
JBUFFER_TRACE(jh, "file as BJ_Reserved");
- spin_lock(&journal->j_list_lock);
__journal_file_buffer(jh, transaction, BJ_Reserved);
- spin_unlock(&journal->j_list_lock);
}
+ spin_unlock(&journal->j_list_lock);
done:
if (need_copy) {
reply other threads:[~2007-09-13 12:27 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20070913122807.GA21154@amitarora.in.ibm.com \
--to=aarora@linux.vnet.ibm.com \
--cc=linux-fsdevel@vger.kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).