From f5113effc2a2ee6b86a4b345ce557353dcbcfffe Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Tue, 4 Jun 2013 12:01:45 -0400 Subject: jbd2: don't create journal_head for temporary journal buffers When writing metadata to the journal, we create temporary buffer heads for that task. We also attach journal heads to these buffer heads but the only purpose of the journal heads is to keep buffers linked in transaction's BJ_IO list. We remove the need for journal heads by reusing buffer_head's b_assoc_buffers list for that purpose. Also since BJ_IO list is just a temporary list for transaction commit, we use a private list in jbd2_journal_commit_transaction() for that thus removing BJ_IO list from transaction completely. Reviewed-by: Zheng Liu Signed-off-by: Jan Kara Signed-off-by: "Theodore Ts'o" --- fs/jbd2/checkpoint.c | 1 - 1 file changed, 1 deletion(-) (limited to 'fs/jbd2/checkpoint.c') diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c index c78841ee81cf..2735fef6e55e 100644 --- a/fs/jbd2/checkpoint.c +++ b/fs/jbd2/checkpoint.c @@ -690,7 +690,6 @@ void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transact J_ASSERT(transaction->t_state == T_FINISHED); J_ASSERT(transaction->t_buffers == NULL); J_ASSERT(transaction->t_forget == NULL); - J_ASSERT(transaction->t_iobuf_list == NULL); J_ASSERT(transaction->t_shadow_list == NULL); J_ASSERT(transaction->t_log_list == NULL); J_ASSERT(transaction->t_checkpoint_list == NULL); -- cgit v1.2.3 From e5a120aeb57f40ae568a5ca1dd6ace53d0213582 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Tue, 4 Jun 2013 12:06:01 -0400 Subject: jbd2: remove journal_head from descriptor buffers Similarly as for metadata buffers, also log descriptor buffers don't really need the journal head. So strip it and remove BJ_LogCtl list. Reviewed-by: Zheng Liu Signed-off-by: Jan Kara Signed-off-by: "Theodore Ts'o" --- fs/jbd2/checkpoint.c | 1 - 1 file changed, 1 deletion(-) (limited to 'fs/jbd2/checkpoint.c') diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c index 2735fef6e55e..65ec076e41f2 100644 --- a/fs/jbd2/checkpoint.c +++ b/fs/jbd2/checkpoint.c @@ -691,7 +691,6 @@ void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transact J_ASSERT(transaction->t_buffers == NULL); J_ASSERT(transaction->t_forget == NULL); J_ASSERT(transaction->t_shadow_list == NULL); - J_ASSERT(transaction->t_log_list == NULL); J_ASSERT(transaction->t_checkpoint_list == NULL); J_ASSERT(transaction->t_checkpoint_io_list == NULL); J_ASSERT(atomic_read(&transaction->t_updates) == 0); -- cgit v1.2.3 From 76c39904561004ac8675f858a290129e439d5168 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Tue, 4 Jun 2013 12:12:57 -0400 Subject: jbd2: cleanup needed free block estimates when starting a transaction __jbd2_log_space_left() and jbd_space_needed() were kind of odd. jbd_space_needed() accounted also credits needed for currently committing transaction while it didn't account for credits needed for control blocks. __jbd2_log_space_left() then accounted for control blocks as a fraction of free space. Since results of these two functions are always only compared against each other, this works correct but is somewhat strange. Move the estimates so that jbd_space_needed() returns number of blocks needed for a transaction including control blocks and __jbd2_log_space_left() returns free space in the journal (with the committing transaction already subtracted). Rename functions to jbd2_log_space_left() and jbd2_space_needed() while we are changing them. Reviewed-by: Zheng Liu Signed-off-by: Jan Kara Signed-off-by: "Theodore Ts'o" --- fs/jbd2/checkpoint.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'fs/jbd2/checkpoint.c') diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c index 65ec076e41f2..a572383bcf99 100644 --- a/fs/jbd2/checkpoint.c +++ b/fs/jbd2/checkpoint.c @@ -120,8 +120,8 @@ void __jbd2_log_wait_for_space(journal_t *journal) int nblocks, space_left; /* assert_spin_locked(&journal->j_state_lock); */ - nblocks = jbd_space_needed(journal); - while (__jbd2_log_space_left(journal) < nblocks) { + nblocks = jbd2_space_needed(journal); + while (jbd2_log_space_left(journal) < nblocks) { if (journal->j_flags & JBD2_ABORT) return; write_unlock(&journal->j_state_lock); @@ -140,8 +140,8 @@ void __jbd2_log_wait_for_space(journal_t *journal) */ write_lock(&journal->j_state_lock); spin_lock(&journal->j_list_lock); - nblocks = jbd_space_needed(journal); - space_left = __jbd2_log_space_left(journal); + nblocks = jbd2_space_needed(journal); + space_left = jbd2_log_space_left(journal); if (space_left < nblocks) { int chkpt = journal->j_checkpoint_transactions != NULL; tid_t tid = 0; -- cgit v1.2.3 From f29fad72105287e6899d9128a9d494514f220e77 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Tue, 4 Jun 2013 12:24:11 -0400 Subject: jbd2: remove unused waitqueues j_wait_logspace and j_wait_checkpoint are unused. Remove them. Reviewed-by: Zheng Liu Signed-off-by: Jan Kara Signed-off-by: "Theodore Ts'o" --- fs/jbd2/checkpoint.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'fs/jbd2/checkpoint.c') diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c index a572383bcf99..75a15f371b00 100644 --- a/fs/jbd2/checkpoint.c +++ b/fs/jbd2/checkpoint.c @@ -625,10 +625,6 @@ int __jbd2_journal_remove_checkpoint(struct journal_head *jh) __jbd2_journal_drop_transaction(journal, transaction); jbd2_journal_free_transaction(transaction); - - /* Just in case anybody was waiting for more transactions to be - checkpointed... */ - wake_up(&journal->j_wait_logspace); ret = 1; out: return ret; -- cgit v1.2.3 From 0ef54180e0187117062939202b96faf04c8673bc Mon Sep 17 00:00:00 2001 From: Paul Gortmaker Date: Wed, 12 Jun 2013 22:47:35 -0400 Subject: jbd2: drop checkpoint mutex when waiting in __jbd2_log_wait_for_space() While trying to debug an an issue under extreme I/O loading on preempt-rt kernels, the following backtrace was observed via SysRQ output: rm D ffff8802203afbc0 4600 4878 4748 0x00000000 ffff8802217bfb78 0000000000000082 ffff88021fc2bb80 ffff88021fc2bb80 ffff88021fc2bb80 ffff8802217bffd8 ffff8802217bffd8 ffff8802217bffd8 ffff88021f1d4c80 ffff88021fc2bb80 ffff8802217bfb88 ffff88022437b000 Call Trace: [] schedule+0x24/0x70 [] jbd2_log_wait_commit+0xbd/0x140 [] ? __init_waitqueue_head+0x50/0x50 [] jbd2_log_do_checkpoint+0xf5/0x520 [] __jbd2_log_wait_for_space+0xa9/0x1f0 [] start_this_handle.isra.10+0x2e0/0x530 [] ? __init_waitqueue_head+0x50/0x50 [] jbd2__journal_start+0xc3/0x110 [] ? ext4_rmdir+0x6e/0x230 [] jbd2_journal_start+0xe/0x10 [] ext4_journal_start_sb+0x5b/0x160 [] ext4_rmdir+0x6e/0x230 [] vfs_rmdir+0xd5/0x140 [] do_rmdir+0xdf/0x120 [] ? task_work_run+0x44/0x80 [] ? do_notify_resume+0x89/0x100 [] ? int_signal+0x12/0x17 [] sys_unlinkat+0x25/0x40 [] system_call_fastpath+0x16/0x1b What is interesting here, is that we call log_wait_commit, from within wait_for_space, but we are still holding the checkpoint_mutex as it surrounds mostly the whole of wait_for_space. And then, as we are waiting, journal_commit_transaction can run, and if the JBD2_FLUSHED bit is set, then we will also try to take the same checkpoint_mutex. It seems that we need to drop the checkpoint_mutex while sitting in jbd2_log_wait_commit, if we want to guarantee that progress can be made by jbd2_journal_commit_transaction(). There does not seem to be anything preempt-rt specific about this, other then perhaps increasing the odds of it happening. Signed-off-by: Paul Gortmaker Signed-off-by: "Theodore Ts'o" --- fs/jbd2/checkpoint.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'fs/jbd2/checkpoint.c') diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c index 75a15f371b00..7f34f4716165 100644 --- a/fs/jbd2/checkpoint.c +++ b/fs/jbd2/checkpoint.c @@ -156,7 +156,15 @@ void __jbd2_log_wait_for_space(journal_t *journal) /* We were able to recover space; yay! */ ; } else if (tid) { + /* + * jbd2_journal_commit_transaction() may want + * to take the checkpoint_mutex if JBD2_FLUSHED + * is set. So we need to temporarily drop it. + */ + mutex_unlock(&journal->j_checkpoint_mutex); jbd2_log_wait_commit(journal, tid); + write_lock(&journal->j_state_lock); + continue; } else { printk(KERN_ERR "%s: needed %d blocks and " "only had %d space available\n", -- cgit v1.2.3