From: Jeff Layton <jlayton@redhat.com> To: Andrew Morton <akpm@linux-foundation.org>, Al Viro <viro@ZenIV.linux.org.uk>, Jan Kara <jack@suse.cz>, tytso@mit.edu, axboe@kernel.dk, mawilcox@microsoft.com, ross.zwisler@linux.intel.com, corbet@lwn.net, Chris Mason <clm@fb.com>, Josef Bacik <jbacik@fb.com>, David Sterba <dsterba@suse.com>, "Darrick J . Wong" <darrick.wong@oracle.com> Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-block@vger.kernel.org Subject: [PATCH v6 04/20] buffer: set errors in mapping at the time that the error occurs Date: Mon, 12 Jun 2017 08:22:56 -0400 [thread overview] Message-ID: <20170612122316.13244-5-jlayton@redhat.com> (raw) In-Reply-To: <20170612122316.13244-1-jlayton@redhat.com> I noticed on xfs that I could still sometimes get back an error on fsync on a fd that was opened after the error condition had been cleared. The problem is that the buffer code sets the write_io_error flag and then later checks that flag to set the error in the mapping. That flag perisists for quite a while however. If the file is later opened with O_TRUNC, the buffers will then be invalidated and the mapping's error set such that a subsequent fsync will return error. I think this is incorrect, as there was no writeback between the open and fsync. Add a new mark_buffer_write_io_error operation that sets the flag and the error in the mapping at the same time. Replace all calls to set_buffer_write_io_error with mark_buffer_write_io_error, and remove the places that check this flag in order to set the error in the mapping. This sets the error in the mapping earlier, at the time that it's first detected. Signed-off-by: Jeff Layton <jlayton@redhat.com> Reviewed-by: Jan Kara <jack@suse.cz> --- fs/buffer.c | 20 +++++++++++++------- fs/gfs2/lops.c | 2 +- include/linux/buffer_head.h | 1 + 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index 4be8b914a222..b946149e8214 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -178,7 +178,7 @@ void end_buffer_write_sync(struct buffer_head *bh, int uptodate) set_buffer_uptodate(bh); } else { buffer_io_error(bh, ", lost sync page write"); - set_buffer_write_io_error(bh); + mark_buffer_write_io_error(bh); clear_buffer_uptodate(bh); } unlock_buffer(bh); @@ -352,8 +352,7 @@ void end_buffer_async_write(struct buffer_head *bh, int uptodate) set_buffer_uptodate(bh); } else { buffer_io_error(bh, ", lost async page write"); - mapping_set_error(page->mapping, -EIO); - set_buffer_write_io_error(bh); + mark_buffer_write_io_error(bh); clear_buffer_uptodate(bh); SetPageError(page); } @@ -481,8 +480,6 @@ static void __remove_assoc_queue(struct buffer_head *bh) { list_del_init(&bh->b_assoc_buffers); WARN_ON(!bh->b_assoc_map); - if (buffer_write_io_error(bh)) - mapping_set_error(bh->b_assoc_map, -EIO); bh->b_assoc_map = NULL; } @@ -1181,6 +1178,17 @@ void mark_buffer_dirty(struct buffer_head *bh) } EXPORT_SYMBOL(mark_buffer_dirty); +void mark_buffer_write_io_error(struct buffer_head *bh) +{ + set_buffer_write_io_error(bh); + /* FIXME: do we need to set this in both places? */ + if (bh->b_page && bh->b_page->mapping) + mapping_set_error(bh->b_page->mapping, -EIO); + if (bh->b_assoc_map) + mapping_set_error(bh->b_assoc_map, -EIO); +} +EXPORT_SYMBOL(mark_buffer_write_io_error); + /* * Decrement a buffer_head's reference count. If all buffers against a page * have zero reference count, are clean and unlocked, and if the page is clean @@ -3279,8 +3287,6 @@ drop_buffers(struct page *page, struct buffer_head **buffers_to_free) bh = head; do { - if (buffer_write_io_error(bh) && page->mapping) - mapping_set_error(page->mapping, -EIO); if (buffer_busy(bh)) goto failed; bh = bh->b_this_page; diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c index b1f9144b42c7..cd7857ab1a6a 100644 --- a/fs/gfs2/lops.c +++ b/fs/gfs2/lops.c @@ -182,7 +182,7 @@ static void gfs2_end_log_write_bh(struct gfs2_sbd *sdp, struct bio_vec *bvec, bh = bh->b_this_page; do { if (error) - set_buffer_write_io_error(bh); + mark_buffer_write_io_error(bh); unlock_buffer(bh); next = bh->b_this_page; size -= bh->b_size; diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h index bd029e52ef5e..e0abeba3ced7 100644 --- a/include/linux/buffer_head.h +++ b/include/linux/buffer_head.h @@ -149,6 +149,7 @@ void buffer_check_dirty_writeback(struct page *page, */ void mark_buffer_dirty(struct buffer_head *bh); +void mark_buffer_write_io_error(struct buffer_head *bh); void init_buffer(struct buffer_head *, bh_end_io_t *, void *); void touch_buffer(struct buffer_head *bh); void set_bh_page(struct buffer_head *bh, -- 2.13.0
WARNING: multiple messages have this Message-ID (diff)
From: Jeff Layton <jlayton@redhat.com> To: Andrew Morton <akpm@linux-foundation.org>, Al Viro <viro@ZenIV.linux.org.uk>, Jan Kara <jack@suse.cz>, tytso@mit.edu, axboe@kernel.dk, mawilcox@microsoft.com, ross.zwisler@linux.intel.com, corbet@lwn.net, Chris Mason <clm@fb.com>, Josef Bacik <jbacik@fb.com>, David Sterba <dsterba@suse.com>, "Darrick J . Wong" <darrick.wong@oracle.com> Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-block@vger.kernel.org Subject: [PATCH v6 04/20] buffer: set errors in mapping at the time that the error occurs Date: Mon, 12 Jun 2017 08:22:56 -0400 [thread overview] Message-ID: <20170612122316.13244-5-jlayton@redhat.com> (raw) In-Reply-To: <20170612122316.13244-1-jlayton@redhat.com> I noticed on xfs that I could still sometimes get back an error on fsync on a fd that was opened after the error condition had been cleared. The problem is that the buffer code sets the write_io_error flag and then later checks that flag to set the error in the mapping. That flag perisists for quite a while however. If the file is later opened with O_TRUNC, the buffers will then be invalidated and the mapping's error set such that a subsequent fsync will return error. I think this is incorrect, as there was no writeback between the open and fsync. Add a new mark_buffer_write_io_error operation that sets the flag and the error in the mapping at the same time. Replace all calls to set_buffer_write_io_error with mark_buffer_write_io_error, and remove the places that check this flag in order to set the error in the mapping. This sets the error in the mapping earlier, at the time that it's first detected. Signed-off-by: Jeff Layton <jlayton@redhat.com> Reviewed-by: Jan Kara <jack@suse.cz> --- fs/buffer.c | 20 +++++++++++++------- fs/gfs2/lops.c | 2 +- include/linux/buffer_head.h | 1 + 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index 4be8b914a222..b946149e8214 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -178,7 +178,7 @@ void end_buffer_write_sync(struct buffer_head *bh, int uptodate) set_buffer_uptodate(bh); } else { buffer_io_error(bh, ", lost sync page write"); - set_buffer_write_io_error(bh); + mark_buffer_write_io_error(bh); clear_buffer_uptodate(bh); } unlock_buffer(bh); @@ -352,8 +352,7 @@ void end_buffer_async_write(struct buffer_head *bh, int uptodate) set_buffer_uptodate(bh); } else { buffer_io_error(bh, ", lost async page write"); - mapping_set_error(page->mapping, -EIO); - set_buffer_write_io_error(bh); + mark_buffer_write_io_error(bh); clear_buffer_uptodate(bh); SetPageError(page); } @@ -481,8 +480,6 @@ static void __remove_assoc_queue(struct buffer_head *bh) { list_del_init(&bh->b_assoc_buffers); WARN_ON(!bh->b_assoc_map); - if (buffer_write_io_error(bh)) - mapping_set_error(bh->b_assoc_map, -EIO); bh->b_assoc_map = NULL; } @@ -1181,6 +1178,17 @@ void mark_buffer_dirty(struct buffer_head *bh) } EXPORT_SYMBOL(mark_buffer_dirty); +void mark_buffer_write_io_error(struct buffer_head *bh) +{ + set_buffer_write_io_error(bh); + /* FIXME: do we need to set this in both places? */ + if (bh->b_page && bh->b_page->mapping) + mapping_set_error(bh->b_page->mapping, -EIO); + if (bh->b_assoc_map) + mapping_set_error(bh->b_assoc_map, -EIO); +} +EXPORT_SYMBOL(mark_buffer_write_io_error); + /* * Decrement a buffer_head's reference count. If all buffers against a page * have zero reference count, are clean and unlocked, and if the page is clean @@ -3279,8 +3287,6 @@ drop_buffers(struct page *page, struct buffer_head **buffers_to_free) bh = head; do { - if (buffer_write_io_error(bh) && page->mapping) - mapping_set_error(page->mapping, -EIO); if (buffer_busy(bh)) goto failed; bh = bh->b_this_page; diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c index b1f9144b42c7..cd7857ab1a6a 100644 --- a/fs/gfs2/lops.c +++ b/fs/gfs2/lops.c @@ -182,7 +182,7 @@ static void gfs2_end_log_write_bh(struct gfs2_sbd *sdp, struct bio_vec *bvec, bh = bh->b_this_page; do { if (error) - set_buffer_write_io_error(bh); + mark_buffer_write_io_error(bh); unlock_buffer(bh); next = bh->b_this_page; size -= bh->b_size; diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h index bd029e52ef5e..e0abeba3ced7 100644 --- a/include/linux/buffer_head.h +++ b/include/linux/buffer_head.h @@ -149,6 +149,7 @@ void buffer_check_dirty_writeback(struct page *page, */ void mark_buffer_dirty(struct buffer_head *bh); +void mark_buffer_write_io_error(struct buffer_head *bh); void init_buffer(struct buffer_head *, bh_end_io_t *, void *); void touch_buffer(struct buffer_head *bh); void set_bh_page(struct buffer_head *bh, -- 2.13.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:[~2017-06-12 12:22 UTC|newest] Thread overview: 85+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-06-12 12:22 [PATCH v6 00/20] fs: enhanced writeback error reporting with errseq_t (pile #1) Jeff Layton 2017-06-12 12:22 ` Jeff Layton 2017-06-12 12:22 ` [PATCH v6 01/20] mm: fix mapping_set_error call in me_pagecache_dirty Jeff Layton 2017-06-12 12:22 ` Jeff Layton 2017-06-12 12:22 ` [PATCH v6 02/20] buffer: use mapping_set_error instead of setting the flag Jeff Layton 2017-06-12 12:22 ` Jeff Layton 2017-06-12 12:22 ` [PATCH v6 03/20] fs: check for writeback errors after syncing out buffers in generic_file_fsync Jeff Layton 2017-06-12 12:22 ` Jeff Layton 2017-06-12 12:22 ` Jeff Layton [this message] 2017-06-12 12:22 ` [PATCH v6 04/20] buffer: set errors in mapping at the time that the error occurs Jeff Layton 2017-06-12 12:22 ` [PATCH v6 05/20] mm: don't TestClearPageError in __filemap_fdatawait_range Jeff Layton 2017-06-12 12:22 ` Jeff Layton 2017-06-12 12:22 ` [PATCH v6 06/20] mm: drop "wait" parameter from write_one_page Jeff Layton 2017-06-12 12:22 ` Jeff Layton 2017-06-12 12:22 ` [PATCH v6 07/20] mm: clean up error handling in write_one_page Jeff Layton 2017-06-12 12:22 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 08/20] lib: add errseq_t type and infrastructure for handling it Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 09/20] fs: new infrastructure for writeback error handling and reporting Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 10/13] ext4: add more robust reporting of metadata writeback errors Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 10/20] mm: tracepoints for writeback error events Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 11/13] Documentation: flesh out the section in vfs.txt on storing and reporting writeback errors Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 11/20] mm: set both AS_EIO/AS_ENOSPC and errseq_t in mapping_set_error Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 12/20] fs: add a new fstype flag to indicate how writeback errors are tracked Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:45 ` Christoph Hellwig 2017-06-12 12:45 ` Christoph Hellwig 2017-06-13 10:24 ` Jeff Layton 2017-06-13 10:24 ` Jeff Layton 2017-06-13 10:24 ` Jeff Layton 2017-06-13 10:24 ` Jeff Layton 2017-06-14 6:47 ` Christoph Hellwig 2017-06-14 6:47 ` Christoph Hellwig 2017-06-14 17:24 ` Jeff Layton 2017-06-14 17:24 ` Jeff Layton 2017-06-14 17:24 ` Jeff Layton 2017-06-14 17:24 ` Jeff Layton 2017-06-15 8:22 ` Christoph Hellwig 2017-06-15 8:22 ` Christoph Hellwig 2017-06-15 10:42 ` Jeff Layton 2017-06-15 10:42 ` Jeff Layton 2017-06-15 10:42 ` Jeff Layton 2017-06-15 10:42 ` Jeff Layton 2017-06-15 14:57 ` Christoph Hellwig 2017-06-15 14:57 ` Christoph Hellwig 2017-06-15 15:03 ` Jeff Layton 2017-06-15 15:03 ` Jeff Layton 2017-06-15 15:03 ` Jeff Layton 2017-06-15 15:03 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 12/13] xfs: minimal conversion to errseq_t writeback error reporting Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 13/13] btrfs: minimal conversion to errseq_t writeback error reporting on fsync Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 13/20] Documentation: flesh out the section in vfs.txt on storing and reporting writeback errors Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 14/20] dax: set errors in mapping when writeback fails Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:44 ` Christoph Hellwig 2017-06-12 12:44 ` Christoph Hellwig 2017-06-12 12:23 ` [PATCH v6 15/20] fs: have call_fsync call filemap_report_wb_err if FS_WB_ERRSEQ is set Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:42 ` Christoph Hellwig 2017-06-12 12:42 ` Christoph Hellwig 2017-06-12 12:23 ` [PATCH v6 16/20] block: convert to errseq_t based writeback error tracking Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 17/20] fs: add f_md_wb_err field to struct file for tracking metadata errors Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 18/20] ext4: use errseq_t based error handling for reporting data writeback errors Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 19/20] xfs: minimal conversion to errseq_t writeback error reporting Jeff Layton 2017-06-12 12:23 ` Jeff Layton 2017-06-13 4:30 ` Darrick J. Wong 2017-06-13 4:30 ` Darrick J. Wong 2017-06-13 10:27 ` Jeff Layton 2017-06-13 10:27 ` Jeff Layton 2017-06-13 10:27 ` Jeff Layton 2017-06-13 10:27 ` Jeff Layton 2017-06-12 12:23 ` [PATCH v6 20/20] btrfs: minimal conversion to errseq_t writeback error reporting on fsync Jeff Layton 2017-06-12 12:23 ` Jeff Layton
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=20170612122316.13244-5-jlayton@redhat.com \ --to=jlayton@redhat.com \ --cc=akpm@linux-foundation.org \ --cc=axboe@kernel.dk \ --cc=clm@fb.com \ --cc=corbet@lwn.net \ --cc=darrick.wong@oracle.com \ --cc=dsterba@suse.com \ --cc=jack@suse.cz \ --cc=jbacik@fb.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-btrfs@vger.kernel.org \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-xfs@vger.kernel.org \ --cc=mawilcox@microsoft.com \ --cc=ross.zwisler@linux.intel.com \ --cc=tytso@mit.edu \ --cc=viro@ZenIV.linux.org.uk \ /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.