Commit 53cf9784 authored by Xiaoguang Wang's avatar Xiaoguang Wang Committed by Theodore Ts'o

jbd2: fix deadlock while checkpoint thread waits commit thread to finish

This issue was found when I tried to put checkpoint work in a separate thread,
the deadlock below happened:
         Thread1                                |   Thread2
__jbd2_log_wait_for_space                       |
jbd2_log_do_checkpoint (hold j_checkpoint_mutex)|
  if (jh->b_transaction != NULL)                |
    ...                                         |
    jbd2_log_start_commit(journal, tid);        |jbd2_update_log_tail
                                                |  will lock j_checkpoint_mutex,
                                                |  but will be blocked here.
                                                |
    jbd2_log_wait_commit(journal, tid);         |
    wait_event(journal->j_wait_done_commit,     |
     !tid_gt(tid, journal->j_commit_sequence)); |
     ...                                        |wake_up(j_wait_done_commit)
  }                                             |

then deadlock occurs, Thread1 will never be waken up.

To fix this issue, drop j_checkpoint_mutex in jbd2_log_do_checkpoint()
when we are going to wait for transaction commit.
Reviewed-by: default avatarJan Kara <jack@suse.cz>
Signed-off-by: default avatarXiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
parent 8fdd60f2
...@@ -113,7 +113,7 @@ void __jbd2_log_wait_for_space(journal_t *journal) ...@@ -113,7 +113,7 @@ void __jbd2_log_wait_for_space(journal_t *journal)
nblocks = jbd2_space_needed(journal); nblocks = jbd2_space_needed(journal);
while (jbd2_log_space_left(journal) < nblocks) { while (jbd2_log_space_left(journal) < nblocks) {
write_unlock(&journal->j_state_lock); write_unlock(&journal->j_state_lock);
mutex_lock(&journal->j_checkpoint_mutex); mutex_lock_io(&journal->j_checkpoint_mutex);
/* /*
* Test again, another process may have checkpointed while we * Test again, another process may have checkpointed while we
...@@ -276,9 +276,22 @@ int jbd2_log_do_checkpoint(journal_t *journal) ...@@ -276,9 +276,22 @@ int jbd2_log_do_checkpoint(journal_t *journal)
"JBD2: %s: Waiting for Godot: block %llu\n", "JBD2: %s: Waiting for Godot: block %llu\n",
journal->j_devname, (unsigned long long) bh->b_blocknr); journal->j_devname, (unsigned long long) bh->b_blocknr);
if (batch_count)
__flush_batch(journal, &batch_count);
jbd2_log_start_commit(journal, tid); jbd2_log_start_commit(journal, tid);
/*
* jbd2_journal_commit_transaction() may want
* to take the checkpoint_mutex if JBD2_FLUSHED
* is set, jbd2_update_log_tail() called by
* jbd2_journal_commit_transaction() may also take
* checkpoint_mutex. So we need to temporarily
* drop it.
*/
mutex_unlock(&journal->j_checkpoint_mutex);
jbd2_log_wait_commit(journal, tid); jbd2_log_wait_commit(journal, tid);
goto retry; mutex_lock_io(&journal->j_checkpoint_mutex);
spin_lock(&journal->j_list_lock);
goto restart;
} }
if (!buffer_dirty(bh)) { if (!buffer_dirty(bh)) {
if (unlikely(buffer_write_io_error(bh)) && !result) if (unlikely(buffer_write_io_error(bh)) && !result)
......
...@@ -2067,7 +2067,7 @@ int jbd2_journal_wipe(journal_t *journal, int write) ...@@ -2067,7 +2067,7 @@ int jbd2_journal_wipe(journal_t *journal, int write)
err = jbd2_journal_skip_recovery(journal); err = jbd2_journal_skip_recovery(journal);
if (write) { if (write) {
/* Lock to make assertions happy... */ /* Lock to make assertions happy... */
mutex_lock(&journal->j_checkpoint_mutex); mutex_lock_io(&journal->j_checkpoint_mutex);
jbd2_mark_journal_empty(journal, REQ_SYNC | REQ_FUA); jbd2_mark_journal_empty(journal, REQ_SYNC | REQ_FUA);
mutex_unlock(&journal->j_checkpoint_mutex); mutex_unlock(&journal->j_checkpoint_mutex);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment