jbd2: call __jbd2_log_start_commit with j_state_lock write locked

On an SMP ARM system running ext4, I've received a report that the
first J_ASSERT in jbd2_journal_commit_transaction has been triggering:

	J_ASSERT(journal->j_running_transaction != NULL);

While investigating possible causes for this problem, I noticed that
__jbd2_log_start_commit() is getting called with j_state_lock only
read-locked, in spite of the fact that it's possible for it might
j_commit_request.  Fix this by grabbing the necessary information so
we can test to see if we need to start a new transaction before
dropping the read lock, and then calling jbd2_log_start_commit() which
will grab the write lock.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

2 files changed, 21 insertions, 9 deletions

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 9e4686900f18..97e73469b2c4 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -473,7 +473,8 @@ int __jbd2_log_space_left(journal_t *journal)
 }
 
 /*
- * Called under j_state_lock.  Returns true if a transaction commit was started.
+ * Called with j_state_lock locked for writing.
+ * Returns true if a transaction commit was started.
  */
 int __jbd2_log_start_commit(journal_t *journal, tid_t target)
 {
@@ -520,11 +521,13 @@ int jbd2_journal_force_commit_nested(journal_t *journal)
 {
         transaction_t *transaction = NULL;
         tid_t tid;
+        int need_to_start = 0;
 
         read_lock(&journal->j_state_lock);
         if (journal->j_running_transaction && !current->journal_info) {
                 transaction = journal->j_running_transaction;
-                __jbd2_log_start_commit(journal, transaction->t_tid);
+                if (!tid_geq(journal->j_commit_request, transaction->t_tid))
+                        need_to_start = 1;
         } else if (journal->j_committing_transaction)
                 transaction = journal->j_committing_transaction;
 
@@ -535,6 +538,8 @@ int jbd2_journal_force_commit_nested(journal_t *journal)
 
         tid = transaction->t_tid;
         read_unlock(&journal->j_state_lock);
+        if (need_to_start)
+                jbd2_log_start_commit(journal, tid);
         jbd2_log_wait_commit(journal, tid);
         return 1;
 }
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index faad2bd787c7..1d1191050f99 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -117,10 +117,10 @@ static inline void update_t_max_wait(transaction_t *transaction)
 static int start_this_handle(journal_t *journal, handle_t *handle,
                              int gfp_mask)
 {
-        transaction_t *transaction;
-        int needed;
-        int nblocks = handle->h_buffer_credits;
-        transaction_t *new_transaction = NULL;
+        transaction_t   *transaction, *new_transaction = NULL;
+        tid_t           tid;
+        int             needed, need_to_start;
+        int             nblocks = handle->h_buffer_credits;
 
         if (nblocks > journal->j_max_transaction_buffers) {
                 printk(KERN_ERR "JBD: %s wants too many credits (%d > %d)\n",
@@ -222,8 +222,11 @@ repeat:
                 atomic_sub(nblocks, &transaction->t_outstanding_credits);
                 prepare_to_wait(&journal->j_wait_transaction_locked, &wait,
                                 TASK_UNINTERRUPTIBLE);
-                __jbd2_log_start_commit(journal, transaction->t_tid);
+                tid = transaction->t_tid;
+                need_to_start = !tid_geq(journal->j_commit_request, tid);
                 read_unlock(&journal->j_state_lock);
+                if (need_to_start)
+                        jbd2_log_start_commit(journal, tid);
                 schedule();
                 finish_wait(&journal->j_wait_transaction_locked, &wait);
                 goto repeat;
@@ -442,7 +445,8 @@ int jbd2__journal_restart(handle_t *handle, int nblocks, int gfp_mask)
 {
         transaction_t *transaction = handle->h_transaction;
         journal_t *journal = transaction->t_journal;
-        int ret;
+        tid_t           tid;
+        int             need_to_start, ret;
 
         /* If we've had an abort of any type, don't even think about
          * actually doing the restart! */
@@ -465,8 +469,11 @@ int jbd2__journal_restart(handle_t *handle, int nblocks, int gfp_mask)
         spin_unlock(&transaction->t_handle_lock);
 
         jbd_debug(2, "restarting handle %p\n", handle);
-        __jbd2_log_start_commit(journal, transaction->t_tid);
+        tid = transaction->t_tid;
+        need_to_start = !tid_geq(journal->j_commit_request, tid);
         read_unlock(&journal->j_state_lock);
+        if (need_to_start)
+                jbd2_log_start_commit(journal, tid);
 
         lock_map_release(&handle->h_lockdep_map);
         handle->h_buffer_credits = nblocks;