[PATCH 0/9] xfs: configurable error behaviour

[PATCH 0/9] xfs: configurable error behaviour

[Dave Chinner]

Hi folks,

I need to restart the discussion and review of this patch series.
There was some discussion of it last time, but nothing really came
from that. I'm posting what I have in my tree right now - treat it
as though it's an initial posting of the code because I can't recall
what I've changed since the first posting.

What I'd like to have to for the next merge window is all the IO
error bits sorted out. The final patch (kmem failure behaviour)
needs more infrastructure (passing mp to every allocation) so that's
a secondary concern right now and I've only included it to
demonstrate how to apply this code ot a different subsystem.

Things that need to be nailed down before I can commit the series:

	- sysfs layout
	- naming conventions for errors and subsystems in sysfs
	- how best to display/change default behaviour

Things that we can change/implement later:

	- default behaviour
	- additional error classes
	- additional error types
	- additional subsystems
	- subsystem error handling implementation
	- communication with other subsystems to dynamically change
	  error behaviour

IOWs, what is important right now is how we present this to
userspace, because we can't change that easily once we've decided on
a presentation structure.

Modifying the code to classify and handle all the different error
types is much less important, as we can change that to fix whatever
problems we have without impacting the presentation to userspace.

There is definite need for this (e.g. handling of ENOSPC on thin
provisioned devices), so I want to get quickly to a consensus on the
userspace facing aspects so that we can get this ball rolling.

The biggest unsolved issue is how to change the default behaviour
persistently. There is no infrastructure in this patch series to do
that, but it is someting that we have to consider so that we don't
require default behaviour to be changed after every mount of every
filesystem on a system. My thoughts on this is we store changes to
the defaults in xattrs on the root inode, but I'm open to ideas here
as there's no code written for it yet. Solving this problem,
however, is not necessary before commiting the initial code; it's
something we can add later once we've worked out all the details.

Discuss!

-Dave.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/9] xfs: configurable error behaviour via sysfs

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

We need to be able to change the way XFS behaviours in error
conditions depending on the type of underlying storage. This is
necessary for handling non-traditional block devices with extended
error cases, such as thin provisioned devices that can return ENOSPC
as an IO error.

Introduce the basic sysfs infrastructure needed to define and
configure error behaviours. This is done to be generic enough to
extend to configuring behaviour in other error conditions, such as
ENOMEM, which also has different desired behaviours according to
machine configuration.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_mount.c | 10 +++++++++-
 fs/xfs/xfs_mount.h | 27 ++++++++++++++++++++++++++
 fs/xfs/xfs_sysfs.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/xfs/xfs_sysfs.h |  3 +++
 4 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 986290c..f6fb5e1 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -708,10 +708,15 @@ xfs_mountfs(
 	if (error)
 		goto out_remove_sysfs;
 
-	error = xfs_uuid_mount(mp);
+	error = xfs_error_sysfs_init(mp);
 	if (error)
 		goto out_del_stats;
 
+
+	error = xfs_uuid_mount(mp);
+	if (error)
+		goto out_remove_error_sysfs;
+
 	/*
 	 * Set the minimum read and write sizes
 	 */
@@ -986,6 +991,8 @@ xfs_mountfs(
 	xfs_da_unmount(mp);
  out_remove_uuid:
 	xfs_uuid_unmount(mp);
+ out_remove_error_sysfs:
+	xfs_error_sysfs_del(mp);
  out_del_stats:
 	xfs_sysfs_del(&mp->m_stats.xs_kobj);
  out_remove_sysfs:
@@ -1074,6 +1081,7 @@ xfs_unmountfs(
 #endif
 	xfs_free_perag(mp);
 
+	xfs_error_sysfs_del(mp);
 	xfs_sysfs_del(&mp->m_stats.xs_kobj);
 	xfs_sysfs_del(&mp->m_kobj);
 }
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index b570984..96c9658 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -37,6 +37,31 @@ enum {
 	XFS_LOWSP_MAX,
 };
 
+/*
+ * Error Configuration
+ *
+ * Error classes define the subsystem the configuration belongs to.
+ * Error numbers define the errors that are configurable.
+ * Failure types describe when an error is considered fatal.
+ */
+enum {
+	XFS_ERR_CLASS_MAX,
+};
+enum {
+	XFS_ERR_ERRNO_MAX,
+};
+enum {
+	XFS_ERR_FAIL_DEFAULT,
+	XFS_ERR_FAIL_NEVER,
+	XFS_ERR_FAIL_SLOW,
+	XFS_ERR_FAIL_FAST,
+};
+
+struct xfs_error_cfg {
+	struct xfs_kobj	kobj;
+	int		fail_speed;
+};
+
 typedef struct xfs_mount {
 	struct super_block	*m_super;
 	xfs_tid_t		m_tid;		/* next unused tid for fs */
@@ -127,6 +152,8 @@ typedef struct xfs_mount {
 	int64_t			m_low_space[XFS_LOWSP_MAX];
 						/* low free space thresholds */
 	struct xfs_kobj		m_kobj;
+	struct xfs_kobj		m_error_kobj;
+	struct xfs_error_cfg	m_error_cfg[XFS_ERR_CLASS_MAX][XFS_ERR_ERRNO_MAX];
 	struct xstats		m_stats;	/* per-fs stats */
 
 	struct workqueue_struct *m_buf_workqueue;
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index 641d625..7d81c1b 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -17,11 +17,15 @@
  */
 
 #include "xfs.h"
-#include "xfs_sysfs.h"
+#include "xfs_shared.h"
+#include "xfs_format.h"
 #include "xfs_log_format.h"
+#include "xfs_trans_resv.h"
+#include "xfs_sysfs.h"
 #include "xfs_log.h"
 #include "xfs_log_priv.h"
 #include "xfs_stats.h"
+#include "xfs_mount.h"
 
 struct xfs_sysfs_attr {
 	struct attribute attr;
@@ -304,3 +308,53 @@ struct kobj_type xfs_log_ktype = {
 	.sysfs_ops = &xfs_sysfs_ops,
 	.default_attrs = xfs_log_attrs,
 };
+
+/*
+ * Metadata IO error configuration
+ *
+ * The sysfs structure here is:
+ *	...xfs/<dev>/error/<class>/<errno>/<error_attrs>
+ *
+ * where <class> allows use to discriminate between data IO and metadata IO,
+ * and any other future type of IO (e.g. special inode or directory error
+ * handling) we care to support.
+ */
+static struct attribute *xfs_error_attrs[] = {
+	NULL,
+};
+
+static inline struct xfs_error_cfg *
+to_error_cfg(struct kobject *kobject)
+{
+	struct xfs_kobj *kobj = to_kobj(kobject);
+	return container_of(kobj, struct xfs_error_cfg, kobj);
+}
+
+struct kobj_type xfs_error_cfg_ktype = {
+	.release = xfs_sysfs_release,
+	.sysfs_ops = &xfs_sysfs_ops,
+	.default_attrs = xfs_error_attrs,
+};
+
+struct kobj_type xfs_error_ktype = {
+	.release = xfs_sysfs_release,
+};
+
+int
+xfs_error_sysfs_init(
+	struct xfs_mount	*mp)
+{
+	int			error;
+
+	/* .../xfs/<dev>/error/ */
+	error = xfs_sysfs_init(&mp->m_error_kobj, &xfs_error_ktype,
+				&mp->m_kobj, "error");
+	return error;
+}
+
+void
+xfs_error_sysfs_del(
+	struct xfs_mount	*mp)
+{
+	xfs_sysfs_del(&mp->m_error_kobj);
+}
diff --git a/fs/xfs/xfs_sysfs.h b/fs/xfs/xfs_sysfs.h
index be692e5..d046371 100644
--- a/fs/xfs/xfs_sysfs.h
+++ b/fs/xfs/xfs_sysfs.h
@@ -58,4 +58,7 @@ xfs_sysfs_del(
 	wait_for_completion(&kobj->complete);
 }
 
+int	xfs_error_sysfs_init(struct xfs_mount *mp);
+void	xfs_error_sysfs_del(struct xfs_mount *mp);
+
 #endif	/* __XFS_SYSFS_H__ */
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/9] xfs: introduce metadata IO error class

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Now we have the basic infrastructure, add the first error class so
we can build up the infrastructure in a meaningful way. Add the
metadata async write IO error class and sysfs entry, and introduce a
default configuration that matches the existing behaviour of
XFS_ERR_FAIL_NEVER for async write metadata buffers.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_mount.h |  3 +++
 fs/xfs/xfs_sysfs.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 96c9658..36e7330 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -45,9 +45,11 @@ enum {
  * Failure types describe when an error is considered fatal.
  */
 enum {
+	XFS_ERR_METADATA,
 	XFS_ERR_CLASS_MAX,
 };
 enum {
+	XFS_ERR_DEFAULT,
 	XFS_ERR_ERRNO_MAX,
 };
 enum {
@@ -153,6 +155,7 @@ typedef struct xfs_mount {
 						/* low free space thresholds */
 	struct xfs_kobj		m_kobj;
 	struct xfs_kobj		m_error_kobj;
+	struct xfs_kobj		m_error_meta_kobj;
 	struct xfs_error_cfg	m_error_cfg[XFS_ERR_CLASS_MAX][XFS_ERR_ERRNO_MAX];
 	struct xstats		m_stats;	/* per-fs stats */
 
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index 7d81c1b..c5f0afa 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -344,11 +344,34 @@ int
 xfs_error_sysfs_init(
 	struct xfs_mount	*mp)
 {
+	struct xfs_error_cfg	*cfg;
 	int			error;
 
 	/* .../xfs/<dev>/error/ */
 	error = xfs_sysfs_init(&mp->m_error_kobj, &xfs_error_ktype,
 				&mp->m_kobj, "error");
+	if (error)
+		return error;
+
+	/* .../xfs/<dev>/error/metadata/ */
+	error = xfs_sysfs_init(&mp->m_error_meta_kobj, &xfs_error_ktype,
+				&mp->m_error_kobj, "metadata");
+	if (error)
+		goto out_error;
+
+	cfg = &mp->m_error_cfg[XFS_ERR_METADATA][XFS_ERR_DEFAULT];
+	error = xfs_sysfs_init(&cfg->kobj, &xfs_error_cfg_ktype,
+				&mp->m_error_meta_kobj, "Default");
+	if (error)
+		goto out_error_meta;
+	cfg->fail_speed = XFS_ERR_FAIL_NEVER;
+
+	return 0;
+
+out_error_meta:
+	xfs_sysfs_del(&mp->m_error_meta_kobj);
+out_error:
+	xfs_sysfs_del(&mp->m_error_kobj);
 	return error;
 }
 
@@ -356,5 +379,16 @@ void
 xfs_error_sysfs_del(
 	struct xfs_mount	*mp)
 {
+	struct xfs_error_cfg	*cfg;
+	int			i, j;
+
+	for (i = 0; i < XFS_ERR_CLASS_MAX; i++) {
+		for (j = 0; j < XFS_ERR_ERRNO_MAX; j++) {
+			cfg = &mp->m_error_cfg[i][j];
+
+			xfs_sysfs_del(&cfg->kobj);
+		}
+	}
+	xfs_sysfs_del(&mp->m_error_meta_kobj);
 	xfs_sysfs_del(&mp->m_error_kobj);
 }
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/9] xfs: add configurable error support to metadata buffers

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

With the error configuration handle for async metadata write errors
in place, we can now add initial support to the IO error processing
in xfs_buf_iodone_error().

Add an infrastructure function to look up the configuration handle,
and rearrange the error handling to prepare the way for different
error handling conigurations to be used.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_buf.h      |   1 +
 fs/xfs/xfs_buf_item.c | 112 ++++++++++++++++++++++++++++++--------------------
 fs/xfs/xfs_mount.h    |   3 ++
 fs/xfs/xfs_sysfs.c    |  17 ++++++++
 4 files changed, 88 insertions(+), 45 deletions(-)

diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 0735096..ad16a1e 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -183,6 +183,7 @@ typedef struct xfs_buf {
 	unsigned int		b_page_count;	/* size of page array */
 	unsigned int		b_offset;	/* page offset in first page */
 	int			b_error;	/* error code on I/O */
+	int			b_last_error;	/* previous async I/O error */
 	const struct xfs_buf_ops	*b_ops;
 
 #ifdef XFS_BUF_LOCK_TRACKING
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 2891134..68e34d1 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -926,35 +926,22 @@ xfs_buf_do_callbacks(
 	}
 }
 
-/*
- * This is the iodone() function for buffers which have had callbacks
- * attached to them by xfs_buf_attach_iodone().  It should remove each
- * log item from the buffer's list and call the callback of each in turn.
- * When done, the buffer's fsprivate field is set to NULL and the buffer
- * is unlocked with a call to iodone().
- */
-void
-xfs_buf_iodone_callbacks(
+static bool
+xfs_buf_iodone_callback_error(
 	struct xfs_buf		*bp)
 {
 	struct xfs_log_item	*lip = bp->b_fspriv;
 	struct xfs_mount	*mp = lip->li_mountp;
 	static ulong		lasttime;
 	static xfs_buftarg_t	*lasttarg;
-
-	if (likely(!bp->b_error))
-		goto do_callbacks;
+	struct xfs_error_cfg	*cfg;
 
 	/*
 	 * If we've already decided to shutdown the filesystem because of
 	 * I/O errors, there's no point in giving this a retry.
 	 */
-	if (XFS_FORCED_SHUTDOWN(mp)) {
-		xfs_buf_stale(bp);
-		bp->b_flags |= XBF_DONE;
-		trace_xfs_buf_item_iodone(bp, _RET_IP_);
-		goto do_callbacks;
-	}
+	if (XFS_FORCED_SHUTDOWN(mp))
+		goto out_stale;
 
 	if (bp->b_target != lasttarg ||
 	    time_after(jiffies, (lasttime + 5*HZ))) {
@@ -963,45 +950,80 @@ xfs_buf_iodone_callbacks(
 	}
 	lasttarg = bp->b_target;
 
+	/* synchronous writes will have callers process the error */
+	if (!(bp->b_flags & XBF_ASYNC))
+		goto out_stale;
+
+	trace_xfs_buf_item_iodone_async(bp, _RET_IP_);
+	ASSERT(bp->b_iodone != NULL);
+
 	/*
 	 * If the write was asynchronous then no one will be looking for the
-	 * error.  Clear the error state and write the buffer out again.
-	 *
-	 * XXX: This helps against transient write errors, but we need to find
-	 * a way to shut the filesystem down if the writes keep failing.
-	 *
-	 * In practice we'll shut the filesystem down soon as non-transient
-	 * errors tend to affect the whole device and a failing log write
-	 * will make us give up.  But we really ought to do better here.
+	 * error.  If this is the first failure of this type, clear the error
+	 * state and write the buffer out again. This means we always retry an
+	 * async write failure at least once, but we also need to set the buffer
+	 * up to behave correctly now for repeated failures.
 	 */
-	if (bp->b_flags & XBF_ASYNC) {
-		ASSERT(bp->b_iodone != NULL);
-
-		trace_xfs_buf_item_iodone_async(bp, _RET_IP_);
+	cfg = xfs_error_get_cfg(mp, XFS_ERR_METADATA, bp->b_error);
+	if (!(bp->b_flags & (XBF_STALE|XBF_WRITE_FAIL)) ||
+	     bp->b_last_error != bp->b_error) {
+		bp->b_flags |= (XBF_WRITE | XBF_ASYNC |
+			        XBF_DONE | XBF_WRITE_FAIL);
+		bp->b_last_error = bp->b_error;
+		xfs_buf_ioerror(bp, 0);
+		xfs_buf_submit(bp);
+		return true;
+	}
 
-		xfs_buf_ioerror(bp, 0); /* errno of 0 unsets the flag */
+	/*
+	 * Repeated failure on an async write. Take action according to the
+	 * error configuration we have been set up to use.
+	 */
+	if (cfg->fail_speed == XFS_ERR_FAIL_FAST)
+		goto permanent_error;
 
-		if (!(bp->b_flags & (XBF_STALE|XBF_WRITE_FAIL))) {
-			bp->b_flags |= XBF_WRITE | XBF_ASYNC |
-				       XBF_DONE | XBF_WRITE_FAIL;
-			xfs_buf_submit(bp);
-		} else {
-			xfs_buf_relse(bp);
-		}
-
-		return;
-	}
+	/* still a transient error, higher layers will retry */
+	xfs_buf_ioerror(bp, 0);
+	xfs_buf_relse(bp);
+	return true;
 
 	/*
-	 * If the write of the buffer was synchronous, we want to make
-	 * sure to return the error to the caller of xfs_bwrite().
+	 * Permanent error - we need to trigger a shutdown if we haven't already
+	 * to indicate that inconsistency will result from this action.
 	 */
+permanent_error:
+	xfs_force_shutdown(mp, SHUTDOWN_META_IO_ERROR);
+out_stale:
 	xfs_buf_stale(bp);
 	bp->b_flags |= XBF_DONE;
-
 	trace_xfs_buf_error_relse(bp, _RET_IP_);
+	return false;
+}
+
+/*
+ * This is the iodone() function for buffers which have had callbacks attached
+ * to them by xfs_buf_attach_iodone(). We need to iterate the items on the
+ * callback list, mark the buffer as having no more callbacks and then push the
+ * buffer through IO completion processing.
+ */
+void
+xfs_buf_iodone_callbacks(
+	struct xfs_buf		*bp)
+{
+	/*
+	 * If there is an error, process it. Some errors require us
+	 * to run callbacks after failure processing is done so we
+	 * detect that and take appropriate action.
+	 */
+	if (bp->b_error && xfs_buf_iodone_callback_error(bp))
+		return;
+
+	/*
+	 * Successful IO or permanent error. Either way, we can clear the
+	 * retry state here in preparation for the next error that may occur.
+	 */
+	bp->b_last_error = 0;
 
-do_callbacks:
 	xfs_buf_do_callbacks(bp);
 	bp->b_fspriv = NULL;
 	bp->b_iodone = NULL;
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 36e7330..9a61f39 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -371,4 +371,7 @@ extern void	xfs_set_low_space_thresholds(struct xfs_mount *);
 int	xfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
 			xfs_off_t count_fsb);
 
+struct xfs_error_cfg * xfs_error_get_cfg(struct xfs_mount *mp,
+		int error_class, int error);
+
 #endif	/* __XFS_MOUNT_H__ */
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index c5f0afa..b8a6804 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -392,3 +392,20 @@ xfs_error_sysfs_del(
 	xfs_sysfs_del(&mp->m_error_meta_kobj);
 	xfs_sysfs_del(&mp->m_error_kobj);
 }
+
+struct xfs_error_cfg *
+xfs_error_get_cfg(
+	struct xfs_mount	*mp,
+	int			error_class,
+	int			error)
+{
+	struct xfs_error_cfg	*cfg;
+
+	switch (error) {
+	default:
+		cfg = &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT];
+		break;
+	}
+
+	return cfg;
+}
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/9] xfs: introduce table-based init for error behaviours

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Before we start expanding the number of error classes and errors we
can configure behaviour for, we need a simple and clear way to
define the default behaviour that we initialised each mount with.
Introduce a table based method for keeping the initial configuration
in, and apply that to the existing initialisation code.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_sysfs.c | 72 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 60 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index b8a6804..27487ce 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -340,11 +340,67 @@ struct kobj_type xfs_error_ktype = {
 	.release = xfs_sysfs_release,
 };
 
+/*
+ * Error initialisation tables. These need to be ordered in the same
+ * order as the enums used to index the array. All class init tables need to
+ * define a "default" behaviour as the first entry, all other entries can be
+ * empty.
+ */
+struct xfs_error_init {
+	char		*name;
+	int		fail_speed;
+};
+
+static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
+	{ .name = "Default",
+	  .fail_speed = XFS_ERR_FAIL_NEVER,
+	},
+};
+
+static int
+xfs_error_sysfs_init_class(
+	struct xfs_mount	*mp,
+	int			class,
+	const char		*parent_name,
+	struct xfs_kobj		*parent_kobj,
+	const struct xfs_error_init init[])
+{
+	struct xfs_error_cfg	*cfg;
+	int			error;
+	int			i;
+
+	ASSERT(class < XFS_ERR_CLASS_MAX);
+
+	error = xfs_sysfs_init(parent_kobj, &xfs_error_ktype,
+				&mp->m_error_kobj, parent_name);
+	if (error)
+		return error;
+
+	for (i = 0; i < XFS_ERR_ERRNO_MAX; i++) {
+		cfg = &mp->m_error_cfg[class][i];
+		error = xfs_sysfs_init(&cfg->kobj, &xfs_error_cfg_ktype,
+					parent_kobj, init[i].name);
+		if (error)
+			goto out_error;
+
+		cfg->fail_speed = init[i].fail_speed;
+	}
+	return 0;
+
+out_error:
+	/* unwind the entries that succeeded */
+	for (i--; i >= 0; i--) {
+		cfg = &mp->m_error_cfg[class][i];
+		xfs_sysfs_del(&cfg->kobj);
+	}
+	xfs_sysfs_del(parent_kobj);
+	return error;
+}
+
 int
 xfs_error_sysfs_init(
 	struct xfs_mount	*mp)
 {
-	struct xfs_error_cfg	*cfg;
 	int			error;
 
 	/* .../xfs/<dev>/error/ */
@@ -354,22 +410,14 @@ xfs_error_sysfs_init(
 		return error;
 
 	/* .../xfs/<dev>/error/metadata/ */
-	error = xfs_sysfs_init(&mp->m_error_meta_kobj, &xfs_error_ktype,
-				&mp->m_error_kobj, "metadata");
+	error = xfs_error_sysfs_init_class(mp, XFS_ERR_METADATA,
+				"metadata", &mp->m_error_meta_kobj,
+				xfs_error_meta_init);
 	if (error)
 		goto out_error;
 
-	cfg = &mp->m_error_cfg[XFS_ERR_METADATA][XFS_ERR_DEFAULT];
-	error = xfs_sysfs_init(&cfg->kobj, &xfs_error_cfg_ktype,
-				&mp->m_error_meta_kobj, "Default");
-	if (error)
-		goto out_error_meta;
-	cfg->fail_speed = XFS_ERR_FAIL_NEVER;
-
 	return 0;
 
-out_error_meta:
-	xfs_sysfs_del(&mp->m_error_meta_kobj);
 out_error:
 	xfs_sysfs_del(&mp->m_error_kobj);
 	return error;
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 5/9] xfs: add configuration of error failure speed

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

On reception of an error, we can fail immediately, perform some
bound amount of retries or retry indefinitely. The current behaviour
we have is to retry forever.

However, we'd like the ability to choose what behaviour we have, and
that requires the ability to configure the behaviour through the new
sysfs interfaces. Add configuration options for fail fast, slow or
never to reflect the three choices above. Fail fast or fail never
don't require any other options, but "fail slow" needs configuration
to bound the retry behaviour. Add both a maximum retry count and a
retry timeout so that we can bound by time and/or physical IO
attempts.

Finally, plumb these into xfs_buf_iodone error processing so that
the error behaviour follows the selected configuration.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_buf.h      |  23 ++++++++-
 fs/xfs/xfs_buf_item.c |  22 ++++++++-
 fs/xfs/xfs_mount.h    |   2 +
 fs/xfs/xfs_sysfs.c    | 128 ++++++++++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 169 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index ad16a1e..1edbe3c 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -183,7 +183,28 @@ typedef struct xfs_buf {
 	unsigned int		b_page_count;	/* size of page array */
 	unsigned int		b_offset;	/* page offset in first page */
 	int			b_error;	/* error code on I/O */
-	int			b_last_error;	/* previous async I/O error */
+
+	/*
+	 * async write failure retry count. Initialised to zero on the first
+	 * failure, then when it exceeds the maximum configured without a
+	 * success the write is considered to be failed permanently and the
+	 * iodone handler will take appropriate action.
+	 *
+	 * For retry timeouts, we record the jiffie of the first failure. This
+	 * means that we can change the retry timeout and it on the next error
+	 * it will be checked against the newly configured timeout. This
+	 * prevents buffers getting stuck in retry loops with a really long
+	 * timeout.
+	 *
+	 * last_error is used to ensure that we are getting repeated errors, not
+	 * different errors. e.g. a block device might change ENOSPC to EIO when
+	 * a failure timeout occurs, so we want to re-initialise the error
+	 * retry behaviour appropriately when that happens.
+	 */
+	int			b_retries;
+	unsigned long		b_first_retry_time; /* in jiffies */
+	int			b_last_error;
+
 	const struct xfs_buf_ops	*b_ops;
 
 #ifdef XFS_BUF_LOCK_TRACKING
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 68e34d1..7afd4d5 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -970,6 +970,9 @@ xfs_buf_iodone_callback_error(
 		bp->b_flags |= (XBF_WRITE | XBF_ASYNC |
 			        XBF_DONE | XBF_WRITE_FAIL);
 		bp->b_last_error = bp->b_error;
+		bp->b_retries = 0;
+		bp->b_first_retry_time = jiffies;
+
 		xfs_buf_ioerror(bp, 0);
 		xfs_buf_submit(bp);
 		return true;
@@ -979,9 +982,25 @@ xfs_buf_iodone_callback_error(
 	 * Repeated failure on an async write. Take action according to the
 	 * error configuration we have been set up to use.
 	 */
-	if (cfg->fail_speed == XFS_ERR_FAIL_FAST)
+	switch (cfg->fail_speed) {
+	case XFS_ERR_FAIL_FAST:
 		goto permanent_error;
 
+	case XFS_ERR_FAIL_SLOW:
+		if (++bp->b_retries > cfg->max_retries)
+			goto permanent_error;
+		if (!cfg->retry_timeout)
+			break;
+		if (time_after(jiffies,
+			       cfg->retry_timeout + bp->b_first_retry_time))
+			goto permanent_error;
+		break;
+
+	case XFS_ERR_FAIL_NEVER:
+	default:
+		break;
+	}
+
 	/* still a transient error, higher layers will retry */
 	xfs_buf_ioerror(bp, 0);
 	xfs_buf_relse(bp);
@@ -1023,6 +1042,7 @@ xfs_buf_iodone_callbacks(
 	 * retry state here in preparation for the next error that may occur.
 	 */
 	bp->b_last_error = 0;
+	bp->b_retries = 0;
 
 	xfs_buf_do_callbacks(bp);
 	bp->b_fspriv = NULL;
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 9a61f39..2a3d178 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -62,6 +62,8 @@ enum {
 struct xfs_error_cfg {
 	struct xfs_kobj	kobj;
 	int		fail_speed;
+	int		max_retries;	/* INT_MAX = retry forever */
+	unsigned long	retry_timeout;	/* in jiffies, 0 = no timeout */
 };
 
 typedef struct xfs_mount {
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index 27487ce..51d9fa7 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -319,10 +319,6 @@ struct kobj_type xfs_log_ktype = {
  * and any other future type of IO (e.g. special inode or directory error
  * handling) we care to support.
  */
-static struct attribute *xfs_error_attrs[] = {
-	NULL,
-};
-
 static inline struct xfs_error_cfg *
 to_error_cfg(struct kobject *kobject)
 {
@@ -330,6 +326,123 @@ to_error_cfg(struct kobject *kobject)
 	return container_of(kobj, struct xfs_error_cfg, kobj);
 }
 
+static ssize_t
+failure_speed_show(
+	struct kobject	*kobject,
+	char		*buf)
+{
+	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
+
+	if (cfg->fail_speed == XFS_ERR_FAIL_FAST)
+		return snprintf(buf, PAGE_SIZE, "[fast] slow never\n");
+	if (cfg->fail_speed == XFS_ERR_FAIL_SLOW)
+		return snprintf(buf, PAGE_SIZE, "fast [slow] never\n");
+	return snprintf(buf, PAGE_SIZE, "fast slow [never]\n");
+}
+
+static ssize_t
+failure_speed_store(
+	struct kobject	*kobject,
+	const char	*buf,
+	size_t		count)
+{
+	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
+	char		*str = kstrdup(buf, GFP_KERNEL);
+	char		*sp;
+	int		len;
+
+	if (!str)
+		return -ENOMEM;
+
+	sp = strstrip(str);
+	len = strlen(sp);
+	if (strncmp(sp, "fast", len) == 0)
+		cfg->fail_speed = XFS_ERR_FAIL_FAST;
+	else if (strncmp(sp, "slow", len) == 0)
+		cfg->fail_speed = XFS_ERR_FAIL_SLOW;
+	else if (strncmp(sp, "never", len) == 0)
+		cfg->fail_speed = XFS_ERR_FAIL_NEVER;
+	else
+		count = -EINVAL;
+	kfree(str);
+	return count;
+}
+XFS_SYSFS_ATTR_RW(failure_speed);
+
+static ssize_t
+max_retries_show(
+	struct kobject	*kobject,
+	char		*buf)
+{
+	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", cfg->max_retries);
+}
+
+static ssize_t
+max_retries_store(
+	struct kobject	*kobject,
+	const char	*buf,
+	size_t		count)
+{
+	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
+	int		ret;
+	int		val;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	if (val < 0 || val > INT_MAX)
+		return -EINVAL;
+
+	cfg->max_retries = val;
+	return count;
+}
+XFS_SYSFS_ATTR_RW(max_retries);
+
+static ssize_t
+retry_timeout_seconds_show(
+	struct kobject	*kobject,
+	char		*buf)
+{
+	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
+
+	return snprintf(buf, PAGE_SIZE, "%ld\n", 
+			jiffies_to_msecs(cfg->retry_timeout) * MSEC_PER_SEC);
+}
+
+static ssize_t
+retry_timeout_seconds_store(
+	struct kobject	*kobject,
+	const char	*buf,
+	size_t		count)
+{
+	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
+	int		ret;
+	int		val;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	/* 1 day timeout maximum */
+	if (val < 0 || val > 86400)
+		return -EINVAL;
+
+	cfg->retry_timeout = msecs_to_jiffies(val * MSEC_PER_SEC);
+	return count;
+}
+XFS_SYSFS_ATTR_RW(retry_timeout_seconds);
+
+static struct attribute *xfs_error_attrs[] = {
+	ATTR_LIST(failure_speed),
+	ATTR_LIST(max_retries),
+	ATTR_LIST(retry_timeout_seconds),
+	NULL,
+};
+
+
 struct kobj_type xfs_error_cfg_ktype = {
 	.release = xfs_sysfs_release,
 	.sysfs_ops = &xfs_sysfs_ops,
@@ -349,11 +462,15 @@ struct kobj_type xfs_error_ktype = {
 struct xfs_error_init {
 	char		*name;
 	int		fail_speed;
+	int		max_retries;
+	int		retry_timeout;	/* in seconds */
 };
 
 static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
 	{ .name = "Default",
 	  .fail_speed = XFS_ERR_FAIL_NEVER,
+	  .max_retries = INT_MAX,
+	  .retry_timeout = 0,
 	},
 };
 
@@ -384,6 +501,9 @@ xfs_error_sysfs_init_class(
 			goto out_error;
 
 		cfg->fail_speed = init[i].fail_speed;
+		cfg->max_retries = init[i].max_retries;
+		cfg->retry_timeout = msecs_to_jiffies(
+					init[i].retry_timeout * MSEC_PER_SEC);
 	}
 	return 0;
 
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 6/9] xfs: add "fail at unmount" error handling configuration

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

If we take "retry forever" literally on metadata IO errors, we can
hang an unmount retries those writes forever. This is the default
behaviour, unfortunately. Add a error configuration option for this
behaviour and default it to "fail" so that an unmount will trigger
actual errors, a shutdown and allow the unmount to succeed. It will
be noisy, though, as it will log the errors and shutdown that
occurs.

To do this, we need to mark the filesystem as being in the process
of unmounting. Do this with a mount flag that is added at the
appropriate time (i.e. before the blocking AIL sync). We also need
to add this flag if mount fails after the initial phase of log
recovery has been run.

The config is done by a separate boolean sysfs option rather than a
new fail_speed enum, as fail_at_unmount is relevant to both
XFS_ERR_FAIL_NEVER and XFS_ERR_FAIL_SLOW options.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_buf_item.c |  4 ++++
 fs/xfs/xfs_mount.c    |  9 +++++++++
 fs/xfs/xfs_mount.h    |  2 ++
 fs/xfs/xfs_sysfs.c    | 36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+)

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 7afd4d5..9220283 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -1001,6 +1001,10 @@ xfs_buf_iodone_callback_error(
 		break;
 	}
 
+	/* At unmount we may treat errors differently */
+	if ((mp->m_flags & XFS_MOUNT_UNMOUNTING) && cfg->fail_at_unmount)
+		goto permanent_error;
+
 	/* still a transient error, higher layers will retry */
 	xfs_buf_ioerror(bp, 0);
 	xfs_buf_relse(bp);
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index f6fb5e1..f8c4a50 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -980,6 +980,7 @@ xfs_mountfs(
 	cancel_delayed_work_sync(&mp->m_reclaim_work);
 	xfs_reclaim_inodes(mp, SYNC_WAIT);
  out_log_dealloc:
+	mp->m_flags |= XFS_MOUNT_UNMOUNTING;
 	xfs_log_mount_cancel(mp);
  out_fail_wait:
 	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
@@ -1031,6 +1032,14 @@ xfs_unmountfs(
 	xfs_log_force(mp, XFS_LOG_SYNC);
 
 	/*
+	 * We now need to tell the world we are unmounting. This will allow
+	 * us to detect that the filesystem is going away and we should error
+	 * out anything that we have been retrying in the background. This will
+	 * prevent neverending retries iin AIL pushing from hanging the unmount.
+	 */
+	mp->m_flags |= XFS_MOUNT_UNMOUNTING;
+
+	/*
 	 * Flush all pending changes from the AIL.
 	 */
 	xfs_ail_push_all_sync(mp->m_ail);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 2a3d178..edeb0b6 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -64,6 +64,7 @@ struct xfs_error_cfg {
 	int		fail_speed;
 	int		max_retries;	/* INT_MAX = retry forever */
 	unsigned long	retry_timeout;	/* in jiffies, 0 = no timeout */
+	bool		fail_at_unmount;
 };
 
 typedef struct xfs_mount {
@@ -187,6 +188,7 @@ typedef struct xfs_mount {
 #define XFS_MOUNT_WSYNC		(1ULL << 0)	/* for nfs - all metadata ops
 						   must be synchronous except
 						   for space allocations */
+#define XFS_MOUNT_UNMOUNTING	(1ULL << 1)	/* filesystem is unmounting */
 #define XFS_MOUNT_WAS_CLEAN	(1ULL << 3)
 #define XFS_MOUNT_FS_SHUTDOWN	(1ULL << 4)	/* atomic stop of all filesystem
 						   operations, typically for
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index 51d9fa7..a5b040a 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -435,10 +435,43 @@ retry_timeout_seconds_store(
 }
 XFS_SYSFS_ATTR_RW(retry_timeout_seconds);
 
+static ssize_t
+fail_at_unmount_show(
+	struct kobject	*kobject,
+	char		*buf)
+{
+	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", cfg->fail_at_unmount);
+}
+
+static ssize_t
+fail_at_unmount_store(
+	struct kobject	*kobject,
+	const char	*buf,
+	size_t		count)
+{
+	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
+	int		ret;
+	int		val;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	if (val < 0 || val > 1)
+		return -EINVAL;
+
+	cfg->fail_at_unmount = val;
+	return count;
+}
+XFS_SYSFS_ATTR_RW(fail_at_unmount);
+
 static struct attribute *xfs_error_attrs[] = {
 	ATTR_LIST(failure_speed),
 	ATTR_LIST(max_retries),
 	ATTR_LIST(retry_timeout_seconds),
+	ATTR_LIST(fail_at_unmount),
 	NULL,
 };
 
@@ -464,6 +497,7 @@ struct xfs_error_init {
 	int		fail_speed;
 	int		max_retries;
 	int		retry_timeout;	/* in seconds */
+	bool		fail_at_unmount;
 };
 
 static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
@@ -471,6 +505,7 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
 	  .fail_speed = XFS_ERR_FAIL_NEVER,
 	  .max_retries = INT_MAX,
 	  .retry_timeout = 0,
+	  .fail_at_unmount = true,
 	},
 };
 
@@ -504,6 +539,7 @@ xfs_error_sysfs_init_class(
 		cfg->max_retries = init[i].max_retries;
 		cfg->retry_timeout = msecs_to_jiffies(
 					init[i].retry_timeout * MSEC_PER_SEC);
+		cfg->fail_at_unmount = init[i].fail_at_unmount;
 	}
 	return 0;
 
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 7/9] xfs: add configuration handles for specific errors

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

now most of the infrastructure is in place, we can start adding
support for configuring specific errors such as ENODEV, ENOSPC, EIO,
etc. Add these error configurations and configure them all to have
appropriate behaviours. That is, all will use the "fail never, fail
at unmount" default except for ENODEV, which is an unrecoverable
error so it will be configured as a "fail fast" error.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_mount.h |  3 +++
 fs/xfs/xfs_sysfs.c | 24 ++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index edeb0b6..c05b500 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -50,6 +50,9 @@ enum {
 };
 enum {
 	XFS_ERR_DEFAULT,
+	XFS_ERR_EIO,
+	XFS_ERR_ENOSPC,
+	XFS_ERR_ENODEV,
 	XFS_ERR_ERRNO_MAX,
 };
 enum {
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index a5b040a..d48dc46 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -507,6 +507,21 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
 	  .retry_timeout = 0,
 	  .fail_at_unmount = true,
 	},
+	{ .name = "EIO",
+	  .fail_speed = XFS_ERR_FAIL_NEVER,
+	  .max_retries = INT_MAX,
+	  .retry_timeout = 0,
+	  .fail_at_unmount = true,
+	},
+	{ .name = "ENOSPC",
+	  .fail_speed = XFS_ERR_FAIL_NEVER,
+	  .max_retries = INT_MAX,
+	  .retry_timeout = 0,
+	  .fail_at_unmount = true,
+	},
+	{ .name = "ENODEV",
+	  .fail_speed = XFS_ERR_FAIL_FAST,
+	},
 };
 
 static int
@@ -606,6 +621,15 @@ xfs_error_get_cfg(
 	struct xfs_error_cfg	*cfg;
 
 	switch (error) {
+	case EIO:
+		cfg = &mp->m_error_cfg[error_class][XFS_ERR_EIO];
+		break;
+	case ENOSPC:
+		cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENOSPC];
+		break;
+	case ENODEV:
+		cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENODEV];
+		break;
 	default:
 		cfg = &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT];
 		break;
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 8/9] xfs: disable specific error configurations

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Different error classes are going to need different error to be
configured, so we don't want them all to be visible in sysfs. Add a
configuration check into the config initialisation an lookup
code to determine if the default should be used for a specific
error. If so, the sysfs entry is not created, and on lookup the
default config is returned.

Add ENOMEM at this point to exercise this code, as it will be used
later when adding a kmem error failure class.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_mount.h |  1 +
 fs/xfs/xfs_sysfs.c | 22 ++++++++++++++++++----
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index c05b500..0ff14a90 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -53,6 +53,7 @@ enum {
 	XFS_ERR_EIO,
 	XFS_ERR_ENOSPC,
 	XFS_ERR_ENODEV,
+	XFS_ERR_ENOMEM,
 	XFS_ERR_ERRNO_MAX,
 };
 enum {
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index d48dc46..c984c3e 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -522,6 +522,9 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
 	{ .name = "ENODEV",
 	  .fail_speed = XFS_ERR_FAIL_FAST,
 	},
+	{ .name = "ENOMEM",
+	  .fail_speed = XFS_ERR_FAIL_DEFAULT,
+	},
 };
 
 static int
@@ -545,12 +548,17 @@ xfs_error_sysfs_init_class(
 
 	for (i = 0; i < XFS_ERR_ERRNO_MAX; i++) {
 		cfg = &mp->m_error_cfg[class][i];
+
+		/* skip errors that are not configurable for this class */
+		cfg->fail_speed = init[i].fail_speed;
+		if (cfg->fail_speed == XFS_ERR_FAIL_DEFAULT)
+			continue;
+
 		error = xfs_sysfs_init(&cfg->kobj, &xfs_error_cfg_ktype,
 					parent_kobj, init[i].name);
 		if (error)
 			goto out_error;
 
-		cfg->fail_speed = init[i].fail_speed;
 		cfg->max_retries = init[i].max_retries;
 		cfg->retry_timeout = msecs_to_jiffies(
 					init[i].retry_timeout * MSEC_PER_SEC);
@@ -605,7 +613,8 @@ xfs_error_sysfs_del(
 		for (j = 0; j < XFS_ERR_ERRNO_MAX; j++) {
 			cfg = &mp->m_error_cfg[i][j];
 
-			xfs_sysfs_del(&cfg->kobj);
+			if (cfg->fail_speed != XFS_ERR_FAIL_DEFAULT)
+				xfs_sysfs_del(&cfg->kobj);
 		}
 	}
 	xfs_sysfs_del(&mp->m_error_meta_kobj);
@@ -630,10 +639,15 @@ xfs_error_get_cfg(
 	case ENODEV:
 		cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENODEV];
 		break;
-	default:
-		cfg = &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT];
+	case ENOMEM:
+		cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENOMEM];
 		break;
+	default:
+		return &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT];
 	}
 
+	/* The error may not be not configurable, so uses default behaviour */
+	if (cfg->fail_speed == XFS_ERR_FAIL_DEFAULT)
+		return &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT];
 	return cfg;
 }
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 9/9] xfs: add kmem error configuration class

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Add a new error configuration class for defining how memory
allocation failures are supposed to be handled, This requires
factoring the initialisation/teardown code to be able to handle
a generic error class object.

I'm not yet sure how to hook it into the memory allocation calls -
that will be done in a later patch; this just demonstrates how
multiple classes are configured and initialised. It may be that we
don't configure specific errors here - instead configure how we
handle specific types of allocation failure e.g. GFP_KERNEL vs
GFP_NOFS vs allocations inside transactions. Either way, we are
going to need to plumb the error config handler into the
memory allocation code in some manner.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_mount.h |  3 ++-
 fs/xfs/xfs_sysfs.c | 77 +++++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 61 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 0ff14a90..62bcdcb 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -46,6 +46,7 @@ enum {
  */
 enum {
 	XFS_ERR_METADATA,
+	XFS_ERR_KMEM,
 	XFS_ERR_CLASS_MAX,
 };
 enum {
@@ -162,7 +163,7 @@ typedef struct xfs_mount {
 						/* low free space thresholds */
 	struct xfs_kobj		m_kobj;
 	struct xfs_kobj		m_error_kobj;
-	struct xfs_kobj		m_error_meta_kobj;
+	struct xfs_kobj		m_error_class_kobj[XFS_ERR_CLASS_MAX];
 	struct xfs_error_cfg	m_error_cfg[XFS_ERR_CLASS_MAX][XFS_ERR_ERRNO_MAX];
 	struct xstats		m_stats;	/* per-fs stats */
 
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index c984c3e..c3dff95 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -522,26 +522,50 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
 	{ .name = "ENODEV",
 	  .fail_speed = XFS_ERR_FAIL_FAST,
 	},
-	{ .name = "ENOMEM",
+	{ /* ENOMEM */
 	  .fail_speed = XFS_ERR_FAIL_DEFAULT,
 	},
 };
 
+static const struct xfs_error_init xfs_error_kmem_init[XFS_ERR_ERRNO_MAX] = {
+	{ .name = "Default",
+	  .fail_speed = XFS_ERR_FAIL_NEVER,
+	  .max_retries = INT_MAX,
+	  .retry_timeout = 0,
+	  .fail_at_unmount = false,
+	},
+	{ /* EIO */
+	  .fail_speed = XFS_ERR_FAIL_DEFAULT,
+	},
+	{ /* ENOSPC */
+	  .fail_speed = XFS_ERR_FAIL_DEFAULT,
+	},
+	{ /* ENODEV */
+	  .fail_speed = XFS_ERR_FAIL_DEFAULT,
+	},
+	{ .name = "ENOMEM",
+	  .fail_speed = XFS_ERR_FAIL_NEVER,
+	  .max_retries = INT_MAX,
+	  .retry_timeout = 0,
+	  .fail_at_unmount = false,
+	},
+};
+
 static int
 xfs_error_sysfs_init_class(
 	struct xfs_mount	*mp,
 	int			class,
 	const char		*parent_name,
-	struct xfs_kobj		*parent_kobj,
 	const struct xfs_error_init init[])
 {
+	struct xfs_kobj		*class_kobj = &mp->m_error_class_kobj[class];
 	struct xfs_error_cfg	*cfg;
 	int			error;
 	int			i;
 
 	ASSERT(class < XFS_ERR_CLASS_MAX);
 
-	error = xfs_sysfs_init(parent_kobj, &xfs_error_ktype,
+	error = xfs_sysfs_init(class_kobj, &xfs_error_ktype,
 				&mp->m_error_kobj, parent_name);
 	if (error)
 		return error;
@@ -554,8 +578,9 @@ xfs_error_sysfs_init_class(
 		if (cfg->fail_speed == XFS_ERR_FAIL_DEFAULT)
 			continue;
 
+		ASSERT(init[i].name);
 		error = xfs_sysfs_init(&cfg->kobj, &xfs_error_cfg_ktype,
-					parent_kobj, init[i].name);
+					class_kobj, init[i].name);
 		if (error)
 			goto out_error;
 
@@ -572,10 +597,28 @@ out_error:
 		cfg = &mp->m_error_cfg[class][i];
 		xfs_sysfs_del(&cfg->kobj);
 	}
-	xfs_sysfs_del(parent_kobj);
+	xfs_sysfs_del(class_kobj);
 	return error;
 }
 
+static void
+xfs_error_sysfs_del_class(
+	struct xfs_mount	*mp,
+	int			class)
+{
+	struct xfs_error_cfg	*cfg;
+	int			i;
+
+	for (i = 0; i < XFS_ERR_ERRNO_MAX; i++) {
+		cfg = &mp->m_error_cfg[class][i];
+
+		if (cfg->fail_speed != XFS_ERR_FAIL_DEFAULT)
+			xfs_sysfs_del(&cfg->kobj);
+	}
+
+	xfs_sysfs_del(&mp->m_error_class_kobj[class]);
+}
+
 int
 xfs_error_sysfs_init(
 	struct xfs_mount	*mp)
@@ -589,14 +632,19 @@ xfs_error_sysfs_init(
 		return error;
 
 	/* .../xfs/<dev>/error/metadata/ */
-	error = xfs_error_sysfs_init_class(mp, XFS_ERR_METADATA,
-				"metadata", &mp->m_error_meta_kobj,
-				xfs_error_meta_init);
+	error = xfs_error_sysfs_init_class(mp, XFS_ERR_METADATA, "metadata",
+					   xfs_error_meta_init);
 	if (error)
 		goto out_error;
 
+	error = xfs_error_sysfs_init_class(mp, XFS_ERR_KMEM, "kmem",
+					   xfs_error_kmem_init);
+	if (error)
+		goto out_error_meta;
 	return 0;
 
+out_error_meta:
+	xfs_error_sysfs_del_class(mp, XFS_ERR_METADATA);
 out_error:
 	xfs_sysfs_del(&mp->m_error_kobj);
 	return error;
@@ -606,18 +654,11 @@ void
 xfs_error_sysfs_del(
 	struct xfs_mount	*mp)
 {
-	struct xfs_error_cfg	*cfg;
-	int			i, j;
+	int			i;
 
-	for (i = 0; i < XFS_ERR_CLASS_MAX; i++) {
-		for (j = 0; j < XFS_ERR_ERRNO_MAX; j++) {
-			cfg = &mp->m_error_cfg[i][j];
+	for (i = 0; i < XFS_ERR_CLASS_MAX; i++)
+		xfs_error_sysfs_del_class(mp, i);
 
-			if (cfg->fail_speed != XFS_ERR_FAIL_DEFAULT)
-				xfs_sysfs_del(&cfg->kobj);
-		}
-	}
-	xfs_sysfs_del(&mp->m_error_meta_kobj);
 	xfs_sysfs_del(&mp->m_error_kobj);
 }
 
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/9] xfs: configurable error behaviour

[Dave Chinner]

ping?

On Fri, Feb 05, 2016 at 12:23:18PM +1100, Dave Chinner wrote:
> Hi folks,
> 
> I need to restart the discussion and review of this patch series.
> There was some discussion of it last time, but nothing really came
> from that. I'm posting what I have in my tree right now - treat it
> as though it's an initial posting of the code because I can't recall
> what I've changed since the first posting.
> 
> What I'd like to have to for the next merge window is all the IO
> error bits sorted out. The final patch (kmem failure behaviour)
> needs more infrastructure (passing mp to every allocation) so that's
> a secondary concern right now and I've only included it to
> demonstrate how to apply this code ot a different subsystem.
> 
> Things that need to be nailed down before I can commit the series:
> 
> 	- sysfs layout
> 	- naming conventions for errors and subsystems in sysfs
> 	- how best to display/change default behaviour
> 
> Things that we can change/implement later:
> 
> 	- default behaviour
> 	- additional error classes
> 	- additional error types
> 	- additional subsystems
> 	- subsystem error handling implementation
> 	- communication with other subsystems to dynamically change
> 	  error behaviour
> 
> IOWs, what is important right now is how we present this to
> userspace, because we can't change that easily once we've decided on
> a presentation structure.
> 
> Modifying the code to classify and handle all the different error
> types is much less important, as we can change that to fix whatever
> problems we have without impacting the presentation to userspace.
> 
> There is definite need for this (e.g. handling of ENOSPC on thin
> provisioned devices), so I want to get quickly to a consensus on the
> userspace facing aspects so that we can get this ball rolling.
> 
> The biggest unsolved issue is how to change the default behaviour
> persistently. There is no infrastructure in this patch series to do
> that, but it is someting that we have to consider so that we don't
> require default behaviour to be changed after every mount of every
> filesystem on a system. My thoughts on this is we store changes to
> the defaults in xattrs on the root inode, but I'm open to ideas here
> as there's no code written for it yet. Solving this problem,
> however, is not necessary before commiting the initial code; it's
> something we can add later once we've worked out all the details.
> 
> Discuss!
> 
> -Dave.
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 5/9] xfs: add configuration of error failure speed

[Brian Foster]

On Fri, Feb 05, 2016 at 12:23:23PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> On reception of an error, we can fail immediately, perform some
> bound amount of retries or retry indefinitely. The current behaviour
> we have is to retry forever.
> 
> However, we'd like the ability to choose what behaviour we have, and
> that requires the ability to configure the behaviour through the new
> sysfs interfaces. Add configuration options for fail fast, slow or
> never to reflect the three choices above. Fail fast or fail never
> don't require any other options, but "fail slow" needs configuration
> to bound the retry behaviour. Add both a maximum retry count and a
> retry timeout so that we can bound by time and/or physical IO
> attempts.
> 
> Finally, plumb these into xfs_buf_iodone error processing so that
> the error behaviour follows the selected configuration.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_buf.h      |  23 ++++++++-
>  fs/xfs/xfs_buf_item.c |  22 ++++++++-
>  fs/xfs/xfs_mount.h    |   2 +
>  fs/xfs/xfs_sysfs.c    | 128 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  4 files changed, 169 insertions(+), 6 deletions(-)
> 
...
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index 68e34d1..7afd4d5 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
...
> @@ -979,9 +982,25 @@ xfs_buf_iodone_callback_error(
>  	 * Repeated failure on an async write. Take action according to the
>  	 * error configuration we have been set up to use.
>  	 */
> -	if (cfg->fail_speed == XFS_ERR_FAIL_FAST)
> +	switch (cfg->fail_speed) {
> +	case XFS_ERR_FAIL_FAST:
>  		goto permanent_error;
>  
> +	case XFS_ERR_FAIL_SLOW:
> +		if (++bp->b_retries > cfg->max_retries)
> +			goto permanent_error;
> +		if (!cfg->retry_timeout)
> +			break;
> +		if (time_after(jiffies,
> +			       cfg->retry_timeout + bp->b_first_retry_time))
> +			goto permanent_error;
> +		break;
> +
> +	case XFS_ERR_FAIL_NEVER:
> +	default:
> +		break;
> +	}
> +

I wonder a bit how granular this system needs to be in terms of user
interface, at least right now. For example, fail fast and fail never
just seem like variants of fail slow with particular tunables. Fail fast
is roughly equivalent to a retry count of one, whereas fail never
implies an infinite (e.g., -1) retry count. Do we really need the higher
level classification?

>  	/* still a transient error, higher layers will retry */
>  	xfs_buf_ioerror(bp, 0);
>  	xfs_buf_relse(bp);
> @@ -1023,6 +1042,7 @@ xfs_buf_iodone_callbacks(
>  	 * retry state here in preparation for the next error that may occur.
>  	 */
>  	bp->b_last_error = 0;
> +	bp->b_retries = 0;
>  
>  	xfs_buf_do_callbacks(bp);
>  	bp->b_fspriv = NULL;
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 9a61f39..2a3d178 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -62,6 +62,8 @@ enum {
>  struct xfs_error_cfg {
>  	struct xfs_kobj	kobj;
>  	int		fail_speed;
> +	int		max_retries;	/* INT_MAX = retry forever */
> +	unsigned long	retry_timeout;	/* in jiffies, 0 = no timeout */
>  };
>  
>  typedef struct xfs_mount {
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index 27487ce..51d9fa7 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
...
> @@ -330,6 +326,123 @@ to_error_cfg(struct kobject *kobject)
...
> +static ssize_t
> +retry_timeout_seconds_show(
> +	struct kobject	*kobject,
> +	char		*buf)
> +{
> +	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
> +
> +	return snprintf(buf, PAGE_SIZE, "%ld\n", 

Trailing whitespace here ^

Brian

> +			jiffies_to_msecs(cfg->retry_timeout) * MSEC_PER_SEC);
> +}
> +
> +static ssize_t
> +retry_timeout_seconds_store(
> +	struct kobject	*kobject,
> +	const char	*buf,
> +	size_t		count)
> +{
> +	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
> +	int		ret;
> +	int		val;
> +
> +	ret = kstrtoint(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	/* 1 day timeout maximum */
> +	if (val < 0 || val > 86400)
> +		return -EINVAL;
> +
> +	cfg->retry_timeout = msecs_to_jiffies(val * MSEC_PER_SEC);
> +	return count;
> +}
> +XFS_SYSFS_ATTR_RW(retry_timeout_seconds);
> +
> +static struct attribute *xfs_error_attrs[] = {
> +	ATTR_LIST(failure_speed),
> +	ATTR_LIST(max_retries),
> +	ATTR_LIST(retry_timeout_seconds),
> +	NULL,
> +};
> +
> +
>  struct kobj_type xfs_error_cfg_ktype = {
>  	.release = xfs_sysfs_release,
>  	.sysfs_ops = &xfs_sysfs_ops,
> @@ -349,11 +462,15 @@ struct kobj_type xfs_error_ktype = {
>  struct xfs_error_init {
>  	char		*name;
>  	int		fail_speed;
> +	int		max_retries;
> +	int		retry_timeout;	/* in seconds */
>  };
>  
>  static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
>  	{ .name = "Default",
>  	  .fail_speed = XFS_ERR_FAIL_NEVER,
> +	  .max_retries = INT_MAX,
> +	  .retry_timeout = 0,
>  	},
>  };
>  
> @@ -384,6 +501,9 @@ xfs_error_sysfs_init_class(
>  			goto out_error;
>  
>  		cfg->fail_speed = init[i].fail_speed;
> +		cfg->max_retries = init[i].max_retries;
> +		cfg->retry_timeout = msecs_to_jiffies(
> +					init[i].retry_timeout * MSEC_PER_SEC);
>  	}
>  	return 0;
>  
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 6/9] xfs: add "fail at unmount" error handling configuration

[Brian Foster]

On Fri, Feb 05, 2016 at 12:23:24PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> If we take "retry forever" literally on metadata IO errors, we can
> hang an unmount retries those writes forever. This is the default
> behaviour, unfortunately. Add a error configuration option for this
> behaviour and default it to "fail" so that an unmount will trigger
> actual errors, a shutdown and allow the unmount to succeed. It will
> be noisy, though, as it will log the errors and shutdown that
> occurs.
> 
> To do this, we need to mark the filesystem as being in the process
> of unmounting. Do this with a mount flag that is added at the
> appropriate time (i.e. before the blocking AIL sync). We also need
> to add this flag if mount fails after the initial phase of log
> recovery has been run.
> 
> The config is done by a separate boolean sysfs option rather than a
> new fail_speed enum, as fail_at_unmount is relevant to both
> XFS_ERR_FAIL_NEVER and XFS_ERR_FAIL_SLOW options.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Similar question of scope/granularity here... why would one want to set
this option for a particular error and not any others? In other words,
it seems more useful as a global (or per-mount) option.

Brian

>  fs/xfs/xfs_buf_item.c |  4 ++++
>  fs/xfs/xfs_mount.c    |  9 +++++++++
>  fs/xfs/xfs_mount.h    |  2 ++
>  fs/xfs/xfs_sysfs.c    | 36 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 51 insertions(+)
> 
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index 7afd4d5..9220283 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -1001,6 +1001,10 @@ xfs_buf_iodone_callback_error(
>  		break;
>  	}
>  
> +	/* At unmount we may treat errors differently */
> +	if ((mp->m_flags & XFS_MOUNT_UNMOUNTING) && cfg->fail_at_unmount)
> +		goto permanent_error;
> +
>  	/* still a transient error, higher layers will retry */
>  	xfs_buf_ioerror(bp, 0);
>  	xfs_buf_relse(bp);
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index f6fb5e1..f8c4a50 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -980,6 +980,7 @@ xfs_mountfs(
>  	cancel_delayed_work_sync(&mp->m_reclaim_work);
>  	xfs_reclaim_inodes(mp, SYNC_WAIT);
>   out_log_dealloc:
> +	mp->m_flags |= XFS_MOUNT_UNMOUNTING;
>  	xfs_log_mount_cancel(mp);
>   out_fail_wait:
>  	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
> @@ -1031,6 +1032,14 @@ xfs_unmountfs(
>  	xfs_log_force(mp, XFS_LOG_SYNC);
>  
>  	/*
> +	 * We now need to tell the world we are unmounting. This will allow
> +	 * us to detect that the filesystem is going away and we should error
> +	 * out anything that we have been retrying in the background. This will
> +	 * prevent neverending retries iin AIL pushing from hanging the unmount.
> +	 */
> +	mp->m_flags |= XFS_MOUNT_UNMOUNTING;
> +
> +	/*
>  	 * Flush all pending changes from the AIL.
>  	 */
>  	xfs_ail_push_all_sync(mp->m_ail);
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 2a3d178..edeb0b6 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -64,6 +64,7 @@ struct xfs_error_cfg {
>  	int		fail_speed;
>  	int		max_retries;	/* INT_MAX = retry forever */
>  	unsigned long	retry_timeout;	/* in jiffies, 0 = no timeout */
> +	bool		fail_at_unmount;
>  };
>  
>  typedef struct xfs_mount {
> @@ -187,6 +188,7 @@ typedef struct xfs_mount {
>  #define XFS_MOUNT_WSYNC		(1ULL << 0)	/* for nfs - all metadata ops
>  						   must be synchronous except
>  						   for space allocations */
> +#define XFS_MOUNT_UNMOUNTING	(1ULL << 1)	/* filesystem is unmounting */
>  #define XFS_MOUNT_WAS_CLEAN	(1ULL << 3)
>  #define XFS_MOUNT_FS_SHUTDOWN	(1ULL << 4)	/* atomic stop of all filesystem
>  						   operations, typically for
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index 51d9fa7..a5b040a 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
> @@ -435,10 +435,43 @@ retry_timeout_seconds_store(
>  }
>  XFS_SYSFS_ATTR_RW(retry_timeout_seconds);
>  
> +static ssize_t
> +fail_at_unmount_show(
> +	struct kobject	*kobject,
> +	char		*buf)
> +{
> +	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", cfg->fail_at_unmount);
> +}
> +
> +static ssize_t
> +fail_at_unmount_store(
> +	struct kobject	*kobject,
> +	const char	*buf,
> +	size_t		count)
> +{
> +	struct xfs_error_cfg *cfg = to_error_cfg(kobject);
> +	int		ret;
> +	int		val;
> +
> +	ret = kstrtoint(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val < 0 || val > 1)
> +		return -EINVAL;
> +
> +	cfg->fail_at_unmount = val;
> +	return count;
> +}
> +XFS_SYSFS_ATTR_RW(fail_at_unmount);
> +
>  static struct attribute *xfs_error_attrs[] = {
>  	ATTR_LIST(failure_speed),
>  	ATTR_LIST(max_retries),
>  	ATTR_LIST(retry_timeout_seconds),
> +	ATTR_LIST(fail_at_unmount),
>  	NULL,
>  };
>  
> @@ -464,6 +497,7 @@ struct xfs_error_init {
>  	int		fail_speed;
>  	int		max_retries;
>  	int		retry_timeout;	/* in seconds */
> +	bool		fail_at_unmount;
>  };
>  
>  static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
> @@ -471,6 +505,7 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
>  	  .fail_speed = XFS_ERR_FAIL_NEVER,
>  	  .max_retries = INT_MAX,
>  	  .retry_timeout = 0,
> +	  .fail_at_unmount = true,
>  	},
>  };
>  
> @@ -504,6 +539,7 @@ xfs_error_sysfs_init_class(
>  		cfg->max_retries = init[i].max_retries;
>  		cfg->retry_timeout = msecs_to_jiffies(
>  					init[i].retry_timeout * MSEC_PER_SEC);
> +		cfg->fail_at_unmount = init[i].fail_at_unmount;
>  	}
>  	return 0;
>  
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 8/9] xfs: disable specific error configurations

[Brian Foster]

On Fri, Feb 05, 2016 at 12:23:26PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Different error classes are going to need different error to be
> configured, so we don't want them all to be visible in sysfs. Add a
> configuration check into the config initialisation an lookup
> code to determine if the default should be used for a specific
> error. If so, the sysfs entry is not created, and on lookup the
> default config is returned.
> 
> Add ENOMEM at this point to exercise this code, as it will be used
> later when adding a kmem error failure class.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_mount.h |  1 +
>  fs/xfs/xfs_sysfs.c | 22 ++++++++++++++++++----
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index c05b500..0ff14a90 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -53,6 +53,7 @@ enum {
>  	XFS_ERR_EIO,
>  	XFS_ERR_ENOSPC,
>  	XFS_ERR_ENODEV,
> +	XFS_ERR_ENOMEM,
>  	XFS_ERR_ERRNO_MAX,
>  };
>  enum {
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index d48dc46..c984c3e 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
> @@ -522,6 +522,9 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = {
>  	{ .name = "ENODEV",
>  	  .fail_speed = XFS_ERR_FAIL_FAST,
>  	},
> +	{ .name = "ENOMEM",
> +	  .fail_speed = XFS_ERR_FAIL_DEFAULT,
> +	},
>  };
>  
>  static int
> @@ -545,12 +548,17 @@ xfs_error_sysfs_init_class(
>  
>  	for (i = 0; i < XFS_ERR_ERRNO_MAX; i++) {
>  		cfg = &mp->m_error_cfg[class][i];
> +
> +		/* skip errors that are not configurable for this class */
> +		cfg->fail_speed = init[i].fail_speed;
> +		if (cfg->fail_speed == XFS_ERR_FAIL_DEFAULT)
> +			continue;
> +

It seems like this confuses a default configuration for an error with a
"tunable" or "configurable" flag (or otherwise has confused me ;). E.g.,
what happens if we want to alter ENOMEM behavior from the "default"
behavior, retain the current definition in the "Default" entry, but
still not expose the ENOMEM configuration to the user?

Brian

>  		error = xfs_sysfs_init(&cfg->kobj, &xfs_error_cfg_ktype,
>  					parent_kobj, init[i].name);
>  		if (error)
>  			goto out_error;
>  
> -		cfg->fail_speed = init[i].fail_speed;
>  		cfg->max_retries = init[i].max_retries;
>  		cfg->retry_timeout = msecs_to_jiffies(
>  					init[i].retry_timeout * MSEC_PER_SEC);
> @@ -605,7 +613,8 @@ xfs_error_sysfs_del(
>  		for (j = 0; j < XFS_ERR_ERRNO_MAX; j++) {
>  			cfg = &mp->m_error_cfg[i][j];
>  
> -			xfs_sysfs_del(&cfg->kobj);
> +			if (cfg->fail_speed != XFS_ERR_FAIL_DEFAULT)
> +				xfs_sysfs_del(&cfg->kobj);
>  		}
>  	}
>  	xfs_sysfs_del(&mp->m_error_meta_kobj);
> @@ -630,10 +639,15 @@ xfs_error_get_cfg(
>  	case ENODEV:
>  		cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENODEV];
>  		break;
> -	default:
> -		cfg = &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT];
> +	case ENOMEM:
> +		cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENOMEM];
>  		break;
> +	default:
> +		return &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT];
>  	}
>  
> +	/* The error may not be not configurable, so uses default behaviour */
> +	if (cfg->fail_speed == XFS_ERR_FAIL_DEFAULT)
> +		return &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT];
>  	return cfg;
>  }
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 6/9] xfs: add "fail at unmount" error handling configuration

[Eric Sandeen]

On 2/16/16 10:44 AM, Brian Foster wrote:
> On Fri, Feb 05, 2016 at 12:23:24PM +1100, Dave Chinner wrote:
>> > From: Dave Chinner <dchinner@redhat.com>
>> > 
>> > If we take "retry forever" literally on metadata IO errors, we can
>> > hang an unmount retries those writes forever. This is the default
>> > behaviour, unfortunately. Add a error configuration option for this
>> > behaviour and default it to "fail" so that an unmount will trigger
>> > actual errors, a shutdown and allow the unmount to succeed. It will
>> > be noisy, though, as it will log the errors and shutdown that
>> > occurs.
>> > 
>> > To do this, we need to mark the filesystem as being in the process
>> > of unmounting. Do this with a mount flag that is added at the
>> > appropriate time (i.e. before the blocking AIL sync). We also need
>> > to add this flag if mount fails after the initial phase of log
>> > recovery has been run.
>> > 
>> > The config is done by a separate boolean sysfs option rather than a
>> > new fail_speed enum, as fail_at_unmount is relevant to both
>> > XFS_ERR_FAIL_NEVER and XFS_ERR_FAIL_SLOW options.
>> > 
>> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
>> > ---
> Similar question of scope/granularity here... why would one want to set
> this option for a particular error and not any others? In other words,
> it seems more useful as a global (or per-mount) option.

I guess my question here is higher-level than that.  Why make this
(fail_at_unmount) configurable at all.  When would one *want* unmount
blocked by pending failure retries?

I guess I could imagine it as sort of a safety net, "I told it to retry
for a day, and the day's not up yet, so we shouldn't stop trying
just because I said unmount!" - but that seems a bit contrived to me.

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/9] xfs: configurable error behaviour

[Carlos Maiolino]

Hi Dave,

Here are a few comments about it, I didn't finish to review all the patches, so,
for now, it's an implementation review, other than code review.

First, I tend to agree with Brian and Eric in most of their comments.

I don't see any point in having a fail fast/slow/never, if we can configure the
number of retries, so, it just looks redundant to me.

Setting 1, -1 or N for fail fast/never/after N times, looks simpler.

Also, I don't see a point in having a fail_at_unmount option, IMHO it should
always fail at unmount, unless, if we expect the sysadmin to extend the
underlying storage device (let's say here, a thin pool), so that we can flush
all the metadata in AIL and clean unmount.

I'm sorry if I'm wrong here, but, shouldn't xfs_log_force also check for the error
configuration here? Otherwise, if, we set the configuration from fail never
to fail fast, after an unmount has been issued, unmount will be stuck waiting
for xfs_log_worker to finish metadata writeback, which will never happen
since we have no space and, it does not check for the error configuration.

I can easily reproduce this behavior trying to unmount a filesystem, then set
the configuration to fail fast after the unmount.

Unmount will be stuck at:

[<ffffffff8138ad56>] xfs_ail_push_all_sync+0xb6/0x100
[<ffffffff813750ee>] xfs_unmountfs+0x6e/0x1b0
[<ffffffff81377bf0>] xfs_fs_put_super+0x30/0x90
[<ffffffff812193da>] generic_shutdown_super+0x6a/0xf0
[<ffffffff81219747>] kill_block_super+0x27/0x70
[<ffffffff81219a83>] deactivate_locked_super+0x43/0x70
[<ffffffff81219b0c>] deactivate_super+0x5c/0x60
[<ffffffff81236baf>] cleanup_mnt+0x3f/0x90
[<ffffffff81236c42>] __cleanup_mnt+0x12/0x20
[<ffffffff810a49c3>] task_work_run+0x73/0x90
[<ffffffff81002242>] exit_to_usermode_loop+0xc2/0xd0
[<ffffffff81002d11>] syscall_return_slowpath+0xa1/0xb0
[<ffffffff818129cc>] int_ret_from_sys_call+0x25/0x8f
[<ffffffffffffffff>] 0xffffffffffffffff

and kernel will be spinning into:

kworker/3:1H-478   [003] ....   361.187791: xfs_log_force: dev 253:4 lsn 0x0
caller xfs_log_worker


or from dmesg output:

[  103.881961] XFS (dm-4): Unmounting Filesystem
[  103.882824] XFS (dm-4): xfs_do_force_shutdown(0x1) called from line 1134 of
file fs/xfs/xfs_buf_item.c.  Return address = 0xffffffff813818d7
[  103.884101] XFS (dm-4): I/O Error Detected. Shutting down filesystem
[  103.884713] XFS (dm-4): Please umount the filesystem and rectify the
problem(s)
[  121.185786] XFS (dm-4): xfs_log_force: error -5 returned.
[  151.185818] XFS (dm-4): xfs_log_force: error -5 returned.



I'll add more comments as soon as I finish to review the patches.



Btw, while testing it, I found a bug in dm-thin and direct-io code, which return
wrong error information to the filesystem, and actually prevented your patchset
to work as expected, I got a few patches to dm-thin and direct io to test, and
the patches fixed the problem I found, but I'm not sure when they will be posted
upstream. Hopefully today? :)



On Fri, Feb 05, 2016 at 12:23:18PM +1100, Dave Chinner wrote:
> Hi folks,
> 
> I need to restart the discussion and review of this patch series.
> There was some discussion of it last time, but nothing really came
> from that. I'm posting what I have in my tree right now - treat it
> as though it's an initial posting of the code because I can't recall
> what I've changed since the first posting.
> 
> What I'd like to have to for the next merge window is all the IO
> error bits sorted out. The final patch (kmem failure behaviour)
> needs more infrastructure (passing mp to every allocation) so that's
> a secondary concern right now and I've only included it to
> demonstrate how to apply this code ot a different subsystem.
> 
> Things that need to be nailed down before I can commit the series:
> 
> 	- sysfs layout
> 	- naming conventions for errors and subsystems in sysfs
> 	- how best to display/change default behaviour
> 
> Things that we can change/implement later:
> 
> 	- default behaviour
> 	- additional error classes
> 	- additional error types
> 	- additional subsystems
> 	- subsystem error handling implementation
> 	- communication with other subsystems to dynamically change
> 	  error behaviour
> 
> IOWs, what is important right now is how we present this to
> userspace, because we can't change that easily once we've decided on
> a presentation structure.
> 
> Modifying the code to classify and handle all the different error
> types is much less important, as we can change that to fix whatever
> problems we have without impacting the presentation to userspace.
> 
> There is definite need for this (e.g. handling of ENOSPC on thin
> provisioned devices), so I want to get quickly to a consensus on the
> userspace facing aspects so that we can get this ball rolling.
> 
> The biggest unsolved issue is how to change the default behaviour
> persistently. There is no infrastructure in this patch series to do
> that, but it is someting that we have to consider so that we don't
> require default behaviour to be changed after every mount of every
> filesystem on a system. My thoughts on this is we store changes to
> the defaults in xattrs on the root inode, but I'm open to ideas here
> as there's no code written for it yet. Solving this problem,
> however, is not necessary before commiting the initial code; it's
> something we can add later once we've worked out all the details.
> 
> Discuss!
> 
> -Dave.
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs