JFS fsync wrong behavior when I/O failure occurs
PATCH 1/1 JFS fsync wrong behavior when I/O failure occurs
The patch has sent to JFS maintainer, and ACKed,
but filtered by LKML, so I send it again.
JFS contains a bug with sys_fsync(jfs_fsync), also with fs/mpage.c:mpage_end_io_write
Symptom
The open-write-fsync-close code will not return error when disk I/O occurs
between open and fsync.
Details
1. sys_fsync does 3 jobs:
submit all pages taged with PAGECACHE_TAG_DIRTY to disk I/O,
do file-system related fsync operations
wait all pages taged with PAGECACHE_TAG_WRITEBACK to complete.
Here, all disk I/O are asynchronous, when finished, mpage_end_io_write will
remove page from mapping's PAGECACHE_TAG_WRITEBACK tree. If I/O fails, it will also set PG_error flag, but it FORGET to set the mapping->flags AS_EIO
flag. So in step 3, sys_fsync won't notice these pages.
2. Besides 1, jfs_fsync should return error also, but it doesn't wait the page
uptodate, e.g.:jfs_fsync->jfs_commit_inode->txCommit->diWrite->read_metapage->__get_metapage ->read_cache_page reads a page from disk. Because read is async, whenread_cache_page: err = filler(data, page), filler will not return error, it just submits I/O request and returns. So, page is not uptodate. only use if(IS_ERROR(mp->page)) is not enough, we should add
"|| !PageUptodate(mp->page)" - remember, mp->data = kmap(mp->page) + page_offst,
later, the outer function will have mem access with mp->data. If mp->data is not
uptodate, dangerous.
Patch
diff -urN linux-2.6.11.7/fs/jfs/jfs_metapage.c linux-2.6.11.7.new/fs/jfs/jfs_metapage.c
--- linux-2.6.11.7/fs/jfs/jfs_metapage.c 2005-04-07 14:57:26.000000000 -0400
+++ linux-2.6.11.7.new/fs/jfs/jfs_metapage.c 2005-04-25 16:33:59.000000000 -0400
@@ -347,7 +347,13 @@
jfs_info("__get_metapage: Calling read_cache_page");
mp->page = read_cache_page(mapping, lblock,
(filler_t *)mapping->a_ops->readpage, NULL);
- if (IS_ERR(mp->page)) {
+ if (IS_ERR(mp->page) || !PageUptodate(mp->page)) {
+ /* Page hasn't been uptodate yet because of
+ * async I/O, in most cases, I/O error has
+ * occurred. We must make sure the page is
+ * uptodate - the outer function will do mem
+ * operations with kmap(mp->page) + page_offset
+ */
jfs_err("read_cache_page failed!");
goto freeit;
} elsefs/mpage.c 2005-04-25 16:38:05.000000000 -0400 renamed from fs/mpage.c 2005-04-07 14:57:25.000000000 -0400 +3 -1
@@ -79,8 +79,10 @@ if (--bvec >= bio->bi_io_vec) prefetchw(&bvec->bv_page->flags); - if (!uptodate) + if (!uptodate){ SetPageError(page); + set_bit(AS_EIO, &page->mapping->flags); + } end_page_writeback(page); } while (bvec >= bio->bi_io_vec); bio_put(bio);
Ref
http://developer.osdl.jp/projects/doubt/fs-consistency-and-coherency/index.html