ceph: make kcephfs use errseq_t for writeback error reporting
cover ceph: make kcephfs use errseq_t for writeback error reporting
From: Jeff Layton <[email protected]>
A couple of small patches to make cephfs use errseq_t for reporting writeback errors. This ensures that when a writeback error occurs that it is reported to all file descriptions that were open at the time. The first patch is what fixes the error reporting.
The second patch (which should probably be considered an RFC) adds a new fault injection switch for cephfs that makes it throw writeback errors even when the writes succeed. I have an xfstest that uses this to help test error handling behavior in cephfs.
Jeff Layton (2):
ceph: use errseq_t for writeback error reporting
ceph: pagecache writeback fault injection switch
fs/ceph/addr.c | 7 +++++++ fs/ceph/caps.c | 2 +- fs/ceph/debugfs.c | 8 +++++++- fs/ceph/super.h | 2 ++ 4 files changed, 17 insertions(+), 2 deletions(-)
Signature · 1 line
2.13.3
PATCH 1/2 ceph: use errseq_t for writeback error reporting
From: Jeff Layton <[email protected]>
Ensure that when writeback errors are marked that we report those to all file descriptions that were open at the time of the error.
--- fs/ceph/caps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ceph/caps.c +1 -1
@@ -2110,7 +2110,7 @@ int ceph_fsync(struct file *file, loff_t start, loff_t end, int datasync)dout ("fsync %p%s\n" , inode, datasync ?" datasync" :"" ); - ret =filemap_write_and_wait_range (inode->i_mapping, start, end); + ret =file_write_and_wait_range (file, start, end);if (ret <0 ) goto out;
Signature · 1 line
2.13.3
Re: [PATCH 1/2] ceph: use errseq_t for writeback error reporting
Quoted 24 lines · Jeff Layton
From: Jeff Layton <[email protected]>
Ensure that when writeback errors are marked that we report those to all file descriptions that were open at the time of the error.
Signed-off-by Jeff Layton <[email protected]>--- fs/ceph/caps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/ceph/caps.c +1
-2@@ -2110,7 +2110,7 @@ int ceph_fsync(struct file *file, loff_t start, loff_t end, int datasync)dout ("fsync %p%s\n" , inode, datasync ?" datasync" :"" ); - ret =filemap_write_and_wait_range (inode->i_mapping, start, end); + ret =file_write_and_wait_range (file, start, end);if (ret <0 ) goto out; --
Quoted 2 lines
2.13.3
PATCH 2/2 ceph: pagecache writeback fault injection switch
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
Testing ceph for proper writeback error handling turns out to be quite difficult. I tried using iptables to block traffic but that didn't give reliable results.
I hacked in this wb_fault switch that makes the filesystem pretend that writeback failed, even when it succeeds. With this, I could verify that cephfs fsync error reporting does work properly.
--- fs/ceph/addr.c | 7 +++++++ fs/ceph/debugfs.c | 8 +++++++- fs/ceph/super.h | 2 ++ 3 files changed, 16 insertions(+), 1 deletion(-)
fs/ceph/addr.c +7 -0
@@ -584,6 +584,10 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc) page_off, len, truncate_seq, truncate_size, &inode->i_mtime, &page, 1); + +if (fsc->wb_fault && err >=0 ) + err = -EIO ; +if (err <0 ) {struct writeback_control tmp_wbc;if (!wbc)
@@ -666,6 +670,9 @@ static void writepages_finish(struct ceph_osd_request *req)struct ceph_fs_client *fsc =ceph_inode_to_client (inode);bool remove_page; +if (fsc->wb_fault && rc >=0 ) + rc = -EIO ; +dout ("writepages_finish %p rc %d\n" , inode, rc);if (rc <0 ) {mapping_set_error (mapping, rc);
fs/ceph/debugfs.c +7 -1
@@ -197,7 +197,6 @@ CEPH_DEFINE_SHOW_FUNC(caps_show)CEPH_DEFINE_SHOW_FUNC (dentry_lru_show)CEPH_DEFINE_SHOW_FUNC (mds_sessions_show) -/* * debugfs */
@@ -231,6 +230,7 @@ void ceph_fs_debugfs_cleanup(struct ceph_fs_client *fsc)debugfs_remove (fsc->debugfs_caps);debugfs_remove (fsc->debugfs_mdsc);debugfs_remove (fsc->debugfs_dentry_lru); +debugfs_remove (fsc->debugfs_wb_fault); }int ceph_fs_debugfs_init (struct ceph_fs_client *fsc)
@@ -298,6 +298,12 @@ int ceph_fs_debugfs_init(struct ceph_fs_client *fsc)if (!fsc->debugfs_dentry_lru) goto out; + fsc->debugfs_wb_fault =debugfs_create_bool ("wb_fault" , +0600 , fsc->client->debugfs_dir, + &fsc->wb_fault); +if (!fsc->debugfs_wb_fault) + goto out; +return 0 ; out:
fs/ceph/super.h +2 -0
@@ -84,6 +84,7 @@ struct ceph_fs_client {unsigned long mount_state;int min_caps;/* min caps i added */ +bool wb_fault;struct ceph_mds_client *mdsc;
@@ -100,6 +101,7 @@ struct ceph_fs_client {struct dentry *debugfs_bdi;struct dentry *debugfs_mdsc, *debugfs_mdsmap;struct dentry *debugfs_mds_sessions; +struct dentry *debugfs_wb_fault;#endif #ifdef CONFIG_CEPH_FSCACHE
Signature · 1 line
2.13.3
Re: [PATCH 2/2] ceph: pagecache writeback fault injection switch
Quoted 88 lines · Jeff Layton
From: Jeff Layton <[email protected]>
Testing ceph for proper writeback error handling turns out to be quite difficult. I tried using iptables to block traffic but that didn't give reliable results.
I hacked in this
wb_faultswitch that makes the filesystem pretend that writeback failed, even when it succeeds. With this, I could verify that cephfs fsync error reporting does work properly.Signed-off-by Jeff Layton <[email protected]>--- fs/ceph/addr.c | 7 +++++++ fs/ceph/debugfs.c | 8 +++++++- fs/ceph/super.h | 2 ++ 3 files changed, 16 insertions(+), 1 deletion(-)fs/ceph/addr.c +7
-0@@ -584,6 +584,10 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc) page_off, len, truncate_seq, truncate_size, &inode->i_mtime, &page, 1); + +if (fsc->wb_fault && err >=0 ) + err = -EIO ; +if (err <0 ) {struct writeback_control tmp_wbc;if (!wbc)@@ -666,6 +670,9 @@ static void writepages_finish(struct ceph_osd_request *req)struct ceph_fs_client *fsc =ceph_inode_to_client (inode);bool remove_page; +if (fsc->wb_fault && rc >=0 ) + rc = -EIO ; +dout ("writepages_finish %p rc %d\n" , inode, rc);if (rc <0 ) {mapping_set_error (mapping, rc);fs/ceph/debugfs.c +7
-1@@ -197,7 +197,6 @@ CEPH_DEFINE_SHOW_FUNC(caps_show)CEPH_DEFINE_SHOW_FUNC (dentry_lru_show)CEPH_DEFINE_SHOW_FUNC (mds_sessions_show) -/* * debugfs */ @@ -231,6 +230,7 @@ void ceph_fs_debugfs_cleanup(struct ceph_fs_client *fsc)debugfs_remove (fsc->debugfs_caps);debugfs_remove (fsc->debugfs_mdsc);debugfs_remove (fsc->debugfs_dentry_lru); +debugfs_remove (fsc->debugfs_wb_fault); }int ceph_fs_debugfs_init (struct ceph_fs_client *fsc)@@ -298,6 +298,12 @@ int ceph_fs_debugfs_init(struct ceph_fs_client *fsc)if (!fsc->debugfs_dentry_lru) goto out; + fsc->debugfs_wb_fault =debugfs_create_bool ("wb_fault" , +0600 , fsc->client->debugfs_dir, + &fsc->wb_fault); +if (!fsc->debugfs_wb_fault) + goto out; +return 0 ; out:fs/ceph/super.h +1
-0@@ -84,6 +84,7 @@ struct ceph_fs_client {unsigned long mount_state;int min_caps;/* min caps i added */ +bool wb_fault;struct ceph_mds_client *mdsc;
fs/ceph/super.h struct ceph_fs_client { · line 106
Quoted 6 lines
fs/ceph/super.h +1
-0@@ -100,6 +101,7 @@ struct ceph_fs_client {struct dentry *debugfs_bdi;struct dentry *debugfs_mdsc, *debugfs_mdsmap;struct dentry *debugfs_mds_sessions; +struct dentry *debugfs_wb_fault;#endif
I think it's better not to enable this feature by default. Enabling it by compilation option or mount option?
Regards
Yan, Zheng
Quoted 4 lines
fs/ceph/super.h +0
-1#ifdef CONFIG_CEPH_FSCACHE --2.13.3
Re: [PATCH 2/2] ceph: pagecache writeback fault injection switch
Quoted 99 lines · Zheng
Quoted 88 lines · Jeff Layton
From: Jeff Layton <[email protected]>
Testing ceph for proper writeback error handling turns out to be quite difficult. I tried using iptables to block traffic but that didn't give reliable results.
I hacked in this
wb_faultswitch that makes the filesystem pretend that writeback failed, even when it succeeds. With this, I could verify that cephfs fsync error reporting does work properly.Signed-off-by Jeff Layton <[email protected]>--- fs/ceph/addr.c | 7 +++++++ fs/ceph/debugfs.c | 8 +++++++- fs/ceph/super.h | 2 ++ 3 files changed, 16 insertions(+), 1 deletion(-)fs/ceph/addr.c +7
-0@@ -584,6 +584,10 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc) page_off, len, truncate_seq, truncate_size, &inode->i_mtime, &page, 1); + +if (fsc->wb_fault && err >=0 ) + err = -EIO ; +if (err <0 ) {struct writeback_control tmp_wbc;if (!wbc)@@ -666,6 +670,9 @@ static void writepages_finish(struct ceph_osd_request *req)struct ceph_fs_client *fsc =ceph_inode_to_client (inode);bool remove_page; +if (fsc->wb_fault && rc >=0 ) + rc = -EIO ; +dout ("writepages_finish %p rc %d\n" , inode, rc);if (rc <0 ) {mapping_set_error (mapping, rc);fs/ceph/debugfs.c +7
-1@@ -197,7 +197,6 @@ CEPH_DEFINE_SHOW_FUNC(caps_show)CEPH_DEFINE_SHOW_FUNC (dentry_lru_show)CEPH_DEFINE_SHOW_FUNC (mds_sessions_show) -/* * debugfs */ @@ -231,6 +230,7 @@ void ceph_fs_debugfs_cleanup(struct ceph_fs_client *fsc)debugfs_remove (fsc->debugfs_caps);debugfs_remove (fsc->debugfs_mdsc);debugfs_remove (fsc->debugfs_dentry_lru); +debugfs_remove (fsc->debugfs_wb_fault); }int ceph_fs_debugfs_init (struct ceph_fs_client *fsc)@@ -298,6 +298,12 @@ int ceph_fs_debugfs_init(struct ceph_fs_client *fsc)if (!fsc->debugfs_dentry_lru) goto out; + fsc->debugfs_wb_fault =debugfs_create_bool ("wb_fault" , +0600 , fsc->client->debugfs_dir, + &fsc->wb_fault); +if (!fsc->debugfs_wb_fault) + goto out; +return 0 ; out:fs/ceph/super.h +1
-0@@ -84,6 +84,7 @@ struct ceph_fs_client {unsigned long mount_state;int min_caps;/* min caps i added */ +bool wb_fault;struct ceph_mds_client *mdsc;fs/ceph/super.h struct ceph_fs_client { · line 106
Quoted 6 lines
fs/ceph/super.h +1
-0@@ -100,6 +101,7 @@ struct ceph_fs_client {struct dentry *debugfs_bdi;struct dentry *debugfs_mdsc, *debugfs_mdsmap;struct dentry *debugfs_mds_sessions; +struct dentry *debugfs_wb_fault;#endif I think it's better not to enable this feature by default. Enabling it by compilation option or mount option?
Quoted 2 lines
Regards
Yan, Zheng
Yes, it probably should be.
I really should have sent this as an RFC patch. I mostly did it to document how patch 1/2 was tested. I'm not at all convinced this is a great idea, though. I'm fine with just dropping this patch for now until we have some time to consider it more. Maybe it would be better to hack something into the OSDs for this?
I think if we did want to start adding fault injection knobs, then we'll probably want to allow other tunables to be added simply. Maybe put them in a "fault" subdir under the per-fsc debugfs dir?
Signature · 1 line
Jeff Layton <[email protected]>