From e6c9f5c1888097c936334bf9740024520ca47b8e Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Tue, 6 Sep 2005 15:19:09 -0700 Subject: [PATCH] Fix JBD race in t_forget list handling Fix race between journal_commit_transaction() and other places as journal_unmap_buffer() that are adding buffers to transaction's t_forget list. We have to protect against such places by holding j_list_lock even when traversing the t_forget list. The fact that other places can only add buffers to the list makes the locking easier. OTOH the lock ranking complicates the stuff... Signed-off-by: Jan Kara Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/jbd/commit.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) (limited to 'fs/jbd/commit.c') diff --git a/fs/jbd/commit.c b/fs/jbd/commit.c index dac720c837a..9d0494dcc57 100644 --- a/fs/jbd/commit.c +++ b/fs/jbd/commit.c @@ -720,11 +720,17 @@ wait_for_iobuf: J_ASSERT(commit_transaction->t_log_list == NULL); restart_loop: + /* + * As there are other places (journal_unmap_buffer()) adding buffers + * to this list we have to be careful and hold the j_list_lock. + */ + spin_lock(&journal->j_list_lock); while (commit_transaction->t_forget) { transaction_t *cp_transaction; struct buffer_head *bh; jh = commit_transaction->t_forget; + spin_unlock(&journal->j_list_lock); bh = jh2bh(jh); jbd_lock_bh_state(bh); J_ASSERT_JH(jh, jh->b_transaction == commit_transaction || @@ -792,9 +798,25 @@ restart_loop: journal_remove_journal_head(bh); /* needs a brelse */ release_buffer_page(bh); } + cond_resched_lock(&journal->j_list_lock); + } + spin_unlock(&journal->j_list_lock); + /* + * This is a bit sleazy. We borrow j_list_lock to protect + * journal->j_committing_transaction in __journal_remove_checkpoint. + * Really, __journal_remove_checkpoint should be using j_state_lock but + * it's a bit hassle to hold that across __journal_remove_checkpoint + */ + spin_lock(&journal->j_state_lock); + spin_lock(&journal->j_list_lock); + /* + * Now recheck if some buffers did not get attached to the transaction + * while the lock was dropped... + */ + if (commit_transaction->t_forget) { spin_unlock(&journal->j_list_lock); - if (cond_resched()) - goto restart_loop; + spin_unlock(&journal->j_state_lock); + goto restart_loop; } /* Done with this transaction! */ @@ -803,14 +825,6 @@ restart_loop: J_ASSERT(commit_transaction->t_state == T_COMMIT); - /* - * This is a bit sleazy. We borrow j_list_lock to protect - * journal->j_committing_transaction in __journal_remove_checkpoint. - * Really, __jornal_remove_checkpoint should be using j_state_lock but - * it's a bit hassle to hold that across __journal_remove_checkpoint - */ - spin_lock(&journal->j_state_lock); - spin_lock(&journal->j_list_lock); commit_transaction->t_state = T_FINISHED; J_ASSERT(commit_transaction == journal->j_committing_transaction); journal->j_commit_sequence = commit_transaction->t_tid; -- cgit v1.2.3-70-g09d2 From 26707699b5337ea471ba1774447e8a1170c99e52 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Tue, 6 Sep 2005 15:19:12 -0700 Subject: [PATCH] Change ll_rw_block() calls in JBD We must be sure that the current data in buffer are sent to disk. Hence we have to call ll_rw_block() with SWRITE. Signed-off-by: Jan Kara Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/jbd/checkpoint.c | 2 +- fs/jbd/commit.c | 4 ++-- fs/jbd/journal.c | 2 +- fs/jbd/revoke.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) (limited to 'fs/jbd/commit.c') diff --git a/fs/jbd/checkpoint.c b/fs/jbd/checkpoint.c index 5a97e346bd9..014a51fd00d 100644 --- a/fs/jbd/checkpoint.c +++ b/fs/jbd/checkpoint.c @@ -204,7 +204,7 @@ __flush_batch(journal_t *journal, struct buffer_head **bhs, int *batch_count) int i; spin_unlock(&journal->j_list_lock); - ll_rw_block(WRITE, *batch_count, bhs); + ll_rw_block(SWRITE, *batch_count, bhs); spin_lock(&journal->j_list_lock); for (i = 0; i < *batch_count; i++) { struct buffer_head *bh = bhs[i]; diff --git a/fs/jbd/commit.c b/fs/jbd/commit.c index 9d0494dcc57..2a3e310f79e 100644 --- a/fs/jbd/commit.c +++ b/fs/jbd/commit.c @@ -358,7 +358,7 @@ write_out_data: jbd_debug(2, "submit %d writes\n", bufs); spin_unlock(&journal->j_list_lock); - ll_rw_block(WRITE, bufs, wbuf); + ll_rw_block(SWRITE, bufs, wbuf); journal_brelse_array(wbuf, bufs); bufs = 0; goto write_out_data; @@ -381,7 +381,7 @@ write_out_data: if (bufs) { spin_unlock(&journal->j_list_lock); - ll_rw_block(WRITE, bufs, wbuf); + ll_rw_block(SWRITE, bufs, wbuf); journal_brelse_array(wbuf, bufs); spin_lock(&journal->j_list_lock); } diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c index 334f4cf0823..7ae2c4fe506 100644 --- a/fs/jbd/journal.c +++ b/fs/jbd/journal.c @@ -957,7 +957,7 @@ void journal_update_superblock(journal_t *journal, int wait) if (wait) sync_dirty_buffer(bh); else - ll_rw_block(WRITE, 1, &bh); + ll_rw_block(SWRITE, 1, &bh); out: /* If we have just flushed the log (by marking s_start==0), then diff --git a/fs/jbd/revoke.c b/fs/jbd/revoke.c index 93b9f45eebd..a5614418346 100644 --- a/fs/jbd/revoke.c +++ b/fs/jbd/revoke.c @@ -614,7 +614,7 @@ static void flush_descriptor(journal_t *journal, set_buffer_jwrite(bh); BUFFER_TRACE(bh, "write"); set_buffer_dirty(bh); - ll_rw_block(WRITE, 1, &bh); + ll_rw_block(SWRITE, 1, &bh); } #endif -- cgit v1.2.3-70-g09d2