There are various places in which JBD is starting a commit against a
transaction without sufficient locking in place to ensure that that
transaction is still alive.
Change it so that log_start_commit() takes a transaction ID instead. Make
the caller take a copy of that ID inside the appropriate locks.
fs/ext3/super.c | 9 +++--
fs/jbd/checkpoint.c | 12 +------
fs/jbd/journal.c | 75 ++++++++++++++++++++++++-----------------------
fs/jbd/transaction.c | 13 +++++---
include/linux/ext3_jbd.h | 11 ------
include/linux/jbd.h | 11 +++---
6 files changed, 62 insertions(+), 69 deletions(-)
diff -puN fs/ext3/super.c~jbd-670-log_start_commit-locking-fix fs/ext3/super.c
--- 25/fs/ext3/super.c~jbd-670-log_start_commit-locking-fix 2003-06-10 20:35:32.000000000 -0700
+++ 25-akpm/fs/ext3/super.c 2003-06-10 20:35:32.000000000 -0700
@@ -1811,7 +1811,7 @@ void ext3_write_super (struct super_bloc
if (down_trylock(&sb->s_lock) == 0)
BUG();
sb->s_dirt = 0;
- log_start_commit(EXT3_SB(sb)->s_journal, NULL);
+ journal_start_commit(EXT3_SB(sb)->s_journal, NULL);
}
static int ext3_sync_fs(struct super_block *sb, int wait)
@@ -1819,9 +1819,10 @@ static int ext3_sync_fs(struct super_blo
tid_t target;
sb->s_dirt = 0;
- target = log_start_commit(EXT3_SB(sb)->s_journal, NULL);
- if (wait)
- log_wait_commit(EXT3_SB(sb)->s_journal, target);
+ if (journal_start_commit(EXT3_SB(sb)->s_journal, &target)) {
+ if (wait)
+ log_wait_commit(EXT3_SB(sb)->s_journal, target);
+ }
return 0;
}
diff -puN fs/jbd/checkpoint.c~jbd-670-log_start_commit-locking-fix fs/jbd/checkpoint.c
--- 25/fs/jbd/checkpoint.c~jbd-670-log_start_commit-locking-fix 2003-06-10 20:35:32.000000000 -0700
+++ 25-akpm/fs/jbd/checkpoint.c 2003-06-10 20:35:32.000000000 -0700
@@ -124,7 +124,6 @@ static void jbd_sync_bh(journal_t *journ
* checkpoint. (journal_remove_checkpoint() deletes the transaction when
* the last checkpoint buffer is cleansed)
*
- * Called with the journal locked.
* Called with j_list_lock held.
*/
static int __cleanup_transaction(journal_t *journal, transaction_t *transaction)
@@ -167,17 +166,12 @@ static int __cleanup_transaction(journal
spin_unlock(&journal->j_list_lock);
jbd_unlock_bh_state(bh);
- log_start_commit(journal, t);
+ log_start_commit(journal, tid);
log_wait_commit(journal, tid);
goto out_return_1;
}
/*
- * We used to test for (jh->b_list != BUF_CLEAN) here.
- * But unmap_underlying_metadata() can place buffer onto
- * BUF_CLEAN.
- */
- /*
* AKPM: I think the buffer_jbddirty test is redundant - it
* shouldn't have NULL b_transaction?
*/
@@ -322,7 +316,7 @@ int log_do_checkpoint(journal_t *journal
int cleanup_ret, retry = 0;
tid_t this_tid;
- transaction = journal->j_checkpoint_transactions->t_cpprev;
+ transaction = journal->j_checkpoint_transactions->t_cpnext;
this_tid = transaction->t_tid;
jh = transaction->t_checkpoint_list;
last_jh = jh->b_cpprev;
@@ -359,7 +353,7 @@ int log_do_checkpoint(journal_t *journal
* If someone cleaned up this transaction while we slept, we're
* done
*/
- if (journal->j_checkpoint_transactions->t_cpprev != transaction)
+ if (journal->j_checkpoint_transactions->t_cpnext != transaction)
continue;
/*
* Maybe it's a new transaction, but it fell at the same
diff -puN fs/jbd/journal.c~jbd-670-log_start_commit-locking-fix fs/jbd/journal.c
--- 25/fs/jbd/journal.c~jbd-670-log_start_commit-locking-fix 2003-06-10 20:35:32.000000000 -0700
+++ 25-akpm/fs/jbd/journal.c 2003-06-10 20:35:32.000000000 -0700
@@ -73,7 +73,7 @@ EXPORT_SYMBOL(journal_errno);
EXPORT_SYMBOL(journal_ack_err);
EXPORT_SYMBOL(journal_clear_err);
EXPORT_SYMBOL(log_wait_commit);
-EXPORT_SYMBOL(log_start_commit);
+EXPORT_SYMBOL(journal_start_commit);
EXPORT_SYMBOL(journal_wipe);
EXPORT_SYMBOL(journal_blocks_per_page);
EXPORT_SYMBOL(journal_invalidatepage);
@@ -433,52 +433,55 @@ int __log_space_left(journal_t *journal)
}
/*
- * Called under j_state_lock.
+ * Called under j_state_lock. Returns true if a transaction was started.
*/
-tid_t __log_start_commit(journal_t *journal, transaction_t *transaction)
+int __log_start_commit(journal_t *journal, tid_t target)
{
- tid_t target = journal->j_commit_request;
-
- /*
- * A NULL transaction asks us to commit the currently running
- * transaction, if there is one.
- */
- if (transaction)
- target = transaction->t_tid;
- else {
- transaction = journal->j_running_transaction;
- if (!transaction)
- goto out;
- target = transaction->t_tid;
- }
-
/*
* Are we already doing a recent enough commit?
*/
- if (tid_geq(journal->j_commit_request, target))
- goto out;
+ if (!tid_geq(journal->j_commit_request, target)) {
+ /*
+ * We want a new commit: OK, mark the request and wakup the
+ * commit thread. We do _not_ do the commit ourselves.
+ */
- /*
- * We want a new commit: OK, mark the request and wakup the
- * commit thread. We do _not_ do the commit ourselves.
- */
+ journal->j_commit_request = target;
+ jbd_debug(1, "JBD: requesting commit %d/%d\n",
+ journal->j_commit_request,
+ journal->j_commit_sequence);
+ wake_up(&journal->j_wait_commit);
+ return 1;
+ }
+ return 0;
+}
- journal->j_commit_request = target;
- jbd_debug(1, "JBD: requesting commit %d/%d\n",
- journal->j_commit_request,
- journal->j_commit_sequence);
- wake_up(&journal->j_wait_commit);
+int log_start_commit(journal_t *journal, tid_t tid)
+{
+ int ret;
-out:
- return target;
+ spin_lock(&journal->j_state_lock);
+ ret = __log_start_commit(journal, tid);
+ spin_unlock(&journal->j_state_lock);
+ return ret;
}
-tid_t log_start_commit(journal_t *journal, transaction_t *transaction)
+/*
+ * Start a commit of the current running transaction (if any). Returns true
+ * if a transaction was started, and fills its tid in at *ptid
+ */
+int journal_start_commit(journal_t *journal, tid_t *ptid)
{
- tid_t ret;
+ int ret = 0;
spin_lock(&journal->j_state_lock);
- ret = __log_start_commit(journal, transaction);
+ if (journal->j_running_transaction) {
+ tid_t tid = journal->j_running_transaction->t_tid;
+
+ ret = __log_start_commit(journal, tid);
+ if (ret && ptid)
+ *ptid = tid;
+ }
spin_unlock(&journal->j_state_lock);
return ret;
}
@@ -1243,7 +1246,7 @@ int journal_flush(journal_t *journal)
/* Force everything buffered to the log... */
if (journal->j_running_transaction) {
transaction = journal->j_running_transaction;
- __log_start_commit(journal, transaction);
+ __log_start_commit(journal, transaction->t_tid);
} else if (journal->j_committing_transaction)
transaction = journal->j_committing_transaction;
@@ -1374,7 +1377,7 @@ void __journal_abort_hard(journal_t *jou
journal->j_flags |= JFS_ABORT;
transaction = journal->j_running_transaction;
if (transaction)
- __log_start_commit(journal, transaction);
+ __log_start_commit(journal, transaction->t_tid);
spin_unlock(&journal->j_state_lock);
}
diff -puN fs/jbd/transaction.c~jbd-670-log_start_commit-locking-fix fs/jbd/transaction.c
--- 25/fs/jbd/transaction.c~jbd-670-log_start_commit-locking-fix 2003-06-10 20:35:32.000000000 -0700
+++ 25-akpm/fs/jbd/transaction.c 2003-06-10 20:35:32.000000000 -0700
@@ -173,7 +173,7 @@ repeat:
spin_unlock(&transaction->t_handle_lock);
prepare_to_wait(&journal->j_wait_transaction_locked, &wait,
TASK_UNINTERRUPTIBLE);
- __log_start_commit(journal, transaction);
+ __log_start_commit(journal, transaction->t_tid);
spin_unlock(&journal->j_state_lock);
schedule();
finish_wait(&journal->j_wait_transaction_locked, &wait);
@@ -396,6 +396,7 @@ int journal_restart(handle_t *handle, in
J_ASSERT(transaction->t_updates > 0);
J_ASSERT(journal_current_handle() == handle);
+ spin_lock(&journal->j_state_lock);
spin_lock(&transaction->t_handle_lock);
transaction->t_outstanding_credits -= handle->h_buffer_credits;
transaction->t_updates--;
@@ -405,7 +406,8 @@ int journal_restart(handle_t *handle, in
spin_unlock(&transaction->t_handle_lock);
jbd_debug(2, "restarting handle %p\n", handle);
- log_start_commit(journal, transaction);
+ __log_start_commit(journal, transaction->t_tid);
+ spin_unlock(&journal->j_state_lock);
handle->h_buffer_credits = nblocks;
ret = start_this_handle(journal, handle);
@@ -1382,6 +1384,7 @@ int journal_stop(handle_t *handle)
}
current->journal_info = NULL;
+ spin_lock(&journal->j_state_lock);
spin_lock(&transaction->t_handle_lock);
transaction->t_outstanding_credits -= handle->h_buffer_credits;
transaction->t_updates--;
@@ -1415,8 +1418,9 @@ int journal_stop(handle_t *handle)
jbd_debug(2, "transaction too old, requesting commit for "
"handle %p\n", handle);
/* This is non-blocking */
- log_start_commit(journal, transaction);
-
+ __log_start_commit(journal, transaction->t_tid);
+ spin_unlock(&journal->j_state_lock);
+
/*
* Special case: JFS_SYNC synchronous updates require us
* to wait for the commit to complete.
@@ -1425,6 +1429,7 @@ int journal_stop(handle_t *handle)
err = log_wait_commit(journal, tid);
} else {
spin_unlock(&transaction->t_handle_lock);
+ spin_unlock(&journal->j_state_lock);
}
jbd_free_handle(handle);
diff -puN include/linux/ext3_jbd.h~jbd-670-log_start_commit-locking-fix include/linux/ext3_jbd.h
--- 25/include/linux/ext3_jbd.h~jbd-670-log_start_commit-locking-fix 2003-06-10 20:35:32.000000000 -0700
+++ 25-akpm/include/linux/ext3_jbd.h 2003-06-10 20:35:32.000000000 -0700
@@ -184,17 +184,6 @@ static inline handle_t *ext3_journal_cur
return journal_current_handle();
}
-static inline void
-ext3_log_start_commit(journal_t *journal, transaction_t *transaction)
-{
- log_start_commit(journal, transaction);
-}
-
-static inline void ext3_log_wait_commit(journal_t *journal, tid_t tid)
-{
- log_wait_commit(journal, tid);
-}
-
static inline int ext3_journal_extend(handle_t *handle, int nblocks)
{
return journal_extend(handle, nblocks);
diff -puN include/linux/jbd.h~jbd-670-log_start_commit-locking-fix include/linux/jbd.h
--- 25/include/linux/jbd.h~jbd-670-log_start_commit-locking-fix 2003-06-10 20:35:32.000000000 -0700
+++ 25-akpm/include/linux/jbd.h 2003-06-10 20:35:32.000000000 -0700
@@ -988,12 +988,13 @@ extern void journal_switch_revoke_table(
*/
int __log_space_left(journal_t *); /* Called with journal locked */
-extern tid_t log_start_commit (journal_t *, transaction_t *);
-extern tid_t __log_start_commit(journal_t *, transaction_t *);
-extern int log_wait_commit (journal_t *, tid_t);
-extern int log_do_checkpoint (journal_t *, int);
+int log_start_commit(journal_t *journal, tid_t tid);
+int __log_start_commit(journal_t *journal, tid_t tid);
+int journal_start_commit(journal_t *journal, tid_t *tid);
+int log_wait_commit(journal_t *journal, tid_t tid);
+int log_do_checkpoint(journal_t *journal, int nblocks);
-void __log_wait_for_space(journal_t *, int nblocks);
+void __log_wait_for_space(journal_t *journal, int nblocks);
extern void __journal_drop_transaction(journal_t *, transaction_t *);
extern int cleanup_journal_tail(journal_t *);
_