diff options
author | Hisashi Hifumi <hifumi.hisashi@oss.ntt.co.jp> | 2008-09-02 17:35:40 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2008-09-02 22:21:37 -0400 |
commit | 6ccfa806a9cfbbf1cd43d5b6aa47ef2c0eb518fd (patch) | |
tree | dd3f17e1aebc802b147627a4151add363f39b77c /mm | |
parent | 344c790e3821dac37eb742ddd0b611a300f78b9a (diff) |
VFS: fix dio write returning EIO when try_to_release_page fails
Dio write returns EIO when try_to_release_page fails because bh is
still referenced.
The patch
commit 3f31fddfa26b7594b44ff2b34f9a04ba409e0f91
Author: Mingming Cao <cmm@us.ibm.com>
Date: Fri Jul 25 01:46:22 2008 -0700
jbd: fix race between free buffer and commit transaction
was merged into 2.6.27-rc1, but I noticed that this patch is not enough
to fix the race.
I did fsstress test heavily to 2.6.27-rc1, and found that dio write still
sometimes got EIO through this test.
The patch above fixed race between freeing buffer(dio) and committing
transaction(jbd) but I discovered that there is another race, freeing
buffer(dio) and ext3/4_ordered_writepage.
: background_writeout()
->write_cache_pages()
->ext3_ordered_writepage()
walk_page_buffers() -> take a bh ref
block_write_full_page() -> unlock_page
: <- end_page_writeback
: <- race! (dio write->try_to_release_page fails)
walk_page_buffers() ->release a bh ref
ext3_ordered_writepage holds bh ref and does unlock_page remaining
taking a bh ref, so this causes the race and failure of
try_to_release_page.
To fix this race, I used the approach of falling back to buffered
writes if try_to_release_page() fails on a page.
[akpm@linux-foundation.org: cleanups]
Signed-off-by: Hisashi Hifumi <hifumi.hisashi@oss.ntt.co.jp>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Mingming Cao <cmm@us.ibm.com>
Cc: Zach Brown <zach.brown@oracle.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'mm')
-rw-r--r-- | mm/filemap.c | 11 | ||||
-rw-r--r-- | mm/truncate.c | 4 |
2 files changed, 11 insertions, 4 deletions
diff --git a/mm/filemap.c b/mm/filemap.c index 54e968650855..876bc595d0f8 100644 --- a/mm/filemap.c +++ b/mm/filemap.c | |||
@@ -2129,13 +2129,20 @@ generic_file_direct_write(struct kiocb *iocb, const struct iovec *iov, | |||
2129 | * After a write we want buffered reads to be sure to go to disk to get | 2129 | * After a write we want buffered reads to be sure to go to disk to get |
2130 | * the new data. We invalidate clean cached page from the region we're | 2130 | * the new data. We invalidate clean cached page from the region we're |
2131 | * about to write. We do this *before* the write so that we can return | 2131 | * about to write. We do this *before* the write so that we can return |
2132 | * -EIO without clobbering -EIOCBQUEUED from ->direct_IO(). | 2132 | * without clobbering -EIOCBQUEUED from ->direct_IO(). |
2133 | */ | 2133 | */ |
2134 | if (mapping->nrpages) { | 2134 | if (mapping->nrpages) { |
2135 | written = invalidate_inode_pages2_range(mapping, | 2135 | written = invalidate_inode_pages2_range(mapping, |
2136 | pos >> PAGE_CACHE_SHIFT, end); | 2136 | pos >> PAGE_CACHE_SHIFT, end); |
2137 | if (written) | 2137 | /* |
2138 | * If a page can not be invalidated, return 0 to fall back | ||
2139 | * to buffered write. | ||
2140 | */ | ||
2141 | if (written) { | ||
2142 | if (written == -EBUSY) | ||
2143 | return 0; | ||
2138 | goto out; | 2144 | goto out; |
2145 | } | ||
2139 | } | 2146 | } |
2140 | 2147 | ||
2141 | written = mapping->a_ops->direct_IO(WRITE, iocb, iov, pos, *nr_segs); | 2148 | written = mapping->a_ops->direct_IO(WRITE, iocb, iov, pos, *nr_segs); |
diff --git a/mm/truncate.c b/mm/truncate.c index 250505091d37..6650c1d878b4 100644 --- a/mm/truncate.c +++ b/mm/truncate.c | |||
@@ -380,7 +380,7 @@ static int do_launder_page(struct address_space *mapping, struct page *page) | |||
380 | * Any pages which are found to be mapped into pagetables are unmapped prior to | 380 | * Any pages which are found to be mapped into pagetables are unmapped prior to |
381 | * invalidation. | 381 | * invalidation. |
382 | * | 382 | * |
383 | * Returns -EIO if any pages could not be invalidated. | 383 | * Returns -EBUSY if any pages could not be invalidated. |
384 | */ | 384 | */ |
385 | int invalidate_inode_pages2_range(struct address_space *mapping, | 385 | int invalidate_inode_pages2_range(struct address_space *mapping, |
386 | pgoff_t start, pgoff_t end) | 386 | pgoff_t start, pgoff_t end) |
@@ -440,7 +440,7 @@ int invalidate_inode_pages2_range(struct address_space *mapping, | |||
440 | ret2 = do_launder_page(mapping, page); | 440 | ret2 = do_launder_page(mapping, page); |
441 | if (ret2 == 0) { | 441 | if (ret2 == 0) { |
442 | if (!invalidate_complete_page2(mapping, page)) | 442 | if (!invalidate_complete_page2(mapping, page)) |
443 | ret2 = -EIO; | 443 | ret2 = -EBUSY; |
444 | } | 444 | } |
445 | if (ret2 < 0) | 445 | if (ret2 < 0) |
446 | ret = ret2; | 446 | ret = ret2; |