[PATCH v2 0/4] vfs: track per-sb writeback errors and report them via fsinfo()

[PATCH v2 0/4] vfs: track per-sb writeback errors and report them via fsinfo()

[Jeff Layton]

v2: drop buffer.c patch to record wb errors when underlying blockdev
flush fails. We may eventually want that, but at this point I don't have
a clear way to test it to determine its efficacy.

At LSF/MM this year, the PostgreSQL developers mentioned that they'd
like to have some mechanism to check whether there have been any
writeback errors on a filesystem, without necessarily flushing any of
the cached data first.

Given that we have a new fsinfo syscall being introduced, we may as well
use it to report writeback errors on a per superblock basis. This allows
us to provide the info that the PostgreSQL developers wanted, without
needing to change an existing interface.

This seems to do the right thing when tested by hand, but I don't yet
have an xfstest for it, since the syscall is still quite new. Once that
goes in and we get fsinfo support in xfs_io, it should be rather
trivial to roll a testcase for this.

Al, if this looks ok, could you pull this into the branch where you
have David's fsinfo patches queued up?

Thanks,
Jeff

Jeff Layton (4):
  vfs: track per-sb writeback errors
  errseq: add a new errseq_scrape function
  vfs: allow fsinfo to fetch the current state of s_wb_err
  samples: extend test-fsinfo to access error_state

 fs/statfs.c                 |  9 +++++++++
 include/linux/errseq.h      |  1 +
 include/linux/fs.h          |  3 +++
 include/linux/pagemap.h     |  5 ++++-
 include/uapi/linux/fsinfo.h | 11 +++++++++++
 lib/errseq.c                | 33 +++++++++++++++++++++++++++++++--
 samples/statx/test-fsinfo.c | 13 +++++++++++++
 7 files changed, 72 insertions(+), 3 deletions(-)

-- 
2.17.1

[PATCH v2 1/4] vfs: track per-sb writeback errors

[Jeff Layton]

From: Jeff Layton <jlayton@redhat.com>

Keep a per-sb errseq_t that tracks whether there have been any writeback
errors on this superblock. When an error is recorded for an inode via
mapping_set_error, record it in s_wb_err as well.

Cc: Andres Freund <andres@anarazel.de>
Cc: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 include/linux/fs.h      | 3 +++
 include/linux/pagemap.h | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 858dd2f3300c..5378a845ca01 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1446,6 +1446,9 @@ struct super_block {
 	/* Being remounted read-only */
 	int s_readonly_remount;
 
+	/* per-sb errseq_t for reporting writeback errors via syncfs */
+	errseq_t s_wb_err;
+
 	/* AIO completions deferred from interrupt context */
 	struct workqueue_struct *s_dio_done_wq;
 	struct hlist_head s_pins;
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 442977811b59..c1e36a7b44d2 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -51,7 +51,10 @@ static inline void mapping_set_error(struct address_space *mapping, int error)
 		return;
 
 	/* Record in wb_err for checkers using errseq_t based tracking */
-	filemap_set_wb_err(mapping, error);
+	__filemap_set_wb_err(mapping, error);
+
+	/* Record it in superblock */
+	errseq_set(&mapping->host->i_sb->s_wb_err, error);
 
 	/* Record it in flags for now, for legacy callers */
 	if (error == -ENOSPC)
-- 
2.17.1

[PATCH v2 2/4] errseq: add a new errseq_scrape function

[Jeff Layton]

From: Jeff Layton <jlayton@redhat.com>

To grab the current value of an errseq_t, mark it as seen and then
return the value with the seen bit masked off.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 include/linux/errseq.h |  1 +
 lib/errseq.c           | 33 +++++++++++++++++++++++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/include/linux/errseq.h b/include/linux/errseq.h
index fc2777770768..de165623fa86 100644
--- a/include/linux/errseq.h
+++ b/include/linux/errseq.h
@@ -9,6 +9,7 @@ typedef u32	errseq_t;
 
 errseq_t errseq_set(errseq_t *eseq, int err);
 errseq_t errseq_sample(errseq_t *eseq);
+errseq_t errseq_scrape(errseq_t *eseq);
 int errseq_check(errseq_t *eseq, errseq_t since);
 int errseq_check_and_advance(errseq_t *eseq, errseq_t *since);
 #endif
diff --git a/lib/errseq.c b/lib/errseq.c
index 81f9e33aa7e7..8ded0920eed3 100644
--- a/lib/errseq.c
+++ b/lib/errseq.c
@@ -108,7 +108,7 @@ errseq_t errseq_set(errseq_t *eseq, int err)
 EXPORT_SYMBOL(errseq_set);
 
 /**
- * errseq_sample() - Grab current errseq_t value.
+ * errseq_sample() - Grab current errseq_t value (or 0 if it hasn't been seen)
  * @eseq: Pointer to errseq_t to be sampled.
  *
  * This function allows callers to initialise their errseq_t variable.
@@ -117,7 +117,7 @@ EXPORT_SYMBOL(errseq_set);
  * see it the next time it checks for an error.
  *
  * Context: Any context.
- * Return: The current errseq value.
+ * Return: The current errseq value or 0 if it wasn't previously seen
  */
 errseq_t errseq_sample(errseq_t *eseq)
 {
@@ -130,6 +130,35 @@ errseq_t errseq_sample(errseq_t *eseq)
 }
 EXPORT_SYMBOL(errseq_sample);
 
+/**
+ * errseq_scrape() - Grab current errseq_t value
+ * @eseq: Pointer to errseq_t to be sampled.
+ *
+ * This function allows callers to scrape the current value of an errseq_t.
+ * Unlike errseq_sample, this will always return the current value with
+ * the SEEN flag unset, even when the value has not yet been seen.
+ *
+ * Context: Any context.
+ * Return: The current errseq value with ERRSEQ_SEEN masked off
+ */
+errseq_t errseq_scrape(errseq_t *eseq)
+{
+	errseq_t old = READ_ONCE(*eseq);
+
+	/*
+	 * For the common case of no errors ever having been set, we can skip
+	 * marking the SEEN bit. Once an error has been set, the value will
+	 * never go back to zero.
+	 */
+	if (old != 0) {
+		errseq_t new = old | ERRSEQ_SEEN;
+		if (old != new)
+			cmpxchg(eseq, old, new);
+	}
+	return old & ~ERRSEQ_SEEN;
+}
+EXPORT_SYMBOL(errseq_scrape);
+
 /**
  * errseq_check() - Has an error occurred since a particular sample point?
  * @eseq: Pointer to errseq_t value to be checked.
-- 
2.17.1

[PATCH v2 3/4] vfs: allow fsinfo to fetch the current state of s_wb_err

[Jeff Layton]

From: Jeff Layton <jlayton@redhat.com>

Add a new "error_state" struct to fsinfo, and teach the kernel to fill
that out from sb->s_wb_info. There are two fields:

wb_error_last: the most recently recorded errno for the filesystem

wb_error_cookie: this value will change vs. the previously fetched
                 value if a new error was recorded since it was last
		 checked

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/statfs.c                 |  9 +++++++++
 include/uapi/linux/fsinfo.h | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/fs/statfs.c b/fs/statfs.c
index fa6be965dce1..fb1f48eb381a 100644
--- a/fs/statfs.c
+++ b/fs/statfs.c
@@ -576,6 +576,14 @@ static int fsinfo_generic_io_size(struct dentry *dentry,
 	return sizeof(*c);
 }
 
+static int fsinfo_generic_error_state(struct dentry *dentry,
+				  struct fsinfo_error_state *c)
+{
+	c->wb_error_cookie = errseq_scrape(&dentry->d_sb->s_wb_err);
+	c->wb_error_last = c->wb_error_cookie & MAX_ERRNO;
+	return sizeof(*c);
+}
+
 /*
  * Implement some queries generically from stuff in the superblock.
  */
@@ -594,6 +602,7 @@ int generic_fsinfo(struct dentry *dentry, struct fsinfo_kparams *params)
 	case _gen(volume_id);
 	case _gen(name_encoding);
 	case _gen(io_size);
+	case _gen(error_state);
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/include/uapi/linux/fsinfo.h b/include/uapi/linux/fsinfo.h
index f2bc5130544d..fbc5d840ea4e 100644
--- a/include/uapi/linux/fsinfo.h
+++ b/include/uapi/linux/fsinfo.h
@@ -35,6 +35,7 @@ enum fsinfo_attribute {
 	fsinfo_attr_name_encoding	= 17,	/* Filename encoding (string) */
 	fsinfo_attr_name_codepage	= 18,	/* Filename codepage (string) */
 	fsinfo_attr_io_size		= 19,	/* Optimal I/O sizes */
+	fsinfo_attr_error_state		= 20,	/* Error state */
 	fsinfo_attr__nr
 };
 
@@ -211,6 +212,16 @@ struct fsinfo_server_address {
 	struct __kernel_sockaddr_storage address;
 };
 
+/*
+ * Information struct for fsinfo(fsinfo_attr_error_state).
+ *
+ * Retrieve the error state for a filesystem.
+ */
+struct fsinfo_error_state {
+	__u32		wb_error_cookie;	/* writeback error cookie */
+	__u32		wb_error_last;		/* latest writeback error */
+};
+
 /*
  * Information struct for fsinfo(fsinfo_attr_io_size).
  *
-- 
2.17.1

[PATCH v2 4/4] samples: extend test-fsinfo to access error_state

[Jeff Layton]

From: Jeff Layton <jlayton@redhat.com>

Add support for error_state struct to test-fsinfo sample program.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 samples/statx/test-fsinfo.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/samples/statx/test-fsinfo.c b/samples/statx/test-fsinfo.c
index 9e9fa62a3b9f..01dd8c6d3c86 100644
--- a/samples/statx/test-fsinfo.c
+++ b/samples/statx/test-fsinfo.c
@@ -60,6 +60,7 @@ static const __u8 fsinfo_buffer_sizes[fsinfo_attr__nr] = {
 	FSINFO_STRING		(name_encoding),
 	FSINFO_STRING		(name_codepage),
 	FSINFO_STRUCT		(io_size),
+	FSINFO_STRUCT		(error_state),
 };
 
 #define FSINFO_NAME(N) [fsinfo_attr_##N] = #N
@@ -84,6 +85,7 @@ static const char *fsinfo_attr_names[fsinfo_attr__nr] = {
 	FSINFO_NAME(name_encoding),
 	FSINFO_NAME(name_codepage),
 	FSINFO_NAME(io_size),
+	FSINFO_NAME(error_state),
 };
 
 union reply {
@@ -98,6 +100,7 @@ union reply {
 	struct fsinfo_volume_uuid uuid;
 	struct fsinfo_server_address srv_addr;
 	struct fsinfo_io_size io_size;
+	struct fsinfo_error_state error_state;
 };
 
 static void dump_hex(unsigned int *data, int from, int to)
@@ -304,6 +307,15 @@ static void dump_attr_io_size(union reply *r, int size)
 	printf("bs=%u\n", f->block_size);
 }
 
+static void dump_attr_error_state(union reply *r, int size)
+{
+	struct fsinfo_error_state *f = &r->error_state;
+
+	printf("err_cookie=0x%x err_last=%u\n",
+			f->wb_error_cookie,
+			f->wb_error_last);
+}
+
 /*
  *
  */
@@ -321,6 +333,7 @@ static const dumper_t fsinfo_attr_dumper[fsinfo_attr__nr] = {
 	FSINFO_DUMPER(volume_uuid),
 	FSINFO_DUMPER(server_address),
 	FSINFO_DUMPER(io_size),
+	FSINFO_DUMPER(error_state),
 };
 
 static void dump_fsinfo(enum fsinfo_attribute attr, __u8 about,
-- 
2.17.1

Re: [PATCH v2 0/4] vfs: track per-sb writeback errors and report them via fsinfo()

[David Howells]

Jeff Layton <jlayton@kernel.org> wrote:

> v2: drop buffer.c patch to record wb errors when underlying blockdev
> flush fails. We may eventually want that, but at this point I don't have
> a clear way to test it to determine its efficacy.
> 
> At LSF/MM this year, the PostgreSQL developers mentioned that they'd
> like to have some mechanism to check whether there have been any
> writeback errors on a filesystem, without necessarily flushing any of
> the cached data first.
> 
> Given that we have a new fsinfo syscall being introduced, we may as well
> use it to report writeback errors on a per superblock basis. This allows
> us to provide the info that the PostgreSQL developers wanted, without
> needing to change an existing interface.
> 
> This seems to do the right thing when tested by hand, but I don't yet
> have an xfstest for it, since the syscall is still quite new. Once that
> goes in and we get fsinfo support in xfs_io, it should be rather
> trivial to roll a testcase for this.

Reviewed-by: David Howells <dhowells@redhat.com>

Re: [PATCH v2 0/4] vfs: track per-sb writeback errors and report them via fsinfo()

[Carlos Maiolino]

On Tue, Jul 10, 2018 at 10:01:23AM -0400, Jeff Layton wrote:
> v2: drop buffer.c patch to record wb errors when underlying blockdev
> flush fails. We may eventually want that, but at this point I don't have
> a clear way to test it to determine its efficacy.
> 
> At LSF/MM this year, the PostgreSQL developers mentioned that they'd
> like to have some mechanism to check whether there have been any
> writeback errors on a filesystem, without necessarily flushing any of
> the cached data first.
> 
> Given that we have a new fsinfo syscall being introduced, we may as well
> use it to report writeback errors on a per superblock basis. This allows
> us to provide the info that the PostgreSQL developers wanted, without
> needing to change an existing interface.
> 
> This seems to do the right thing when tested by hand, but I don't yet
> have an xfstest for it, since the syscall is still quite new. Once that
> goes in and we get fsinfo support in xfs_io, it should be rather
> trivial to roll a testcase for this.
> 

Whole patch sounds fine, you can add:

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

Cheers

> Al, if this looks ok, could you pull this into the branch where you
> have David's fsinfo patches queued up?
> 
> Thanks,
> Jeff
> 
> Jeff Layton (4):
>   vfs: track per-sb writeback errors
>   errseq: add a new errseq_scrape function
>   vfs: allow fsinfo to fetch the current state of s_wb_err
>   samples: extend test-fsinfo to access error_state
> 
>  fs/statfs.c                 |  9 +++++++++
>  include/linux/errseq.h      |  1 +
>  include/linux/fs.h          |  3 +++
>  include/linux/pagemap.h     |  5 ++++-
>  include/uapi/linux/fsinfo.h | 11 +++++++++++
>  lib/errseq.c                | 33 +++++++++++++++++++++++++++++++--
>  samples/statx/test-fsinfo.c | 13 +++++++++++++
>  7 files changed, 72 insertions(+), 3 deletions(-)
> 
> -- 
> 2.17.1
> 

-- 
Carlos