Skip to content

Commit

Permalink
VFS: fix dio write returning EIO when try_to_release_page fails
Browse files Browse the repository at this point in the history
Dio write returns EIO when try_to_release_page fails because bh is
still referenced.

The patch

    commit 3f31fdd
    Author: Mingming Cao <[email protected]>
    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.

[[email protected]: cleanups]
Signed-off-by: Hisashi Hifumi <[email protected]>
Cc: Chris Mason <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Mingming Cao <[email protected]>
Cc: Zach Brown <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Hisashi Hifumi authored and torvalds committed Sep 3, 2008
1 parent 344c790 commit 6ccfa80
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
11 changes: 9 additions & 2 deletions mm/filemap.c
Original file line number Diff line number Diff line change
Expand Up @@ -2129,13 +2129,20 @@ generic_file_direct_write(struct kiocb *iocb, const struct iovec *iov,
* After a write we want buffered reads to be sure to go to disk to get
* the new data. We invalidate clean cached page from the region we're
* about to write. We do this *before* the write so that we can return
* -EIO without clobbering -EIOCBQUEUED from ->direct_IO().
* without clobbering -EIOCBQUEUED from ->direct_IO().
*/
if (mapping->nrpages) {
written = invalidate_inode_pages2_range(mapping,
pos >> PAGE_CACHE_SHIFT, end);
if (written)
/*
* If a page can not be invalidated, return 0 to fall back
* to buffered write.
*/
if (written) {
if (written == -EBUSY)
return 0;
goto out;
}
}

written = mapping->a_ops->direct_IO(WRITE, iocb, iov, pos, *nr_segs);
Expand Down
4 changes: 2 additions & 2 deletions mm/truncate.c
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ static int do_launder_page(struct address_space *mapping, struct page *page)
* Any pages which are found to be mapped into pagetables are unmapped prior to
* invalidation.
*
* Returns -EIO if any pages could not be invalidated.
* Returns -EBUSY if any pages could not be invalidated.
*/
int invalidate_inode_pages2_range(struct address_space *mapping,
pgoff_t start, pgoff_t end)
Expand Down Expand Up @@ -440,7 +440,7 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
ret2 = do_launder_page(mapping, page);
if (ret2 == 0) {
if (!invalidate_complete_page2(mapping, page))
ret2 = -EIO;
ret2 = -EBUSY;
}
if (ret2 < 0)
ret = ret2;
Expand Down

0 comments on commit 6ccfa80

Please sign in to comment.