[v2,04/17] ext2: don't test/clear AS_EIO flag
diff mbox

Message ID 20170412120614.6111-5-jlayton@redhat.com
State New
Headers show

Commit Message

Jeff Layton April 12, 2017, 12:06 p.m. UTC
ext2 does a test+clear of AS_EIO flag. With the new error reporting
infrastructure, we don't need to clear anything. Sample the file's
current error code, and after writeback just report whether any
errors have been seen since then.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/ext2/file.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Jan Kara April 12, 2017, 12:29 p.m. UTC | #1
On Wed 12-04-17 08:06:01, Jeff Layton wrote:
> ext2 does a test+clear of AS_EIO flag. With the new error reporting
> infrastructure, we don't need to clear anything. Sample the file's
> current error code, and after writeback just report whether any
> errors have been seen since then.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/ext2/file.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext2/file.c b/fs/ext2/file.c
> index b21891a6bfca..0ca77d337c94 100644
> --- a/fs/ext2/file.c
> +++ b/fs/ext2/file.c
> @@ -177,7 +177,12 @@ int ext2_fsync(struct file *file, loff_t start, loff_t end, int datasync)
>  	struct address_space *mapping = sb->s_bdev->bd_inode->i_mapping;
>  
>  	ret = generic_file_fsync(file, start, end, datasync);
> -	if (ret == -EIO || test_and_clear_bit(AS_EIO, &mapping->flags)) {
> +	if (ret == -EIO)
> +		ret = -EIO;
> +	else
> +		ret = filemap_check_errors(mapping);
> +

IMO making __generic_file_fsync() perform the filemap_check_errors() after
sync_mapping_buffers() is better and deals with all filesystem using
generic_file_fsync(). Then we can just remove the AS_EIO check here.

								Honza

> +	if (ret) {
>  		/* We don't really know where the IO error happened... */
>  		ext2_error(sb, __func__,
>  			   "detected IO error when writing metadata buffers");
> -- 
> 2.9.3
>
Jeff Layton April 12, 2017, 12:30 p.m. UTC | #2
On Wed, 2017-04-12 at 14:29 +0200, Jan Kara wrote:
> On Wed 12-04-17 08:06:01, Jeff Layton wrote:
> > ext2 does a test+clear of AS_EIO flag. With the new error reporting
> > infrastructure, we don't need to clear anything. Sample the file's
> > current error code, and after writeback just report whether any
> > errors have been seen since then.
> > 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/ext2/file.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/ext2/file.c b/fs/ext2/file.c
> > index b21891a6bfca..0ca77d337c94 100644
> > --- a/fs/ext2/file.c
> > +++ b/fs/ext2/file.c
> > @@ -177,7 +177,12 @@ int ext2_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> >  	struct address_space *mapping = sb->s_bdev->bd_inode->i_mapping;
> >  
> >  	ret = generic_file_fsync(file, start, end, datasync);
> > -	if (ret == -EIO || test_and_clear_bit(AS_EIO, &mapping->flags)) {
> > +	if (ret == -EIO)
> > +		ret = -EIO;
> > +	else
> > +		ret = filemap_check_errors(mapping);
> > +
> 
> IMO making __generic_file_fsync() perform the filemap_check_errors() after
> sync_mapping_buffers() is better and deals with all filesystem using
> generic_file_fsync(). Then we can just remove the AS_EIO check here.
> 
> 								Honza
> 

Thanks, I was just looking at this patch and thinking that it wasn't
quite right. I'll take your approach on the next set.

> > +	if (ret) {
> >  		/* We don't really know where the IO error happened... */
> >  		ext2_error(sb, __func__,
> >  			   "detected IO error when writing metadata buffers");
> > -- 
> > 2.9.3
> >

Patch
diff mbox

diff --git a/fs/ext2/file.c b/fs/ext2/file.c
index b21891a6bfca..0ca77d337c94 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -177,7 +177,12 @@  int ext2_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 	struct address_space *mapping = sb->s_bdev->bd_inode->i_mapping;
 
 	ret = generic_file_fsync(file, start, end, datasync);
-	if (ret == -EIO || test_and_clear_bit(AS_EIO, &mapping->flags)) {
+	if (ret == -EIO)
+		ret = -EIO;
+	else
+		ret = filemap_check_errors(mapping);
+
+	if (ret) {
 		/* We don't really know where the IO error happened... */
 		ext2_error(sb, __func__,
 			   "detected IO error when writing metadata buffers");