* [PATCH 0/2] ceph: make kcephfs use errseq_t for writeback error reporting
@ 2017-07-25 14:50 Jeff Layton
2017-07-25 14:50 ` [PATCH 1/2] ceph: " Jeff Layton
2017-07-25 14:50 ` [PATCH 2/2] ceph: pagecache writeback fault injection switch Jeff Layton
0 siblings, 2 replies; 6+ messages in thread
From: Jeff Layton @ 2017-07-25 14:50 UTC (permalink / raw)
To: Yan, Zheng, Sage Weil, Ilya Dryomov; +Cc: ceph-devel, linux-kernel
From: Jeff Layton <jlayton@redhat.com>
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(-)
--
2.13.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] ceph: use errseq_t for writeback error reporting
2017-07-25 14:50 [PATCH 0/2] ceph: make kcephfs use errseq_t for writeback error reporting Jeff Layton
@ 2017-07-25 14:50 ` Jeff Layton
2017-07-26 13:09 ` Yan, Zheng
2017-07-25 14:50 ` [PATCH 2/2] ceph: pagecache writeback fault injection switch Jeff Layton
1 sibling, 1 reply; 6+ messages in thread
From: Jeff Layton @ 2017-07-25 14:50 UTC (permalink / raw)
To: Yan, Zheng, Sage Weil, Ilya Dryomov; +Cc: ceph-devel, linux-kernel
From: Jeff Layton <jlayton@redhat.com>
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 <jlayton@redhat.com>
---
fs/ceph/caps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 7007ae2a5ad2..13f6edf24acd 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -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;
--
2.13.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] ceph: pagecache writeback fault injection switch
2017-07-25 14:50 [PATCH 0/2] ceph: make kcephfs use errseq_t for writeback error reporting Jeff Layton
2017-07-25 14:50 ` [PATCH 1/2] ceph: " Jeff Layton
@ 2017-07-25 14:50 ` Jeff Layton
2017-07-26 13:08 ` Yan, Zheng
1 sibling, 1 reply; 6+ messages in thread
From: Jeff Layton @ 2017-07-25 14:50 UTC (permalink / raw)
To: Yan, Zheng, Sage Weil, Ilya Dryomov; +Cc: ceph-devel, linux-kernel
From: Jeff Layton <jlayton@redhat.com>
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.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/ceph/addr.c | 7 +++++++
fs/ceph/debugfs.c | 8 +++++++-
fs/ceph/super.h | 2 ++
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 50836280a6f8..a3831d100e16 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -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);
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 4e2d112c982f..e1e6eaa12031 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -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:
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index f02a2225fe42..a38fd6203b77 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -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
--
2.13.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ceph: pagecache writeback fault injection switch
2017-07-25 14:50 ` [PATCH 2/2] ceph: pagecache writeback fault injection switch Jeff Layton
@ 2017-07-26 13:08 ` Yan, Zheng
2017-07-31 12:11 ` Jeff Layton
0 siblings, 1 reply; 6+ messages in thread
From: Yan, Zheng @ 2017-07-26 13:08 UTC (permalink / raw)
To: Jeff Layton
Cc: Yan, Zheng, Sage Weil, Ilya Dryomov, ceph-devel,
Linux Kernel Mailing List
On Tue, Jul 25, 2017 at 10:50 PM, Jeff Layton <jlayton@kernel.org> wrote:
> From: Jeff Layton <jlayton@redhat.com>
>
> 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.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/ceph/addr.c | 7 +++++++
> fs/ceph/debugfs.c | 8 +++++++-
> fs/ceph/super.h | 2 ++
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 50836280a6f8..a3831d100e16 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -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);
> diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
> index 4e2d112c982f..e1e6eaa12031 100644
> --- a/fs/ceph/debugfs.c
> +++ b/fs/ceph/debugfs.c
> @@ -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:
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index f02a2225fe42..a38fd6203b77 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -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
>
I think it's better not to enable this feature by default. Enabling it
by compilation option or mount option?
Regards
Yan, Zheng
> #ifdef CONFIG_CEPH_FSCACHE
> --
> 2.13.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ceph: use errseq_t for writeback error reporting
2017-07-25 14:50 ` [PATCH 1/2] ceph: " Jeff Layton
@ 2017-07-26 13:09 ` Yan, Zheng
0 siblings, 0 replies; 6+ messages in thread
From: Yan, Zheng @ 2017-07-26 13:09 UTC (permalink / raw)
To: Jeff Layton
Cc: Yan, Zheng, Sage Weil, Ilya Dryomov, ceph-devel,
Linux Kernel Mailing List
On Tue, Jul 25, 2017 at 10:50 PM, Jeff Layton <jlayton@kernel.org> wrote:
> From: Jeff Layton <jlayton@redhat.com>
>
> 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 <jlayton@redhat.com>
> ---
> fs/ceph/caps.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 7007ae2a5ad2..13f6edf24acd 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -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;
>
> --
> 2.13.3
>
Reviewed-by: "Yan, Zheng" <zyan@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ceph: pagecache writeback fault injection switch
2017-07-26 13:08 ` Yan, Zheng
@ 2017-07-31 12:11 ` Jeff Layton
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2017-07-31 12:11 UTC (permalink / raw)
To: Yan, Zheng, Jeff Layton
Cc: Yan, Zheng, Sage Weil, Ilya Dryomov, ceph-devel,
Linux Kernel Mailing List
On Wed, 2017-07-26 at 21:08 +0800, Yan, Zheng wrote:
> On Tue, Jul 25, 2017 at 10:50 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > From: Jeff Layton <jlayton@redhat.com>
> >
> > 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.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/ceph/addr.c | 7 +++++++
> > fs/ceph/debugfs.c | 8 +++++++-
> > fs/ceph/super.h | 2 ++
> > 3 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> > index 50836280a6f8..a3831d100e16 100644
> > --- a/fs/ceph/addr.c
> > +++ b/fs/ceph/addr.c
> > @@ -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);
> > diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
> > index 4e2d112c982f..e1e6eaa12031 100644
> > --- a/fs/ceph/debugfs.c
> > +++ b/fs/ceph/debugfs.c
> > @@ -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:
> > diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> > index f02a2225fe42..a38fd6203b77 100644
> > --- a/fs/ceph/super.h
> > +++ b/fs/ceph/super.h
> > @@ -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
> >
>
> I think it's better not to enable this feature by default. Enabling it
> by compilation option or mount option?
>
> 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?
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-07-31 12:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-25 14:50 [PATCH 0/2] ceph: make kcephfs use errseq_t for writeback error reporting Jeff Layton
2017-07-25 14:50 ` [PATCH 1/2] ceph: " Jeff Layton
2017-07-26 13:09 ` Yan, Zheng
2017-07-25 14:50 ` [PATCH 2/2] ceph: pagecache writeback fault injection switch Jeff Layton
2017-07-26 13:08 ` Yan, Zheng
2017-07-31 12:11 ` Jeff Layton
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.