Go through all sites which use j_committing_transaction and ensure that the deisgned locking is correctly implemented there. 25-akpm/fs/jbd/checkpoint.c | 44 +++++++++++++++++++++---------------------- 25-akpm/fs/jbd/commit.c | 8 ++++--- 25-akpm/fs/jbd/transaction.c | 4 +-- fs/jbd/journal.c | 0 4 files changed, 29 insertions(+), 27 deletions(-) diff -puN fs/jbd/checkpoint.c~jbd-170-j_committing_transaction-locking fs/jbd/checkpoint.c --- 25/fs/jbd/checkpoint.c~jbd-170-j_committing_transaction-locking Thu Jun 5 15:14:25 2003 +++ 25-akpm/fs/jbd/checkpoint.c Thu Jun 5 15:14:25 2003 @@ -519,7 +519,6 @@ void __journal_remove_checkpoint(struct JBUFFER_TRACE(jh, "not on transaction"); goto out; } - journal = transaction->t_journal; __buffer_unlink(jh); @@ -528,11 +527,14 @@ void __journal_remove_checkpoint(struct 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_ 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); } diff -puN fs/jbd/commit.c~jbd-170-j_committing_transaction-locking fs/jbd/commit.c --- 25/fs/jbd/commit.c~jbd-170-j_committing_transaction-locking Thu Jun 5 15:14:25 2003 +++ 25-akpm/fs/jbd/commit.c Thu Jun 5 15:14:25 2003 @@ -722,12 +722,14 @@ skip_commit: /* The journal should be un 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) { diff -puN fs/jbd/journal.c~jbd-170-j_committing_transaction-locking fs/jbd/journal.c diff -puN fs/jbd/transaction.c~jbd-170-j_committing_transaction-locking fs/jbd/transaction.c --- 25/fs/jbd/transaction.c~jbd-170-j_committing_transaction-locking Thu Jun 5 15:14:25 2003 +++ 25-akpm/fs/jbd/transaction.c Thu Jun 5 15:14:25 2003 @@ -948,7 +948,7 @@ out: * 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 *ha * 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; _