Commit e4471831 authored by Theodore Ts'o's avatar Theodore Ts'o
Browse files

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: default avatar"Theodore Ts'o" <tytso@mit.edu>
parent e9e3bcec

fs/jbd2/journal.c

+7 −2
Original line number Diff line number Diff line
@@ -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;

}

fs/jbd2/transaction.c

+14 −7
Original line number Diff line number Diff line
@@ -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;

	transaction_t	*transaction, *new_transaction = NULL;

	tid_t		tid;

	int		needed, need_to_start;

	int		nblocks = handle->h_buffer_credits;

	transaction_t *new_transaction = NULL;



	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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE