| Message ID | 20170412120614.6111-5-jlayton@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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 > >
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");
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(-)