Protect update of b_transaction by j_list_lock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>

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) {
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux