[PATCH] JBD: implement j_committing_transaction locking

Go through all sites which use j_committing_transaction and ensure that the
deisgned locking is correctly implemented there.

fs/jbd/checkpoint.c

@@ -519,7 +519,6 @@ void __journal_remove_checkpoint(struct journal_head *jh)
 		JBUFFER_TRACE(jh, "not on transaction");
 		goto out;
 	}
-
 	journal = transaction->t_journal;
 
 	__buffer_unlink(jh);
@@ -528,11 +527,14 @@ void __journal_remove_checkpoint(struct journal_head *jh)
 		goto out;
 	JBUFFER_TRACE(jh, "transaction has no more buffers");
 
-	/* There is one special case to worry about: if we have just
-           pulled the buffer off a committing transaction's forget list,
-           then even if the checkpoint list is empty, the transaction
-           obviously cannot be dropped! */
-
+	/*
+	 * There is one special case to worry about: if we have just pulled the
+	 * buffer off a committing transaction's forget list, then even if the
+	 * checkpoint list is empty, the transaction obviously cannot be
+	 * dropped!
+	 *
+	 * AKPM2: locking here around j_committing_transaction is a bit flakey.
+	 */
 	if (transaction == journal->j_committing_transaction) {
 		JBUFFER_TRACE(jh, "belongs to committing transaction");
 		goto out;
@@ -601,21 +603,19 @@ void __journal_drop_transaction(journal_t *journal, transaction_t *transaction)
 			journal->j_checkpoint_transactions = NULL;
 	}
 
-	J_ASSERT (transaction->t_buffers == NULL);
-	J_ASSERT (transaction->t_sync_datalist == NULL);
-	J_ASSERT (transaction->t_forget == NULL);
-	J_ASSERT (transaction->t_iobuf_list == NULL);
-	J_ASSERT (transaction->t_shadow_list == NULL);
-	J_ASSERT (transaction->t_log_list == NULL);
-	J_ASSERT (transaction->t_checkpoint_list == NULL);
-	J_ASSERT (transaction->t_updates == 0);
-	J_ASSERT (list_empty(&transaction->t_jcb));
-
-	J_ASSERT (transaction->t_journal->j_committing_transaction !=
-					transaction);
-	
-	jbd_debug (1, "Dropping transaction %d, all done\n", 
-		   transaction->t_tid);
-	kfree (transaction);
+	J_ASSERT(transaction->t_buffers == NULL);
+	J_ASSERT(transaction->t_sync_datalist == NULL);
+	J_ASSERT(transaction->t_forget == NULL);
+	J_ASSERT(transaction->t_iobuf_list == NULL);
+	J_ASSERT(transaction->t_shadow_list == NULL);
+	J_ASSERT(transaction->t_log_list == NULL);
+	J_ASSERT(transaction->t_checkpoint_list == NULL);
+	J_ASSERT(transaction->t_updates == 0);
+	J_ASSERT(list_empty(&transaction->t_jcb));
+
+	J_ASSERT(journal->j_committing_transaction != transaction);
+
+	jbd_debug(1, "Dropping transaction %d, all done\n", transaction->t_tid);
+	kfree(transaction);
 }
 

fs/jbd/commit.c

@@ -722,12 +722,14 @@ void journal_commit_transaction(journal_t *journal)
 
 	jbd_debug(3, "JBD: commit phase 8\n");
 
-	J_ASSERT (commit_transaction->t_state == T_COMMIT);
-	commit_transaction->t_state = T_FINISHED;
+	J_ASSERT(commit_transaction->t_state == T_COMMIT);
 
-	J_ASSERT (commit_transaction == journal->j_committing_transaction);
+	spin_lock(&journal->j_state_lock);
+	commit_transaction->t_state = T_FINISHED;
+	J_ASSERT(commit_transaction == journal->j_committing_transaction);
 	journal->j_commit_sequence = commit_transaction->t_tid;
 	journal->j_committing_transaction = NULL;
+	spin_unlock(&journal->j_state_lock);
 
 	spin_lock(&journal->j_list_lock);
 	if (commit_transaction->t_checkpoint_list == NULL) {

fs/jbd/transaction.c

@@ -948,7 +948,7 @@ int journal_get_undo_access(handle_t *handle, struct buffer_head *bh)
  * by kswapd.  So it cannot block.  Happily, there's nothing here
  * which needs lock_journal if `async' is set.
  */
-int journal_dirty_data (handle_t *handle, struct buffer_head *bh)
+int journal_dirty_data(handle_t *handle, struct buffer_head *bh)
 {
 	journal_t *journal = handle->h_transaction->t_journal;
 	int need_brelse = 0;
@@ -1217,7 +1217,7 @@ void journal_release_buffer(handle_t *handle, struct buffer_head *bh)
  * Allow this call even if the handle has aborted --- it may be part of
  * the caller's cleanup after an abort.
  */
-void journal_forget (handle_t *handle, struct buffer_head *bh)
+void journal_forget(handle_t *handle, struct buffer_head *bh)
 {
 	transaction_t *transaction = handle->h_transaction;
 	journal_t *journal = transaction->t_journal;