From: Junichi Nomura <j-nomura@ce.jp.nec.com> To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, "linux-mm@kvack.org" <linux-mm@kvack.org> Cc: "akpm@linux-foundation.org" <akpm@linux-foundation.org>, "andi@firstfloor.org" <andi@firstfloor.org>, "fengguang.wu@intel.com" <fengguang.wu@intel.com>, "tony.luck@intel.com" <tony.luck@intel.com>, "liwanp@linux.vnet.ibm.com" <liwanp@linux.vnet.ibm.com>, "david@fromorbit.com" <david@fromorbit.com>, Tejun Heo <tj@kernel.org>, "Naoya Horiguchi" <n-horiguchi@ah.jp.nec.com> Subject: [PATCH 1/1] fs: global sync to not clear error status of individual inodes Date: Tue, 15 Sep 2015 09:54:13 +0000 [thread overview] Message-ID: <20150915095412.GD13399@xzibit.linux.bs1.fc.nec.co.jp> (raw) In-Reply-To: <20150915094638.GA13399@xzibit.linux.bs1.fc.nec.co.jp> filemap_fdatawait() is a function to wait for on-going writeback to complete but also consume and clear error status of the mapping set during writeback. The latter functionality is critical for applications to detect writeback error with system calls like fsync(2)/fdatasync(2). However filemap_fdatawait() is also used by sync(2) or FIFREEZE ioctl, which don't check error status of individual mappings. As a result, fsync() may not be able to detect writeback error if events happen in the following order: Application System admin ---------------------------------------------------------- write data on page cache Run sync command writeback completes with error filemap_fdatawait() clears error fsync returns success (but the data is not on disk) This patch adds filemap_fdatawait_keep_errors() for call sites where writeback error is not handled so that they don't clear error status. Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com> --- fs/fs-writeback.c | 8 +++++++- fs/sync.c | 2 +- include/linux/fs.h | 1 + mm/filemap.c | 35 ++++++++++++++++++++++++++++++++--- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 587ac08..df52aad 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -2121,7 +2121,13 @@ static void wait_sb_inodes(struct super_block *sb) iput(old_inode); old_inode = inode; - filemap_fdatawait(mapping); + /* + * Wait for on-going writeback to complete + * but not consume error status on this mapping. + * Otherwise application may fail to catch writeback error + * using fsync(2). + */ + filemap_fdatawait_keep_errors(mapping); cond_resched(); diff --git a/fs/sync.c b/fs/sync.c index fbc98ee..e2b7a77 100644 --- a/fs/sync.c +++ b/fs/sync.c @@ -86,7 +86,7 @@ static void fdatawrite_one_bdev(struct block_device *bdev, void *arg) static void fdatawait_one_bdev(struct block_device *bdev, void *arg) { - filemap_fdatawait(bdev->bd_inode->i_mapping); + filemap_fdatawait_keep_errors(bdev->bd_inode->i_mapping); } /* diff --git a/include/linux/fs.h b/include/linux/fs.h index 72d8a84..9355f37 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2422,6 +2422,7 @@ extern int write_inode_now(struct inode *, int); extern int filemap_fdatawrite(struct address_space *); extern int filemap_flush(struct address_space *); extern int filemap_fdatawait(struct address_space *); +extern void filemap_fdatawait_keep_errors(struct address_space *); extern int filemap_fdatawait_range(struct address_space *, loff_t lstart, loff_t lend); extern int filemap_write_and_wait(struct address_space *mapping); diff --git a/mm/filemap.c b/mm/filemap.c index 72940fb..059050a 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -340,14 +340,14 @@ EXPORT_SYMBOL(filemap_flush); * Walk the list of under-writeback pages of the given address space * in the given range and wait for all of them. */ -int filemap_fdatawait_range(struct address_space *mapping, loff_t start_byte, - loff_t end_byte) +static int __filemap_fdatawait_range(struct address_space *mapping, + loff_t start_byte, loff_t end_byte) { pgoff_t index = start_byte >> PAGE_CACHE_SHIFT; pgoff_t end = end_byte >> PAGE_CACHE_SHIFT; struct pagevec pvec; int nr_pages; - int ret2, ret = 0; + int ret = 0; if (end_byte < start_byte) goto out; @@ -374,6 +374,15 @@ int filemap_fdatawait_range(struct address_space *mapping, loff_t start_byte, cond_resched(); } out: + return ret; +} + +int filemap_fdatawait_range(struct address_space *mapping, loff_t start_byte, + loff_t end_byte) +{ + int ret, ret2; + + ret = __filemap_fdatawait_range(mapping, start_byte, end_byte); ret2 = filemap_check_errors(mapping); if (!ret) ret = ret2; @@ -382,6 +391,26 @@ out: } EXPORT_SYMBOL(filemap_fdatawait_range); +/* + * As filemap_check_errors() consumes and clears error status of mapping, + * filemap_fdatawait() should be used only when the caller is responsible + * for handling the error. + * + * Use filemap_fdatawait_keep_errors() if callers just want to wait for + * witeback and don't handle errors themselves. + * Expected call sites are system-wide / filesystem-wide data flushers: + * e.g. sync(2), fsfreeze(8) + */ +void filemap_fdatawait_keep_errors(struct address_space *mapping) +{ + loff_t i_size = i_size_read(mapping->host); + + if (i_size == 0) + return; + + __filemap_fdatawait_range(mapping, 0, i_size - 1); +} + /** * filemap_fdatawait - wait for all under-writeback pages to complete * @mapping: address space structure to wait for -- 2.1.0
WARNING: multiple messages have this Message-ID (diff)
From: Junichi Nomura <j-nomura@ce.jp.nec.com> To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, "linux-mm@kvack.org" <linux-mm@kvack.org> Cc: "akpm@linux-foundation.org" <akpm@linux-foundation.org>, "andi@firstfloor.org" <andi@firstfloor.org>, "fengguang.wu@intel.com" <fengguang.wu@intel.com>, "tony.luck@intel.com" <tony.luck@intel.com>, "liwanp@linux.vnet.ibm.com" <liwanp@linux.vnet.ibm.com>, "david@fromorbit.com" <david@fromorbit.com>, Tejun Heo <tj@kernel.org>, "Naoya Horiguchi" <n-horiguchi@ah.jp.nec.com> Subject: [PATCH 1/1] fs: global sync to not clear error status of individual inodes Date: Tue, 15 Sep 2015 09:54:13 +0000 [thread overview] Message-ID: <20150915095412.GD13399@xzibit.linux.bs1.fc.nec.co.jp> (raw) In-Reply-To: <20150915094638.GA13399@xzibit.linux.bs1.fc.nec.co.jp> filemap_fdatawait() is a function to wait for on-going writeback to complete but also consume and clear error status of the mapping set during writeback. The latter functionality is critical for applications to detect writeback error with system calls like fsync(2)/fdatasync(2). However filemap_fdatawait() is also used by sync(2) or FIFREEZE ioctl, which don't check error status of individual mappings. As a result, fsync() may not be able to detect writeback error if events happen in the following order: Application System admin ---------------------------------------------------------- write data on page cache Run sync command writeback completes with error filemap_fdatawait() clears error fsync returns success (but the data is not on disk) This patch adds filemap_fdatawait_keep_errors() for call sites where writeback error is not handled so that they don't clear error status. Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com> --- fs/fs-writeback.c | 8 +++++++- fs/sync.c | 2 +- include/linux/fs.h | 1 + mm/filemap.c | 35 ++++++++++++++++++++++++++++++++--- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 587ac08..df52aad 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -2121,7 +2121,13 @@ static void wait_sb_inodes(struct super_block *sb) iput(old_inode); old_inode = inode; - filemap_fdatawait(mapping); + /* + * Wait for on-going writeback to complete + * but not consume error status on this mapping. + * Otherwise application may fail to catch writeback error + * using fsync(2). + */ + filemap_fdatawait_keep_errors(mapping); cond_resched(); diff --git a/fs/sync.c b/fs/sync.c index fbc98ee..e2b7a77 100644 --- a/fs/sync.c +++ b/fs/sync.c @@ -86,7 +86,7 @@ static void fdatawrite_one_bdev(struct block_device *bdev, void *arg) static void fdatawait_one_bdev(struct block_device *bdev, void *arg) { - filemap_fdatawait(bdev->bd_inode->i_mapping); + filemap_fdatawait_keep_errors(bdev->bd_inode->i_mapping); } /* diff --git a/include/linux/fs.h b/include/linux/fs.h index 72d8a84..9355f37 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2422,6 +2422,7 @@ extern int write_inode_now(struct inode *, int); extern int filemap_fdatawrite(struct address_space *); extern int filemap_flush(struct address_space *); extern int filemap_fdatawait(struct address_space *); +extern void filemap_fdatawait_keep_errors(struct address_space *); extern int filemap_fdatawait_range(struct address_space *, loff_t lstart, loff_t lend); extern int filemap_write_and_wait(struct address_space *mapping); diff --git a/mm/filemap.c b/mm/filemap.c index 72940fb..059050a 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -340,14 +340,14 @@ EXPORT_SYMBOL(filemap_flush); * Walk the list of under-writeback pages of the given address space * in the given range and wait for all of them. */ -int filemap_fdatawait_range(struct address_space *mapping, loff_t start_byte, - loff_t end_byte) +static int __filemap_fdatawait_range(struct address_space *mapping, + loff_t start_byte, loff_t end_byte) { pgoff_t index = start_byte >> PAGE_CACHE_SHIFT; pgoff_t end = end_byte >> PAGE_CACHE_SHIFT; struct pagevec pvec; int nr_pages; - int ret2, ret = 0; + int ret = 0; if (end_byte < start_byte) goto out; @@ -374,6 +374,15 @@ int filemap_fdatawait_range(struct address_space *mapping, loff_t start_byte, cond_resched(); } out: + return ret; +} + +int filemap_fdatawait_range(struct address_space *mapping, loff_t start_byte, + loff_t end_byte) +{ + int ret, ret2; + + ret = __filemap_fdatawait_range(mapping, start_byte, end_byte); ret2 = filemap_check_errors(mapping); if (!ret) ret = ret2; @@ -382,6 +391,26 @@ out: } EXPORT_SYMBOL(filemap_fdatawait_range); +/* + * As filemap_check_errors() consumes and clears error status of mapping, + * filemap_fdatawait() should be used only when the caller is responsible + * for handling the error. + * + * Use filemap_fdatawait_keep_errors() if callers just want to wait for + * witeback and don't handle errors themselves. + * Expected call sites are system-wide / filesystem-wide data flushers: + * e.g. sync(2), fsfreeze(8) + */ +void filemap_fdatawait_keep_errors(struct address_space *mapping) +{ + loff_t i_size = i_size_read(mapping->host); + + if (i_size == 0) + return; + + __filemap_fdatawait_range(mapping, 0, i_size - 1); +} + /** * filemap_fdatawait - wait for all under-writeback pages to complete * @mapping: address space structure to wait for -- 2.1.0 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2015-09-15 9:58 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-09-15 9:46 [PATCH 0/1] Fix false-negative error reporting from fsync/fdatasync Junichi Nomura 2015-09-15 9:46 ` Junichi Nomura 2015-09-15 9:49 ` Test program: check if fsync() can detect I/O error (1/2) Junichi Nomura 2015-09-15 9:49 ` Junichi Nomura 2015-09-15 9:52 ` Test program: check if fsync() can detect I/O error (2/2) Junichi Nomura 2015-09-15 9:52 ` Junichi Nomura 2015-09-15 9:54 ` Junichi Nomura [this message] 2015-09-15 9:54 ` [PATCH 1/1] fs: global sync to not clear error status of individual inodes Junichi Nomura 2015-09-15 14:37 ` Andi Kleen 2015-09-15 14:37 ` Andi Kleen 2015-09-15 22:02 ` Andrew Morton 2015-09-15 22:02 ` Andrew Morton 2015-09-16 0:45 ` Junichi Nomura 2015-09-16 0:45 ` Junichi Nomura 2015-09-16 10:45 ` xfstests: test data-writeback error detection with fsync Junichi Nomura 2015-09-16 10:45 ` Junichi Nomura 2015-09-15 15:20 ` [PATCH 1/1] fs: global sync to not clear error status of individual inodes Tejun Heo 2015-09-15 15:20 ` Tejun Heo 2015-09-15 16:52 ` Andi Kleen 2015-09-15 16:52 ` Andi Kleen 2015-09-16 0:59 ` Junichi Nomura 2015-09-16 0:59 ` Junichi Nomura 2015-09-16 8:39 ` [PATCH v2] " Junichi Nomura 2015-09-16 8:39 ` Junichi Nomura 2015-09-16 17:47 ` Tejun Heo 2015-09-16 17:47 ` Tejun Heo
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20150915095412.GD13399@xzibit.linux.bs1.fc.nec.co.jp \ --to=j-nomura@ce.jp.nec.com \ --cc=akpm@linux-foundation.org \ --cc=andi@firstfloor.org \ --cc=david@fromorbit.com \ --cc=fengguang.wu@intel.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=liwanp@linux.vnet.ibm.com \ --cc=n-horiguchi@ah.jp.nec.com \ --cc=tj@kernel.org \ --cc=tony.luck@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.