Commit 90153a16 authored by Andrew Morton's avatar Andrew Morton Committed by Linus Torvalds

[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.
parent 610a61e0
...@@ -169,10 +169,23 @@ void journal_commit_transaction(journal_t *journal) ...@@ -169,10 +169,23 @@ void journal_commit_transaction(journal_t *journal)
* that multiple journal_get_write_access() calls to the same * that multiple journal_get_write_access() calls to the same
* buffer are perfectly permissable. * buffer are perfectly permissable.
*/ */
{
int nr = 0;
while (commit_transaction->t_reserved_list) { while (commit_transaction->t_reserved_list) {
jh = commit_transaction->t_reserved_list; jh = commit_transaction->t_reserved_list;
JBUFFER_TRACE(jh, "reserved, unused: refile"); JBUFFER_TRACE(jh, "reserved, unused: refile");
journal_refile_buffer(journal, jh); journal_refile_buffer(journal, jh);
nr++;
}
if (nr) {
static int noisy;
if (noisy < 10) {
noisy++;
printk("%s: freed %d reserved buffers\n",
__FUNCTION__, nr);
}
}
} }
/* /*
......
...@@ -1168,37 +1168,24 @@ int journal_dirty_metadata(handle_t *handle, struct buffer_head *bh) ...@@ -1168,37 +1168,24 @@ int journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
* journal_release_buffer: undo a get_write_access without any buffer * 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. * 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 * The caller passes in the number of credits which should be put back for
* this buffer (zero or one). * 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 void
journal_release_buffer(handle_t *handle, struct buffer_head *bh, int credits) journal_release_buffer(handle_t *handle, struct buffer_head *bh, int credits)
{ {
transaction_t *transaction = handle->h_transaction; BUFFER_TRACE(bh, "entry");
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);
handle->h_buffer_credits += credits; handle->h_buffer_credits += credits;
JBUFFER_TRACE(jh, "exit");
} }
/** /**
......
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