[PATCH] ext3: fix journal_release_buffer() race

		CPU0				CPU1

	journal_get_write_access(bh)
	 (Add buffer to t_reserved_list)

					journal_get_write_access(bh)
					 (It's already on t_reserved_list:
					  nothing to do)

	 (We decide we don't want to
	  journal the buffer after all)
	journal_release_buffer()
	 (It gets pulled off the transaction)


					journal_dirty_metadata()
					 (The buffer isn't on the reserved
					  list!  The kernel explodes)


Simple fix: just leave the buffer on t_reserved_list in
journal_release_buffer().  If nobody ends up claiming the buffer then it will
get thrown away at start of transaction commit.

2 files changed, 28 insertions, 28 deletions

diff --git a/fs/jbd/commit.c b/fs/jbd/commit.c
index 54f7862a3717..2580162cad52 100644
--- a/fs/jbd/commit.c
+++ b/fs/jbd/commit.c
@@ -169,10 +169,23 @@ void journal_commit_transaction(journal_t *journal)
 	 * that multiple journal_get_write_access() calls to the same
 	 * buffer are perfectly permissable.
 	 */
-	while (commit_transaction->t_reserved_list) {
-		jh = commit_transaction->t_reserved_list;
-		JBUFFER_TRACE(jh, "reserved, unused: refile");
-		journal_refile_buffer(journal, jh);
+	{
+		int nr = 0;
+		while (commit_transaction->t_reserved_list) {
+			jh = commit_transaction->t_reserved_list;
+			JBUFFER_TRACE(jh, "reserved, unused: refile");
+			journal_refile_buffer(journal, jh);
+			nr++;
+		}
+		if (nr) {
+			static int noisy;
+
+			if (noisy < 10) {
+				noisy++;
+				printk("%s: freed %d reserved buffers\n",
+					__FUNCTION__, nr);
+			}
+		}
 	}
 
 	/*
diff --git a/fs/jbd/transaction.c b/fs/jbd/transaction.c
index 54e16b97fdaa..12ad174e7662 100644
--- a/fs/jbd/transaction.c
+++ b/fs/jbd/transaction.c
@@ -1168,37 +1168,24 @@ out:
  * journal_release_buffer: undo a get_write_access without any buffer
  * updates, if the update decided in the end that it didn't need access.
  *
- * journal_get_write_access() can block, so it is quite possible for a
- * journaling component to decide after the write access is returned
- * that global state has changed and the update is no longer required.
- *
  * The caller passes in the number of credits which should be put back for
  * this buffer (zero or one).
+ *
+ * We leave the buffer attached to t_reserved_list because even though this
+ * handle doesn't want it, some other concurrent handle may want to journal
+ * this buffer.  If that handle is curently in between get_write_access() and
+ * journal_dirty_metadata() then it expects the buffer to be reserved.  If
+ * we were to rip it off t_reserved_list here, the other handle will explode
+ * when journal_dirty_metadata is presented with a non-reserved buffer.
+ *
+ * If nobody really wants to journal this buffer then it will be thrown
+ * away at the start of commit.
  */
 void
 journal_release_buffer(handle_t *handle, struct buffer_head *bh, int credits)
 {
-	transaction_t *transaction = handle->h_transaction;
-	journal_t *journal = transaction->t_journal;
-	struct journal_head *jh = bh2jh(bh);
-
-	JBUFFER_TRACE(jh, "entry");
-
-	/* If the buffer is reserved but not modified by this
-	 * transaction, then it is safe to release it.  In all other
-	 * cases, just leave the buffer as it is. */
-
-	jbd_lock_bh_state(bh);
-	spin_lock(&journal->j_list_lock);
-	if (jh->b_jlist == BJ_Reserved && jh->b_transaction == transaction &&
-	    !buffer_jbddirty(jh2bh(jh))) {
-		JBUFFER_TRACE(jh, "unused: refiling it");
-		__journal_refile_buffer(jh);
-	}
-	spin_unlock(&journal->j_list_lock);
-	jbd_unlock_bh_state(bh);
+	BUFFER_TRACE(bh, "entry");
 	handle->h_buffer_credits += credits;
-	JBUFFER_TRACE(jh, "exit");
 }
 
 /**