diff mbox

[5/6] ext4: Call journal commit callback without holding j_list_lock

Message ID 1225471859-19718-5-git-send-email-aneesh.kumar@linux.vnet.ibm.com
State Superseded, archived
Headers show

Commit Message

Aneesh Kumar K.V Oct. 31, 2008, 4:50 p.m. UTC 
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/jbd2/checkpoint.c |    6 ++++--
 fs/jbd2/commit.c     |   16 ++++++++++------
 include/linux/jbd2.h |    9 +++++++--
 3 files changed, 21 insertions(+), 10 deletions(-)

Comments

Theodore Ts'o Nov. 3, 2008, 2:42 a.m. UTC | #1 
Justification for this patch, please?  Is there a lockdep violation
that you're trying to avoid? 

Also, given that __jbd2_journal_drop_transaction() is called exactly
twice, instead of passing in an extra free, it might be simpler just
to transfer the responsibility of calling kfree() to the caller.

   	    		       	  	  - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 9203c33..cbe7227 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -716,7 +716,8 @@  void __jbd2_journal_insert_checkpoint(struct journal_head *jh,
  * Called with j_list_lock held.
  */
 
-void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transaction)
+void ___jbd2_journal_drop_transaction(journal_t *journal,
+					transaction_t *transaction, int free)
 {
 	assert_spin_locked(&journal->j_list_lock);
 	if (transaction->t_cpnext) {
@@ -742,5 +743,6 @@  void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transact
 	J_ASSERT(journal->j_running_transaction != transaction);
 
 	jbd_debug(1, "Dropping transaction %d, all done\n", transaction->t_tid);
-	kfree(transaction);
+	if (free)
+		kfree(transaction);
 }
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index ebc667b..35f016c 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -338,7 +338,7 @@  void jbd2_journal_commit_transaction(journal_t *journal)
 	int space_left = 0;
 	int first_tag = 0;
 	int tag_flag;
-	int i;
+	int i, to_free = 0;
 	int tag_bytes = journal_tag_bytes(journal);
 	struct buffer_head *cbh = NULL; /* For transactional checksums */
 	__u32 crc32_sum = ~0;
@@ -974,12 +974,11 @@  void jbd2_journal_commit_transaction(journal_t *journal)
 	journal->j_committing_transaction = NULL;
 	spin_unlock(&journal->j_state_lock);
 
-	if (journal->j_commit_callback)
-		journal->j_commit_callback(journal, commit_transaction);
-
 	if (commit_transaction->t_checkpoint_list == NULL &&
 	    commit_transaction->t_checkpoint_io_list == NULL) {
-		__jbd2_journal_drop_transaction(journal, commit_transaction);
+		___jbd2_journal_drop_transaction(journal,
+						commit_transaction, 0);
+		to_free = 1;
 	} else {
 		if (journal->j_checkpoint_transactions == NULL) {
 			journal->j_checkpoint_transactions = commit_transaction;
@@ -998,11 +997,16 @@  void jbd2_journal_commit_transaction(journal_t *journal)
 	}
 	spin_unlock(&journal->j_list_lock);
 
+	if (journal->j_commit_callback)
+		journal->j_commit_callback(journal, commit_transaction);
+
 	trace_mark(jbd2_end_commit, "dev %s transaction %d head %d",
-		   journal->j_devname, journal->j_commit_sequence,
+		   journal->j_devname, commit_transaction->t_tid,
 		   journal->j_tail_sequence);
 	jbd_debug(1, "JBD: commit %d complete, head %d\n",
 		  journal->j_commit_sequence, journal->j_tail_sequence);
+	if (to_free)
+		kfree(commit_transaction);
 
 	wake_up(&journal->j_wait_done_commit);
 }
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 973db89..0ff7209 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1146,8 +1146,8 @@  int jbd2_log_wait_commit(journal_t *journal, tid_t tid);
 int jbd2_log_do_checkpoint(journal_t *journal);
 
 void __jbd2_log_wait_for_space(journal_t *journal);
-extern void	__jbd2_journal_drop_transaction(journal_t *, transaction_t *);
-extern int	jbd2_cleanup_journal_tail(journal_t *);
+extern void ___jbd2_journal_drop_transaction(journal_t *, transaction_t *, int);
+extern int jbd2_cleanup_journal_tail(journal_t *);
 
 /* Debugging code only: */
 
@@ -1217,6 +1217,11 @@  static inline int jbd_space_needed(journal_t *journal)
 					t_outstanding_credits;
 	return nblocks;
 }
+static inline void __jbd2_journal_drop_transaction(journal_t *journal,
+						transaction_t *transaction)
+{
+	return ___jbd2_journal_drop_transaction(journal, transaction, 1);
+}
 
 /*
  * Definitions which augment the buffer_head layer