All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] xfs: more configurable error injection
@ 2017-06-23 16:35 Darrick J. Wong
  2017-06-23 16:35 ` [PATCH 1/4] xfs: make errortag a per-mountpoint structure Darrick J. Wong
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Darrick J. Wong @ 2017-06-23 16:35 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

Hi all,

This is a small patch set that exposes all the error injection tags
under /sys/fs/xfs/$dev/errortag/.  Each tag gets its own sysfs
attribute; you can enable a tag via:

# echo $num > /sys/fs/xfs/$dev/errortag/$tagname

The tag names are the same ones as xfs_io's inject command, and the
number units are the denominator of the occurrence frequency, which is
to say that they're the same units as XFS_RANDOM_*.  1 = all the time,
2 = half the time, 100 = 1% of the time, "default" = whatever the
ioctl would do, etc.

The purpose of this patch series is twofold -- first, the old ioctl
interface doesn't allow us to control how often errors actually get
injected.  The extra flexibility enables us to write regression tests
where we set up the next operation to fail, as well as tests that set a
low frequency of trouble and try to run longer tests.  Second, with
error injection knobs appearing in sysfs we might as well move them all
to the same place.  Third, now we can individually /disable/ error
injectors, whereas the ioctl interface can only turn all of them off at
once.

Note that the old ioctls are not going away; their behavior is
unchanged.

Comments and questions are, as always, welcome.

--D

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 1/4] xfs: make errortag a per-mountpoint structure
  2017-06-23 16:35 [PATCH v2 0/4] xfs: more configurable error injection Darrick J. Wong
@ 2017-06-23 16:35 ` Darrick J. Wong
  2017-06-26 11:10   ` Brian Foster
  2017-06-27  7:35   ` Carlos Maiolino
  2017-06-23 16:35 ` [PATCH 2/4] xfs: expose errortag knobs via sysfs Darrick J. Wong
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Darrick J. Wong @ 2017-06-23 16:35 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Remove the xfs_etest structure in favor of a per-mountpoint structure.
This will give us the flexibility to set as many error injection points
as we want, and later enable us to set up sysfs knobs to set the trigger
frequency as we wish.  This comes at a cost of higher memory use, but
unti we hit 1024 injection points (we're at 29) or a lot of mounts this
shouldn't be a huge issue.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_error.c |  154 +++++++++++++++++++++++++++-------------------------
 fs/xfs/xfs_error.h |   25 +++++---
 fs/xfs/xfs_ioctl.c |    4 +
 fs/xfs/xfs_mount.c |   10 +++
 fs/xfs/xfs_mount.h |    7 ++
 5 files changed, 111 insertions(+), 89 deletions(-)


diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index ed7ee4e..52f75bc 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -25,100 +25,106 @@
 
 #ifdef DEBUG
 
-int	xfs_etest[XFS_NUM_INJECT_ERROR];
-int64_t	xfs_etest_fsid[XFS_NUM_INJECT_ERROR];
-char *	xfs_etest_fsname[XFS_NUM_INJECT_ERROR];
-int	xfs_error_test_active;
+static unsigned int xfs_errortag_random_default[] = {
+	XFS_RANDOM_DEFAULT,
+	XFS_RANDOM_IFLUSH_1,
+	XFS_RANDOM_IFLUSH_2,
+	XFS_RANDOM_IFLUSH_3,
+	XFS_RANDOM_IFLUSH_4,
+	XFS_RANDOM_IFLUSH_5,
+	XFS_RANDOM_IFLUSH_6,
+	XFS_RANDOM_DA_READ_BUF,
+	XFS_RANDOM_BTREE_CHECK_LBLOCK,
+	XFS_RANDOM_BTREE_CHECK_SBLOCK,
+	XFS_RANDOM_ALLOC_READ_AGF,
+	XFS_RANDOM_IALLOC_READ_AGI,
+	XFS_RANDOM_ITOBP_INOTOBP,
+	XFS_RANDOM_IUNLINK,
+	XFS_RANDOM_IUNLINK_REMOVE,
+	XFS_RANDOM_DIR_INO_VALIDATE,
+	XFS_RANDOM_BULKSTAT_READ_CHUNK,
+	XFS_RANDOM_IODONE_IOERR,
+	XFS_RANDOM_STRATREAD_IOERR,
+	XFS_RANDOM_STRATCMPL_IOERR,
+	XFS_RANDOM_DIOWRITE_IOERR,
+	XFS_RANDOM_BMAPIFORMAT,
+	XFS_RANDOM_FREE_EXTENT,
+	XFS_RANDOM_RMAP_FINISH_ONE,
+	XFS_RANDOM_REFCOUNT_CONTINUE_UPDATE,
+	XFS_RANDOM_REFCOUNT_FINISH_ONE,
+	XFS_RANDOM_BMAP_FINISH_ONE,
+	XFS_RANDOM_AG_RESV_CRITICAL,
+};
 
 int
-xfs_error_test(int error_tag, int *fsidp, char *expression,
-	       int line, char *file, unsigned long randfactor)
+xfs_errortag_init(
+	struct xfs_mount	*mp)
 {
-	int i;
-	int64_t fsid;
+	mp->m_errortag = kmem_zalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
+			KM_SLEEP | KM_MAYFAIL);
+	if (!mp->m_errortag)
+		return -ENOMEM;
+	return 0;
+}
 
-	if (prandom_u32() % randfactor)
-		return 0;
+void
+xfs_errortag_del(
+	struct xfs_mount	*mp)
+{
+	kmem_free(mp->m_errortag);
+}
 
-	memcpy(&fsid, fsidp, sizeof(xfs_fsid_t));
+bool
+xfs_errortag_test(
+	struct xfs_mount	*mp,
+	const char		*expression,
+	const char		*file,
+	int			line,
+	unsigned int		error_tag)
+{
+	unsigned int		randfactor;
 
-	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++)  {
-		if (xfs_etest[i] == error_tag && xfs_etest_fsid[i] == fsid) {
-			xfs_warn(NULL,
-	"Injecting error (%s) at file %s, line %d, on filesystem \"%s\"",
-				expression, file, line, xfs_etest_fsname[i]);
-			return 1;
-		}
-	}
+	ASSERT(error_tag < XFS_ERRTAG_MAX);
+	randfactor = mp->m_errortag[error_tag];
+	if (!randfactor || prandom_u32() % randfactor)
+		return false;
 
-	return 0;
+	xfs_warn_ratelimited(mp,
+"Injecting error (%s) at file %s, line %d, on filesystem \"%s\"",
+			expression, file, line, mp->m_fsname);
+	return true;
 }
 
 int
-xfs_errortag_add(unsigned int error_tag, xfs_mount_t *mp)
+xfs_errortag_set(
+	struct xfs_mount	*mp,
+	unsigned int		error_tag,
+	unsigned int		tag_value)
 {
-	int i;
-	int len;
-	int64_t fsid;
-
 	if (error_tag >= XFS_ERRTAG_MAX)
 		return -EINVAL;
 
-	memcpy(&fsid, mp->m_fixedfsid, sizeof(xfs_fsid_t));
-
-	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++)  {
-		if (xfs_etest_fsid[i] == fsid && xfs_etest[i] == error_tag) {
-			xfs_warn(mp, "error tag #%d on", error_tag);
-			return 0;
-		}
-	}
-
-	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++)  {
-		if (xfs_etest[i] == 0) {
-			xfs_warn(mp, "Turned on XFS error tag #%d",
-				error_tag);
-			xfs_etest[i] = error_tag;
-			xfs_etest_fsid[i] = fsid;
-			len = strlen(mp->m_fsname);
-			xfs_etest_fsname[i] = kmem_alloc(len + 1, KM_SLEEP);
-			strcpy(xfs_etest_fsname[i], mp->m_fsname);
-			xfs_error_test_active++;
-			return 0;
-		}
-	}
-
-	xfs_warn(mp, "error tag overflow, too many turned on");
-
-	return 1;
+	mp->m_errortag[error_tag] = tag_value;
+	return 0;
 }
 
 int
-xfs_errortag_clearall(xfs_mount_t *mp, int loud)
+xfs_errortag_add(
+	struct xfs_mount	*mp,
+	unsigned int		error_tag)
 {
-	int64_t fsid;
-	int cleared = 0;
-	int i;
-
-	memcpy(&fsid, mp->m_fixedfsid, sizeof(xfs_fsid_t));
-
-
-	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++) {
-		if ((fsid == 0LL || xfs_etest_fsid[i] == fsid) &&
-		     xfs_etest[i] != 0) {
-			cleared = 1;
-			xfs_warn(mp, "Clearing XFS error tag #%d",
-				xfs_etest[i]);
-			xfs_etest[i] = 0;
-			xfs_etest_fsid[i] = 0LL;
-			kmem_free(xfs_etest_fsname[i]);
-			xfs_etest_fsname[i] = NULL;
-			xfs_error_test_active--;
-		}
-	}
+	if (error_tag >= XFS_ERRTAG_MAX)
+		return -EINVAL;
 
-	if (loud || cleared)
-		xfs_warn(mp, "Cleared all XFS error tags for filesystem");
+	return xfs_errortag_set(mp, error_tag,
+			xfs_errortag_random_default[error_tag]);
+}
 
+int
+xfs_errortag_clearall(
+	struct xfs_mount	*mp)
+{
+	memset(mp->m_errortag, 0, sizeof(unsigned int) * XFS_ERRTAG_MAX);
 	return 0;
 }
 #endif /* DEBUG */
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index 05f8666..b4316d3 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -131,21 +131,24 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
 #define XFS_RANDOM_AG_RESV_CRITICAL			4
 
 #ifdef DEBUG
-extern int xfs_error_test_active;
-extern int xfs_error_test(int, int *, char *, int, char *, unsigned long);
-
-#define	XFS_NUM_INJECT_ERROR				10
+extern int xfs_errortag_init(struct xfs_mount *mp);
+extern void xfs_errortag_del(struct xfs_mount *mp);
+extern bool xfs_errortag_test(struct xfs_mount *mp, const char *expression,
+		const char *file, int line, unsigned int error_tag);
 #define XFS_TEST_ERROR(expr, mp, tag, rf)		\
-	((expr) || (xfs_error_test_active && \
-	 xfs_error_test((tag), (mp)->m_fixedfsid, "expr", __LINE__, __FILE__, \
-			(rf))))
+	((expr) || xfs_errortag_test((mp), #expr, __FILE__, __LINE__, (tag)))
 
-extern int xfs_errortag_add(unsigned int error_tag, struct xfs_mount *mp);
-extern int xfs_errortag_clearall(struct xfs_mount *mp, int loud);
+extern int xfs_errortag_set(struct xfs_mount *mp, unsigned int error_tag,
+		unsigned int tag_value);
+extern int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
+extern int xfs_errortag_clearall(struct xfs_mount *mp);
 #else
+#define xfs_errortag_init(mp)			(0)
+#define xfs_errortag_del(mp)
 #define XFS_TEST_ERROR(expr, mp, tag, rf)	(expr)
-#define xfs_errortag_add(tag, mp)		(ENOSYS)
-#define xfs_errortag_clearall(mp, loud)		(ENOSYS)
+#define xfs_errortag_set(mp, tag, val)		(ENOSYS)
+#define xfs_errortag_add(mp, tag)		(ENOSYS)
+#define xfs_errortag_clearall(mp)		(ENOSYS)
 #endif /* DEBUG */
 
 /*
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 8ffe4ea..9c0c7a9 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -2037,14 +2037,14 @@ xfs_file_ioctl(
 		if (copy_from_user(&in, arg, sizeof(in)))
 			return -EFAULT;
 
-		return xfs_errortag_add(in.errtag, mp);
+		return xfs_errortag_add(mp, in.errtag);
 	}
 
 	case XFS_IOC_ERROR_CLEARALL:
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
 
-		return xfs_errortag_clearall(mp, 1);
+		return xfs_errortag_clearall(mp);
 
 	case XFS_IOC_FREE_EOFBLOCKS: {
 		struct xfs_fs_eofblocks eofb;
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index cc6789d..1a98c35 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -720,10 +720,13 @@ xfs_mountfs(
 	if (error)
 		goto out_del_stats;
 
+	error = xfs_errortag_init(mp);
+	if (error)
+		goto out_remove_error_sysfs;
 
 	error = xfs_uuid_mount(mp);
 	if (error)
-		goto out_remove_error_sysfs;
+		goto out_remove_errortag;
 
 	/*
 	 * Set the minimum read and write sizes
@@ -1042,6 +1045,8 @@ xfs_mountfs(
 	xfs_da_unmount(mp);
  out_remove_uuid:
 	xfs_uuid_unmount(mp);
+ out_remove_errortag:
+	xfs_errortag_del(mp);
  out_remove_error_sysfs:
 	xfs_error_sysfs_del(mp);
  out_del_stats:
@@ -1145,10 +1150,11 @@ xfs_unmountfs(
 	xfs_uuid_unmount(mp);
 
 #if defined(DEBUG)
-	xfs_errortag_clearall(mp, 0);
+	xfs_errortag_clearall(mp);
 #endif
 	xfs_free_perag(mp);
 
+	xfs_errortag_del(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 305d953..e002ac5 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -199,6 +199,13 @@ typedef struct xfs_mount {
 	bool			m_fail_unmount;
 #ifdef DEBUG
 	/*
+	 * Frequency with which errors are injected.  Replaces xfs_etest; the
+	 * value stored in here is the inverse of the frequency with which the
+	 * error triggers.  1 = always, 2 = half the time, etc.
+	 */
+	unsigned int		*m_errortag;
+
+	/*
 	 * DEBUG mode instrumentation to test and/or trigger delayed allocation
 	 * block killing in the event of failed writes. When enabled, all
 	 * buffered writes are silenty dropped and handled as if they failed.


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 2/4] xfs: expose errortag knobs via sysfs
  2017-06-23 16:35 [PATCH v2 0/4] xfs: more configurable error injection Darrick J. Wong
  2017-06-23 16:35 ` [PATCH 1/4] xfs: make errortag a per-mountpoint structure Darrick J. Wong
@ 2017-06-23 16:35 ` Darrick J. Wong
  2017-06-26 11:10   ` Brian Foster
  2017-06-27 10:26   ` Carlos Maiolino
  2017-06-23 16:35 ` [PATCH 3/4] xfs: remove unneeded parameter from XFS_TEST_ERROR Darrick J. Wong
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Darrick J. Wong @ 2017-06-23 16:35 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Creates a /sys/fs/xfs/$dev/errortag/ directory to control the errortag
values directly.  This enables us to control the randomness values,
rather than having to accept the defaults.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_error.c |  156 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_error.h |    1 
 fs/xfs/xfs_mount.h |    1 
 3 files changed, 157 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 52f75bc..e2278af6 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -22,6 +22,7 @@
 #include "xfs_trans_resv.h"
 #include "xfs_mount.h"
 #include "xfs_error.h"
+#include "xfs_sysfs.h"
 
 #ifdef DEBUG
 
@@ -56,6 +57,145 @@ static unsigned int xfs_errortag_random_default[] = {
 	XFS_RANDOM_AG_RESV_CRITICAL,
 };
 
+struct xfs_errortag_attr {
+	struct attribute	attr;
+	unsigned int		tag;
+};
+
+static inline struct xfs_errortag_attr *
+to_attr(struct attribute *attr)
+{
+	return container_of(attr, struct xfs_errortag_attr, attr);
+}
+
+static inline struct xfs_mount *
+to_mp(struct kobject *kobject)
+{
+	struct xfs_kobj *kobj = to_kobj(kobject);
+
+	return container_of(kobj, struct xfs_mount, m_errortag_kobj);
+}
+
+STATIC ssize_t
+xfs_errortag_attr_store(
+	struct kobject		*kobject,
+	struct attribute	*attr,
+	const char		*buf,
+	size_t			count)
+{
+	struct xfs_mount	*mp = to_mp(kobject);
+	struct xfs_errortag_attr *xfs_attr = to_attr(attr);
+	int			ret;
+	unsigned int		val;
+
+	if (strcmp(buf, "default") == 0) {
+		val = xfs_errortag_random_default[xfs_attr->tag];
+	} else {
+		ret = kstrtouint(buf, 0, &val);
+		if (ret)
+			return ret;
+	}
+
+	ret = xfs_errortag_set(mp, xfs_attr->tag, val);
+	if (ret)
+		return ret;
+	return count;
+}
+
+STATIC ssize_t
+xfs_errortag_attr_show(
+	struct kobject		*kobject,
+	struct attribute	*attr,
+	char			*buf)
+{
+	struct xfs_mount	*mp = to_mp(kobject);
+	struct xfs_errortag_attr *xfs_attr = to_attr(attr);
+
+	return snprintf(buf, PAGE_SIZE, "%u\n",
+			xfs_errortag_get(mp, xfs_attr->tag));
+}
+
+static const struct sysfs_ops xfs_errortag_sysfs_ops = {
+	.show = xfs_errortag_attr_show,
+	.store = xfs_errortag_attr_store,
+};
+
+#define XFS_ERRORTAG_ATTR_RW(_name, _tag) \
+static struct xfs_errortag_attr xfs_errortag_attr_##_name = {		\
+	.attr = {.name = __stringify(_name),				\
+		 .mode = VERIFY_OCTAL_PERMISSIONS(S_IWUSR | S_IRUGO) },	\
+	.tag	= (_tag),						\
+}
+
+#define XFS_ERRORTAG_ATTR_LIST(_name) &xfs_errortag_attr_##_name.attr
+
+XFS_ERRORTAG_ATTR_RW(noerror,		XFS_ERRTAG_NOERROR);
+XFS_ERRORTAG_ATTR_RW(iflush1,		XFS_ERRTAG_IFLUSH_1);
+XFS_ERRORTAG_ATTR_RW(iflush2,		XFS_ERRTAG_IFLUSH_2);
+XFS_ERRORTAG_ATTR_RW(iflush3,		XFS_ERRTAG_IFLUSH_3);
+XFS_ERRORTAG_ATTR_RW(iflush4,		XFS_ERRTAG_IFLUSH_4);
+XFS_ERRORTAG_ATTR_RW(iflush5,		XFS_ERRTAG_IFLUSH_5);
+XFS_ERRORTAG_ATTR_RW(iflush6,		XFS_ERRTAG_IFLUSH_6);
+XFS_ERRORTAG_ATTR_RW(dareadbuf,		XFS_ERRTAG_DA_READ_BUF);
+XFS_ERRORTAG_ATTR_RW(btree_chk_lblk,	XFS_ERRTAG_BTREE_CHECK_LBLOCK);
+XFS_ERRORTAG_ATTR_RW(btree_chk_sblk,	XFS_ERRTAG_BTREE_CHECK_SBLOCK);
+XFS_ERRORTAG_ATTR_RW(readagf,		XFS_ERRTAG_ALLOC_READ_AGF);
+XFS_ERRORTAG_ATTR_RW(readagi,		XFS_ERRTAG_IALLOC_READ_AGI);
+XFS_ERRORTAG_ATTR_RW(itobp,		XFS_ERRTAG_ITOBP_INOTOBP);
+XFS_ERRORTAG_ATTR_RW(iunlink,		XFS_ERRTAG_IUNLINK);
+XFS_ERRORTAG_ATTR_RW(iunlinkrm,		XFS_ERRTAG_IUNLINK_REMOVE);
+XFS_ERRORTAG_ATTR_RW(dirinovalid,	XFS_ERRTAG_DIR_INO_VALIDATE);
+XFS_ERRORTAG_ATTR_RW(bulkstat,		XFS_ERRTAG_BULKSTAT_READ_CHUNK);
+XFS_ERRORTAG_ATTR_RW(logiodone,		XFS_ERRTAG_IODONE_IOERR);
+XFS_ERRORTAG_ATTR_RW(stratread,		XFS_ERRTAG_STRATREAD_IOERR);
+XFS_ERRORTAG_ATTR_RW(stratcmpl,		XFS_ERRTAG_STRATCMPL_IOERR);
+XFS_ERRORTAG_ATTR_RW(diowrite,		XFS_ERRTAG_DIOWRITE_IOERR);
+XFS_ERRORTAG_ATTR_RW(bmapifmt,		XFS_ERRTAG_BMAPIFORMAT);
+XFS_ERRORTAG_ATTR_RW(free_extent,	XFS_ERRTAG_FREE_EXTENT);
+XFS_ERRORTAG_ATTR_RW(rmap_finish_one,	XFS_ERRTAG_RMAP_FINISH_ONE);
+XFS_ERRORTAG_ATTR_RW(refcount_continue_update,	XFS_ERRTAG_REFCOUNT_CONTINUE_UPDATE);
+XFS_ERRORTAG_ATTR_RW(refcount_finish_one,	XFS_ERRTAG_REFCOUNT_FINISH_ONE);
+XFS_ERRORTAG_ATTR_RW(bmap_finish_one,	XFS_ERRTAG_BMAP_FINISH_ONE);
+XFS_ERRORTAG_ATTR_RW(ag_resv_critical,	XFS_ERRTAG_AG_RESV_CRITICAL);
+
+static struct attribute *xfs_errortag_attrs[] = {
+	XFS_ERRORTAG_ATTR_LIST(noerror),
+	XFS_ERRORTAG_ATTR_LIST(iflush1),
+	XFS_ERRORTAG_ATTR_LIST(iflush2),
+	XFS_ERRORTAG_ATTR_LIST(iflush3),
+	XFS_ERRORTAG_ATTR_LIST(iflush4),
+	XFS_ERRORTAG_ATTR_LIST(iflush5),
+	XFS_ERRORTAG_ATTR_LIST(iflush6),
+	XFS_ERRORTAG_ATTR_LIST(dareadbuf),
+	XFS_ERRORTAG_ATTR_LIST(btree_chk_lblk),
+	XFS_ERRORTAG_ATTR_LIST(btree_chk_sblk),
+	XFS_ERRORTAG_ATTR_LIST(readagf),
+	XFS_ERRORTAG_ATTR_LIST(readagi),
+	XFS_ERRORTAG_ATTR_LIST(itobp),
+	XFS_ERRORTAG_ATTR_LIST(iunlink),
+	XFS_ERRORTAG_ATTR_LIST(iunlinkrm),
+	XFS_ERRORTAG_ATTR_LIST(dirinovalid),
+	XFS_ERRORTAG_ATTR_LIST(bulkstat),
+	XFS_ERRORTAG_ATTR_LIST(logiodone),
+	XFS_ERRORTAG_ATTR_LIST(stratread),
+	XFS_ERRORTAG_ATTR_LIST(stratcmpl),
+	XFS_ERRORTAG_ATTR_LIST(diowrite),
+	XFS_ERRORTAG_ATTR_LIST(bmapifmt),
+	XFS_ERRORTAG_ATTR_LIST(free_extent),
+	XFS_ERRORTAG_ATTR_LIST(rmap_finish_one),
+	XFS_ERRORTAG_ATTR_LIST(refcount_continue_update),
+	XFS_ERRORTAG_ATTR_LIST(refcount_finish_one),
+	XFS_ERRORTAG_ATTR_LIST(bmap_finish_one),
+	XFS_ERRORTAG_ATTR_LIST(ag_resv_critical),
+	NULL,
+};
+
+struct kobj_type xfs_errortag_ktype = {
+	.release = xfs_sysfs_release,
+	.sysfs_ops = &xfs_errortag_sysfs_ops,
+	.default_attrs = xfs_errortag_attrs,
+};
+
 int
 xfs_errortag_init(
 	struct xfs_mount	*mp)
@@ -64,13 +204,16 @@ xfs_errortag_init(
 			KM_SLEEP | KM_MAYFAIL);
 	if (!mp->m_errortag)
 		return -ENOMEM;
-	return 0;
+
+	return xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
+			       &mp->m_kobj, "errortag");
 }
 
 void
 xfs_errortag_del(
 	struct xfs_mount	*mp)
 {
+	xfs_sysfs_del(&mp->m_errortag_kobj);
 	kmem_free(mp->m_errortag);
 }
 
@@ -96,6 +239,17 @@ xfs_errortag_test(
 }
 
 int
+xfs_errortag_get(
+	struct xfs_mount	*mp,
+	unsigned int		error_tag)
+{
+	if (error_tag >= XFS_ERRTAG_MAX)
+		return -EINVAL;
+
+	return mp->m_errortag[error_tag];
+}
+
+int
 xfs_errortag_set(
 	struct xfs_mount	*mp,
 	unsigned int		error_tag,
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index b4316d3..8915bde 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -138,6 +138,7 @@ extern bool xfs_errortag_test(struct xfs_mount *mp, const char *expression,
 #define XFS_TEST_ERROR(expr, mp, tag, rf)		\
 	((expr) || xfs_errortag_test((mp), #expr, __FILE__, __LINE__, (tag)))
 
+extern int xfs_errortag_get(struct xfs_mount *mp, unsigned int error_tag);
 extern int xfs_errortag_set(struct xfs_mount *mp, unsigned int error_tag,
 		unsigned int tag_value);
 extern int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index e002ac5..931e9fc 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -204,6 +204,7 @@ typedef struct xfs_mount {
 	 * error triggers.  1 = always, 2 = half the time, etc.
 	 */
 	unsigned int		*m_errortag;
+	struct xfs_kobj		m_errortag_kobj;
 
 	/*
 	 * DEBUG mode instrumentation to test and/or trigger delayed allocation


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 3/4] xfs: remove unneeded parameter from XFS_TEST_ERROR
  2017-06-23 16:35 [PATCH v2 0/4] xfs: more configurable error injection Darrick J. Wong
  2017-06-23 16:35 ` [PATCH 1/4] xfs: make errortag a per-mountpoint structure Darrick J. Wong
  2017-06-23 16:35 ` [PATCH 2/4] xfs: expose errortag knobs via sysfs Darrick J. Wong
@ 2017-06-23 16:35 ` Darrick J. Wong
  2017-06-27 10:33   ` Carlos Maiolino
  2017-06-23 16:35 ` [PATCH 4/4] xfs: convert drop_writes to use the errortag mechanism Darrick J. Wong
  2017-06-27 12:35 ` [PATCH] xfs: replace log_badcrc_factor knob with error injection tag Brian Foster
  4 siblings, 1 reply; 15+ messages in thread
From: Darrick J. Wong @ 2017-06-23 16:35 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, Brian Foster

From: Darrick J. Wong <darrick.wong@oracle.com>

Since we moved the injected error frequency controls to the mountpoint,
we can get rid of the last argument to XFS_TEST_ERROR.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ag_resv.c   |    3 +--
 fs/xfs/libxfs/xfs_alloc.c     |    6 ++----
 fs/xfs/libxfs/xfs_bmap.c      |   13 ++++++-------
 fs/xfs/libxfs/xfs_btree.c     |    6 ++----
 fs/xfs/libxfs/xfs_dir2.c      |    3 +--
 fs/xfs/libxfs/xfs_ialloc.c    |    3 +--
 fs/xfs/libxfs/xfs_inode_buf.c |    3 +--
 fs/xfs/libxfs/xfs_refcount.c  |    6 ++----
 fs/xfs/libxfs/xfs_rmap.c      |    3 +--
 fs/xfs/xfs_error.h            |    4 ++--
 fs/xfs/xfs_inode.c            |   11 +++++------
 fs/xfs/xfs_iomap.c            |    2 +-
 fs/xfs/xfs_log.c              |    3 +--
 13 files changed, 26 insertions(+), 40 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c
index 33db69b..b008ff3 100644
--- a/fs/xfs/libxfs/xfs_ag_resv.c
+++ b/fs/xfs/libxfs/xfs_ag_resv.c
@@ -111,8 +111,7 @@ xfs_ag_resv_critical(
 
 	/* Critically low if less than 10% or max btree height remains. */
 	return XFS_TEST_ERROR(avail < orig / 10 || avail < XFS_BTREE_MAXLEVELS,
-			pag->pag_mount, XFS_ERRTAG_AG_RESV_CRITICAL,
-			XFS_RANDOM_AG_RESV_CRITICAL);
+			pag->pag_mount, XFS_ERRTAG_AG_RESV_CRITICAL);
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index fefa8da..744dcae 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -2454,8 +2454,7 @@ xfs_agf_read_verify(
 	    !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF))
 		xfs_buf_ioerror(bp, -EFSBADCRC);
 	else if (XFS_TEST_ERROR(!xfs_agf_verify(mp, bp), mp,
-				XFS_ERRTAG_ALLOC_READ_AGF,
-				XFS_RANDOM_ALLOC_READ_AGF))
+				XFS_ERRTAG_ALLOC_READ_AGF))
 		xfs_buf_ioerror(bp, -EFSCORRUPTED);
 
 	if (bp->b_error)
@@ -2842,8 +2841,7 @@ xfs_free_extent(
 	ASSERT(type != XFS_AG_RESV_AGFL);
 
 	if (XFS_TEST_ERROR(false, mp,
-			XFS_ERRTAG_FREE_EXTENT,
-			XFS_RANDOM_FREE_EXTENT))
+			XFS_ERRTAG_FREE_EXTENT))
 		return -EIO;
 
 	error = xfs_free_extent_fix_freelist(tp, agno, &agbp);
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 19480ed..0a98807 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3992,7 +3992,7 @@ xfs_bmapi_read(
 	if (unlikely(XFS_TEST_ERROR(
 	    (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
 	     XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
-	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
+	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
 		XFS_ERROR_REPORT("xfs_bmapi_read", XFS_ERRLEVEL_LOW, mp);
 		return -EFSCORRUPTED;
 	}
@@ -4473,7 +4473,7 @@ xfs_bmapi_write(
 	if (unlikely(XFS_TEST_ERROR(
 	    (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
 	     XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
-	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
+	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
 		XFS_ERROR_REPORT("xfs_bmapi_write", XFS_ERRLEVEL_LOW, mp);
 		return -EFSCORRUPTED;
 	}
@@ -4694,7 +4694,7 @@ xfs_bmapi_remap(
 	if (unlikely(XFS_TEST_ERROR(
 	    (XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_EXTENTS &&
 	     XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_BTREE),
-	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
+	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
 		XFS_ERROR_REPORT("xfs_bmapi_remap", XFS_ERRLEVEL_LOW, mp);
 		return -EFSCORRUPTED;
 	}
@@ -6098,7 +6098,7 @@ xfs_bmap_shift_extents(
 	if (unlikely(XFS_TEST_ERROR(
 	    (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
 	     XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
-	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
+	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
 		XFS_ERROR_REPORT("xfs_bmap_shift_extents",
 				 XFS_ERRLEVEL_LOW, mp);
 		return -EFSCORRUPTED;
@@ -6250,7 +6250,7 @@ xfs_bmap_split_extent_at(
 	if (unlikely(XFS_TEST_ERROR(
 	    (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
 	     XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
-	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
+	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
 		XFS_ERROR_REPORT("xfs_bmap_split_extent_at",
 				 XFS_ERRLEVEL_LOW, mp);
 		return -EFSCORRUPTED;
@@ -6508,8 +6508,7 @@ xfs_bmap_finish_one(
 		return -EFSCORRUPTED;
 
 	if (XFS_TEST_ERROR(false, tp->t_mountp,
-			XFS_ERRTAG_BMAP_FINISH_ONE,
-			XFS_RANDOM_BMAP_FINISH_ONE))
+			XFS_ERRTAG_BMAP_FINISH_ONE))
 		return -EIO;
 
 	switch (type) {
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 2f8075a..4da85ff 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -101,8 +101,7 @@ xfs_btree_check_lblock(
 			be64_to_cpu(block->bb_u.l.bb_rightsib)));
 
 	if (unlikely(XFS_TEST_ERROR(!lblock_ok, mp,
-			XFS_ERRTAG_BTREE_CHECK_LBLOCK,
-			XFS_RANDOM_BTREE_CHECK_LBLOCK))) {
+			XFS_ERRTAG_BTREE_CHECK_LBLOCK))) {
 		if (bp)
 			trace_xfs_btree_corrupt(bp, _RET_IP_);
 		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
@@ -153,8 +152,7 @@ xfs_btree_check_sblock(
 		block->bb_u.s.bb_rightsib;
 
 	if (unlikely(XFS_TEST_ERROR(!sblock_ok, mp,
-			XFS_ERRTAG_BTREE_CHECK_SBLOCK,
-			XFS_RANDOM_BTREE_CHECK_SBLOCK))) {
+			XFS_ERRTAG_BTREE_CHECK_SBLOCK))) {
 		if (bp)
 			trace_xfs_btree_corrupt(bp, _RET_IP_);
 		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index 2f389d36..ccf9783 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -218,8 +218,7 @@ xfs_dir_ino_validate(
 		agblkno != 0 &&
 		ioff < (1 << mp->m_sb.sb_inopblog) &&
 		XFS_AGINO_TO_INO(mp, agno, agino) == ino;
-	if (unlikely(XFS_TEST_ERROR(!ino_ok, mp, XFS_ERRTAG_DIR_INO_VALIDATE,
-			XFS_RANDOM_DIR_INO_VALIDATE))) {
+	if (unlikely(XFS_TEST_ERROR(!ino_ok, mp, XFS_ERRTAG_DIR_INO_VALIDATE))) {
 		xfs_warn(mp, "Invalid inode number 0x%Lx",
 				(unsigned long long) ino);
 		XFS_ERROR_REPORT("xfs_dir_ino_validate", XFS_ERRLEVEL_LOW, mp);
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index c514fe9..ffd5a15 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -2552,8 +2552,7 @@ xfs_agi_read_verify(
 	    !xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF))
 		xfs_buf_ioerror(bp, -EFSBADCRC);
 	else if (XFS_TEST_ERROR(!xfs_agi_verify(bp), mp,
-				XFS_ERRTAG_IALLOC_READ_AGI,
-				XFS_RANDOM_IALLOC_READ_AGI))
+				XFS_ERRTAG_IALLOC_READ_AGI))
 		xfs_buf_ioerror(bp, -EFSCORRUPTED);
 
 	if (bp->b_error)
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 0c970cf..378f8fb 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -105,8 +105,7 @@ xfs_inode_buf_verify(
 		di_ok = dip->di_magic == cpu_to_be16(XFS_DINODE_MAGIC) &&
 			xfs_dinode_good_version(mp, dip->di_version);
 		if (unlikely(XFS_TEST_ERROR(!di_ok, mp,
-						XFS_ERRTAG_ITOBP_INOTOBP,
-						XFS_RANDOM_ITOBP_INOTOBP))) {
+						XFS_ERRTAG_ITOBP_INOTOBP))) {
 			if (readahead) {
 				bp->b_flags &= ~XBF_DONE;
 				xfs_buf_ioerror(bp, -EIO);
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index e170161..900ea23 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -805,8 +805,7 @@ xfs_refcount_still_have_space(
 	 */
 	if (cur->bc_private.a.priv.refc.nr_ops > 2 &&
 	    XFS_TEST_ERROR(false, cur->bc_mp,
-			XFS_ERRTAG_REFCOUNT_CONTINUE_UPDATE,
-			XFS_RANDOM_REFCOUNT_CONTINUE_UPDATE))
+			XFS_ERRTAG_REFCOUNT_CONTINUE_UPDATE))
 		return false;
 
 	if (cur->bc_private.a.priv.refc.nr_ops == 0)
@@ -1068,8 +1067,7 @@ xfs_refcount_finish_one(
 			blockcount);
 
 	if (XFS_TEST_ERROR(false, mp,
-			XFS_ERRTAG_REFCOUNT_FINISH_ONE,
-			XFS_RANDOM_REFCOUNT_FINISH_ONE))
+			XFS_ERRTAG_REFCOUNT_FINISH_ONE))
 		return -EIO;
 
 	/*
diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
index eda275b..55c88a7 100644
--- a/fs/xfs/libxfs/xfs_rmap.c
+++ b/fs/xfs/libxfs/xfs_rmap.c
@@ -2087,8 +2087,7 @@ xfs_rmap_finish_one(
 			startoff, blockcount, state);
 
 	if (XFS_TEST_ERROR(false, mp,
-			XFS_ERRTAG_RMAP_FINISH_ONE,
-			XFS_RANDOM_RMAP_FINISH_ONE))
+			XFS_ERRTAG_RMAP_FINISH_ONE))
 		return -EIO;
 
 	/*
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index 8915bde..ae8935b 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -135,7 +135,7 @@ extern int xfs_errortag_init(struct xfs_mount *mp);
 extern void xfs_errortag_del(struct xfs_mount *mp);
 extern bool xfs_errortag_test(struct xfs_mount *mp, const char *expression,
 		const char *file, int line, unsigned int error_tag);
-#define XFS_TEST_ERROR(expr, mp, tag, rf)		\
+#define XFS_TEST_ERROR(expr, mp, tag)		\
 	((expr) || xfs_errortag_test((mp), #expr, __FILE__, __LINE__, (tag)))
 
 extern int xfs_errortag_get(struct xfs_mount *mp, unsigned int error_tag);
@@ -146,7 +146,7 @@ extern int xfs_errortag_clearall(struct xfs_mount *mp);
 #else
 #define xfs_errortag_init(mp)			(0)
 #define xfs_errortag_del(mp)
-#define XFS_TEST_ERROR(expr, mp, tag, rf)	(expr)
+#define XFS_TEST_ERROR(expr, mp, tag)		(expr)
 #define xfs_errortag_set(mp, tag, val)		(ENOSYS)
 #define xfs_errortag_add(mp, tag)		(ENOSYS)
 #define xfs_errortag_clearall(mp)		(ENOSYS)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index ffbfe7d..67bd563 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -3489,7 +3489,7 @@ xfs_iflush_int(
 	dip = xfs_buf_offset(bp, ip->i_imap.im_boffset);
 
 	if (XFS_TEST_ERROR(dip->di_magic != cpu_to_be16(XFS_DINODE_MAGIC),
-			       mp, XFS_ERRTAG_IFLUSH_1, XFS_RANDOM_IFLUSH_1)) {
+			       mp, XFS_ERRTAG_IFLUSH_1)) {
 		xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
 			"%s: Bad inode %Lu magic number 0x%x, ptr 0x%p",
 			__func__, ip->i_ino, be16_to_cpu(dip->di_magic), dip);
@@ -3499,7 +3499,7 @@ xfs_iflush_int(
 		if (XFS_TEST_ERROR(
 		    (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS) &&
 		    (ip->i_d.di_format != XFS_DINODE_FMT_BTREE),
-		    mp, XFS_ERRTAG_IFLUSH_3, XFS_RANDOM_IFLUSH_3)) {
+		    mp, XFS_ERRTAG_IFLUSH_3)) {
 			xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
 				"%s: Bad regular inode %Lu, ptr 0x%p",
 				__func__, ip->i_ino, ip);
@@ -3510,7 +3510,7 @@ xfs_iflush_int(
 		    (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS) &&
 		    (ip->i_d.di_format != XFS_DINODE_FMT_BTREE) &&
 		    (ip->i_d.di_format != XFS_DINODE_FMT_LOCAL),
-		    mp, XFS_ERRTAG_IFLUSH_4, XFS_RANDOM_IFLUSH_4)) {
+		    mp, XFS_ERRTAG_IFLUSH_4)) {
 			xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
 				"%s: Bad directory inode %Lu, ptr 0x%p",
 				__func__, ip->i_ino, ip);
@@ -3518,8 +3518,7 @@ xfs_iflush_int(
 		}
 	}
 	if (XFS_TEST_ERROR(ip->i_d.di_nextents + ip->i_d.di_anextents >
-				ip->i_d.di_nblocks, mp, XFS_ERRTAG_IFLUSH_5,
-				XFS_RANDOM_IFLUSH_5)) {
+				ip->i_d.di_nblocks, mp, XFS_ERRTAG_IFLUSH_5)) {
 		xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
 			"%s: detected corrupt incore inode %Lu, "
 			"total extents = %d, nblocks = %Ld, ptr 0x%p",
@@ -3529,7 +3528,7 @@ xfs_iflush_int(
 		goto corrupt_out;
 	}
 	if (XFS_TEST_ERROR(ip->i_d.di_forkoff > mp->m_sb.sb_inodesize,
-				mp, XFS_ERRTAG_IFLUSH_6, XFS_RANDOM_IFLUSH_6)) {
+				mp, XFS_ERRTAG_IFLUSH_6)) {
 		xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
 			"%s: bad inode %Lu, forkoff 0x%x, ptr 0x%p",
 			__func__, ip->i_ino, ip->i_d.di_forkoff, ip);
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 94e5bdf..304b79d 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -543,7 +543,7 @@ xfs_file_iomap_begin_delay(
 	if (unlikely(XFS_TEST_ERROR(
 	    (XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_EXTENTS &&
 	     XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_BTREE),
-	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
+	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
 		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
 		error = -EFSCORRUPTED;
 		goto out_unlock;
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 98b39cb..2d1112e 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1189,8 +1189,7 @@ xlog_iodone(xfs_buf_t *bp)
 	 * IOABORT state. The IOABORT state is only set in DEBUG mode to inject
 	 * CRC errors into log recovery.
 	 */
-	if (XFS_TEST_ERROR(bp->b_error, l->l_mp, XFS_ERRTAG_IODONE_IOERR,
-			   XFS_RANDOM_IODONE_IOERR) ||
+	if (XFS_TEST_ERROR(bp->b_error, l->l_mp, XFS_ERRTAG_IODONE_IOERR) ||
 	    iclog->ic_state & XLOG_STATE_IOABORT) {
 		if (iclog->ic_state & XLOG_STATE_IOABORT)
 			iclog->ic_state &= ~XLOG_STATE_IOABORT;


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 4/4] xfs: convert drop_writes to use the errortag mechanism
  2017-06-23 16:35 [PATCH v2 0/4] xfs: more configurable error injection Darrick J. Wong
                   ` (2 preceding siblings ...)
  2017-06-23 16:35 ` [PATCH 3/4] xfs: remove unneeded parameter from XFS_TEST_ERROR Darrick J. Wong
@ 2017-06-23 16:35 ` Darrick J. Wong
  2017-06-27 10:51   ` Carlos Maiolino
  2017-06-27 12:35 ` [PATCH] xfs: replace log_badcrc_factor knob with error injection tag Brian Foster
  4 siblings, 1 reply; 15+ messages in thread
From: Darrick J. Wong @ 2017-06-23 16:35 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, Brian Foster

From: Darrick J. Wong <darrick.wong@oracle.com>

We now have enhanced error injection that can control the frequency
with which errors happen, so convert drop_writes to use this.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_error.c |    3 +++
 fs/xfs/xfs_error.h |   14 +++++++++++++-
 fs/xfs/xfs_iomap.c |    2 +-
 fs/xfs/xfs_mount.h |   24 ------------------------
 fs/xfs/xfs_sysfs.c |   42 ------------------------------------------
 5 files changed, 17 insertions(+), 68 deletions(-)


diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index e2278af6..a2f23d2 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -55,6 +55,7 @@ static unsigned int xfs_errortag_random_default[] = {
 	XFS_RANDOM_REFCOUNT_FINISH_ONE,
 	XFS_RANDOM_BMAP_FINISH_ONE,
 	XFS_RANDOM_AG_RESV_CRITICAL,
+	XFS_RANDOM_DROP_WRITES,
 };
 
 struct xfs_errortag_attr {
@@ -157,6 +158,7 @@ XFS_ERRORTAG_ATTR_RW(refcount_continue_update,	XFS_ERRTAG_REFCOUNT_CONTINUE_UPDA
 XFS_ERRORTAG_ATTR_RW(refcount_finish_one,	XFS_ERRTAG_REFCOUNT_FINISH_ONE);
 XFS_ERRORTAG_ATTR_RW(bmap_finish_one,	XFS_ERRTAG_BMAP_FINISH_ONE);
 XFS_ERRORTAG_ATTR_RW(ag_resv_critical,	XFS_ERRTAG_AG_RESV_CRITICAL);
+XFS_ERRORTAG_ATTR_RW(drop_writes,	XFS_ERRTAG_DROP_WRITES);
 
 static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(noerror),
@@ -187,6 +189,7 @@ static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(refcount_finish_one),
 	XFS_ERRORTAG_ATTR_LIST(bmap_finish_one),
 	XFS_ERRORTAG_ATTR_LIST(ag_resv_critical),
+	XFS_ERRORTAG_ATTR_LIST(drop_writes),
 	NULL,
 };
 
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index ae8935b..ad9b24a 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -96,7 +96,18 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
 #define XFS_ERRTAG_REFCOUNT_FINISH_ONE			25
 #define XFS_ERRTAG_BMAP_FINISH_ONE			26
 #define XFS_ERRTAG_AG_RESV_CRITICAL			27
-#define XFS_ERRTAG_MAX					28
+
+/*
+ * DEBUG mode instrumentation to test and/or trigger delayed allocation
+ * block killing in the event of failed writes. When enabled, all
+ * buffered writes are silenty dropped and handled as if they failed.
+ * All delalloc blocks in the range of the write (including pre-existing
+ * delalloc blocks!) are tossed as part of the write failure error
+ * handling sequence.
+ */
+#define XFS_ERRTAG_DROP_WRITES				28
+
+#define XFS_ERRTAG_MAX					29
 
 /*
  * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -129,6 +140,7 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
 #define XFS_RANDOM_REFCOUNT_FINISH_ONE			1
 #define XFS_RANDOM_BMAP_FINISH_ONE			1
 #define XFS_RANDOM_AG_RESV_CRITICAL			4
+#define XFS_RANDOM_DROP_WRITES				1
 
 #ifdef DEBUG
 extern int xfs_errortag_init(struct xfs_mount *mp);
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 304b79d..86f1a9f 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1097,7 +1097,7 @@ xfs_file_iomap_end_delalloc(
 	 * Behave as if the write failed if drop writes is enabled. Set the NEW
 	 * flag to force delalloc cleanup.
 	 */
-	if (xfs_mp_drop_writes(mp)) {
+	if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_DROP_WRITES)) {
 		iomap->flags |= IOMAP_F_NEW;
 		written = 0;
 	}
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 931e9fc..e0792d0 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -205,16 +205,6 @@ typedef struct xfs_mount {
 	 */
 	unsigned int		*m_errortag;
 	struct xfs_kobj		m_errortag_kobj;
-
-	/*
-	 * DEBUG mode instrumentation to test and/or trigger delayed allocation
-	 * block killing in the event of failed writes. When enabled, all
-	 * buffered writes are silenty dropped and handled as if they failed.
-	 * All delalloc blocks in the range of the write (including pre-existing
-	 * delalloc blocks!) are tossed as part of the write failure error
-	 * handling sequence.
-	 */
-	bool			m_drop_writes;
 #endif
 } xfs_mount_t;
 
@@ -333,20 +323,6 @@ xfs_daddr_to_agbno(struct xfs_mount *mp, xfs_daddr_t d)
 	return (xfs_agblock_t) do_div(ld, mp->m_sb.sb_agblocks);
 }
 
-#ifdef DEBUG
-static inline bool
-xfs_mp_drop_writes(struct xfs_mount *mp)
-{
-	return mp->m_drop_writes;
-}
-#else
-static inline bool
-xfs_mp_drop_writes(struct xfs_mount *mp)
-{
-	return 0;
-}
-#endif
-
 /* per-AG block reservation data structures*/
 enum xfs_ag_resv_type {
 	XFS_AG_RESV_NONE = 0,
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index ec6e0e2..56610a9 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -90,49 +90,7 @@ to_mp(struct kobject *kobject)
 	return container_of(kobj, struct xfs_mount, m_kobj);
 }
 
-#ifdef DEBUG
-
-STATIC ssize_t
-drop_writes_store(
-	struct kobject		*kobject,
-	const char		*buf,
-	size_t			count)
-{
-	struct xfs_mount	*mp = to_mp(kobject);
-	int			ret;
-	int			val;
-
-	ret = kstrtoint(buf, 0, &val);
-	if (ret)
-		return ret;
-
-	if (val == 1)
-		mp->m_drop_writes = true;
-	else if (val == 0)
-		mp->m_drop_writes = false;
-	else
-		return -EINVAL;
-
-	return count;
-}
-
-STATIC ssize_t
-drop_writes_show(
-	struct kobject		*kobject,
-	char			*buf)
-{
-	struct xfs_mount	*mp = to_mp(kobject);
-
-	return snprintf(buf, PAGE_SIZE, "%d\n", mp->m_drop_writes ? 1 : 0);
-}
-XFS_SYSFS_ATTR_RW(drop_writes);
-
-#endif /* DEBUG */
-
 static struct attribute *xfs_mp_attrs[] = {
-#ifdef DEBUG
-	ATTR_LIST(drop_writes),
-#endif
 	NULL,
 };
 


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] xfs: make errortag a per-mountpoint structure
  2017-06-23 16:35 ` [PATCH 1/4] xfs: make errortag a per-mountpoint structure Darrick J. Wong
@ 2017-06-26 11:10   ` Brian Foster
  2017-06-27  7:35   ` Carlos Maiolino
  1 sibling, 0 replies; 15+ messages in thread
From: Brian Foster @ 2017-06-26 11:10 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Fri, Jun 23, 2017 at 09:35:15AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Remove the xfs_etest structure in favor of a per-mountpoint structure.
> This will give us the flexibility to set as many error injection points
> as we want, and later enable us to set up sysfs knobs to set the trigger
> frequency as we wish.  This comes at a cost of higher memory use, but
> unti we hit 1024 injection points (we're at 29) or a lot of mounts this
> shouldn't be a huge issue.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_error.c |  154 +++++++++++++++++++++++++++-------------------------
>  fs/xfs/xfs_error.h |   25 +++++---
>  fs/xfs/xfs_ioctl.c |    4 +
>  fs/xfs/xfs_mount.c |   10 +++
>  fs/xfs/xfs_mount.h |    7 ++
>  5 files changed, 111 insertions(+), 89 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index ed7ee4e..52f75bc 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -25,100 +25,106 @@
>  
>  #ifdef DEBUG
>  
> -int	xfs_etest[XFS_NUM_INJECT_ERROR];
> -int64_t	xfs_etest_fsid[XFS_NUM_INJECT_ERROR];
> -char *	xfs_etest_fsname[XFS_NUM_INJECT_ERROR];
> -int	xfs_error_test_active;
> +static unsigned int xfs_errortag_random_default[] = {
> +	XFS_RANDOM_DEFAULT,
> +	XFS_RANDOM_IFLUSH_1,
> +	XFS_RANDOM_IFLUSH_2,
> +	XFS_RANDOM_IFLUSH_3,
> +	XFS_RANDOM_IFLUSH_4,
> +	XFS_RANDOM_IFLUSH_5,
> +	XFS_RANDOM_IFLUSH_6,
> +	XFS_RANDOM_DA_READ_BUF,
> +	XFS_RANDOM_BTREE_CHECK_LBLOCK,
> +	XFS_RANDOM_BTREE_CHECK_SBLOCK,
> +	XFS_RANDOM_ALLOC_READ_AGF,
> +	XFS_RANDOM_IALLOC_READ_AGI,
> +	XFS_RANDOM_ITOBP_INOTOBP,
> +	XFS_RANDOM_IUNLINK,
> +	XFS_RANDOM_IUNLINK_REMOVE,
> +	XFS_RANDOM_DIR_INO_VALIDATE,
> +	XFS_RANDOM_BULKSTAT_READ_CHUNK,
> +	XFS_RANDOM_IODONE_IOERR,
> +	XFS_RANDOM_STRATREAD_IOERR,
> +	XFS_RANDOM_STRATCMPL_IOERR,
> +	XFS_RANDOM_DIOWRITE_IOERR,
> +	XFS_RANDOM_BMAPIFORMAT,
> +	XFS_RANDOM_FREE_EXTENT,
> +	XFS_RANDOM_RMAP_FINISH_ONE,
> +	XFS_RANDOM_REFCOUNT_CONTINUE_UPDATE,
> +	XFS_RANDOM_REFCOUNT_FINISH_ONE,
> +	XFS_RANDOM_BMAP_FINISH_ONE,
> +	XFS_RANDOM_AG_RESV_CRITICAL,
> +};
>  
>  int
> -xfs_error_test(int error_tag, int *fsidp, char *expression,
> -	       int line, char *file, unsigned long randfactor)
> +xfs_errortag_init(
> +	struct xfs_mount	*mp)
>  {
> -	int i;
> -	int64_t fsid;
> +	mp->m_errortag = kmem_zalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
> +			KM_SLEEP | KM_MAYFAIL);
> +	if (!mp->m_errortag)
> +		return -ENOMEM;
> +	return 0;
> +}
>  
> -	if (prandom_u32() % randfactor)
> -		return 0;
> +void
> +xfs_errortag_del(
> +	struct xfs_mount	*mp)
> +{
> +	kmem_free(mp->m_errortag);
> +}
>  
> -	memcpy(&fsid, fsidp, sizeof(xfs_fsid_t));
> +bool
> +xfs_errortag_test(
> +	struct xfs_mount	*mp,
> +	const char		*expression,
> +	const char		*file,
> +	int			line,
> +	unsigned int		error_tag)
> +{
> +	unsigned int		randfactor;
>  
> -	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++)  {
> -		if (xfs_etest[i] == error_tag && xfs_etest_fsid[i] == fsid) {
> -			xfs_warn(NULL,
> -	"Injecting error (%s) at file %s, line %d, on filesystem \"%s\"",
> -				expression, file, line, xfs_etest_fsname[i]);
> -			return 1;
> -		}
> -	}
> +	ASSERT(error_tag < XFS_ERRTAG_MAX);
> +	randfactor = mp->m_errortag[error_tag];
> +	if (!randfactor || prandom_u32() % randfactor)
> +		return false;
>  
> -	return 0;
> +	xfs_warn_ratelimited(mp,
> +"Injecting error (%s) at file %s, line %d, on filesystem \"%s\"",
> +			expression, file, line, mp->m_fsname);
> +	return true;
>  }
>  
>  int
> -xfs_errortag_add(unsigned int error_tag, xfs_mount_t *mp)
> +xfs_errortag_set(
> +	struct xfs_mount	*mp,
> +	unsigned int		error_tag,
> +	unsigned int		tag_value)
>  {
> -	int i;
> -	int len;
> -	int64_t fsid;
> -
>  	if (error_tag >= XFS_ERRTAG_MAX)
>  		return -EINVAL;
>  
> -	memcpy(&fsid, mp->m_fixedfsid, sizeof(xfs_fsid_t));
> -
> -	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++)  {
> -		if (xfs_etest_fsid[i] == fsid && xfs_etest[i] == error_tag) {
> -			xfs_warn(mp, "error tag #%d on", error_tag);
> -			return 0;
> -		}
> -	}
> -
> -	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++)  {
> -		if (xfs_etest[i] == 0) {
> -			xfs_warn(mp, "Turned on XFS error tag #%d",
> -				error_tag);
> -			xfs_etest[i] = error_tag;
> -			xfs_etest_fsid[i] = fsid;
> -			len = strlen(mp->m_fsname);
> -			xfs_etest_fsname[i] = kmem_alloc(len + 1, KM_SLEEP);
> -			strcpy(xfs_etest_fsname[i], mp->m_fsname);
> -			xfs_error_test_active++;
> -			return 0;
> -		}
> -	}
> -
> -	xfs_warn(mp, "error tag overflow, too many turned on");
> -
> -	return 1;
> +	mp->m_errortag[error_tag] = tag_value;
> +	return 0;
>  }
>  
>  int
> -xfs_errortag_clearall(xfs_mount_t *mp, int loud)
> +xfs_errortag_add(
> +	struct xfs_mount	*mp,
> +	unsigned int		error_tag)
>  {
> -	int64_t fsid;
> -	int cleared = 0;
> -	int i;
> -
> -	memcpy(&fsid, mp->m_fixedfsid, sizeof(xfs_fsid_t));
> -
> -
> -	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++) {
> -		if ((fsid == 0LL || xfs_etest_fsid[i] == fsid) &&
> -		     xfs_etest[i] != 0) {
> -			cleared = 1;
> -			xfs_warn(mp, "Clearing XFS error tag #%d",
> -				xfs_etest[i]);
> -			xfs_etest[i] = 0;
> -			xfs_etest_fsid[i] = 0LL;
> -			kmem_free(xfs_etest_fsname[i]);
> -			xfs_etest_fsname[i] = NULL;
> -			xfs_error_test_active--;
> -		}
> -	}
> +	if (error_tag >= XFS_ERRTAG_MAX)
> +		return -EINVAL;
>  
> -	if (loud || cleared)
> -		xfs_warn(mp, "Cleared all XFS error tags for filesystem");
> +	return xfs_errortag_set(mp, error_tag,
> +			xfs_errortag_random_default[error_tag]);
> +}
>  
> +int
> +xfs_errortag_clearall(
> +	struct xfs_mount	*mp)
> +{
> +	memset(mp->m_errortag, 0, sizeof(unsigned int) * XFS_ERRTAG_MAX);
>  	return 0;
>  }
>  #endif /* DEBUG */
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index 05f8666..b4316d3 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -131,21 +131,24 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
>  #define XFS_RANDOM_AG_RESV_CRITICAL			4
>  
>  #ifdef DEBUG
> -extern int xfs_error_test_active;
> -extern int xfs_error_test(int, int *, char *, int, char *, unsigned long);
> -
> -#define	XFS_NUM_INJECT_ERROR				10
> +extern int xfs_errortag_init(struct xfs_mount *mp);
> +extern void xfs_errortag_del(struct xfs_mount *mp);
> +extern bool xfs_errortag_test(struct xfs_mount *mp, const char *expression,
> +		const char *file, int line, unsigned int error_tag);
>  #define XFS_TEST_ERROR(expr, mp, tag, rf)		\
> -	((expr) || (xfs_error_test_active && \
> -	 xfs_error_test((tag), (mp)->m_fixedfsid, "expr", __LINE__, __FILE__, \
> -			(rf))))
> +	((expr) || xfs_errortag_test((mp), #expr, __FILE__, __LINE__, (tag)))
>  
> -extern int xfs_errortag_add(unsigned int error_tag, struct xfs_mount *mp);
> -extern int xfs_errortag_clearall(struct xfs_mount *mp, int loud);
> +extern int xfs_errortag_set(struct xfs_mount *mp, unsigned int error_tag,
> +		unsigned int tag_value);
> +extern int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
> +extern int xfs_errortag_clearall(struct xfs_mount *mp);
>  #else
> +#define xfs_errortag_init(mp)			(0)
> +#define xfs_errortag_del(mp)
>  #define XFS_TEST_ERROR(expr, mp, tag, rf)	(expr)
> -#define xfs_errortag_add(tag, mp)		(ENOSYS)
> -#define xfs_errortag_clearall(mp, loud)		(ENOSYS)
> +#define xfs_errortag_set(mp, tag, val)		(ENOSYS)
> +#define xfs_errortag_add(mp, tag)		(ENOSYS)
> +#define xfs_errortag_clearall(mp)		(ENOSYS)
>  #endif /* DEBUG */
>  
>  /*
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 8ffe4ea..9c0c7a9 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -2037,14 +2037,14 @@ xfs_file_ioctl(
>  		if (copy_from_user(&in, arg, sizeof(in)))
>  			return -EFAULT;
>  
> -		return xfs_errortag_add(in.errtag, mp);
> +		return xfs_errortag_add(mp, in.errtag);
>  	}
>  
>  	case XFS_IOC_ERROR_CLEARALL:
>  		if (!capable(CAP_SYS_ADMIN))
>  			return -EPERM;
>  
> -		return xfs_errortag_clearall(mp, 1);
> +		return xfs_errortag_clearall(mp);
>  
>  	case XFS_IOC_FREE_EOFBLOCKS: {
>  		struct xfs_fs_eofblocks eofb;
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index cc6789d..1a98c35 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -720,10 +720,13 @@ xfs_mountfs(
>  	if (error)
>  		goto out_del_stats;
>  
> +	error = xfs_errortag_init(mp);
> +	if (error)
> +		goto out_remove_error_sysfs;
>  
>  	error = xfs_uuid_mount(mp);
>  	if (error)
> -		goto out_remove_error_sysfs;
> +		goto out_remove_errortag;
>  
>  	/*
>  	 * Set the minimum read and write sizes
> @@ -1042,6 +1045,8 @@ xfs_mountfs(
>  	xfs_da_unmount(mp);
>   out_remove_uuid:
>  	xfs_uuid_unmount(mp);
> + out_remove_errortag:
> +	xfs_errortag_del(mp);
>   out_remove_error_sysfs:
>  	xfs_error_sysfs_del(mp);
>   out_del_stats:
> @@ -1145,10 +1150,11 @@ xfs_unmountfs(
>  	xfs_uuid_unmount(mp);
>  
>  #if defined(DEBUG)
> -	xfs_errortag_clearall(mp, 0);
> +	xfs_errortag_clearall(mp);
>  #endif
>  	xfs_free_perag(mp);
>  
> +	xfs_errortag_del(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 305d953..e002ac5 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -199,6 +199,13 @@ typedef struct xfs_mount {
>  	bool			m_fail_unmount;
>  #ifdef DEBUG
>  	/*
> +	 * Frequency with which errors are injected.  Replaces xfs_etest; the
> +	 * value stored in here is the inverse of the frequency with which the
> +	 * error triggers.  1 = always, 2 = half the time, etc.
> +	 */
> +	unsigned int		*m_errortag;
> +
> +	/*
>  	 * DEBUG mode instrumentation to test and/or trigger delayed allocation
>  	 * block killing in the event of failed writes. When enabled, all
>  	 * buffered writes are silenty dropped and handled as if they failed.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] xfs: expose errortag knobs via sysfs
  2017-06-23 16:35 ` [PATCH 2/4] xfs: expose errortag knobs via sysfs Darrick J. Wong
@ 2017-06-26 11:10   ` Brian Foster
  2017-06-27 10:26   ` Carlos Maiolino
  1 sibling, 0 replies; 15+ messages in thread
From: Brian Foster @ 2017-06-26 11:10 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Fri, Jun 23, 2017 at 09:35:29AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Creates a /sys/fs/xfs/$dev/errortag/ directory to control the errortag
> values directly.  This enables us to control the randomness values,
> rather than having to accept the defaults.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_error.c |  156 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_error.h |    1 
>  fs/xfs/xfs_mount.h |    1 
>  3 files changed, 157 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 52f75bc..e2278af6 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -22,6 +22,7 @@
>  #include "xfs_trans_resv.h"
>  #include "xfs_mount.h"
>  #include "xfs_error.h"
> +#include "xfs_sysfs.h"
>  
>  #ifdef DEBUG
>  
> @@ -56,6 +57,145 @@ static unsigned int xfs_errortag_random_default[] = {
>  	XFS_RANDOM_AG_RESV_CRITICAL,
>  };
>  
> +struct xfs_errortag_attr {
> +	struct attribute	attr;
> +	unsigned int		tag;
> +};
> +
> +static inline struct xfs_errortag_attr *
> +to_attr(struct attribute *attr)
> +{
> +	return container_of(attr, struct xfs_errortag_attr, attr);
> +}
> +
> +static inline struct xfs_mount *
> +to_mp(struct kobject *kobject)
> +{
> +	struct xfs_kobj *kobj = to_kobj(kobject);
> +
> +	return container_of(kobj, struct xfs_mount, m_errortag_kobj);
> +}
> +
> +STATIC ssize_t
> +xfs_errortag_attr_store(
> +	struct kobject		*kobject,
> +	struct attribute	*attr,
> +	const char		*buf,
> +	size_t			count)
> +{
> +	struct xfs_mount	*mp = to_mp(kobject);
> +	struct xfs_errortag_attr *xfs_attr = to_attr(attr);
> +	int			ret;
> +	unsigned int		val;
> +
> +	if (strcmp(buf, "default") == 0) {
> +		val = xfs_errortag_random_default[xfs_attr->tag];
> +	} else {
> +		ret = kstrtouint(buf, 0, &val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = xfs_errortag_set(mp, xfs_attr->tag, val);
> +	if (ret)
> +		return ret;
> +	return count;
> +}
> +
> +STATIC ssize_t
> +xfs_errortag_attr_show(
> +	struct kobject		*kobject,
> +	struct attribute	*attr,
> +	char			*buf)
> +{
> +	struct xfs_mount	*mp = to_mp(kobject);
> +	struct xfs_errortag_attr *xfs_attr = to_attr(attr);
> +
> +	return snprintf(buf, PAGE_SIZE, "%u\n",
> +			xfs_errortag_get(mp, xfs_attr->tag));
> +}
> +
> +static const struct sysfs_ops xfs_errortag_sysfs_ops = {
> +	.show = xfs_errortag_attr_show,
> +	.store = xfs_errortag_attr_store,
> +};
> +
> +#define XFS_ERRORTAG_ATTR_RW(_name, _tag) \
> +static struct xfs_errortag_attr xfs_errortag_attr_##_name = {		\
> +	.attr = {.name = __stringify(_name),				\
> +		 .mode = VERIFY_OCTAL_PERMISSIONS(S_IWUSR | S_IRUGO) },	\
> +	.tag	= (_tag),						\
> +}
> +
> +#define XFS_ERRORTAG_ATTR_LIST(_name) &xfs_errortag_attr_##_name.attr
> +
> +XFS_ERRORTAG_ATTR_RW(noerror,		XFS_ERRTAG_NOERROR);
> +XFS_ERRORTAG_ATTR_RW(iflush1,		XFS_ERRTAG_IFLUSH_1);
> +XFS_ERRORTAG_ATTR_RW(iflush2,		XFS_ERRTAG_IFLUSH_2);
> +XFS_ERRORTAG_ATTR_RW(iflush3,		XFS_ERRTAG_IFLUSH_3);
> +XFS_ERRORTAG_ATTR_RW(iflush4,		XFS_ERRTAG_IFLUSH_4);
> +XFS_ERRORTAG_ATTR_RW(iflush5,		XFS_ERRTAG_IFLUSH_5);
> +XFS_ERRORTAG_ATTR_RW(iflush6,		XFS_ERRTAG_IFLUSH_6);
> +XFS_ERRORTAG_ATTR_RW(dareadbuf,		XFS_ERRTAG_DA_READ_BUF);
> +XFS_ERRORTAG_ATTR_RW(btree_chk_lblk,	XFS_ERRTAG_BTREE_CHECK_LBLOCK);
> +XFS_ERRORTAG_ATTR_RW(btree_chk_sblk,	XFS_ERRTAG_BTREE_CHECK_SBLOCK);
> +XFS_ERRORTAG_ATTR_RW(readagf,		XFS_ERRTAG_ALLOC_READ_AGF);
> +XFS_ERRORTAG_ATTR_RW(readagi,		XFS_ERRTAG_IALLOC_READ_AGI);
> +XFS_ERRORTAG_ATTR_RW(itobp,		XFS_ERRTAG_ITOBP_INOTOBP);
> +XFS_ERRORTAG_ATTR_RW(iunlink,		XFS_ERRTAG_IUNLINK);
> +XFS_ERRORTAG_ATTR_RW(iunlinkrm,		XFS_ERRTAG_IUNLINK_REMOVE);
> +XFS_ERRORTAG_ATTR_RW(dirinovalid,	XFS_ERRTAG_DIR_INO_VALIDATE);
> +XFS_ERRORTAG_ATTR_RW(bulkstat,		XFS_ERRTAG_BULKSTAT_READ_CHUNK);
> +XFS_ERRORTAG_ATTR_RW(logiodone,		XFS_ERRTAG_IODONE_IOERR);
> +XFS_ERRORTAG_ATTR_RW(stratread,		XFS_ERRTAG_STRATREAD_IOERR);
> +XFS_ERRORTAG_ATTR_RW(stratcmpl,		XFS_ERRTAG_STRATCMPL_IOERR);
> +XFS_ERRORTAG_ATTR_RW(diowrite,		XFS_ERRTAG_DIOWRITE_IOERR);
> +XFS_ERRORTAG_ATTR_RW(bmapifmt,		XFS_ERRTAG_BMAPIFORMAT);
> +XFS_ERRORTAG_ATTR_RW(free_extent,	XFS_ERRTAG_FREE_EXTENT);
> +XFS_ERRORTAG_ATTR_RW(rmap_finish_one,	XFS_ERRTAG_RMAP_FINISH_ONE);
> +XFS_ERRORTAG_ATTR_RW(refcount_continue_update,	XFS_ERRTAG_REFCOUNT_CONTINUE_UPDATE);
> +XFS_ERRORTAG_ATTR_RW(refcount_finish_one,	XFS_ERRTAG_REFCOUNT_FINISH_ONE);
> +XFS_ERRORTAG_ATTR_RW(bmap_finish_one,	XFS_ERRTAG_BMAP_FINISH_ONE);
> +XFS_ERRORTAG_ATTR_RW(ag_resv_critical,	XFS_ERRTAG_AG_RESV_CRITICAL);
> +
> +static struct attribute *xfs_errortag_attrs[] = {
> +	XFS_ERRORTAG_ATTR_LIST(noerror),
> +	XFS_ERRORTAG_ATTR_LIST(iflush1),
> +	XFS_ERRORTAG_ATTR_LIST(iflush2),
> +	XFS_ERRORTAG_ATTR_LIST(iflush3),
> +	XFS_ERRORTAG_ATTR_LIST(iflush4),
> +	XFS_ERRORTAG_ATTR_LIST(iflush5),
> +	XFS_ERRORTAG_ATTR_LIST(iflush6),
> +	XFS_ERRORTAG_ATTR_LIST(dareadbuf),
> +	XFS_ERRORTAG_ATTR_LIST(btree_chk_lblk),
> +	XFS_ERRORTAG_ATTR_LIST(btree_chk_sblk),
> +	XFS_ERRORTAG_ATTR_LIST(readagf),
> +	XFS_ERRORTAG_ATTR_LIST(readagi),
> +	XFS_ERRORTAG_ATTR_LIST(itobp),
> +	XFS_ERRORTAG_ATTR_LIST(iunlink),
> +	XFS_ERRORTAG_ATTR_LIST(iunlinkrm),
> +	XFS_ERRORTAG_ATTR_LIST(dirinovalid),
> +	XFS_ERRORTAG_ATTR_LIST(bulkstat),
> +	XFS_ERRORTAG_ATTR_LIST(logiodone),
> +	XFS_ERRORTAG_ATTR_LIST(stratread),
> +	XFS_ERRORTAG_ATTR_LIST(stratcmpl),
> +	XFS_ERRORTAG_ATTR_LIST(diowrite),
> +	XFS_ERRORTAG_ATTR_LIST(bmapifmt),
> +	XFS_ERRORTAG_ATTR_LIST(free_extent),
> +	XFS_ERRORTAG_ATTR_LIST(rmap_finish_one),
> +	XFS_ERRORTAG_ATTR_LIST(refcount_continue_update),
> +	XFS_ERRORTAG_ATTR_LIST(refcount_finish_one),
> +	XFS_ERRORTAG_ATTR_LIST(bmap_finish_one),
> +	XFS_ERRORTAG_ATTR_LIST(ag_resv_critical),
> +	NULL,
> +};
> +
> +struct kobj_type xfs_errortag_ktype = {
> +	.release = xfs_sysfs_release,
> +	.sysfs_ops = &xfs_errortag_sysfs_ops,
> +	.default_attrs = xfs_errortag_attrs,
> +};
> +
>  int
>  xfs_errortag_init(
>  	struct xfs_mount	*mp)
> @@ -64,13 +204,16 @@ xfs_errortag_init(
>  			KM_SLEEP | KM_MAYFAIL);
>  	if (!mp->m_errortag)
>  		return -ENOMEM;
> -	return 0;
> +
> +	return xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
> +			       &mp->m_kobj, "errortag");
>  }
>  
>  void
>  xfs_errortag_del(
>  	struct xfs_mount	*mp)
>  {
> +	xfs_sysfs_del(&mp->m_errortag_kobj);
>  	kmem_free(mp->m_errortag);
>  }
>  
> @@ -96,6 +239,17 @@ xfs_errortag_test(
>  }
>  
>  int
> +xfs_errortag_get(
> +	struct xfs_mount	*mp,
> +	unsigned int		error_tag)
> +{
> +	if (error_tag >= XFS_ERRTAG_MAX)
> +		return -EINVAL;
> +
> +	return mp->m_errortag[error_tag];
> +}
> +
> +int
>  xfs_errortag_set(
>  	struct xfs_mount	*mp,
>  	unsigned int		error_tag,
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index b4316d3..8915bde 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -138,6 +138,7 @@ extern bool xfs_errortag_test(struct xfs_mount *mp, const char *expression,
>  #define XFS_TEST_ERROR(expr, mp, tag, rf)		\
>  	((expr) || xfs_errortag_test((mp), #expr, __FILE__, __LINE__, (tag)))
>  
> +extern int xfs_errortag_get(struct xfs_mount *mp, unsigned int error_tag);
>  extern int xfs_errortag_set(struct xfs_mount *mp, unsigned int error_tag,
>  		unsigned int tag_value);
>  extern int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index e002ac5..931e9fc 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -204,6 +204,7 @@ typedef struct xfs_mount {
>  	 * error triggers.  1 = always, 2 = half the time, etc.
>  	 */
>  	unsigned int		*m_errortag;
> +	struct xfs_kobj		m_errortag_kobj;
>  
>  	/*
>  	 * DEBUG mode instrumentation to test and/or trigger delayed allocation
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] xfs: make errortag a per-mountpoint structure
  2017-06-23 16:35 ` [PATCH 1/4] xfs: make errortag a per-mountpoint structure Darrick J. Wong
  2017-06-26 11:10   ` Brian Foster
@ 2017-06-27  7:35   ` Carlos Maiolino
  1 sibling, 0 replies; 15+ messages in thread
From: Carlos Maiolino @ 2017-06-27  7:35 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Fri, Jun 23, 2017 at 09:35:15AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Remove the xfs_etest structure in favor of a per-mountpoint structure.
> This will give us the flexibility to set as many error injection points
> as we want, and later enable us to set up sysfs knobs to set the trigger
> frequency as we wish.  This comes at a cost of higher memory use, but
> unti we hit 1024 injection points (we're at 29) or a lot of mounts this
> shouldn't be a huge issue.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_error.c |  154 +++++++++++++++++++++++++++-------------------------
>  fs/xfs/xfs_error.h |   25 +++++---
>  fs/xfs/xfs_ioctl.c |    4 +
>  fs/xfs/xfs_mount.c |   10 +++
>  fs/xfs/xfs_mount.h |    7 ++
>  5 files changed, 111 insertions(+), 89 deletions(-)
> 

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

> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index ed7ee4e..52f75bc 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -25,100 +25,106 @@
>  
>  #ifdef DEBUG
>  
> -int	xfs_etest[XFS_NUM_INJECT_ERROR];
> -int64_t	xfs_etest_fsid[XFS_NUM_INJECT_ERROR];
> -char *	xfs_etest_fsname[XFS_NUM_INJECT_ERROR];
> -int	xfs_error_test_active;
> +static unsigned int xfs_errortag_random_default[] = {
> +	XFS_RANDOM_DEFAULT,
> +	XFS_RANDOM_IFLUSH_1,
> +	XFS_RANDOM_IFLUSH_2,
> +	XFS_RANDOM_IFLUSH_3,
> +	XFS_RANDOM_IFLUSH_4,
> +	XFS_RANDOM_IFLUSH_5,
> +	XFS_RANDOM_IFLUSH_6,
> +	XFS_RANDOM_DA_READ_BUF,
> +	XFS_RANDOM_BTREE_CHECK_LBLOCK,
> +	XFS_RANDOM_BTREE_CHECK_SBLOCK,
> +	XFS_RANDOM_ALLOC_READ_AGF,
> +	XFS_RANDOM_IALLOC_READ_AGI,
> +	XFS_RANDOM_ITOBP_INOTOBP,
> +	XFS_RANDOM_IUNLINK,
> +	XFS_RANDOM_IUNLINK_REMOVE,
> +	XFS_RANDOM_DIR_INO_VALIDATE,
> +	XFS_RANDOM_BULKSTAT_READ_CHUNK,
> +	XFS_RANDOM_IODONE_IOERR,
> +	XFS_RANDOM_STRATREAD_IOERR,
> +	XFS_RANDOM_STRATCMPL_IOERR,
> +	XFS_RANDOM_DIOWRITE_IOERR,
> +	XFS_RANDOM_BMAPIFORMAT,
> +	XFS_RANDOM_FREE_EXTENT,
> +	XFS_RANDOM_RMAP_FINISH_ONE,
> +	XFS_RANDOM_REFCOUNT_CONTINUE_UPDATE,
> +	XFS_RANDOM_REFCOUNT_FINISH_ONE,
> +	XFS_RANDOM_BMAP_FINISH_ONE,
> +	XFS_RANDOM_AG_RESV_CRITICAL,
> +};
>  
>  int
> -xfs_error_test(int error_tag, int *fsidp, char *expression,
> -	       int line, char *file, unsigned long randfactor)
> +xfs_errortag_init(
> +	struct xfs_mount	*mp)
>  {
> -	int i;
> -	int64_t fsid;
> +	mp->m_errortag = kmem_zalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
> +			KM_SLEEP | KM_MAYFAIL);
> +	if (!mp->m_errortag)
> +		return -ENOMEM;
> +	return 0;
> +}
>  
> -	if (prandom_u32() % randfactor)
> -		return 0;
> +void
> +xfs_errortag_del(
> +	struct xfs_mount	*mp)
> +{
> +	kmem_free(mp->m_errortag);
> +}
>  
> -	memcpy(&fsid, fsidp, sizeof(xfs_fsid_t));
> +bool
> +xfs_errortag_test(
> +	struct xfs_mount	*mp,
> +	const char		*expression,
> +	const char		*file,
> +	int			line,
> +	unsigned int		error_tag)
> +{
> +	unsigned int		randfactor;
>  
> -	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++)  {
> -		if (xfs_etest[i] == error_tag && xfs_etest_fsid[i] == fsid) {
> -			xfs_warn(NULL,
> -	"Injecting error (%s) at file %s, line %d, on filesystem \"%s\"",
> -				expression, file, line, xfs_etest_fsname[i]);
> -			return 1;
> -		}
> -	}
> +	ASSERT(error_tag < XFS_ERRTAG_MAX);
> +	randfactor = mp->m_errortag[error_tag];
> +	if (!randfactor || prandom_u32() % randfactor)
> +		return false;
>  
> -	return 0;
> +	xfs_warn_ratelimited(mp,
> +"Injecting error (%s) at file %s, line %d, on filesystem \"%s\"",
> +			expression, file, line, mp->m_fsname);
> +	return true;
>  }
>  
>  int
> -xfs_errortag_add(unsigned int error_tag, xfs_mount_t *mp)
> +xfs_errortag_set(
> +	struct xfs_mount	*mp,
> +	unsigned int		error_tag,
> +	unsigned int		tag_value)
>  {
> -	int i;
> -	int len;
> -	int64_t fsid;
> -
>  	if (error_tag >= XFS_ERRTAG_MAX)
>  		return -EINVAL;
>  
> -	memcpy(&fsid, mp->m_fixedfsid, sizeof(xfs_fsid_t));
> -
> -	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++)  {
> -		if (xfs_etest_fsid[i] == fsid && xfs_etest[i] == error_tag) {
> -			xfs_warn(mp, "error tag #%d on", error_tag);
> -			return 0;
> -		}
> -	}
> -
> -	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++)  {
> -		if (xfs_etest[i] == 0) {
> -			xfs_warn(mp, "Turned on XFS error tag #%d",
> -				error_tag);
> -			xfs_etest[i] = error_tag;
> -			xfs_etest_fsid[i] = fsid;
> -			len = strlen(mp->m_fsname);
> -			xfs_etest_fsname[i] = kmem_alloc(len + 1, KM_SLEEP);
> -			strcpy(xfs_etest_fsname[i], mp->m_fsname);
> -			xfs_error_test_active++;
> -			return 0;
> -		}
> -	}
> -
> -	xfs_warn(mp, "error tag overflow, too many turned on");
> -
> -	return 1;
> +	mp->m_errortag[error_tag] = tag_value;
> +	return 0;
>  }
>  
>  int
> -xfs_errortag_clearall(xfs_mount_t *mp, int loud)
> +xfs_errortag_add(
> +	struct xfs_mount	*mp,
> +	unsigned int		error_tag)
>  {
> -	int64_t fsid;
> -	int cleared = 0;
> -	int i;
> -
> -	memcpy(&fsid, mp->m_fixedfsid, sizeof(xfs_fsid_t));
> -
> -
> -	for (i = 0; i < XFS_NUM_INJECT_ERROR; i++) {
> -		if ((fsid == 0LL || xfs_etest_fsid[i] == fsid) &&
> -		     xfs_etest[i] != 0) {
> -			cleared = 1;
> -			xfs_warn(mp, "Clearing XFS error tag #%d",
> -				xfs_etest[i]);
> -			xfs_etest[i] = 0;
> -			xfs_etest_fsid[i] = 0LL;
> -			kmem_free(xfs_etest_fsname[i]);
> -			xfs_etest_fsname[i] = NULL;
> -			xfs_error_test_active--;
> -		}
> -	}
> +	if (error_tag >= XFS_ERRTAG_MAX)
> +		return -EINVAL;
>  
> -	if (loud || cleared)
> -		xfs_warn(mp, "Cleared all XFS error tags for filesystem");
> +	return xfs_errortag_set(mp, error_tag,
> +			xfs_errortag_random_default[error_tag]);
> +}
>  
> +int
> +xfs_errortag_clearall(
> +	struct xfs_mount	*mp)
> +{
> +	memset(mp->m_errortag, 0, sizeof(unsigned int) * XFS_ERRTAG_MAX);
>  	return 0;
>  }
>  #endif /* DEBUG */
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index 05f8666..b4316d3 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -131,21 +131,24 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
>  #define XFS_RANDOM_AG_RESV_CRITICAL			4
>  
>  #ifdef DEBUG
> -extern int xfs_error_test_active;
> -extern int xfs_error_test(int, int *, char *, int, char *, unsigned long);
> -
> -#define	XFS_NUM_INJECT_ERROR				10
> +extern int xfs_errortag_init(struct xfs_mount *mp);
> +extern void xfs_errortag_del(struct xfs_mount *mp);
> +extern bool xfs_errortag_test(struct xfs_mount *mp, const char *expression,
> +		const char *file, int line, unsigned int error_tag);
>  #define XFS_TEST_ERROR(expr, mp, tag, rf)		\
> -	((expr) || (xfs_error_test_active && \
> -	 xfs_error_test((tag), (mp)->m_fixedfsid, "expr", __LINE__, __FILE__, \
> -			(rf))))
> +	((expr) || xfs_errortag_test((mp), #expr, __FILE__, __LINE__, (tag)))
>  
> -extern int xfs_errortag_add(unsigned int error_tag, struct xfs_mount *mp);
> -extern int xfs_errortag_clearall(struct xfs_mount *mp, int loud);
> +extern int xfs_errortag_set(struct xfs_mount *mp, unsigned int error_tag,
> +		unsigned int tag_value);
> +extern int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
> +extern int xfs_errortag_clearall(struct xfs_mount *mp);
>  #else
> +#define xfs_errortag_init(mp)			(0)
> +#define xfs_errortag_del(mp)
>  #define XFS_TEST_ERROR(expr, mp, tag, rf)	(expr)
> -#define xfs_errortag_add(tag, mp)		(ENOSYS)
> -#define xfs_errortag_clearall(mp, loud)		(ENOSYS)
> +#define xfs_errortag_set(mp, tag, val)		(ENOSYS)
> +#define xfs_errortag_add(mp, tag)		(ENOSYS)
> +#define xfs_errortag_clearall(mp)		(ENOSYS)
>  #endif /* DEBUG */
>  
>  /*
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 8ffe4ea..9c0c7a9 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -2037,14 +2037,14 @@ xfs_file_ioctl(
>  		if (copy_from_user(&in, arg, sizeof(in)))
>  			return -EFAULT;
>  
> -		return xfs_errortag_add(in.errtag, mp);
> +		return xfs_errortag_add(mp, in.errtag);
>  	}
>  
>  	case XFS_IOC_ERROR_CLEARALL:
>  		if (!capable(CAP_SYS_ADMIN))
>  			return -EPERM;
>  
> -		return xfs_errortag_clearall(mp, 1);
> +		return xfs_errortag_clearall(mp);
>  
>  	case XFS_IOC_FREE_EOFBLOCKS: {
>  		struct xfs_fs_eofblocks eofb;
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index cc6789d..1a98c35 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -720,10 +720,13 @@ xfs_mountfs(
>  	if (error)
>  		goto out_del_stats;
>  
> +	error = xfs_errortag_init(mp);
> +	if (error)
> +		goto out_remove_error_sysfs;
>  
>  	error = xfs_uuid_mount(mp);
>  	if (error)
> -		goto out_remove_error_sysfs;
> +		goto out_remove_errortag;
>  
>  	/*
>  	 * Set the minimum read and write sizes
> @@ -1042,6 +1045,8 @@ xfs_mountfs(
>  	xfs_da_unmount(mp);
>   out_remove_uuid:
>  	xfs_uuid_unmount(mp);
> + out_remove_errortag:
> +	xfs_errortag_del(mp);
>   out_remove_error_sysfs:
>  	xfs_error_sysfs_del(mp);
>   out_del_stats:
> @@ -1145,10 +1150,11 @@ xfs_unmountfs(
>  	xfs_uuid_unmount(mp);
>  
>  #if defined(DEBUG)
> -	xfs_errortag_clearall(mp, 0);
> +	xfs_errortag_clearall(mp);
>  #endif
>  	xfs_free_perag(mp);
>  
> +	xfs_errortag_del(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 305d953..e002ac5 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -199,6 +199,13 @@ typedef struct xfs_mount {
>  	bool			m_fail_unmount;
>  #ifdef DEBUG
>  	/*
> +	 * Frequency with which errors are injected.  Replaces xfs_etest; the
> +	 * value stored in here is the inverse of the frequency with which the
> +	 * error triggers.  1 = always, 2 = half the time, etc.
> +	 */
> +	unsigned int		*m_errortag;
> +
> +	/*
>  	 * DEBUG mode instrumentation to test and/or trigger delayed allocation
>  	 * block killing in the event of failed writes. When enabled, all
>  	 * buffered writes are silenty dropped and handled as if they failed.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] xfs: expose errortag knobs via sysfs
  2017-06-23 16:35 ` [PATCH 2/4] xfs: expose errortag knobs via sysfs Darrick J. Wong
  2017-06-26 11:10   ` Brian Foster
@ 2017-06-27 10:26   ` Carlos Maiolino
  1 sibling, 0 replies; 15+ messages in thread
From: Carlos Maiolino @ 2017-06-27 10:26 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Fri, Jun 23, 2017 at 09:35:29AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Creates a /sys/fs/xfs/$dev/errortag/ directory to control the errortag
> values directly.  This enables us to control the randomness values,
> rather than having to accept the defaults.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_error.c |  156 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_error.h |    1 
>  fs/xfs/xfs_mount.h |    1 
>  3 files changed, 157 insertions(+), 1 deletion(-)
> 

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

> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 52f75bc..e2278af6 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -22,6 +22,7 @@
>  #include "xfs_trans_resv.h"
>  #include "xfs_mount.h"
>  #include "xfs_error.h"
> +#include "xfs_sysfs.h"
>  
>  #ifdef DEBUG
>  
> @@ -56,6 +57,145 @@ static unsigned int xfs_errortag_random_default[] = {
>  	XFS_RANDOM_AG_RESV_CRITICAL,
>  };
>  
> +struct xfs_errortag_attr {
> +	struct attribute	attr;
> +	unsigned int		tag;
> +};
> +
> +static inline struct xfs_errortag_attr *
> +to_attr(struct attribute *attr)
> +{
> +	return container_of(attr, struct xfs_errortag_attr, attr);
> +}
> +
> +static inline struct xfs_mount *
> +to_mp(struct kobject *kobject)
> +{
> +	struct xfs_kobj *kobj = to_kobj(kobject);
> +
> +	return container_of(kobj, struct xfs_mount, m_errortag_kobj);
> +}
> +
> +STATIC ssize_t
> +xfs_errortag_attr_store(
> +	struct kobject		*kobject,
> +	struct attribute	*attr,
> +	const char		*buf,
> +	size_t			count)
> +{
> +	struct xfs_mount	*mp = to_mp(kobject);
> +	struct xfs_errortag_attr *xfs_attr = to_attr(attr);
> +	int			ret;
> +	unsigned int		val;
> +
> +	if (strcmp(buf, "default") == 0) {
> +		val = xfs_errortag_random_default[xfs_attr->tag];
> +	} else {
> +		ret = kstrtouint(buf, 0, &val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = xfs_errortag_set(mp, xfs_attr->tag, val);
> +	if (ret)
> +		return ret;
> +	return count;
> +}
> +
> +STATIC ssize_t
> +xfs_errortag_attr_show(
> +	struct kobject		*kobject,
> +	struct attribute	*attr,
> +	char			*buf)
> +{
> +	struct xfs_mount	*mp = to_mp(kobject);
> +	struct xfs_errortag_attr *xfs_attr = to_attr(attr);
> +
> +	return snprintf(buf, PAGE_SIZE, "%u\n",
> +			xfs_errortag_get(mp, xfs_attr->tag));
> +}
> +
> +static const struct sysfs_ops xfs_errortag_sysfs_ops = {
> +	.show = xfs_errortag_attr_show,
> +	.store = xfs_errortag_attr_store,
> +};
> +
> +#define XFS_ERRORTAG_ATTR_RW(_name, _tag) \
> +static struct xfs_errortag_attr xfs_errortag_attr_##_name = {		\
> +	.attr = {.name = __stringify(_name),				\
> +		 .mode = VERIFY_OCTAL_PERMISSIONS(S_IWUSR | S_IRUGO) },	\
> +	.tag	= (_tag),						\
> +}
> +
> +#define XFS_ERRORTAG_ATTR_LIST(_name) &xfs_errortag_attr_##_name.attr
> +
> +XFS_ERRORTAG_ATTR_RW(noerror,		XFS_ERRTAG_NOERROR);
> +XFS_ERRORTAG_ATTR_RW(iflush1,		XFS_ERRTAG_IFLUSH_1);
> +XFS_ERRORTAG_ATTR_RW(iflush2,		XFS_ERRTAG_IFLUSH_2);
> +XFS_ERRORTAG_ATTR_RW(iflush3,		XFS_ERRTAG_IFLUSH_3);
> +XFS_ERRORTAG_ATTR_RW(iflush4,		XFS_ERRTAG_IFLUSH_4);
> +XFS_ERRORTAG_ATTR_RW(iflush5,		XFS_ERRTAG_IFLUSH_5);
> +XFS_ERRORTAG_ATTR_RW(iflush6,		XFS_ERRTAG_IFLUSH_6);
> +XFS_ERRORTAG_ATTR_RW(dareadbuf,		XFS_ERRTAG_DA_READ_BUF);
> +XFS_ERRORTAG_ATTR_RW(btree_chk_lblk,	XFS_ERRTAG_BTREE_CHECK_LBLOCK);
> +XFS_ERRORTAG_ATTR_RW(btree_chk_sblk,	XFS_ERRTAG_BTREE_CHECK_SBLOCK);
> +XFS_ERRORTAG_ATTR_RW(readagf,		XFS_ERRTAG_ALLOC_READ_AGF);
> +XFS_ERRORTAG_ATTR_RW(readagi,		XFS_ERRTAG_IALLOC_READ_AGI);
> +XFS_ERRORTAG_ATTR_RW(itobp,		XFS_ERRTAG_ITOBP_INOTOBP);
> +XFS_ERRORTAG_ATTR_RW(iunlink,		XFS_ERRTAG_IUNLINK);
> +XFS_ERRORTAG_ATTR_RW(iunlinkrm,		XFS_ERRTAG_IUNLINK_REMOVE);
> +XFS_ERRORTAG_ATTR_RW(dirinovalid,	XFS_ERRTAG_DIR_INO_VALIDATE);
> +XFS_ERRORTAG_ATTR_RW(bulkstat,		XFS_ERRTAG_BULKSTAT_READ_CHUNK);
> +XFS_ERRORTAG_ATTR_RW(logiodone,		XFS_ERRTAG_IODONE_IOERR);
> +XFS_ERRORTAG_ATTR_RW(stratread,		XFS_ERRTAG_STRATREAD_IOERR);
> +XFS_ERRORTAG_ATTR_RW(stratcmpl,		XFS_ERRTAG_STRATCMPL_IOERR);
> +XFS_ERRORTAG_ATTR_RW(diowrite,		XFS_ERRTAG_DIOWRITE_IOERR);
> +XFS_ERRORTAG_ATTR_RW(bmapifmt,		XFS_ERRTAG_BMAPIFORMAT);
> +XFS_ERRORTAG_ATTR_RW(free_extent,	XFS_ERRTAG_FREE_EXTENT);
> +XFS_ERRORTAG_ATTR_RW(rmap_finish_one,	XFS_ERRTAG_RMAP_FINISH_ONE);
> +XFS_ERRORTAG_ATTR_RW(refcount_continue_update,	XFS_ERRTAG_REFCOUNT_CONTINUE_UPDATE);
> +XFS_ERRORTAG_ATTR_RW(refcount_finish_one,	XFS_ERRTAG_REFCOUNT_FINISH_ONE);
> +XFS_ERRORTAG_ATTR_RW(bmap_finish_one,	XFS_ERRTAG_BMAP_FINISH_ONE);
> +XFS_ERRORTAG_ATTR_RW(ag_resv_critical,	XFS_ERRTAG_AG_RESV_CRITICAL);
> +
> +static struct attribute *xfs_errortag_attrs[] = {
> +	XFS_ERRORTAG_ATTR_LIST(noerror),
> +	XFS_ERRORTAG_ATTR_LIST(iflush1),
> +	XFS_ERRORTAG_ATTR_LIST(iflush2),
> +	XFS_ERRORTAG_ATTR_LIST(iflush3),
> +	XFS_ERRORTAG_ATTR_LIST(iflush4),
> +	XFS_ERRORTAG_ATTR_LIST(iflush5),
> +	XFS_ERRORTAG_ATTR_LIST(iflush6),
> +	XFS_ERRORTAG_ATTR_LIST(dareadbuf),
> +	XFS_ERRORTAG_ATTR_LIST(btree_chk_lblk),
> +	XFS_ERRORTAG_ATTR_LIST(btree_chk_sblk),
> +	XFS_ERRORTAG_ATTR_LIST(readagf),
> +	XFS_ERRORTAG_ATTR_LIST(readagi),
> +	XFS_ERRORTAG_ATTR_LIST(itobp),
> +	XFS_ERRORTAG_ATTR_LIST(iunlink),
> +	XFS_ERRORTAG_ATTR_LIST(iunlinkrm),
> +	XFS_ERRORTAG_ATTR_LIST(dirinovalid),
> +	XFS_ERRORTAG_ATTR_LIST(bulkstat),
> +	XFS_ERRORTAG_ATTR_LIST(logiodone),
> +	XFS_ERRORTAG_ATTR_LIST(stratread),
> +	XFS_ERRORTAG_ATTR_LIST(stratcmpl),
> +	XFS_ERRORTAG_ATTR_LIST(diowrite),
> +	XFS_ERRORTAG_ATTR_LIST(bmapifmt),
> +	XFS_ERRORTAG_ATTR_LIST(free_extent),
> +	XFS_ERRORTAG_ATTR_LIST(rmap_finish_one),
> +	XFS_ERRORTAG_ATTR_LIST(refcount_continue_update),
> +	XFS_ERRORTAG_ATTR_LIST(refcount_finish_one),
> +	XFS_ERRORTAG_ATTR_LIST(bmap_finish_one),
> +	XFS_ERRORTAG_ATTR_LIST(ag_resv_critical),
> +	NULL,
> +};
> +
> +struct kobj_type xfs_errortag_ktype = {
> +	.release = xfs_sysfs_release,
> +	.sysfs_ops = &xfs_errortag_sysfs_ops,
> +	.default_attrs = xfs_errortag_attrs,
> +};
> +
>  int
>  xfs_errortag_init(
>  	struct xfs_mount	*mp)
> @@ -64,13 +204,16 @@ xfs_errortag_init(
>  			KM_SLEEP | KM_MAYFAIL);
>  	if (!mp->m_errortag)
>  		return -ENOMEM;
> -	return 0;
> +
> +	return xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
> +			       &mp->m_kobj, "errortag");
>  }
>  
>  void
>  xfs_errortag_del(
>  	struct xfs_mount	*mp)
>  {
> +	xfs_sysfs_del(&mp->m_errortag_kobj);
>  	kmem_free(mp->m_errortag);
>  }
>  
> @@ -96,6 +239,17 @@ xfs_errortag_test(
>  }
>  
>  int
> +xfs_errortag_get(
> +	struct xfs_mount	*mp,
> +	unsigned int		error_tag)
> +{
> +	if (error_tag >= XFS_ERRTAG_MAX)
> +		return -EINVAL;
> +
> +	return mp->m_errortag[error_tag];
> +}
> +
> +int
>  xfs_errortag_set(
>  	struct xfs_mount	*mp,
>  	unsigned int		error_tag,
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index b4316d3..8915bde 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -138,6 +138,7 @@ extern bool xfs_errortag_test(struct xfs_mount *mp, const char *expression,
>  #define XFS_TEST_ERROR(expr, mp, tag, rf)		\
>  	((expr) || xfs_errortag_test((mp), #expr, __FILE__, __LINE__, (tag)))
>  
> +extern int xfs_errortag_get(struct xfs_mount *mp, unsigned int error_tag);
>  extern int xfs_errortag_set(struct xfs_mount *mp, unsigned int error_tag,
>  		unsigned int tag_value);
>  extern int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index e002ac5..931e9fc 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -204,6 +204,7 @@ typedef struct xfs_mount {
>  	 * error triggers.  1 = always, 2 = half the time, etc.
>  	 */
>  	unsigned int		*m_errortag;
> +	struct xfs_kobj		m_errortag_kobj;
>  
>  	/*
>  	 * DEBUG mode instrumentation to test and/or trigger delayed allocation
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] xfs: remove unneeded parameter from XFS_TEST_ERROR
  2017-06-23 16:35 ` [PATCH 3/4] xfs: remove unneeded parameter from XFS_TEST_ERROR Darrick J. Wong
@ 2017-06-27 10:33   ` Carlos Maiolino
  0 siblings, 0 replies; 15+ messages in thread
From: Carlos Maiolino @ 2017-06-27 10:33 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, Brian Foster

On Fri, Jun 23, 2017 at 09:35:36AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Since we moved the injected error frequency controls to the mountpoint,
> we can get rid of the last argument to XFS_TEST_ERROR.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

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

> ---
>  fs/xfs/libxfs/xfs_ag_resv.c   |    3 +--
>  fs/xfs/libxfs/xfs_alloc.c     |    6 ++----
>  fs/xfs/libxfs/xfs_bmap.c      |   13 ++++++-------
>  fs/xfs/libxfs/xfs_btree.c     |    6 ++----
>  fs/xfs/libxfs/xfs_dir2.c      |    3 +--
>  fs/xfs/libxfs/xfs_ialloc.c    |    3 +--
>  fs/xfs/libxfs/xfs_inode_buf.c |    3 +--
>  fs/xfs/libxfs/xfs_refcount.c  |    6 ++----
>  fs/xfs/libxfs/xfs_rmap.c      |    3 +--
>  fs/xfs/xfs_error.h            |    4 ++--
>  fs/xfs/xfs_inode.c            |   11 +++++------
>  fs/xfs/xfs_iomap.c            |    2 +-
>  fs/xfs/xfs_log.c              |    3 +--
>  13 files changed, 26 insertions(+), 40 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c
> index 33db69b..b008ff3 100644
> --- a/fs/xfs/libxfs/xfs_ag_resv.c
> +++ b/fs/xfs/libxfs/xfs_ag_resv.c
> @@ -111,8 +111,7 @@ xfs_ag_resv_critical(
>  
>  	/* Critically low if less than 10% or max btree height remains. */
>  	return XFS_TEST_ERROR(avail < orig / 10 || avail < XFS_BTREE_MAXLEVELS,
> -			pag->pag_mount, XFS_ERRTAG_AG_RESV_CRITICAL,
> -			XFS_RANDOM_AG_RESV_CRITICAL);
> +			pag->pag_mount, XFS_ERRTAG_AG_RESV_CRITICAL);
>  }
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index fefa8da..744dcae 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2454,8 +2454,7 @@ xfs_agf_read_verify(
>  	    !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF))
>  		xfs_buf_ioerror(bp, -EFSBADCRC);
>  	else if (XFS_TEST_ERROR(!xfs_agf_verify(mp, bp), mp,
> -				XFS_ERRTAG_ALLOC_READ_AGF,
> -				XFS_RANDOM_ALLOC_READ_AGF))
> +				XFS_ERRTAG_ALLOC_READ_AGF))
>  		xfs_buf_ioerror(bp, -EFSCORRUPTED);
>  
>  	if (bp->b_error)
> @@ -2842,8 +2841,7 @@ xfs_free_extent(
>  	ASSERT(type != XFS_AG_RESV_AGFL);
>  
>  	if (XFS_TEST_ERROR(false, mp,
> -			XFS_ERRTAG_FREE_EXTENT,
> -			XFS_RANDOM_FREE_EXTENT))
> +			XFS_ERRTAG_FREE_EXTENT))
>  		return -EIO;
>  
>  	error = xfs_free_extent_fix_freelist(tp, agno, &agbp);
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 19480ed..0a98807 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3992,7 +3992,7 @@ xfs_bmapi_read(
>  	if (unlikely(XFS_TEST_ERROR(
>  	    (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
>  	     XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
> -	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
> +	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
>  		XFS_ERROR_REPORT("xfs_bmapi_read", XFS_ERRLEVEL_LOW, mp);
>  		return -EFSCORRUPTED;
>  	}
> @@ -4473,7 +4473,7 @@ xfs_bmapi_write(
>  	if (unlikely(XFS_TEST_ERROR(
>  	    (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
>  	     XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
> -	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
> +	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
>  		XFS_ERROR_REPORT("xfs_bmapi_write", XFS_ERRLEVEL_LOW, mp);
>  		return -EFSCORRUPTED;
>  	}
> @@ -4694,7 +4694,7 @@ xfs_bmapi_remap(
>  	if (unlikely(XFS_TEST_ERROR(
>  	    (XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_EXTENTS &&
>  	     XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_BTREE),
> -	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
> +	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
>  		XFS_ERROR_REPORT("xfs_bmapi_remap", XFS_ERRLEVEL_LOW, mp);
>  		return -EFSCORRUPTED;
>  	}
> @@ -6098,7 +6098,7 @@ xfs_bmap_shift_extents(
>  	if (unlikely(XFS_TEST_ERROR(
>  	    (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
>  	     XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
> -	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
> +	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
>  		XFS_ERROR_REPORT("xfs_bmap_shift_extents",
>  				 XFS_ERRLEVEL_LOW, mp);
>  		return -EFSCORRUPTED;
> @@ -6250,7 +6250,7 @@ xfs_bmap_split_extent_at(
>  	if (unlikely(XFS_TEST_ERROR(
>  	    (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
>  	     XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
> -	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
> +	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
>  		XFS_ERROR_REPORT("xfs_bmap_split_extent_at",
>  				 XFS_ERRLEVEL_LOW, mp);
>  		return -EFSCORRUPTED;
> @@ -6508,8 +6508,7 @@ xfs_bmap_finish_one(
>  		return -EFSCORRUPTED;
>  
>  	if (XFS_TEST_ERROR(false, tp->t_mountp,
> -			XFS_ERRTAG_BMAP_FINISH_ONE,
> -			XFS_RANDOM_BMAP_FINISH_ONE))
> +			XFS_ERRTAG_BMAP_FINISH_ONE))
>  		return -EIO;
>  
>  	switch (type) {
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 2f8075a..4da85ff 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -101,8 +101,7 @@ xfs_btree_check_lblock(
>  			be64_to_cpu(block->bb_u.l.bb_rightsib)));
>  
>  	if (unlikely(XFS_TEST_ERROR(!lblock_ok, mp,
> -			XFS_ERRTAG_BTREE_CHECK_LBLOCK,
> -			XFS_RANDOM_BTREE_CHECK_LBLOCK))) {
> +			XFS_ERRTAG_BTREE_CHECK_LBLOCK))) {
>  		if (bp)
>  			trace_xfs_btree_corrupt(bp, _RET_IP_);
>  		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
> @@ -153,8 +152,7 @@ xfs_btree_check_sblock(
>  		block->bb_u.s.bb_rightsib;
>  
>  	if (unlikely(XFS_TEST_ERROR(!sblock_ok, mp,
> -			XFS_ERRTAG_BTREE_CHECK_SBLOCK,
> -			XFS_RANDOM_BTREE_CHECK_SBLOCK))) {
> +			XFS_ERRTAG_BTREE_CHECK_SBLOCK))) {
>  		if (bp)
>  			trace_xfs_btree_corrupt(bp, _RET_IP_);
>  		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index 2f389d36..ccf9783 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -218,8 +218,7 @@ xfs_dir_ino_validate(
>  		agblkno != 0 &&
>  		ioff < (1 << mp->m_sb.sb_inopblog) &&
>  		XFS_AGINO_TO_INO(mp, agno, agino) == ino;
> -	if (unlikely(XFS_TEST_ERROR(!ino_ok, mp, XFS_ERRTAG_DIR_INO_VALIDATE,
> -			XFS_RANDOM_DIR_INO_VALIDATE))) {
> +	if (unlikely(XFS_TEST_ERROR(!ino_ok, mp, XFS_ERRTAG_DIR_INO_VALIDATE))) {
>  		xfs_warn(mp, "Invalid inode number 0x%Lx",
>  				(unsigned long long) ino);
>  		XFS_ERROR_REPORT("xfs_dir_ino_validate", XFS_ERRLEVEL_LOW, mp);
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index c514fe9..ffd5a15 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2552,8 +2552,7 @@ xfs_agi_read_verify(
>  	    !xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF))
>  		xfs_buf_ioerror(bp, -EFSBADCRC);
>  	else if (XFS_TEST_ERROR(!xfs_agi_verify(bp), mp,
> -				XFS_ERRTAG_IALLOC_READ_AGI,
> -				XFS_RANDOM_IALLOC_READ_AGI))
> +				XFS_ERRTAG_IALLOC_READ_AGI))
>  		xfs_buf_ioerror(bp, -EFSCORRUPTED);
>  
>  	if (bp->b_error)
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 0c970cf..378f8fb 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -105,8 +105,7 @@ xfs_inode_buf_verify(
>  		di_ok = dip->di_magic == cpu_to_be16(XFS_DINODE_MAGIC) &&
>  			xfs_dinode_good_version(mp, dip->di_version);
>  		if (unlikely(XFS_TEST_ERROR(!di_ok, mp,
> -						XFS_ERRTAG_ITOBP_INOTOBP,
> -						XFS_RANDOM_ITOBP_INOTOBP))) {
> +						XFS_ERRTAG_ITOBP_INOTOBP))) {
>  			if (readahead) {
>  				bp->b_flags &= ~XBF_DONE;
>  				xfs_buf_ioerror(bp, -EIO);
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index e170161..900ea23 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -805,8 +805,7 @@ xfs_refcount_still_have_space(
>  	 */
>  	if (cur->bc_private.a.priv.refc.nr_ops > 2 &&
>  	    XFS_TEST_ERROR(false, cur->bc_mp,
> -			XFS_ERRTAG_REFCOUNT_CONTINUE_UPDATE,
> -			XFS_RANDOM_REFCOUNT_CONTINUE_UPDATE))
> +			XFS_ERRTAG_REFCOUNT_CONTINUE_UPDATE))
>  		return false;
>  
>  	if (cur->bc_private.a.priv.refc.nr_ops == 0)
> @@ -1068,8 +1067,7 @@ xfs_refcount_finish_one(
>  			blockcount);
>  
>  	if (XFS_TEST_ERROR(false, mp,
> -			XFS_ERRTAG_REFCOUNT_FINISH_ONE,
> -			XFS_RANDOM_REFCOUNT_FINISH_ONE))
> +			XFS_ERRTAG_REFCOUNT_FINISH_ONE))
>  		return -EIO;
>  
>  	/*
> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> index eda275b..55c88a7 100644
> --- a/fs/xfs/libxfs/xfs_rmap.c
> +++ b/fs/xfs/libxfs/xfs_rmap.c
> @@ -2087,8 +2087,7 @@ xfs_rmap_finish_one(
>  			startoff, blockcount, state);
>  
>  	if (XFS_TEST_ERROR(false, mp,
> -			XFS_ERRTAG_RMAP_FINISH_ONE,
> -			XFS_RANDOM_RMAP_FINISH_ONE))
> +			XFS_ERRTAG_RMAP_FINISH_ONE))
>  		return -EIO;
>  
>  	/*
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index 8915bde..ae8935b 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -135,7 +135,7 @@ extern int xfs_errortag_init(struct xfs_mount *mp);
>  extern void xfs_errortag_del(struct xfs_mount *mp);
>  extern bool xfs_errortag_test(struct xfs_mount *mp, const char *expression,
>  		const char *file, int line, unsigned int error_tag);
> -#define XFS_TEST_ERROR(expr, mp, tag, rf)		\
> +#define XFS_TEST_ERROR(expr, mp, tag)		\
>  	((expr) || xfs_errortag_test((mp), #expr, __FILE__, __LINE__, (tag)))
>  
>  extern int xfs_errortag_get(struct xfs_mount *mp, unsigned int error_tag);
> @@ -146,7 +146,7 @@ extern int xfs_errortag_clearall(struct xfs_mount *mp);
>  #else
>  #define xfs_errortag_init(mp)			(0)
>  #define xfs_errortag_del(mp)
> -#define XFS_TEST_ERROR(expr, mp, tag, rf)	(expr)
> +#define XFS_TEST_ERROR(expr, mp, tag)		(expr)
>  #define xfs_errortag_set(mp, tag, val)		(ENOSYS)
>  #define xfs_errortag_add(mp, tag)		(ENOSYS)
>  #define xfs_errortag_clearall(mp)		(ENOSYS)
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index ffbfe7d..67bd563 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3489,7 +3489,7 @@ xfs_iflush_int(
>  	dip = xfs_buf_offset(bp, ip->i_imap.im_boffset);
>  
>  	if (XFS_TEST_ERROR(dip->di_magic != cpu_to_be16(XFS_DINODE_MAGIC),
> -			       mp, XFS_ERRTAG_IFLUSH_1, XFS_RANDOM_IFLUSH_1)) {
> +			       mp, XFS_ERRTAG_IFLUSH_1)) {
>  		xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
>  			"%s: Bad inode %Lu magic number 0x%x, ptr 0x%p",
>  			__func__, ip->i_ino, be16_to_cpu(dip->di_magic), dip);
> @@ -3499,7 +3499,7 @@ xfs_iflush_int(
>  		if (XFS_TEST_ERROR(
>  		    (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS) &&
>  		    (ip->i_d.di_format != XFS_DINODE_FMT_BTREE),
> -		    mp, XFS_ERRTAG_IFLUSH_3, XFS_RANDOM_IFLUSH_3)) {
> +		    mp, XFS_ERRTAG_IFLUSH_3)) {
>  			xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
>  				"%s: Bad regular inode %Lu, ptr 0x%p",
>  				__func__, ip->i_ino, ip);
> @@ -3510,7 +3510,7 @@ xfs_iflush_int(
>  		    (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS) &&
>  		    (ip->i_d.di_format != XFS_DINODE_FMT_BTREE) &&
>  		    (ip->i_d.di_format != XFS_DINODE_FMT_LOCAL),
> -		    mp, XFS_ERRTAG_IFLUSH_4, XFS_RANDOM_IFLUSH_4)) {
> +		    mp, XFS_ERRTAG_IFLUSH_4)) {
>  			xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
>  				"%s: Bad directory inode %Lu, ptr 0x%p",
>  				__func__, ip->i_ino, ip);
> @@ -3518,8 +3518,7 @@ xfs_iflush_int(
>  		}
>  	}
>  	if (XFS_TEST_ERROR(ip->i_d.di_nextents + ip->i_d.di_anextents >
> -				ip->i_d.di_nblocks, mp, XFS_ERRTAG_IFLUSH_5,
> -				XFS_RANDOM_IFLUSH_5)) {
> +				ip->i_d.di_nblocks, mp, XFS_ERRTAG_IFLUSH_5)) {
>  		xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
>  			"%s: detected corrupt incore inode %Lu, "
>  			"total extents = %d, nblocks = %Ld, ptr 0x%p",
> @@ -3529,7 +3528,7 @@ xfs_iflush_int(
>  		goto corrupt_out;
>  	}
>  	if (XFS_TEST_ERROR(ip->i_d.di_forkoff > mp->m_sb.sb_inodesize,
> -				mp, XFS_ERRTAG_IFLUSH_6, XFS_RANDOM_IFLUSH_6)) {
> +				mp, XFS_ERRTAG_IFLUSH_6)) {
>  		xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
>  			"%s: bad inode %Lu, forkoff 0x%x, ptr 0x%p",
>  			__func__, ip->i_ino, ip->i_d.di_forkoff, ip);
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 94e5bdf..304b79d 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -543,7 +543,7 @@ xfs_file_iomap_begin_delay(
>  	if (unlikely(XFS_TEST_ERROR(
>  	    (XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_EXTENTS &&
>  	     XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_BTREE),
> -	     mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
> +	     mp, XFS_ERRTAG_BMAPIFORMAT))) {
>  		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
>  		error = -EFSCORRUPTED;
>  		goto out_unlock;
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 98b39cb..2d1112e 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1189,8 +1189,7 @@ xlog_iodone(xfs_buf_t *bp)
>  	 * IOABORT state. The IOABORT state is only set in DEBUG mode to inject
>  	 * CRC errors into log recovery.
>  	 */
> -	if (XFS_TEST_ERROR(bp->b_error, l->l_mp, XFS_ERRTAG_IODONE_IOERR,
> -			   XFS_RANDOM_IODONE_IOERR) ||
> +	if (XFS_TEST_ERROR(bp->b_error, l->l_mp, XFS_ERRTAG_IODONE_IOERR) ||
>  	    iclog->ic_state & XLOG_STATE_IOABORT) {
>  		if (iclog->ic_state & XLOG_STATE_IOABORT)
>  			iclog->ic_state &= ~XLOG_STATE_IOABORT;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] xfs: convert drop_writes to use the errortag mechanism
  2017-06-23 16:35 ` [PATCH 4/4] xfs: convert drop_writes to use the errortag mechanism Darrick J. Wong
@ 2017-06-27 10:51   ` Carlos Maiolino
  0 siblings, 0 replies; 15+ messages in thread
From: Carlos Maiolino @ 2017-06-27 10:51 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, Brian Foster

On Fri, Jun 23, 2017 at 09:35:42AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> We now have enhanced error injection that can control the frequency
> with which errors happen, so convert drop_writes to use this.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/xfs/xfs_error.c |    3 +++
>  fs/xfs/xfs_error.h |   14 +++++++++++++-
>  fs/xfs/xfs_iomap.c |    2 +-
>  fs/xfs/xfs_mount.h |   24 ------------------------
>  fs/xfs/xfs_sysfs.c |   42 ------------------------------------------
>  5 files changed, 17 insertions(+), 68 deletions(-)
> 

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

> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index e2278af6..a2f23d2 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -55,6 +55,7 @@ static unsigned int xfs_errortag_random_default[] = {
>  	XFS_RANDOM_REFCOUNT_FINISH_ONE,
>  	XFS_RANDOM_BMAP_FINISH_ONE,
>  	XFS_RANDOM_AG_RESV_CRITICAL,
> +	XFS_RANDOM_DROP_WRITES,
>  };
>  
>  struct xfs_errortag_attr {
> @@ -157,6 +158,7 @@ XFS_ERRORTAG_ATTR_RW(refcount_continue_update,	XFS_ERRTAG_REFCOUNT_CONTINUE_UPDA
>  XFS_ERRORTAG_ATTR_RW(refcount_finish_one,	XFS_ERRTAG_REFCOUNT_FINISH_ONE);
>  XFS_ERRORTAG_ATTR_RW(bmap_finish_one,	XFS_ERRTAG_BMAP_FINISH_ONE);
>  XFS_ERRORTAG_ATTR_RW(ag_resv_critical,	XFS_ERRTAG_AG_RESV_CRITICAL);
> +XFS_ERRORTAG_ATTR_RW(drop_writes,	XFS_ERRTAG_DROP_WRITES);
>  
>  static struct attribute *xfs_errortag_attrs[] = {
>  	XFS_ERRORTAG_ATTR_LIST(noerror),
> @@ -187,6 +189,7 @@ static struct attribute *xfs_errortag_attrs[] = {
>  	XFS_ERRORTAG_ATTR_LIST(refcount_finish_one),
>  	XFS_ERRORTAG_ATTR_LIST(bmap_finish_one),
>  	XFS_ERRORTAG_ATTR_LIST(ag_resv_critical),
> +	XFS_ERRORTAG_ATTR_LIST(drop_writes),
>  	NULL,
>  };
>  
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index ae8935b..ad9b24a 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -96,7 +96,18 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
>  #define XFS_ERRTAG_REFCOUNT_FINISH_ONE			25
>  #define XFS_ERRTAG_BMAP_FINISH_ONE			26
>  #define XFS_ERRTAG_AG_RESV_CRITICAL			27
> -#define XFS_ERRTAG_MAX					28
> +
> +/*
> + * DEBUG mode instrumentation to test and/or trigger delayed allocation
> + * block killing in the event of failed writes. When enabled, all
> + * buffered writes are silenty dropped and handled as if they failed.
> + * All delalloc blocks in the range of the write (including pre-existing
> + * delalloc blocks!) are tossed as part of the write failure error
> + * handling sequence.
> + */
> +#define XFS_ERRTAG_DROP_WRITES				28
> +
> +#define XFS_ERRTAG_MAX					29
>  
>  /*
>   * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
> @@ -129,6 +140,7 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
>  #define XFS_RANDOM_REFCOUNT_FINISH_ONE			1
>  #define XFS_RANDOM_BMAP_FINISH_ONE			1
>  #define XFS_RANDOM_AG_RESV_CRITICAL			4
> +#define XFS_RANDOM_DROP_WRITES				1
>  
>  #ifdef DEBUG
>  extern int xfs_errortag_init(struct xfs_mount *mp);
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 304b79d..86f1a9f 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1097,7 +1097,7 @@ xfs_file_iomap_end_delalloc(
>  	 * Behave as if the write failed if drop writes is enabled. Set the NEW
>  	 * flag to force delalloc cleanup.
>  	 */
> -	if (xfs_mp_drop_writes(mp)) {
> +	if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_DROP_WRITES)) {
>  		iomap->flags |= IOMAP_F_NEW;
>  		written = 0;
>  	}
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 931e9fc..e0792d0 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -205,16 +205,6 @@ typedef struct xfs_mount {
>  	 */
>  	unsigned int		*m_errortag;
>  	struct xfs_kobj		m_errortag_kobj;
> -
> -	/*
> -	 * DEBUG mode instrumentation to test and/or trigger delayed allocation
> -	 * block killing in the event of failed writes. When enabled, all
> -	 * buffered writes are silenty dropped and handled as if they failed.
> -	 * All delalloc blocks in the range of the write (including pre-existing
> -	 * delalloc blocks!) are tossed as part of the write failure error
> -	 * handling sequence.
> -	 */
> -	bool			m_drop_writes;
>  #endif
>  } xfs_mount_t;
>  
> @@ -333,20 +323,6 @@ xfs_daddr_to_agbno(struct xfs_mount *mp, xfs_daddr_t d)
>  	return (xfs_agblock_t) do_div(ld, mp->m_sb.sb_agblocks);
>  }
>  
> -#ifdef DEBUG
> -static inline bool
> -xfs_mp_drop_writes(struct xfs_mount *mp)
> -{
> -	return mp->m_drop_writes;
> -}
> -#else
> -static inline bool
> -xfs_mp_drop_writes(struct xfs_mount *mp)
> -{
> -	return 0;
> -}
> -#endif
> -
>  /* per-AG block reservation data structures*/
>  enum xfs_ag_resv_type {
>  	XFS_AG_RESV_NONE = 0,
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index ec6e0e2..56610a9 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
> @@ -90,49 +90,7 @@ to_mp(struct kobject *kobject)
>  	return container_of(kobj, struct xfs_mount, m_kobj);
>  }
>  
> -#ifdef DEBUG
> -
> -STATIC ssize_t
> -drop_writes_store(
> -	struct kobject		*kobject,
> -	const char		*buf,
> -	size_t			count)
> -{
> -	struct xfs_mount	*mp = to_mp(kobject);
> -	int			ret;
> -	int			val;
> -
> -	ret = kstrtoint(buf, 0, &val);
> -	if (ret)
> -		return ret;
> -
> -	if (val == 1)
> -		mp->m_drop_writes = true;
> -	else if (val == 0)
> -		mp->m_drop_writes = false;
> -	else
> -		return -EINVAL;
> -
> -	return count;
> -}
> -
> -STATIC ssize_t
> -drop_writes_show(
> -	struct kobject		*kobject,
> -	char			*buf)
> -{
> -	struct xfs_mount	*mp = to_mp(kobject);
> -
> -	return snprintf(buf, PAGE_SIZE, "%d\n", mp->m_drop_writes ? 1 : 0);
> -}
> -XFS_SYSFS_ATTR_RW(drop_writes);
> -
> -#endif /* DEBUG */
> -
>  static struct attribute *xfs_mp_attrs[] = {
> -#ifdef DEBUG
> -	ATTR_LIST(drop_writes),
> -#endif
>  	NULL,
>  };
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH] xfs: replace log_badcrc_factor knob with error injection tag
  2017-06-23 16:35 [PATCH v2 0/4] xfs: more configurable error injection Darrick J. Wong
                   ` (3 preceding siblings ...)
  2017-06-23 16:35 ` [PATCH 4/4] xfs: convert drop_writes to use the errortag mechanism Darrick J. Wong
@ 2017-06-27 12:35 ` Brian Foster
  2017-06-27 16:51   ` Darrick J. Wong
  4 siblings, 1 reply; 15+ messages in thread
From: Brian Foster @ 2017-06-27 12:35 UTC (permalink / raw)
  To: linux-xfs; +Cc: darrick.wong

Now that error injection tags support dynamic frequency adjustment,
replace the debug mode sysfs knob that controls log record CRC error
injection with an error injection tag.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---

Darrick,

Care to tack something like this onto this series? We might as well fix
this one too before I create another test that relies on it. I'll follow
up with an update to xfs/141 to support either knob.

Brian

 fs/xfs/xfs_error.c |  3 +++
 fs/xfs/xfs_error.h |  4 +++-
 fs/xfs/xfs_log.c   |  5 +----
 fs/xfs/xfs_sysfs.c | 36 ------------------------------------
 4 files changed, 7 insertions(+), 41 deletions(-)

diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index a2f23d2..26c32bc 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -56,6 +56,7 @@ static unsigned int xfs_errortag_random_default[] = {
 	XFS_RANDOM_BMAP_FINISH_ONE,
 	XFS_RANDOM_AG_RESV_CRITICAL,
 	XFS_RANDOM_DROP_WRITES,
+	XFS_RANDOM_LOG_BAD_CRC,
 };
 
 struct xfs_errortag_attr {
@@ -159,6 +160,7 @@ XFS_ERRORTAG_ATTR_RW(refcount_finish_one,	XFS_ERRTAG_REFCOUNT_FINISH_ONE);
 XFS_ERRORTAG_ATTR_RW(bmap_finish_one,	XFS_ERRTAG_BMAP_FINISH_ONE);
 XFS_ERRORTAG_ATTR_RW(ag_resv_critical,	XFS_ERRTAG_AG_RESV_CRITICAL);
 XFS_ERRORTAG_ATTR_RW(drop_writes,	XFS_ERRTAG_DROP_WRITES);
+XFS_ERRORTAG_ATTR_RW(log_bad_crc,	XFS_ERRTAG_LOG_BAD_CRC);
 
 static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(noerror),
@@ -190,6 +192,7 @@ static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(bmap_finish_one),
 	XFS_ERRORTAG_ATTR_LIST(ag_resv_critical),
 	XFS_ERRORTAG_ATTR_LIST(drop_writes),
+	XFS_ERRORTAG_ATTR_LIST(log_bad_crc),
 	NULL,
 };
 
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index ad9b24a..0874d6b 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -106,8 +106,9 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
  * handling sequence.
  */
 #define XFS_ERRTAG_DROP_WRITES				28
+#define XFS_ERRTAG_LOG_BAD_CRC				29
 
-#define XFS_ERRTAG_MAX					29
+#define XFS_ERRTAG_MAX					30
 
 /*
  * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -141,6 +142,7 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
 #define XFS_RANDOM_BMAP_FINISH_ONE			1
 #define XFS_RANDOM_AG_RESV_CRITICAL			4
 #define XFS_RANDOM_DROP_WRITES				1
+#define XFS_RANDOM_LOG_BAD_CRC				1
 
 #ifdef DEBUG
 extern int xfs_errortag_init(struct xfs_mount *mp);
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 2d1112e..31f11be 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1841,7 +1841,6 @@ xlog_sync(
 	/* calculcate the checksum */
 	iclog->ic_header.h_crc = xlog_cksum(log, &iclog->ic_header,
 					    iclog->ic_datap, size);
-#ifdef DEBUG
 	/*
 	 * Intentionally corrupt the log record CRC based on the error injection
 	 * frequency, if defined. This facilitates testing log recovery in the
@@ -1849,15 +1848,13 @@ xlog_sync(
 	 * write on I/O completion and shutdown the fs. The subsequent mount
 	 * detects the bad CRC and attempts to recover.
 	 */
-	if (log->l_badcrc_factor &&
-	    (prandom_u32() % log->l_badcrc_factor == 0)) {
+	if (XFS_TEST_ERROR(false, log->l_mp, XFS_ERRTAG_LOG_BAD_CRC)) {
 		iclog->ic_header.h_crc &= cpu_to_le32(0xAAAAAAAA);
 		iclog->ic_state |= XLOG_STATE_IOABORT;
 		xfs_warn(log->l_mp,
 	"Intentionally corrupted log record at LSN 0x%llx. Shutdown imminent.",
 			 be64_to_cpu(iclog->ic_header.h_lsn));
 	}
-#endif
 
 	bp->b_io_length = BTOBB(count);
 	bp->b_fspriv = iclog;
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index 56610a9..8b2ccc2 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -305,47 +305,11 @@ write_grant_head_show(
 }
 XFS_SYSFS_ATTR_RO(write_grant_head);
 
-#ifdef DEBUG
-STATIC ssize_t
-log_badcrc_factor_store(
-	struct kobject	*kobject,
-	const char	*buf,
-	size_t		count)
-{
-	struct xlog	*log = to_xlog(kobject);
-	int		ret;
-	uint32_t	val;
-
-	ret = kstrtouint(buf, 0, &val);
-	if (ret)
-		return ret;
-
-	log->l_badcrc_factor = val;
-
-	return count;
-}
-
-STATIC ssize_t
-log_badcrc_factor_show(
-	struct kobject	*kobject,
-	char		*buf)
-{
-	struct xlog	*log = to_xlog(kobject);
-
-	return snprintf(buf, PAGE_SIZE, "%d\n", log->l_badcrc_factor);
-}
-
-XFS_SYSFS_ATTR_RW(log_badcrc_factor);
-#endif	/* DEBUG */
-
 static struct attribute *xfs_log_attrs[] = {
 	ATTR_LIST(log_head_lsn),
 	ATTR_LIST(log_tail_lsn),
 	ATTR_LIST(reserve_grant_head),
 	ATTR_LIST(write_grant_head),
-#ifdef DEBUG
-	ATTR_LIST(log_badcrc_factor),
-#endif
 	NULL,
 };
 
-- 
2.7.5


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH] xfs: replace log_badcrc_factor knob with error injection tag
  2017-06-27 12:35 ` [PATCH] xfs: replace log_badcrc_factor knob with error injection tag Brian Foster
@ 2017-06-27 16:51   ` Darrick J. Wong
  0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2017-06-27 16:51 UTC (permalink / raw)
  To: Brian Foster; +Cc: linux-xfs

On Tue, Jun 27, 2017 at 08:35:05AM -0400, Brian Foster wrote:
> Now that error injection tags support dynamic frequency adjustment,
> replace the debug mode sysfs knob that controls log record CRC error
> injection with an error injection tag.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
> 
> Darrick,
> 
> Care to tack something like this onto this series? We might as well fix
> this one too before I create another test that relies on it. I'll follow
> up with an update to xfs/141 to support either knob.

Looks ok to me, will test...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> 
> Brian
> 
>  fs/xfs/xfs_error.c |  3 +++
>  fs/xfs/xfs_error.h |  4 +++-
>  fs/xfs/xfs_log.c   |  5 +----
>  fs/xfs/xfs_sysfs.c | 36 ------------------------------------
>  4 files changed, 7 insertions(+), 41 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index a2f23d2..26c32bc 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -56,6 +56,7 @@ static unsigned int xfs_errortag_random_default[] = {
>  	XFS_RANDOM_BMAP_FINISH_ONE,
>  	XFS_RANDOM_AG_RESV_CRITICAL,
>  	XFS_RANDOM_DROP_WRITES,
> +	XFS_RANDOM_LOG_BAD_CRC,
>  };
>  
>  struct xfs_errortag_attr {
> @@ -159,6 +160,7 @@ XFS_ERRORTAG_ATTR_RW(refcount_finish_one,	XFS_ERRTAG_REFCOUNT_FINISH_ONE);
>  XFS_ERRORTAG_ATTR_RW(bmap_finish_one,	XFS_ERRTAG_BMAP_FINISH_ONE);
>  XFS_ERRORTAG_ATTR_RW(ag_resv_critical,	XFS_ERRTAG_AG_RESV_CRITICAL);
>  XFS_ERRORTAG_ATTR_RW(drop_writes,	XFS_ERRTAG_DROP_WRITES);
> +XFS_ERRORTAG_ATTR_RW(log_bad_crc,	XFS_ERRTAG_LOG_BAD_CRC);
>  
>  static struct attribute *xfs_errortag_attrs[] = {
>  	XFS_ERRORTAG_ATTR_LIST(noerror),
> @@ -190,6 +192,7 @@ static struct attribute *xfs_errortag_attrs[] = {
>  	XFS_ERRORTAG_ATTR_LIST(bmap_finish_one),
>  	XFS_ERRORTAG_ATTR_LIST(ag_resv_critical),
>  	XFS_ERRORTAG_ATTR_LIST(drop_writes),
> +	XFS_ERRORTAG_ATTR_LIST(log_bad_crc),
>  	NULL,
>  };
>  
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index ad9b24a..0874d6b 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -106,8 +106,9 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
>   * handling sequence.
>   */
>  #define XFS_ERRTAG_DROP_WRITES				28
> +#define XFS_ERRTAG_LOG_BAD_CRC				29
>  
> -#define XFS_ERRTAG_MAX					29
> +#define XFS_ERRTAG_MAX					30
>  
>  /*
>   * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
> @@ -141,6 +142,7 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
>  #define XFS_RANDOM_BMAP_FINISH_ONE			1
>  #define XFS_RANDOM_AG_RESV_CRITICAL			4
>  #define XFS_RANDOM_DROP_WRITES				1
> +#define XFS_RANDOM_LOG_BAD_CRC				1
>  
>  #ifdef DEBUG
>  extern int xfs_errortag_init(struct xfs_mount *mp);
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 2d1112e..31f11be 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1841,7 +1841,6 @@ xlog_sync(
>  	/* calculcate the checksum */
>  	iclog->ic_header.h_crc = xlog_cksum(log, &iclog->ic_header,
>  					    iclog->ic_datap, size);
> -#ifdef DEBUG
>  	/*
>  	 * Intentionally corrupt the log record CRC based on the error injection
>  	 * frequency, if defined. This facilitates testing log recovery in the
> @@ -1849,15 +1848,13 @@ xlog_sync(
>  	 * write on I/O completion and shutdown the fs. The subsequent mount
>  	 * detects the bad CRC and attempts to recover.
>  	 */
> -	if (log->l_badcrc_factor &&
> -	    (prandom_u32() % log->l_badcrc_factor == 0)) {
> +	if (XFS_TEST_ERROR(false, log->l_mp, XFS_ERRTAG_LOG_BAD_CRC)) {
>  		iclog->ic_header.h_crc &= cpu_to_le32(0xAAAAAAAA);
>  		iclog->ic_state |= XLOG_STATE_IOABORT;
>  		xfs_warn(log->l_mp,
>  	"Intentionally corrupted log record at LSN 0x%llx. Shutdown imminent.",
>  			 be64_to_cpu(iclog->ic_header.h_lsn));
>  	}
> -#endif
>  
>  	bp->b_io_length = BTOBB(count);
>  	bp->b_fspriv = iclog;
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index 56610a9..8b2ccc2 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
> @@ -305,47 +305,11 @@ write_grant_head_show(
>  }
>  XFS_SYSFS_ATTR_RO(write_grant_head);
>  
> -#ifdef DEBUG
> -STATIC ssize_t
> -log_badcrc_factor_store(
> -	struct kobject	*kobject,
> -	const char	*buf,
> -	size_t		count)
> -{
> -	struct xlog	*log = to_xlog(kobject);
> -	int		ret;
> -	uint32_t	val;
> -
> -	ret = kstrtouint(buf, 0, &val);
> -	if (ret)
> -		return ret;
> -
> -	log->l_badcrc_factor = val;
> -
> -	return count;
> -}
> -
> -STATIC ssize_t
> -log_badcrc_factor_show(
> -	struct kobject	*kobject,
> -	char		*buf)
> -{
> -	struct xlog	*log = to_xlog(kobject);
> -
> -	return snprintf(buf, PAGE_SIZE, "%d\n", log->l_badcrc_factor);
> -}
> -
> -XFS_SYSFS_ATTR_RW(log_badcrc_factor);
> -#endif	/* DEBUG */
> -
>  static struct attribute *xfs_log_attrs[] = {
>  	ATTR_LIST(log_head_lsn),
>  	ATTR_LIST(log_tail_lsn),
>  	ATTR_LIST(reserve_grant_head),
>  	ATTR_LIST(write_grant_head),
> -#ifdef DEBUG
> -	ATTR_LIST(log_badcrc_factor),
> -#endif
>  	NULL,
>  };
>  
> -- 
> 2.7.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] xfs: convert drop_writes to use the errortag mechanism
  2017-06-21  1:11 ` [PATCH 4/4] xfs: convert drop_writes to use the errortag mechanism Darrick J. Wong
@ 2017-06-21 18:19   ` Brian Foster
  0 siblings, 0 replies; 15+ messages in thread
From: Brian Foster @ 2017-06-21 18:19 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Tue, Jun 20, 2017 at 06:11:48PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> We now have enhanced error injection that can control the frequency
> with which errors happen, so convert drop_writes to use this.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Nice. Looks like a simple tweak to xfs/196 too...

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_error.c |    3 +++
>  fs/xfs/xfs_error.h |   14 +++++++++++++-
>  fs/xfs/xfs_iomap.c |    2 +-
>  fs/xfs/xfs_mount.h |   24 ------------------------
>  fs/xfs/xfs_sysfs.c |   42 ------------------------------------------
>  5 files changed, 17 insertions(+), 68 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 81f260f..40a15e2 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -55,6 +55,7 @@ static unsigned int xfs_errortag_random_default[] = {
>  	XFS_RANDOM_REFCOUNT_FINISH_ONE,
>  	XFS_RANDOM_BMAP_FINISH_ONE,
>  	XFS_RANDOM_AG_RESV_CRITICAL,
> +	XFS_RANDOM_DROP_WRITES,
>  };
>  
>  struct xfs_errortag_attr {
> @@ -157,6 +158,7 @@ XFS_ERRORTAG_ATTR_RW(refcount_continue_update,	XFS_ERRTAG_REFCOUNT_CONTINUE_UPDA
>  XFS_ERRORTAG_ATTR_RW(refcount_finish_one,	XFS_ERRTAG_REFCOUNT_FINISH_ONE);
>  XFS_ERRORTAG_ATTR_RW(bmap_finish_one,	XFS_ERRTAG_BMAP_FINISH_ONE);
>  XFS_ERRORTAG_ATTR_RW(ag_resv_critical,	XFS_ERRTAG_AG_RESV_CRITICAL);
> +XFS_ERRORTAG_ATTR_RW(drop_writes,	XFS_ERRTAG_DROP_WRITES);
>  
>  static struct attribute *xfs_errortag_attrs[] = {
>  	XFS_ERRORTAG_ATTR_LIST(noerror),
> @@ -187,6 +189,7 @@ static struct attribute *xfs_errortag_attrs[] = {
>  	XFS_ERRORTAG_ATTR_LIST(refcount_finish_one),
>  	XFS_ERRORTAG_ATTR_LIST(bmap_finish_one),
>  	XFS_ERRORTAG_ATTR_LIST(ag_resv_critical),
> +	XFS_ERRORTAG_ATTR_LIST(drop_writes),
>  	NULL,
>  };
>  
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index 083a6fc..cd7881f 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -96,7 +96,18 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
>  #define XFS_ERRTAG_REFCOUNT_FINISH_ONE			25
>  #define XFS_ERRTAG_BMAP_FINISH_ONE			26
>  #define XFS_ERRTAG_AG_RESV_CRITICAL			27
> -#define XFS_ERRTAG_MAX					28
> +
> +/*
> + * DEBUG mode instrumentation to test and/or trigger delayed allocation
> + * block killing in the event of failed writes. When enabled, all
> + * buffered writes are silenty dropped and handled as if they failed.
> + * All delalloc blocks in the range of the write (including pre-existing
> + * delalloc blocks!) are tossed as part of the write failure error
> + * handling sequence.
> + */
> +#define XFS_ERRTAG_DROP_WRITES				28
> +
> +#define XFS_ERRTAG_MAX					29
>  
>  /*
>   * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
> @@ -129,6 +140,7 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
>  #define XFS_RANDOM_REFCOUNT_FINISH_ONE			1
>  #define XFS_RANDOM_BMAP_FINISH_ONE			1
>  #define XFS_RANDOM_AG_RESV_CRITICAL			4
> +#define XFS_RANDOM_DROP_WRITES				1
>  
>  #ifdef DEBUG
>  extern int xfs_errortag_init(struct xfs_mount *mp);
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 304b79d..86f1a9f 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1097,7 +1097,7 @@ xfs_file_iomap_end_delalloc(
>  	 * Behave as if the write failed if drop writes is enabled. Set the NEW
>  	 * flag to force delalloc cleanup.
>  	 */
> -	if (xfs_mp_drop_writes(mp)) {
> +	if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_DROP_WRITES)) {
>  		iomap->flags |= IOMAP_F_NEW;
>  		written = 0;
>  	}
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 931e9fc..e0792d0 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -205,16 +205,6 @@ typedef struct xfs_mount {
>  	 */
>  	unsigned int		*m_errortag;
>  	struct xfs_kobj		m_errortag_kobj;
> -
> -	/*
> -	 * DEBUG mode instrumentation to test and/or trigger delayed allocation
> -	 * block killing in the event of failed writes. When enabled, all
> -	 * buffered writes are silenty dropped and handled as if they failed.
> -	 * All delalloc blocks in the range of the write (including pre-existing
> -	 * delalloc blocks!) are tossed as part of the write failure error
> -	 * handling sequence.
> -	 */
> -	bool			m_drop_writes;
>  #endif
>  } xfs_mount_t;
>  
> @@ -333,20 +323,6 @@ xfs_daddr_to_agbno(struct xfs_mount *mp, xfs_daddr_t d)
>  	return (xfs_agblock_t) do_div(ld, mp->m_sb.sb_agblocks);
>  }
>  
> -#ifdef DEBUG
> -static inline bool
> -xfs_mp_drop_writes(struct xfs_mount *mp)
> -{
> -	return mp->m_drop_writes;
> -}
> -#else
> -static inline bool
> -xfs_mp_drop_writes(struct xfs_mount *mp)
> -{
> -	return 0;
> -}
> -#endif
> -
>  /* per-AG block reservation data structures*/
>  enum xfs_ag_resv_type {
>  	XFS_AG_RESV_NONE = 0,
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index b86148a..3e52662 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
> @@ -90,49 +90,7 @@ to_mp(struct kobject *kobject)
>  	return container_of(kobj, struct xfs_mount, m_kobj);
>  }
>  
> -#ifdef DEBUG
> -
> -STATIC ssize_t
> -drop_writes_store(
> -	struct kobject		*kobject,
> -	const char		*buf,
> -	size_t			count)
> -{
> -	struct xfs_mount	*mp = to_mp(kobject);
> -	int			ret;
> -	int			val;
> -
> -	ret = kstrtoint(buf, 0, &val);
> -	if (ret)
> -		return ret;
> -
> -	if (val == 1)
> -		mp->m_drop_writes = true;
> -	else if (val == 0)
> -		mp->m_drop_writes = false;
> -	else
> -		return -EINVAL;
> -
> -	return count;
> -}
> -
> -STATIC ssize_t
> -drop_writes_show(
> -	struct kobject		*kobject,
> -	char			*buf)
> -{
> -	struct xfs_mount	*mp = to_mp(kobject);
> -
> -	return snprintf(buf, PAGE_SIZE, "%d\n", mp->m_drop_writes ? 1 : 0);
> -}
> -XFS_SYSFS_ATTR_RW(drop_writes);
> -
> -#endif /* DEBUG */
> -
>  static struct attribute *xfs_mp_attrs[] = {
> -#ifdef DEBUG
> -	ATTR_LIST(drop_writes),
> -#endif
>  	NULL,
>  };
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 4/4] xfs: convert drop_writes to use the errortag mechanism
  2017-06-21  1:11 [RFC PATCH 0/4] xfs: more configurable error injection Darrick J. Wong
@ 2017-06-21  1:11 ` Darrick J. Wong
  2017-06-21 18:19   ` Brian Foster
  0 siblings, 1 reply; 15+ messages in thread
From: Darrick J. Wong @ 2017-06-21  1:11 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, bfoster

From: Darrick J. Wong <darrick.wong@oracle.com>

We now have enhanced error injection that can control the frequency
with which errors happen, so convert drop_writes to use this.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_error.c |    3 +++
 fs/xfs/xfs_error.h |   14 +++++++++++++-
 fs/xfs/xfs_iomap.c |    2 +-
 fs/xfs/xfs_mount.h |   24 ------------------------
 fs/xfs/xfs_sysfs.c |   42 ------------------------------------------
 5 files changed, 17 insertions(+), 68 deletions(-)


diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 81f260f..40a15e2 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -55,6 +55,7 @@ static unsigned int xfs_errortag_random_default[] = {
 	XFS_RANDOM_REFCOUNT_FINISH_ONE,
 	XFS_RANDOM_BMAP_FINISH_ONE,
 	XFS_RANDOM_AG_RESV_CRITICAL,
+	XFS_RANDOM_DROP_WRITES,
 };
 
 struct xfs_errortag_attr {
@@ -157,6 +158,7 @@ XFS_ERRORTAG_ATTR_RW(refcount_continue_update,	XFS_ERRTAG_REFCOUNT_CONTINUE_UPDA
 XFS_ERRORTAG_ATTR_RW(refcount_finish_one,	XFS_ERRTAG_REFCOUNT_FINISH_ONE);
 XFS_ERRORTAG_ATTR_RW(bmap_finish_one,	XFS_ERRTAG_BMAP_FINISH_ONE);
 XFS_ERRORTAG_ATTR_RW(ag_resv_critical,	XFS_ERRTAG_AG_RESV_CRITICAL);
+XFS_ERRORTAG_ATTR_RW(drop_writes,	XFS_ERRTAG_DROP_WRITES);
 
 static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(noerror),
@@ -187,6 +189,7 @@ static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(refcount_finish_one),
 	XFS_ERRORTAG_ATTR_LIST(bmap_finish_one),
 	XFS_ERRORTAG_ATTR_LIST(ag_resv_critical),
+	XFS_ERRORTAG_ATTR_LIST(drop_writes),
 	NULL,
 };
 
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index 083a6fc..cd7881f 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -96,7 +96,18 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
 #define XFS_ERRTAG_REFCOUNT_FINISH_ONE			25
 #define XFS_ERRTAG_BMAP_FINISH_ONE			26
 #define XFS_ERRTAG_AG_RESV_CRITICAL			27
-#define XFS_ERRTAG_MAX					28
+
+/*
+ * DEBUG mode instrumentation to test and/or trigger delayed allocation
+ * block killing in the event of failed writes. When enabled, all
+ * buffered writes are silenty dropped and handled as if they failed.
+ * All delalloc blocks in the range of the write (including pre-existing
+ * delalloc blocks!) are tossed as part of the write failure error
+ * handling sequence.
+ */
+#define XFS_ERRTAG_DROP_WRITES				28
+
+#define XFS_ERRTAG_MAX					29
 
 /*
  * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -129,6 +140,7 @@ extern void xfs_verifier_error(struct xfs_buf *bp);
 #define XFS_RANDOM_REFCOUNT_FINISH_ONE			1
 #define XFS_RANDOM_BMAP_FINISH_ONE			1
 #define XFS_RANDOM_AG_RESV_CRITICAL			4
+#define XFS_RANDOM_DROP_WRITES				1
 
 #ifdef DEBUG
 extern int xfs_errortag_init(struct xfs_mount *mp);
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 304b79d..86f1a9f 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1097,7 +1097,7 @@ xfs_file_iomap_end_delalloc(
 	 * Behave as if the write failed if drop writes is enabled. Set the NEW
 	 * flag to force delalloc cleanup.
 	 */
-	if (xfs_mp_drop_writes(mp)) {
+	if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_DROP_WRITES)) {
 		iomap->flags |= IOMAP_F_NEW;
 		written = 0;
 	}
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 931e9fc..e0792d0 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -205,16 +205,6 @@ typedef struct xfs_mount {
 	 */
 	unsigned int		*m_errortag;
 	struct xfs_kobj		m_errortag_kobj;
-
-	/*
-	 * DEBUG mode instrumentation to test and/or trigger delayed allocation
-	 * block killing in the event of failed writes. When enabled, all
-	 * buffered writes are silenty dropped and handled as if they failed.
-	 * All delalloc blocks in the range of the write (including pre-existing
-	 * delalloc blocks!) are tossed as part of the write failure error
-	 * handling sequence.
-	 */
-	bool			m_drop_writes;
 #endif
 } xfs_mount_t;
 
@@ -333,20 +323,6 @@ xfs_daddr_to_agbno(struct xfs_mount *mp, xfs_daddr_t d)
 	return (xfs_agblock_t) do_div(ld, mp->m_sb.sb_agblocks);
 }
 
-#ifdef DEBUG
-static inline bool
-xfs_mp_drop_writes(struct xfs_mount *mp)
-{
-	return mp->m_drop_writes;
-}
-#else
-static inline bool
-xfs_mp_drop_writes(struct xfs_mount *mp)
-{
-	return 0;
-}
-#endif
-
 /* per-AG block reservation data structures*/
 enum xfs_ag_resv_type {
 	XFS_AG_RESV_NONE = 0,
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index b86148a..3e52662 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -90,49 +90,7 @@ to_mp(struct kobject *kobject)
 	return container_of(kobj, struct xfs_mount, m_kobj);
 }
 
-#ifdef DEBUG
-
-STATIC ssize_t
-drop_writes_store(
-	struct kobject		*kobject,
-	const char		*buf,
-	size_t			count)
-{
-	struct xfs_mount	*mp = to_mp(kobject);
-	int			ret;
-	int			val;
-
-	ret = kstrtoint(buf, 0, &val);
-	if (ret)
-		return ret;
-
-	if (val == 1)
-		mp->m_drop_writes = true;
-	else if (val == 0)
-		mp->m_drop_writes = false;
-	else
-		return -EINVAL;
-
-	return count;
-}
-
-STATIC ssize_t
-drop_writes_show(
-	struct kobject		*kobject,
-	char			*buf)
-{
-	struct xfs_mount	*mp = to_mp(kobject);
-
-	return snprintf(buf, PAGE_SIZE, "%d\n", mp->m_drop_writes ? 1 : 0);
-}
-XFS_SYSFS_ATTR_RW(drop_writes);
-
-#endif /* DEBUG */
-
 static struct attribute *xfs_mp_attrs[] = {
-#ifdef DEBUG
-	ATTR_LIST(drop_writes),
-#endif
 	NULL,
 };
 


^ permalink raw reply related	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-06-27 16:51 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-23 16:35 [PATCH v2 0/4] xfs: more configurable error injection Darrick J. Wong
2017-06-23 16:35 ` [PATCH 1/4] xfs: make errortag a per-mountpoint structure Darrick J. Wong
2017-06-26 11:10   ` Brian Foster
2017-06-27  7:35   ` Carlos Maiolino
2017-06-23 16:35 ` [PATCH 2/4] xfs: expose errortag knobs via sysfs Darrick J. Wong
2017-06-26 11:10   ` Brian Foster
2017-06-27 10:26   ` Carlos Maiolino
2017-06-23 16:35 ` [PATCH 3/4] xfs: remove unneeded parameter from XFS_TEST_ERROR Darrick J. Wong
2017-06-27 10:33   ` Carlos Maiolino
2017-06-23 16:35 ` [PATCH 4/4] xfs: convert drop_writes to use the errortag mechanism Darrick J. Wong
2017-06-27 10:51   ` Carlos Maiolino
2017-06-27 12:35 ` [PATCH] xfs: replace log_badcrc_factor knob with error injection tag Brian Foster
2017-06-27 16:51   ` Darrick J. Wong
  -- strict thread matches above, loose matches on Subject: below --
2017-06-21  1:11 [RFC PATCH 0/4] xfs: more configurable error injection Darrick J. Wong
2017-06-21  1:11 ` [PATCH 4/4] xfs: convert drop_writes to use the errortag mechanism Darrick J. Wong
2017-06-21 18:19   ` Brian Foster

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.