diff options
| author | Jan Kara <jack@suse.cz> | 2005-09-06 18:19:17 -0400 |
|---|---|---|
| committer | Linus Torvalds <torvalds@g5.osdl.org> | 2005-09-07 19:57:57 -0400 |
| commit | 4407c2b6b297339e296facf62e020cf66e55053d (patch) | |
| tree | 485d60b1cb5c6013d09a0327355e216b202bd8ed /fs | |
| parent | e39f07c83bac96850265b87a69dfc5c90ed4f1f5 (diff) | |
[PATCH] Fix race in do_get_write_access()
attached patch should fix the following race:
Proc 1 Proc 2
__flush_batch()
ll_rw_block()
do_get_write_access()
lock_buffer
jh is only waiting for checkpoint
-> b_transaction == NULL ->
do nothing
unlock_buffer
test_set_buffer_locked()
test_clear_buffer_dirty()
__journal_file_buffer()
change the data
submit_bh()
and we have sent wrong data to disk... We now clean the dirty buffer flag
under buffer lock in all cases and hence we know that whenever a buffer is
starting to be journaled we either finish the pending write-out before
attaching a buffer to a transaction or we won't write the buffer until the
transaction is going to be committed.
The test in jbd_unexpected_dirty_buffer() is redundant - remove it.
Furthermore we have to clear the buffer dirty bit under the buffer lock to
prevent races with buffer write-out (and hence prevent returning a buffer with
IO happening).
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Diffstat (limited to 'fs')
| -rw-r--r-- | fs/jbd/transaction.c | 39 |
1 files changed, 21 insertions, 18 deletions
diff --git a/fs/jbd/transaction.c b/fs/jbd/transaction.c index 77b7662b840b..c6ec66fd8766 100644 --- a/fs/jbd/transaction.c +++ b/fs/jbd/transaction.c | |||
| @@ -490,23 +490,21 @@ void journal_unlock_updates (journal_t *journal) | |||
| 490 | */ | 490 | */ |
| 491 | static void jbd_unexpected_dirty_buffer(struct journal_head *jh) | 491 | static void jbd_unexpected_dirty_buffer(struct journal_head *jh) |
| 492 | { | 492 | { |
| 493 | struct buffer_head *bh = jh2bh(jh); | ||
| 494 | int jlist; | 493 | int jlist; |
| 495 | 494 | ||
| 496 | if (buffer_dirty(bh)) { | 495 | /* If this buffer is one which might reasonably be dirty |
| 497 | /* If this buffer is one which might reasonably be dirty | 496 | * --- ie. data, or not part of this journal --- then |
| 498 | * --- ie. data, or not part of this journal --- then | 497 | * we're OK to leave it alone, but otherwise we need to |
| 499 | * we're OK to leave it alone, but otherwise we need to | 498 | * move the dirty bit to the journal's own internal |
| 500 | * move the dirty bit to the journal's own internal | 499 | * JBDDirty bit. */ |
| 501 | * JBDDirty bit. */ | 500 | jlist = jh->b_jlist; |
| 502 | jlist = jh->b_jlist; | 501 | |
| 503 | 502 | if (jlist == BJ_Metadata || jlist == BJ_Reserved || | |
| 504 | if (jlist == BJ_Metadata || jlist == BJ_Reserved || | 503 | jlist == BJ_Shadow || jlist == BJ_Forget) { |
| 505 | jlist == BJ_Shadow || jlist == BJ_Forget) { | 504 | struct buffer_head *bh = jh2bh(jh); |
| 506 | if (test_clear_buffer_dirty(jh2bh(jh))) { | 505 | |
| 507 | set_bit(BH_JBDDirty, &jh2bh(jh)->b_state); | 506 | if (test_clear_buffer_dirty(bh)) |
| 508 | } | 507 | set_buffer_jbddirty(bh); |
| 509 | } | ||
| 510 | } | 508 | } |
| 511 | } | 509 | } |
| 512 | 510 | ||
| @@ -574,9 +572,14 @@ repeat: | |||
| 574 | if (jh->b_next_transaction) | 572 | if (jh->b_next_transaction) |
| 575 | J_ASSERT_JH(jh, jh->b_next_transaction == | 573 | J_ASSERT_JH(jh, jh->b_next_transaction == |
| 576 | transaction); | 574 | transaction); |
| 577 | JBUFFER_TRACE(jh, "Unexpected dirty buffer"); | 575 | } |
| 578 | jbd_unexpected_dirty_buffer(jh); | 576 | /* |
| 579 | } | 577 | * In any case we need to clean the dirty flag and we must |
| 578 | * do it under the buffer lock to be sure we don't race | ||
| 579 | * with running write-out. | ||
| 580 | */ | ||
| 581 | JBUFFER_TRACE(jh, "Unexpected dirty buffer"); | ||
| 582 | jbd_unexpected_dirty_buffer(jh); | ||
| 580 | } | 583 | } |
| 581 | 584 | ||
| 582 | unlock_buffer(bh); | 585 | unlock_buffer(bh); |
