nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [ndctl PATCH v2 0/6] add an inject-error command to ndctl
@ 2017-10-13  0:09 Vishal Verma
  2017-10-13  0:09 ` [ndctl PATCH v2 1/6] libndctl: add APIs to get scrub count and to wait for a scrub Vishal Verma
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Vishal Verma @ 2017-10-13  0:09 UTC (permalink / raw)
  To: linux-nvdimm

v2:
 patch 1:
   - use poll() instead of a sleep loop when waiting for a scrub. (Dan)
 patch 2 (new):
   - move the acpi nfit specific routines into generic wrappers exported
     by the library. (Dan)
 patch 3:
   - rename 'clear' to 'uninject' use 'block' instead of 'sector' (Dan)
   - cleanup the manpage to remove ACPI/NFIT references.
 patch 4:
   - Add the actual inject-error unit test instead of just test
     boilerplate


These patches add a new command to ndctl for error injection. They are
implemented such that the interface provided to a user is consistent
with the kernel - i.e. all media errors are expected/displayed in terms
of 512 byte sectors. The underlying ACPI DSMs need and provide byte
relative offsets/lengths, but these are converted to 512B sectors for
consistency.

These also update unit tests to use the new error injection commands,
and add two new unit tests - first, to test the error injection commands
themselves, and second, to test BTT error clearing.


Vishal Verma (6):
  libndctl: add APIs to get scrub count and to wait for a scrub
  libndctl: add error injection related interfaces
  ndctl: add an inject-error command
  ndctl/test: add a new unit test for inject-error
  ndctl/test: update existing unit tests to use inject-error
  ndctl/test: add a new unit test for BTT error clearing

 Documentation/ndctl/Makefile.am            |   1 +
 Documentation/ndctl/ndctl-inject-error.txt | 108 ++++++++
 Documentation/ndctl/ndctl.txt              |   1 +
 builtin.h                                  |   1 +
 contrib/ndctl                              |   5 +-
 ndctl/Makefile.am                          |   3 +-
 ndctl/inject-error.c                       | 373 +++++++++++++++++++++++++++
 ndctl/lib/Makefile.am                      |   1 +
 ndctl/lib/inject.c                         | 391 +++++++++++++++++++++++++++++
 ndctl/lib/libndctl.c                       | 147 +++++++----
 ndctl/lib/libndctl.sym                     |  11 +
 ndctl/lib/nfit.c                           |  89 +++++++
 ndctl/lib/private.h                        |  54 ++++
 ndctl/libndctl-nfit.h                      |  10 +
 ndctl/libndctl.h.in                        |  25 ++
 ndctl/ndctl.c                              |   1 +
 test/Makefile.am                           |   4 +-
 test/btt-errors.sh                         | 149 +++++++++++
 test/clear.sh                              |   5 +-
 test/dax-errors.sh                         |   5 +-
 test/daxdev-errors.sh                      |  17 +-
 test/inject-error.sh                       | 120 +++++++++
 util/json.c                                |  26 ++
 util/json.h                                |   3 +
 util/list.h                                |  50 ++++
 util/size.h                                |   1 +
 26 files changed, 1551 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/ndctl/ndctl-inject-error.txt
 create mode 100644 ndctl/inject-error.c
 create mode 100644 ndctl/lib/inject.c
 create mode 100755 test/btt-errors.sh
 create mode 100755 test/inject-error.sh
 create mode 100644 util/list.h

-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ndctl PATCH v2 1/6] libndctl: add APIs to get scrub count and to wait for a scrub
  2017-10-13  0:09 [ndctl PATCH v2 0/6] add an inject-error command to ndctl Vishal Verma
@ 2017-10-13  0:09 ` Vishal Verma
  2017-10-13 23:01   ` Dan Williams
  2017-10-13  0:10 ` [ndctl PATCH v2 2/6] libndctl: add error injection related interfaces Vishal Verma
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Vishal Verma @ 2017-10-13  0:09 UTC (permalink / raw)
  To: linux-nvdimm

The kernel uses sysfs to notify userspace of the number of completed
Address Range Scrubs, as well as any ongoing scrubs. Add libndctl
helpers to get the scrub count, and to wait for an in-progress scrub to
complete.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 ndctl/lib/libndctl.c   | 100 +++++++++++++++++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym |   2 +
 ndctl/lib/private.h    |   1 +
 ndctl/libndctl.h.in    |   2 +
 4 files changed, 105 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 60143d9..c878383 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -10,7 +10,9 @@
  * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
  * more details.
  */
+#include <poll.h>
 #include <stdio.h>
+#include <signal.h>
 #include <stdlib.h>
 #include <stddef.h>
 #include <stdarg.h>
@@ -555,6 +557,7 @@ static void free_bus(struct ndctl_bus *bus, struct list_head *head)
 	free(bus->bus_path);
 	free(bus->bus_buf);
 	free(bus->wait_probe_path);
+	free(bus->scrub_path);
 	free(bus);
 }
 
@@ -785,6 +788,11 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
 	if (!bus->wait_probe_path)
 		goto err_read;
 
+	sprintf(path, "%s/device/nfit/scrub", ctl_base);
+	bus->scrub_path = strdup(path);
+	if (!bus->scrub_path)
+		goto err_read;
+
 	bus->bus_path = parent_dev_path("char", bus->major, bus->minor);
 	if (!bus->bus_path)
 		goto err_dev_path;
@@ -810,6 +818,7 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
  err_dev_path:
  err_read:
 	free(bus->wait_probe_path);
+	free(bus->scrub_path);
 	free(bus->provider);
 	free(bus->bus_buf);
 	free(bus);
@@ -1082,6 +1091,97 @@ NDCTL_EXPORT int ndctl_bus_wait_probe(struct ndctl_bus *bus)
 	return rc < 0 ? -ENXIO : 0;
 }
 
+NDCTL_EXPORT unsigned int ndctl_bus_get_scrub_count(struct ndctl_bus *bus)
+{
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
+	char buf[SYSFS_ATTR_SIZE];
+	unsigned int scrub_count;
+	char in_progress = '\0';
+	int rc;
+
+	rc = sysfs_read_attr(ctx, bus->scrub_path, buf);
+	if (rc < 0)
+		return UINT_MAX;
+
+	rc = sscanf(buf, "%u%c", &scrub_count, &in_progress);
+	if (rc < 0)
+		return UINT_MAX;
+	if (rc == 0) {
+		/* unable to read scrub count */
+		return UINT_MAX;
+	}
+	if (rc >= 1)
+		return scrub_count;
+
+	return UINT_MAX;
+}
+
+/**
+ * ndctl_bus_wait_for_scrub - wait for a scrub to complete
+ * @bus: bus for which to check whether a scrub is in progress
+ *
+ * Upon return this bus has completed any in-progress scrubs. This is
+ * different from ndctl_cmd_ars_in_progress in that the latter checks
+ * the output of an ars_status command to see if the in-progress flag
+ * is set, i.e. provides the firmware's view of whether a scrub is in
+ * progress. ndctl_bus_wait_for_scrub instead checks the kernel's view
+ * of whether a scrub is in progress by looking at the 'scrub' file in
+ * sysfs.
+ */
+NDCTL_EXPORT int ndctl_bus_wait_for_scrub_completion(struct ndctl_bus *bus)
+{
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
+	unsigned int tmo = 120, scrub_count;
+	char buf[SYSFS_ATTR_SIZE];
+	char in_progress = '\0';
+	struct pollfd fds;
+	int fd = 0, rc;
+
+	fd = open(bus->scrub_path, O_RDONLY|O_CLOEXEC);
+	fds.fd = fd;
+	fds.events =  POLLPRI | POLLIN;
+	do {
+		rc = sysfs_read_attr(ctx, bus->scrub_path, buf);
+		if (rc < 0)
+			break;
+
+		rc = sscanf(buf, "%u%c", &scrub_count, &in_progress);
+		if (rc < 0)
+			break;
+		else if (rc <= 1) {
+			/* scrub complete, break successfully */
+			rc = 0;
+			break;
+		} else if (rc == 2 && in_progress == '+') {
+			/* scrub in progress, wait */
+			rc = poll(&fds, 1, tmo);
+			if (rc < 0) {
+				dbg(ctx, "poll error: %d\n", errno);
+				break;
+			} else if (rc == 0) {
+				dbg(ctx, "poll timeout after: %d seconds", tmo);
+				rc = -ENXIO;
+				break;
+			}
+			if (fds.revents & (POLLERR | POLLHUP | POLLNVAL)) {
+				dbg(ctx, "poll error, revents: %d\n",
+					fds.revents);
+				rc = -ENXIO;
+				break;
+			}
+		} else {
+			/* unknown condition */
+			rc = -ENXIO;
+			break;
+		}
+	} while (in_progress);
+
+	dbg(ctx, "bus%d: scrub complete\n", ndctl_bus_get_id(bus));
+	if (fd)
+		close (fd);
+	return rc < 0 ? -ENXIO : 0;
+}
+
 static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
 		const char *devname);
 static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 58db669..4c22c0f 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -286,6 +286,8 @@ global:
 	ndctl_bus_get_region_by_physical_address;
 	ndctl_bus_get_dimm_by_physical_address;
 	ndctl_bus_is_nfit_cmd_supported;
+	ndctl_bus_wait_for_scrub_completion;
+	ndctl_bus_get_scrub_count;
 	ndctl_dimm_read_labels;
 	ndctl_dimm_validate_labels;
 	ndctl_dimm_init_labels;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 2de314c..6b909aa 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -152,6 +152,7 @@ struct ndctl_bus {
 	char *bus_buf;
 	size_t buf_len;
 	char *wait_probe_path;
+	char *scrub_path;
 	unsigned long dsm_mask;
 	unsigned long nfit_dsm_mask;
 };
diff --git a/ndctl/libndctl.h.in b/ndctl/libndctl.h.in
index 42e53c6..a4fbe33 100644
--- a/ndctl/libndctl.h.in
+++ b/ndctl/libndctl.h.in
@@ -113,6 +113,8 @@ unsigned int ndctl_bus_get_revision(struct ndctl_bus *bus);
 unsigned int ndctl_bus_get_id(struct ndctl_bus *bus);
 const char *ndctl_bus_get_provider(struct ndctl_bus *bus);
 int ndctl_bus_wait_probe(struct ndctl_bus *bus);
+int ndctl_bus_wait_for_scrub_completion(struct ndctl_bus *bus);
+unsigned int ndctl_bus_get_scrub_count(struct ndctl_bus *bus);
 
 struct ndctl_dimm;
 struct ndctl_dimm *ndctl_dimm_get_first(struct ndctl_bus *bus);
-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [ndctl PATCH v2 2/6] libndctl: add error injection related interfaces
  2017-10-13  0:09 [ndctl PATCH v2 0/6] add an inject-error command to ndctl Vishal Verma
  2017-10-13  0:09 ` [ndctl PATCH v2 1/6] libndctl: add APIs to get scrub count and to wait for a scrub Vishal Verma
@ 2017-10-13  0:10 ` Vishal Verma
  2017-10-13 23:07   ` Dan Williams
  2017-10-13  0:10 ` [ndctl PATCH v2 3/6] ndctl: add an inject-error command Vishal Verma
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Vishal Verma @ 2017-10-13  0:10 UTC (permalink / raw)
  To: linux-nvdimm

Add interfaces to enable error injection commands. Add nfit specific
error injection helpers in ndctl/lib/nfit.c, and generic wrappers for
them in libndctl.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 ndctl/lib/Makefile.am  |   1 +
 ndctl/lib/inject.c     | 391 +++++++++++++++++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.c   |  47 +-----
 ndctl/lib/libndctl.sym |   9 ++
 ndctl/lib/nfit.c       |  89 +++++++++++
 ndctl/lib/private.h    |  53 +++++++
 ndctl/libndctl-nfit.h  |   2 +
 ndctl/libndctl.h.in    |  23 +++
 8 files changed, 573 insertions(+), 42 deletions(-)
 create mode 100644 ndctl/lib/inject.c

diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
index 9a7734d..3a6024e 100644
--- a/ndctl/lib/Makefile.am
+++ b/ndctl/lib/Makefile.am
@@ -18,6 +18,7 @@ libndctl_la_SOURCES =\
 	../../util/sysfs.c \
 	../../util/sysfs.h \
 	dimm.c \
+	inject.c \
 	libndctl.c
 
 libndctl_la_LIBADD =\
diff --git a/ndctl/lib/inject.c b/ndctl/lib/inject.c
new file mode 100644
index 0000000..2660384
--- /dev/null
+++ b/ndctl/lib/inject.c
@@ -0,0 +1,391 @@
+/*
+ * Copyright (c) 2014-2017, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU Lesser General Public License,
+ * version 2.1, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
+ * more details.
+ */
+#include <stdlib.h>
+#include <limits.h>
+#include <util/list.h>
+#include <util/size.h>
+#include <ndctl/libndctl.h>
+#include <ccan/list/list.h>
+#include <ndctl/libndctl-nfit.h>
+#include <ccan/short_types/short_types.h>
+#include "private.h"
+
+NDCTL_EXPORT int ndctl_bus_has_error_injection(struct ndctl_bus *bus)
+{
+	/* Currently, only nfit buses have error injection */
+	if (!bus || !ndctl_bus_has_nfit(bus))
+		return 0;
+
+	if (ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_ARS_INJECT_SET) &&
+		ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_ARS_INJECT_GET) &&
+		ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_ARS_INJECT_CLEAR))
+		return 1;
+
+	return 0;
+}
+
+NDCTL_EXPORT void ndctl_namespace_get_injection_bounds(
+		struct ndctl_namespace *ndns, unsigned long long *ns_offset,
+		unsigned long long *ns_size)
+{
+	struct ndctl_pfn *pfn = ndctl_namespace_get_pfn(ndns);
+	struct ndctl_dax *dax = ndctl_namespace_get_dax(ndns);
+
+	if (!ns_offset || !ns_size)
+		return;
+
+	if (pfn) {
+		*ns_offset = ndctl_pfn_get_resource(pfn);
+		*ns_size = ndctl_pfn_get_size(pfn);
+		return;
+	} else if (dax) {
+		*ns_offset = ndctl_dax_get_resource(dax);
+		*ns_size = ndctl_dax_get_size(dax);
+		return;
+	}
+	/* raw or btt */
+	*ns_offset = ndctl_namespace_get_resource(ndns);
+	*ns_size = ndctl_namespace_get_size(ndns);
+}
+
+static int block_to_spa_offset(struct ndctl_namespace *ndns,
+		unsigned long long block, unsigned long long count,
+		u64 *offset, u64 *length)
+{
+	struct ndctl_ctx *ctx = ndctl_namespace_get_ctx(ndns);
+	unsigned long long ns_offset, ns_size;
+
+	ndctl_namespace_get_injection_bounds(ndns, &ns_offset, &ns_size);
+	*offset = ns_offset + block * 512;
+	*length = count * 512;
+
+	/* check bounds */
+	if (*offset + *length > ns_offset + ns_size) {
+		dbg(ctx, "Error: block %#llx, count %#llx are out of bounds\n",
+			block, count);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int translate_status(u32 status)
+{
+	switch (status) {
+	case ND_ARS_ERR_INJ_STATUS_NOT_SUPP:
+		return -EOPNOTSUPP;
+	case ND_ARS_ERR_INJ_STATUS_INVALID_PARAM:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+NDCTL_EXPORT int ndctl_namespace_inject_error(struct ndctl_namespace *ndns,
+		unsigned long long block, unsigned long long count, bool notify)
+{
+	struct ndctl_bus *bus = ndctl_namespace_get_bus(ndns);
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
+	struct nd_cmd_ars_err_inj *err_inj;
+	struct nd_cmd_pkg *pkg;
+	struct ndctl_cmd *cmd;
+	int rc = -EOPNOTSUPP;
+
+	if (!ndctl_bus_has_error_injection(bus))
+		return -EOPNOTSUPP;
+
+	if (ndctl_bus_has_nfit(bus)) {
+		u64 offset, length;
+
+		rc = block_to_spa_offset(ndns, block, count, &offset, &length);
+		if (rc)
+			return rc;
+		cmd = ndctl_bus_cmd_new_err_inj(bus);
+		if (!cmd)
+			return -ENOMEM;
+
+		pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+		err_inj = (struct nd_cmd_ars_err_inj *)&pkg->nd_payload[0];
+		err_inj->err_inj_spa_range_base = offset;
+		err_inj->err_inj_spa_range_length = length;
+		if (notify)
+			err_inj->err_inj_options |=
+				(1 << ND_ARS_ERR_INJ_OPT_NOTIFY);
+
+		rc = ndctl_cmd_submit(cmd);
+		if (rc) {
+			dbg(ctx, "Error submitting command: %d\n", rc);
+			goto out;
+		}
+		rc = translate_status(err_inj->status);
+ out:
+		ndctl_cmd_unref(cmd);
+	}
+	return rc;
+}
+
+NDCTL_EXPORT int ndctl_namespace_uninject_error(struct ndctl_namespace *ndns,
+		unsigned long long block, unsigned long long count)
+{
+	struct ndctl_bus *bus = ndctl_namespace_get_bus(ndns);
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
+	struct nd_cmd_ars_err_inj_clr *err_inj_clr;
+	struct nd_cmd_pkg *pkg;
+	struct ndctl_cmd *cmd;
+	int rc = -EOPNOTSUPP;
+
+	if (!ndctl_bus_has_error_injection(bus))
+		return -EOPNOTSUPP;
+
+	if (ndctl_bus_has_nfit(bus)) {
+		u64 offset, length;
+
+		rc = block_to_spa_offset(ndns, block, count, &offset, &length);
+		if (rc)
+			return rc;
+		cmd = ndctl_bus_cmd_new_err_inj_clr(bus);
+		if (!cmd)
+			return -ENOMEM;
+
+		pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+		err_inj_clr =
+			(struct nd_cmd_ars_err_inj_clr *)&pkg->nd_payload[0];
+		err_inj_clr->err_inj_clr_spa_range_base = offset;
+		err_inj_clr->err_inj_clr_spa_range_length = length;
+
+		rc = ndctl_cmd_submit(cmd);
+		if (rc) {
+			dbg(ctx, "Error submitting command: %d\n", rc);
+			goto out;
+		}
+		rc = translate_status(err_inj_clr->status);
+ out:
+		ndctl_cmd_unref(cmd);
+	}
+	return rc;
+}
+
+static int bb_add_record(struct list_head *h, u64 block, u64 count)
+{
+	struct ndctl_bb *bb, *bb_iter, *bb_next, *bb_prev;
+	int merged = 0;
+
+	bb = calloc(1, sizeof(*bb));
+	if (bb == NULL)
+		return -ENOMEM;
+	bb->block = block;
+	bb->count = count;
+
+	if (list_empty(h)) {
+		list_add(h, &bb->list);
+		return 0;
+	}
+
+	/* add 'bb' to the list such that it remains sorted */
+	list_for_each(h, bb_iter, list) {
+		/* Find insertion point */
+		bb_prev = list_prev(h, bb_iter, list);
+		bb_next = list_next(h, bb_iter, list);
+
+		if (bb_prev == NULL) {
+			/* bb_iter is the first entry */
+			if (bb->block < bb_iter->block) {
+				list_add(h, &bb->list);
+				break;
+			}
+		}
+		if (bb_next == NULL) {
+			/*
+			 * bb_iter is the last entry. If we've reached here,
+			 * the only option is to add to the tail as the case
+			 * for "tail - 1" should have been covered by the
+			 * following checks for the previous iteration.
+			 */
+			list_add_tail(h, &bb->list);
+			break;
+		}
+		/* Add to the left of bb_iter */
+		if (bb->block <= bb_iter->block) {
+			if (bb_prev && (bb_prev->block <= bb->block)) {
+				list_add_after(h, &bb_prev->list, &bb->list);
+				break;
+			}
+		}
+		/* Add to the right of bb_iter */
+		if (bb_iter->block <= bb->block) {
+			if (bb_next && (bb->block <= bb_next->block)) {
+				list_add_after(h, &bb_iter->list, &bb->list);
+				break;
+			}
+		}
+	}
+
+	/* second pass over the list looking for mergeable entries */
+	list_for_each(h, bb_iter, list) {
+		u64 cur_end, next_end, cur_start, next_start;
+
+		/*
+		 * test for merges in a loop here because one addition can
+		 * potentially have a cascading merge effect on multiple
+		 * remaining entries
+		 */
+		do {
+			/* reset the merged flag */
+			merged = 0;
+
+			bb_next = list_next(h, bb_iter, list);
+			if (bb_next == NULL)
+				break;
+
+			cur_start = bb_iter->block;
+			next_start = bb_next->block;
+			cur_end = bb_iter->block + bb_iter->count - 1;
+			next_end = bb_next->block + bb_next->count - 1;
+
+			if (cur_end >= next_start) {
+				/* overlapping records that can be merged */
+				if (next_end > cur_end) {
+					/* next extends cur */
+					bb_iter->count =
+						next_end - cur_start + 1;
+				} else {
+					/* next is contained in cur */
+					;
+				}
+				/* next is now redundant */
+				list_del_from(h, &bb_next->list);
+				free(bb_next);
+				merged = 1;
+				continue;
+			}
+			if (next_start == cur_end + 1) {
+				/* adjoining records that can be merged */
+				bb_iter->count = next_end - cur_start + 1;
+				list_del_from(h, &bb_next->list);
+				free(bb_next);
+				merged = 1;
+				continue;
+			}
+		} while (merged);
+	}
+
+	return 0;
+}
+
+static int injection_status_to_bb(struct ndctl_namespace *ndns,
+		struct nd_cmd_ars_err_inj_stat *stat, u64 ns_spa, u64 ns_size)
+{
+	unsigned int i;
+	int rc = 0;
+
+	for (i = 0; i < stat->inj_err_rec_count; i++) {
+		u64 ns_off, rec_off, rec_len;
+		u64 block, count, start_pad;
+
+		rec_off = stat->record[i].err_inj_stat_spa_range_base;
+		rec_len = stat->record[i].err_inj_stat_spa_range_length;
+		/* discard ranges outside the provided namespace */
+		if (rec_off < ns_spa)
+			continue;
+		if (rec_off >= ns_spa + ns_size)
+			continue;
+
+		/* translate spa offset to namespace offset */
+		ns_off = rec_off - ns_spa;
+
+		block = ALIGN_DOWN(ns_off, 512)/512;
+		start_pad = ns_off - (block * 512);
+		count = ALIGN(start_pad + rec_len, 512)/512;
+		rc = bb_add_record(&ndns->injected_bb, block, count);
+		if (rc)
+			break;
+	}
+	return rc;
+}
+
+NDCTL_EXPORT int ndctl_namespace_injection_status(struct ndctl_namespace *ndns)
+{
+	struct ndctl_bus *bus = ndctl_namespace_get_bus(ndns);
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
+	struct nd_cmd_ars_err_inj_stat *err_inj_stat;
+	unsigned long long ns_offset, ns_size;
+	int rc = -EOPNOTSUPP, buf_size;
+	struct ndctl_cmd *cmd = NULL;
+	struct nd_cmd_pkg *pkg;
+
+	if (!ndctl_bus_has_error_injection(bus))
+		return -EOPNOTSUPP;
+
+	if (ndctl_bus_has_nfit(bus)) {
+		ndctl_namespace_get_injection_bounds(ndns, &ns_offset,
+			&ns_size);
+
+		cmd = ndctl_bus_cmd_new_ars_cap(bus, ns_offset, ns_size);
+		rc = ndctl_cmd_submit(cmd);
+		if (rc) {
+			dbg(ctx, "Error submitting ars_cap: %d\n", rc);
+			goto out;
+		}
+		buf_size = ndctl_cmd_ars_cap_get_size(cmd);
+		if (buf_size == 0) {
+			dbg(ctx, "Got an invalid max_ars_out from ars_cap\n");
+			rc = -EINVAL;
+			goto out;
+		}
+		ndctl_cmd_unref(cmd);
+
+		cmd = ndctl_bus_cmd_new_err_inj_stat(bus, buf_size);
+		if (!cmd)
+			return -ENOMEM;
+
+		pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+		err_inj_stat =
+			(struct nd_cmd_ars_err_inj_stat *)&pkg->nd_payload[0];
+
+		rc = ndctl_cmd_submit(cmd);
+		if (rc) {
+			dbg(ctx, "Error submitting command: %d\n", rc);
+			goto out;
+		}
+		rc = injection_status_to_bb(ndns, err_inj_stat,
+			ns_offset, ns_size);
+	}
+
+ out:
+	ndctl_cmd_unref(cmd);
+	return rc;
+}
+
+NDCTL_EXPORT struct ndctl_bb *ndctl_namespace_injection_get_first_bb(
+		struct ndctl_namespace *ndns)
+{
+	return list_top(&ndns->injected_bb, struct ndctl_bb, list);
+}
+
+NDCTL_EXPORT struct ndctl_bb *ndctl_namespace_injection_get_next_bb(
+		struct ndctl_namespace *ndns, struct ndctl_bb *bb)
+{
+	return list_next(&ndns->injected_bb, bb, list);
+}
+
+NDCTL_EXPORT unsigned long long ndctl_bb_get_block(struct ndctl_bb *bb)
+{
+	if (bb)
+		return bb->block;
+	return ULLONG_MAX;
+}
+
+NDCTL_EXPORT unsigned long long ndctl_bb_get_count(struct ndctl_bb *bb)
+{
+	if (bb)
+		return bb->count;
+	return ULLONG_MAX;
+}
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index c878383..511e965 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -157,48 +157,6 @@ struct ndctl_region {
 };
 
 /**
- * struct ndctl_lbasize - lbasize info for btt and blk-namespace devices
- * @select: currently selected sector_size
- * @supported: possible sector_size options
- * @num: number of entries in @supported
- */
-struct ndctl_lbasize {
-	int select;
-	unsigned int *supported;
-	int num;
-};
-
-/**
- * struct ndctl_namespace - device claimed by the nd_blk or nd_pmem driver
- * @module: kernel module
- * @type: integer nd-bus device-type
- * @type_name: 'namespace_io', 'namespace_pmem', or 'namespace_block'
- * @namespace_path: devpath for namespace device
- * @bdev: associated block_device of a namespace
- * @size: unsigned
- * @numa_node: numa node attribute
- *
- * A 'namespace' is the resulting device after region-aliasing and
- * label-parsing is resolved.
- */
-struct ndctl_namespace {
-	struct kmod_module *module;
-	struct ndctl_region *region;
-	struct list_node list;
-	char *ndns_path;
-	char *ndns_buf;
-	char *bdev;
-	int type, id, buf_len, raw_mode;
-	int generation;
-	unsigned long long resource, size;
-	enum ndctl_namespace_mode enforce_mode;
-	char *alt_name;
-	uuid_t uuid;
-	struct ndctl_lbasize lbasize;
-	int numa_node;
-};
-
-/**
  * struct ndctl_btt - stacked block device provided sector atomicity
  * @module: kernel module (nd_btt)
  * @lbasize: sector size info
@@ -391,8 +349,12 @@ NDCTL_EXPORT struct ndctl_ctx *ndctl_ref(struct ndctl_ctx *ctx)
 
 static void free_namespace(struct ndctl_namespace *ndns, struct list_head *head)
 {
+	struct ndctl_bb *bb, *next;
+
 	if (head)
 		list_del_from(head, &ndns->list);
+	list_for_each_safe(&ndns->injected_bb, bb, next, list)
+		free(bb);
 	free(ndns->lbasize.supported);
 	free(ndns->ndns_path);
 	free(ndns->ndns_buf);
@@ -2955,6 +2917,7 @@ static void *add_namespace(void *parent, int id, const char *ndns_base)
 	ndns->id = id;
 	ndns->region = region;
 	ndns->generation = region->generation;
+	list_head_init(&ndns->injected_bb);
 
 	sprintf(path, "%s/nstype", ndns_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 4c22c0f..eeed947 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -288,6 +288,7 @@ global:
 	ndctl_bus_is_nfit_cmd_supported;
 	ndctl_bus_wait_for_scrub_completion;
 	ndctl_bus_get_scrub_count;
+	ndctl_bus_has_error_injection;
 	ndctl_dimm_read_labels;
 	ndctl_dimm_validate_labels;
 	ndctl_dimm_init_labels;
@@ -295,4 +296,12 @@ global:
 	ndctl_mapping_get_position;
 	ndctl_namespace_set_enforce_mode;
 	ndctl_namespace_get_enforce_mode;
+	ndctl_namespace_get_injection_bounds;
+	ndctl_namespace_inject_error;
+	ndctl_namespace_uninject_error;
+	ndctl_namespace_injection_status;
+	ndctl_namespace_injection_get_first_bb;
+	ndctl_namespace_injection_get_next_bb;
+	ndctl_bb_get_block;
+	ndctl_bb_get_count;
 } LIBNDCTL_3;
diff --git a/ndctl/lib/nfit.c b/ndctl/lib/nfit.c
index f3806a4..fb6af32 100644
--- a/ndctl/lib/nfit.c
+++ b/ndctl/lib/nfit.c
@@ -143,3 +143,92 @@ int ndctl_bus_nfit_translate_spa(struct ndctl_bus *bus,
 
 	return rc;
 }
+
+struct ndctl_cmd *ndctl_bus_cmd_new_err_inj(struct ndctl_bus *bus)
+{
+	struct nd_cmd_ars_err_inj *err_inj;
+	size_t size, cmd_length;
+	struct nd_cmd_pkg *pkg;
+	struct ndctl_cmd *cmd;
+
+	cmd_length = sizeof(struct nd_cmd_ars_err_inj);
+	size = sizeof(*cmd) + sizeof(*pkg) + cmd_length;
+	cmd = calloc(1, size);
+	if (!cmd)
+		return NULL;
+
+	cmd->bus = bus;
+	ndctl_cmd_ref(cmd);
+	cmd->type = ND_CMD_CALL;
+	cmd->size = size;
+	cmd->status = 1;
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	pkg->nd_command = NFIT_CMD_ARS_INJECT_SET;
+	pkg->nd_size_in = (2 * sizeof(u64)) + sizeof(u32);
+	pkg->nd_size_out = cmd_length;
+	pkg->nd_fw_size = cmd_length;
+	err_inj = (struct nd_cmd_ars_err_inj *)&pkg->nd_payload[0];
+	cmd->firmware_status = &err_inj->status;
+
+	return cmd;
+}
+
+struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_clr(struct ndctl_bus *bus)
+{
+	struct nd_cmd_ars_err_inj_clr *err_inj_clr;
+	size_t size, cmd_length;
+	struct nd_cmd_pkg *pkg;
+	struct ndctl_cmd *cmd;
+
+	cmd_length = sizeof(struct nd_cmd_ars_err_inj_clr);
+	size = sizeof(*cmd) + sizeof(*pkg) + cmd_length;
+	cmd = calloc(1, size);
+	if (!cmd)
+		return NULL;
+
+	cmd->bus = bus;
+	ndctl_cmd_ref(cmd);
+	cmd->type = ND_CMD_CALL;
+	cmd->size = size;
+	cmd->status = 1;
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	pkg->nd_command = NFIT_CMD_ARS_INJECT_CLEAR;
+	pkg->nd_size_in = 2 * sizeof(u64);
+	pkg->nd_size_out = cmd_length;
+	pkg->nd_fw_size = cmd_length;
+	err_inj_clr = (struct nd_cmd_ars_err_inj_clr *)&pkg->nd_payload[0];
+	cmd->firmware_status = &err_inj_clr->status;
+
+	return cmd;
+}
+
+struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_stat(struct ndctl_bus *bus,
+	u32 buf_size)
+{
+	struct nd_cmd_ars_err_inj_stat *err_inj_stat;
+	size_t size, cmd_length;
+	struct nd_cmd_pkg *pkg;
+	struct ndctl_cmd *cmd;
+
+
+	cmd_length = sizeof(struct nd_cmd_ars_err_inj_stat);
+	size = sizeof(*cmd) + sizeof(*pkg) + cmd_length + buf_size;
+	cmd = calloc(1, size);
+	if (!cmd)
+		return NULL;
+
+	cmd->bus = bus;
+	ndctl_cmd_ref(cmd);
+	cmd->type = ND_CMD_CALL;
+	cmd->size = size;
+	cmd->status = 1;
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	pkg->nd_command = NFIT_CMD_ARS_INJECT_GET;
+	pkg->nd_size_in = cmd_length;
+	pkg->nd_size_out = cmd_length + buf_size;
+	pkg->nd_fw_size = cmd_length + buf_size;
+	err_inj_stat = (struct nd_cmd_ars_err_inj_stat *)&pkg->nd_payload[0];
+	cmd->firmware_status = &err_inj_stat->status;
+
+	return cmd;
+}
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 6b909aa..29505fd 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -158,6 +158,49 @@ struct ndctl_bus {
 };
 
 /**
+ * struct ndctl_lbasize - lbasize info for btt and blk-namespace devices
+ * @select: currently selected sector_size
+ * @supported: possible sector_size options
+ * @num: number of entries in @supported
+ */
+struct ndctl_lbasize {
+	int select;
+	unsigned int *supported;
+	int num;
+};
+
+/**
+ * struct ndctl_namespace - device claimed by the nd_blk or nd_pmem driver
+ * @module: kernel module
+ * @type: integer nd-bus device-type
+ * @type_name: 'namespace_io', 'namespace_pmem', or 'namespace_block'
+ * @namespace_path: devpath for namespace device
+ * @bdev: associated block_device of a namespace
+ * @size: unsigned
+ * @numa_node: numa node attribute
+ *
+ * A 'namespace' is the resulting device after region-aliasing and
+ * label-parsing is resolved.
+ */
+struct ndctl_namespace {
+	struct kmod_module *module;
+	struct ndctl_region *region;
+	struct list_node list;
+	char *ndns_path;
+	char *ndns_buf;
+	char *bdev;
+	int type, id, buf_len, raw_mode;
+	int generation;
+	unsigned long long resource, size;
+	enum ndctl_namespace_mode enforce_mode;
+	char *alt_name;
+	uuid_t uuid;
+	struct ndctl_lbasize lbasize;
+	int numa_node;
+	struct list_head injected_bb;
+};
+
+/**
  * struct ndctl_cmd - device-specific-method (_DSM ioctl) container
  * @dimm: set if the command is relative to a dimm, NULL otherwise
  * @bus: set if the command is relative to a bus (like ARS), NULL otherwise
@@ -217,6 +260,12 @@ struct ndctl_cmd {
 	};
 };
 
+struct ndctl_bb {
+	u64 block;
+	u64 count;
+	struct list_node list;
+};
+
 struct ndctl_smart_ops {
 	struct ndctl_cmd *(*new_smart)(struct ndctl_dimm *);
 	unsigned int (*smart_get_flags)(struct ndctl_cmd *);
@@ -280,5 +329,9 @@ static inline int check_kmod(struct kmod_ctx *kmod_ctx)
 
 int ndctl_bus_nfit_translate_spa(struct ndctl_bus *bus, unsigned long long addr,
 		unsigned int *handle, unsigned long long *dpa);
+struct ndctl_cmd *ndctl_bus_cmd_new_err_inj(struct ndctl_bus *bus);
+struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_clr(struct ndctl_bus *bus);
+struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_stat(struct ndctl_bus *bus,
+	u32 buf_size);
 
 #endif /* _LIBNDCTL_PRIVATE_H_ */
diff --git a/ndctl/libndctl-nfit.h b/ndctl/libndctl-nfit.h
index ae9e5ce..6b730aa 100644
--- a/ndctl/libndctl-nfit.h
+++ b/ndctl/libndctl-nfit.h
@@ -16,6 +16,8 @@
 #ifndef __LIBNDCTL_NFIT_H__
 #define __LIBNDCTL_NFIT_H__
 
+#include <linux/types.h>
+
 /*
  * libndctl-nfit.h : definitions for NFIT related commands/functions.
  */
diff --git a/ndctl/libndctl.h.in b/ndctl/libndctl.h.in
index a4fbe33..5ecd07b 100644
--- a/ndctl/libndctl.h.in
+++ b/ndctl/libndctl.h.in
@@ -115,6 +115,7 @@ const char *ndctl_bus_get_provider(struct ndctl_bus *bus);
 int ndctl_bus_wait_probe(struct ndctl_bus *bus);
 int ndctl_bus_wait_for_scrub_completion(struct ndctl_bus *bus);
 unsigned int ndctl_bus_get_scrub_count(struct ndctl_bus *bus);
+int ndctl_bus_has_error_injection(struct ndctl_bus *bus);
 
 struct ndctl_dimm;
 struct ndctl_dimm *ndctl_dimm_get_first(struct ndctl_bus *bus);
@@ -541,6 +542,28 @@ int ndctl_namespace_set_sector_size(struct ndctl_namespace *ndns,
 int ndctl_namespace_get_raw_mode(struct ndctl_namespace *ndns);
 int ndctl_namespace_set_raw_mode(struct ndctl_namespace *ndns, int raw_mode);
 int ndctl_namespace_get_numa_node(struct ndctl_namespace *ndns);
+void ndctl_namespace_get_injection_bounds(
+		struct ndctl_namespace *ndns, unsigned long long *ns_offset,
+		unsigned long long *ns_size);
+int ndctl_namespace_inject_error(struct ndctl_namespace *ndns,
+		unsigned long long block, unsigned long long count,
+		bool notify);
+int ndctl_namespace_uninject_error(struct ndctl_namespace *ndns,
+		unsigned long long block, unsigned long long count);
+int ndctl_namespace_injection_status(struct ndctl_namespace *ndns);
+
+struct ndctl_bb;
+unsigned long long ndctl_bb_get_block(struct ndctl_bb *bb);
+unsigned long long ndctl_bb_get_count(struct ndctl_bb *bb);
+struct ndctl_bb *ndctl_namespace_injection_get_first_bb(
+		struct ndctl_namespace *ndns);
+struct ndctl_bb *ndctl_namespace_injection_get_next_bb(
+		struct ndctl_namespace *ndns, struct ndctl_bb *bb);
+#define ndctl_namespace_bb_foreach(ndns, bb) \
+        for (bb = ndctl_namespace_injection_get_first_bb(ndns); \
+             bb != NULL; \
+             bb = ndctl_namespace_injection_get_next_bb(ndns, bb))
+
 struct ndctl_btt;
 struct ndctl_btt *ndctl_btt_get_first(struct ndctl_region *region);
 struct ndctl_btt *ndctl_btt_get_next(struct ndctl_btt *btt);
-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [ndctl PATCH v2 3/6] ndctl: add an inject-error command
  2017-10-13  0:09 [ndctl PATCH v2 0/6] add an inject-error command to ndctl Vishal Verma
  2017-10-13  0:09 ` [ndctl PATCH v2 1/6] libndctl: add APIs to get scrub count and to wait for a scrub Vishal Verma
  2017-10-13  0:10 ` [ndctl PATCH v2 2/6] libndctl: add error injection related interfaces Vishal Verma
@ 2017-10-13  0:10 ` Vishal Verma
  2017-10-13 22:24   ` Dan Williams
  2017-10-13  0:10 ` [ndctl PATCH v2 4/6] ndctl/test: add a new unit test for inject-error Vishal Verma
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Vishal Verma @ 2017-10-13  0:10 UTC (permalink / raw)
  To: linux-nvdimm

Add an inject-error command to ndctl. This uses the error injection DSMs
in ACPI6.2 to provide a generic error injection and management
interface. Once can inject errors, and view as well as clear injected
errors using these commands.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 Documentation/ndctl/Makefile.am            |   1 +
 Documentation/ndctl/ndctl-inject-error.txt | 108 +++++++++
 Documentation/ndctl/ndctl.txt              |   1 +
 builtin.h                                  |   1 +
 contrib/ndctl                              |   5 +-
 ndctl/Makefile.am                          |   3 +-
 ndctl/inject-error.c                       | 373 +++++++++++++++++++++++++++++
 ndctl/libndctl-nfit.h                      |   8 +
 ndctl/ndctl.c                              |   1 +
 util/json.c                                |  26 ++
 util/json.h                                |   3 +
 util/list.h                                |  50 ++++
 util/size.h                                |   1 +
 13 files changed, 579 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ndctl/ndctl-inject-error.txt
 create mode 100644 ndctl/inject-error.c
 create mode 100644 util/list.h

diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index 229d908..615baf0 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -30,6 +30,7 @@ man1_MANS = \
 	ndctl-create-namespace.1 \
 	ndctl-destroy-namespace.1 \
 	ndctl-check-namespace.1 \
+	ndctl-inject-error.1 \
 	ndctl-list.1
 
 CLEANFILES = $(man1_MANS)
diff --git a/Documentation/ndctl/ndctl-inject-error.txt b/Documentation/ndctl/ndctl-inject-error.txt
new file mode 100644
index 0000000..a7a8959
--- /dev/null
+++ b/Documentation/ndctl/ndctl-inject-error.txt
@@ -0,0 +1,108 @@
+ndctl-inject-error(1)
+=====================
+
+NAME
+----
+ndctl-inject-error - inject media errors at a namespace offset
+
+SYNOPSIS
+--------
+[verse]
+'ndctl inject-error' <namespace> [<options>]
+
+include::namespace-description.txt[]
+
+ndctl-inject-error can be used to ask the platform to simulate media errors
+in the NVDIMM address space to aid debugging and development of features
+related to error handling.
+
+WARNING: These commands are DANGEROUS and can cause data loss. They are
+only provided for testing and debugging purposes.
+
+EXAMPLES
+--------
+
+Inject errors in namespace0.0 at block 12 for a 2 blocks (i.e. 12, 13)
+[verse]
+ndctl inject-error --block=12 --count=2 namespace0.0
+
+Check status of injected errors on namespace0.0
+[verse]
+ndctl inject-error --status namespace0.0
+
+Uninject errors at block 12 for 2 blocks on namespace0.0
+[verse]
+ndctl inject-error --uninject --block=12 --count=2 namespace0.0
+
+OPTIONS
+-------
+-B::
+--block=::
+	Namespace block offset in 512 byte sized blocks where the error is
+	to be injected.
+
+	NOTE: The offset is interpreted in different ways based on the "mode"
+	of the namespace. For "raw" mode, the offset is the base namespace
+	offset. For "memory" mode (i.e. a "pfn" namespace), the offset is
+	relative to the user-visible part of the namespace, and the offset
+	introduced by the kernel's metadata will be accounted for. For a
+	"sector" mode namespace (i.e. a "BTT" namespace), the offset is
+	relative to the base namespace, as the BTT translation details are
+	internal to the kernel, and can't be accounted for while injecting
+	errors.
+
+-n::
+--count=::
+	Number of blocks to inject as errors. This is also in terms of fixed,
+	512 byte blocks.
+
+-d::
+--uninject::
+	This option will ask the platform to remove any injected errors for the
+	specified block offset, and count.
+
+	WARNING: This will not clear the kernel's internal badblock tracking,
+	those can only be cleared by doing a write to the affected locations.
+	Hence use the --clear option only if you know exactly what you are
+	doing. For normal usage, injected errors should only be cleared by
+	doing writes. Do not expect have the original data intact after
+	injecting an error, and clearing it using --clear - it will be lost,
+	as the only "real" way to clear the error location is to write to it
+	or zero it (truncate/hole-punch).
+
+-t::
+--status::
+	This option will retrieve the status of injected errors. Note that
+	this will not retrieve all known/latent errors (i.e. non injected
+	ones), and is NOT equivalent to performing an Address Range Scrub.
+
+-N::
+--no-notify::
+	This option is only valid when injecting errors. By default, the error
+	inject command and will ask platform firmware to trigger a notification
+	in the kernel, asking it to update its state of known errors.
+	With this option, the error will still be injected, the kernel will not
+	get a notification, and the error will appear as a latent media error
+	when the location is accessed. If the platform firmware does not
+	support this feature, this will have no effect.
+
+-v::
+--verbose::
+	Emit debug messages for the error injection process
+
+include::human-option.txt[]
+
+-r::
+--region=::
+include::xable-region-options.txt[]
+
+COPYRIGHT
+---------
+Copyright (c) 2016 - 2017, Intel Corporation. License GPLv2: GNU GPL
+version 2 <http://gnu.org/licenses/gpl.html>.  This is free software:
+you are free to change and redistribute it.  There is NO WARRANTY, to
+the extent permitted by law.
+
+SEE ALSO
+--------
+linkndctl:ndctl-list[1],
diff --git a/Documentation/ndctl/ndctl.txt b/Documentation/ndctl/ndctl.txt
index b02f613..b2e2ab9 100644
--- a/Documentation/ndctl/ndctl.txt
+++ b/Documentation/ndctl/ndctl.txt
@@ -50,6 +50,7 @@ linkndctl:ndctl-enable-namespace[1],
 linkndctl:ndctl-disable-namespace[1],
 linkndctl:ndctl-zero-labels[1],
 linkndctl:ndctl-read-labels[1],
+linkndctl:ndctl-inject-error[1],
 linkndctl:ndctl-list[1],
 https://www.kernel.org/doc/Documentation/nvdimm/nvdimm.txt[LIBNVDIMM
 Overview],
diff --git a/builtin.h b/builtin.h
index 5c8b611..5e1b7ef 100644
--- a/builtin.h
+++ b/builtin.h
@@ -35,6 +35,7 @@ int cmd_read_labels(int argc, const char **argv, void *ctx);
 int cmd_write_labels(int argc, const char **argv, void *ctx);
 int cmd_init_labels(int argc, const char **argv, void *ctx);
 int cmd_check_labels(int argc, const char **argv, void *ctx);
+int cmd_inject_error(int argc, const char **argv, void *ctx);
 int cmd_list(int argc, const char **argv, void *ctx);
 #ifdef ENABLE_TEST
 int cmd_test(int argc, const char **argv, void *ctx);
diff --git a/contrib/ndctl b/contrib/ndctl
index c7d1b67..86718eb 100755
--- a/contrib/ndctl
+++ b/contrib/ndctl
@@ -91,7 +91,7 @@ __ndctlcomp()
 
 	COMPREPLY=( $( compgen -W "$1" -- "$2" ) )
 	for cword in "${COMPREPLY[@]}"; do
-		if [[ "$cword" == @(--bus|--region|--type|--mode|--size|--dimm|--reconfig|--uuid|--name|--sector-size|--map|--namespace|--input|--output|--label-version|--align) ]]; then
+		if [[ "$cword" == @(--bus|--region|--type|--mode|--size|--dimm|--reconfig|--uuid|--name|--sector-size|--map|--namespace|--input|--output|--label-version|--align|--block|--count) ]]; then
 			COMPREPLY[$i]="${cword}="
 		else
 			COMPREPLY[$i]="${cword} "
@@ -257,6 +257,9 @@ __ndctl_comp_non_option_args()
 	zero-labels)
 		opts="$(__ndctl_get_dimms -i) all"
 		;;
+	inject-error)
+		opts="$(__ndctl_get_ns -i)"
+		;;
 	*)
 		return
 		;;
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index d346c04..a0cf500 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -11,7 +11,8 @@ ndctl_SOURCES = ndctl.c \
 		 ../util/log.c \
 		list.c \
 		test.c \
-		../util/json.c
+		../util/json.c \
+		inject-error.c
 
 if ENABLE_SMART
 ndctl_SOURCES += util/json-smart.c
diff --git a/ndctl/inject-error.c b/ndctl/inject-error.c
new file mode 100644
index 0000000..4c45902
--- /dev/null
+++ b/ndctl/inject-error.c
@@ -0,0 +1,373 @@
+/*
+ * Copyright(c) 2015-2017 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+#include <stdio.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <limits.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <util/log.h>
+#include <util/size.h>
+#include <util/json.h>
+#include <json-c/json.h>
+#include <util/filter.h>
+#include <ndctl/libndctl.h>
+#include <util/parse-options.h>
+#include <ccan/array_size/array_size.h>
+#include <ccan/short_types/short_types.h>
+#ifdef HAVE_NDCTL_H
+#include <linux/ndctl.h>
+#else
+#include <ndctl.h>
+#endif
+
+#include "private.h"
+#include <builtin.h>
+#include <test.h>
+
+static bool verbose;
+static struct parameters {
+	const char *bus;
+	const char *region;
+	const char *namespace;
+	const char *block;
+	const char *count;
+	bool clear;
+	bool status;
+	bool no_notify;
+	bool human;
+} param;
+
+static struct inject_ctx {
+	u64 block;
+	u64 count;
+	unsigned int op_mask;
+	unsigned long flags;
+	bool notify;
+} ictx;
+
+#define BASE_OPTIONS() \
+OPT_STRING('b', "bus", &param.bus, "bus-id", \
+	"limit namespace to a bus with an id or provider of <bus-id>"), \
+OPT_STRING('r', "region", &param.region, "region-id", \
+	"limit namespace to a region with an id or name of <region-id>"), \
+OPT_BOOLEAN('v', "verbose", &verbose, "emit extra debug messages to stderr")
+
+#define INJECT_OPTIONS() \
+OPT_STRING('B', "block", &param.block, "namespace block offset (512B)", \
+	"specify the block at which to (un)inject the error"), \
+OPT_STRING('n', "count", &param.count, "count", \
+	"specify the number of blocks of errors to (un)inject"), \
+OPT_BOOLEAN('d', "uninject", &param.clear, \
+	"un-inject a previously injected error"), \
+OPT_BOOLEAN('t', "status", &param.status, "get error injection status"), \
+OPT_BOOLEAN('N', "no-notify", &param.no_notify, "firmware should not notify OS"), \
+OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats ")
+
+static const struct option inject_options[] = {
+	BASE_OPTIONS(),
+	INJECT_OPTIONS(),
+	OPT_END(),
+};
+
+enum {
+	OP_INJECT = 0,
+	OP_CLEAR,
+	OP_STATUS,
+};
+
+static int inject_init(void)
+{
+	if (!param.clear && !param.status) {
+		ictx.op_mask |= 1 << OP_INJECT;
+		ictx.notify = true;
+		if (param.no_notify)
+			ictx.notify = false;
+	}
+	if (param.clear) {
+		if (param.status) {
+			error("status is invalid with inject or uninject\n");
+			return -EINVAL;
+		}
+		ictx.op_mask |= 1 << OP_CLEAR;
+	}
+	if (param.status) {
+		if (param.block || param.count) {
+			error("status is invalid with inject or uninject\n");
+			return -EINVAL;
+		}
+		ictx.op_mask |= 1 << OP_STATUS;
+	}
+
+	if (ictx.op_mask == 0) {
+		error("Unable to determine operation\n");
+		return -EINVAL;
+	}
+	ictx.op_mask &= (
+		(1 << OP_INJECT) |
+		(1 << OP_CLEAR) |
+		(1 << OP_STATUS));
+
+	if (param.block) {
+		ictx.block = parse_size64(param.block);
+		if (ictx.block == ULLONG_MAX) {
+			error("Invalid block: %s\n", param.block);
+			return -EINVAL;
+		}
+	}
+	if (param.count) {
+		ictx.count = parse_size64(param.count);
+		if (ictx.count == ULLONG_MAX) {
+			error("Invalid count: %s\n", param.count);
+			return -EINVAL;
+		}
+	}
+
+	/* For inject or clear, an block and count are required */
+	if (ictx.op_mask & ((1 << OP_INJECT) | (1 << OP_CLEAR))) {
+		if (!param.block || !param.count) {
+			error("block and count required for inject/uninject\n");
+			return -EINVAL;
+		}
+	}
+
+	if (param.human)
+		ictx.flags |= UTIL_JSON_HUMAN;
+
+	return 0;
+}
+
+static int ns_errors_to_json(struct ndctl_namespace *ndns,
+		unsigned int start_count)
+{
+	unsigned long flags = ictx.flags | UTIL_JSON_MEDIA_ERRORS;
+	struct ndctl_bus *bus = ndctl_namespace_get_bus(ndns);
+	struct json_object *jndns;
+	unsigned int count;
+	int rc, tmo = 30;
+
+	/* only wait for scrubs for the inject and notify case */
+	if ((ictx.op_mask & (1 << OP_INJECT)) && ictx.notify) {
+		do {
+			/* wait for a scrub to start */
+			count = ndctl_bus_get_scrub_count(bus);
+			if (count == UINT_MAX) {
+				fprintf(stderr, "Unable to get scrub count\n");
+				return -ENXIO;
+			}
+			sleep(1);
+		} while (count <= start_count && --tmo > 0);
+
+		rc = ndctl_bus_wait_for_scrub_completion(bus);
+		if (rc) {
+			fprintf(stderr, "Error waiting for scrub\n");
+			return rc;
+		}
+	}
+
+	jndns = util_namespace_to_json(ndns, flags);
+	if (jndns)
+		printf("%s\n", json_object_to_json_string_ext(jndns,
+				JSON_C_TO_STRING_PRETTY));
+	return 0;
+}
+
+static int inject_error(struct ndctl_namespace *ndns, u64 offset, u64 length,
+		bool notify)
+{
+	struct ndctl_bus *bus = ndctl_namespace_get_bus(ndns);
+	unsigned int scrub_count;
+	int rc;
+
+	scrub_count = ndctl_bus_get_scrub_count(bus);
+	if (scrub_count == UINT_MAX) {
+		fprintf(stderr, "Unable to get scrub count\n");
+		return -ENXIO;
+	}
+
+	rc = ndctl_namespace_inject_error(ndns, offset, length, notify);
+	if (rc) {
+		fprintf(stderr, "Unable to inject error: %d\n", rc);
+		return rc;
+	}
+
+	return ns_errors_to_json(ndns, scrub_count);
+}
+
+static int uninject_error(struct ndctl_namespace *ndns, u64 offset, u64 length)
+{
+	int rc;
+
+	rc = ndctl_namespace_uninject_error(ndns, offset, length);
+	if (rc) {
+		fprintf(stderr, "Unable to uninject error: %d\n", rc);
+		return rc;
+	}
+
+	printf("Warning: Un-injecting previously injected errors here will\n");
+	printf("not cause the kernel to 'forget' its badblock entries. Those\n");
+	printf("have to be cleared through the normal process of writing\n");
+	printf("the affected blocks\n\n");
+	return ns_errors_to_json(ndns, 0);
+}
+
+static int injection_status(struct ndctl_namespace *ndns)
+{
+	unsigned long long block, count, bbs = 0;
+	struct json_object *jbbs, *jbb, *jobj;
+	struct ndctl_bb *bb;
+	int rc;
+
+	rc = ndctl_namespace_injection_status(ndns);
+	if (rc) {
+		fprintf(stderr, "Unable to get injection status: %d\n", rc);
+		return rc;
+	}
+
+	jobj = json_object_new_object();
+	if (!jobj)
+		return -ENOMEM;
+	jbbs = json_object_new_array();
+	if (!jbbs) {
+		json_object_put(jobj);
+		return -ENOMEM;
+	}
+
+	ndctl_namespace_bb_foreach(ndns, bb) {
+		if (!bb)
+			break;
+
+		block = ndctl_bb_get_block(bb);
+		count = ndctl_bb_get_count(bb);
+		jbb = util_badblock_rec_to_json(block, count, ictx.flags);
+		if (!jbb)
+			break;
+		json_object_array_add(jbbs, jbb);
+		bbs++;
+	}
+
+	if (bbs) {
+		json_object_object_add(jobj, "badblocks", jbbs);
+		printf("%s\n", json_object_to_json_string_ext(jobj,
+			JSON_C_TO_STRING_PRETTY));
+	}
+
+	json_object_put(jbbs);
+	json_object_put(jobj);
+
+	return rc;
+}
+
+static int err_inject_ns(struct ndctl_namespace *ndns)
+{
+	unsigned int op_mask;
+	int rc;
+
+	op_mask = ictx.op_mask;
+	while (op_mask) {
+		if (op_mask & (1 << OP_INJECT)) {
+			rc = inject_error(ndns, ictx.block, ictx.count,
+				ictx.notify);
+			if (rc)
+				return rc;
+			op_mask &= ~(1 << OP_INJECT);
+		}
+		if (op_mask & (1 << OP_CLEAR)) {
+			rc = uninject_error(ndns, ictx.block, ictx.count);
+			if (rc)
+				return rc;
+			op_mask &= ~(1 << OP_CLEAR);
+		}
+		if (op_mask & (1 << OP_STATUS)) {
+			rc = injection_status(ndns);
+			if (rc)
+				return rc;
+			op_mask &= ~(1 << OP_STATUS);
+		}
+	}
+
+	return rc;
+}
+
+static int do_inject(const char *namespace, struct ndctl_ctx *ctx)
+{
+	struct ndctl_namespace *ndns;
+	struct ndctl_region *region;
+	const char *ndns_name;
+	struct ndctl_bus *bus;
+	int rc = -ENXIO;
+
+	if (namespace == NULL)
+		return rc;
+
+	if (verbose)
+		ndctl_set_log_priority(ctx, LOG_DEBUG);
+
+        ndctl_bus_foreach(ctx, bus) {
+		if (!util_bus_filter(bus, param.bus))
+			continue;
+
+		ndctl_region_foreach(bus, region) {
+			if (!util_region_filter(region, param.region))
+				continue;
+
+			ndctl_namespace_foreach(region, ndns) {
+				ndns_name = ndctl_namespace_get_devname(ndns);
+
+				if (strcmp(namespace, ndns_name) != 0)
+					continue;
+
+				if (!ndctl_bus_has_error_injection(bus)) {
+					fprintf(stderr,
+						"%s: error injection not supported\n",
+						ndns_name);
+					return -EOPNOTSUPP;
+				}
+				return err_inject_ns(ndns);
+			}
+		}
+	}
+
+	return 0;
+}
+
+int cmd_inject_error(int argc, const char **argv, void *ctx)
+{
+	const char * const u[] = {
+		"ndctl inject-error <namespace> [<options>]",
+		NULL
+	};
+	int i, rc;
+
+        argc = parse_options(argc, argv, inject_options, u, 0);
+	rc = inject_init();
+	if (rc)
+		return rc;
+
+	if (argc == 0)
+		error("specify a namespace to inject error to\n");
+	for (i = 1; i < argc; i++)
+		error("unknown extra parameter \"%s\"\n", argv[i]);
+	if (argc == 0 || argc > 1) {
+		usage_with_options(u, inject_options);
+		return -ENODEV; /* we won't return from usage_with_options() */
+	}
+
+	return do_inject(argv[0], ctx);
+}
diff --git a/ndctl/libndctl-nfit.h b/ndctl/libndctl-nfit.h
index 6b730aa..d5335c2 100644
--- a/ndctl/libndctl-nfit.h
+++ b/ndctl/libndctl-nfit.h
@@ -33,6 +33,14 @@ enum {
 /* error number of Translate SPA by firmware  */
 #define ND_TRANSLATE_SPA_STATUS_INVALID_SPA  2
 
+/* status definitions for error injection */
+#define ND_ARS_ERR_INJ_STATUS_NOT_SUPP 1
+#define ND_ARS_ERR_INJ_STATUS_INVALID_PARAM 2
+
+enum err_inj_options {
+	ND_ARS_ERR_INJ_OPT_NOTIFY = 0,
+};
+
 /*
  * The following structures are command packages which are
  * defined by ACPI 6.2 (or later).
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index d10718e..0f748e1 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -83,6 +83,7 @@ static struct cmd_struct commands[] = {
 	{ "write-labels", cmd_write_labels },
 	{ "init-labels", cmd_init_labels },
 	{ "check-labels", cmd_check_labels },
+	{ "inject-error", cmd_inject_error },
 	{ "list", cmd_list },
 	{ "help", cmd_help },
 	#ifdef ENABLE_TEST
diff --git a/util/json.c b/util/json.c
index 9b7773e..2f18163 100644
--- a/util/json.c
+++ b/util/json.c
@@ -20,6 +20,7 @@
 #include <ndctl/libndctl.h>
 #include <daxctl/libdaxctl.h>
 #include <ccan/array_size/array_size.h>
+#include <ccan/short_types/short_types.h>
 
 #ifdef HAVE_NDCTL_H
 #include <linux/ndctl.h>
@@ -845,3 +846,28 @@ struct json_object *util_mapping_to_json(struct ndctl_mapping *mapping,
 	json_object_put(jmapping);
 	return NULL;
 }
+
+struct json_object *util_badblock_rec_to_json(u64 block, u64 count,
+		unsigned long flags)
+{
+	struct json_object *jerr = json_object_new_object();
+	struct json_object *jobj;
+
+	if (!jerr)
+		return NULL;
+
+	jobj = util_json_object_hex(block, flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jerr, "block", jobj);
+
+	jobj = util_json_object_hex(count, flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jerr, "count", jobj);
+
+	return jerr;
+ err:
+	json_object_put(jerr);
+	return NULL;
+}
diff --git a/util/json.h b/util/json.h
index d934b2e..de55127 100644
--- a/util/json.h
+++ b/util/json.h
@@ -15,6 +15,7 @@
 #include <stdio.h>
 #include <stdbool.h>
 #include <ndctl/libndctl.h>
+#include <ccan/short_types/short_types.h>
 
 enum util_json_flags {
 	UTIL_JSON_IDLE = (1 << 0),
@@ -33,6 +34,8 @@ struct json_object *util_mapping_to_json(struct ndctl_mapping *mapping,
 		unsigned long flags);
 struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 		unsigned long flags);
+struct json_object *util_badblock_rec_to_json(u64 block, u64 count,
+		unsigned long flags);
 struct daxctl_region;
 struct daxctl_dev;
 struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
diff --git a/util/list.h b/util/list.h
new file mode 100644
index 0000000..6439aef
--- /dev/null
+++ b/util/list.h
@@ -0,0 +1,50 @@
+/*
+ * Copyright(c) 2015-2017 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+#ifndef _NDCTL_LIST_H_
+#define _NDCTL_LIST_H_
+
+#include <ccan/list/list.h>
+
+/**
+ * list_add_after - add an entry after the given node in the linked list.
+ * @h: the list_head to add the node to
+ * @l: the list_node after which to add to
+ * @n: the list_node to add to the list.
+ *
+ * The list_node does not need to be initialized; it will be overwritten.
+ * Example:
+ *	struct child *child = malloc(sizeof(*child));
+ *
+ *	child->name = "geoffrey";
+ *	list_add_after(&parent->children, &child1->list, &child->list);
+ *	parent->num_children++;
+ */
+#define list_add_after(h, l, n) list_add_after_(h, l, n, LIST_LOC)
+static inline void list_add_after_(struct list_head *h,
+				   struct list_node *l,
+				   struct list_node *n,
+				   const char *abortstr)
+{
+	if (l->next == &h->n) {
+		/* l is the last element, this becomes a list_add_tail */
+		list_add_tail(h, n);
+		return;
+	}
+	n->next = l->next;
+	n->prev = l;
+	l->next->prev = n;
+	l->next = n;
+	(void)list_debug(h, abortstr);
+}
+
+#endif /* _NDCTL_LIST_H_ */
diff --git a/util/size.h b/util/size.h
index 3c27079..34fac58 100644
--- a/util/size.h
+++ b/util/size.h
@@ -28,6 +28,7 @@ unsigned long long parse_size64(const char *str);
 unsigned long long __parse_size64(const char *str, unsigned long long *units);
 
 #define ALIGN(x, a) ((((unsigned long long) x) + (a - 1)) & ~(a - 1))
+#define ALIGN_DOWN(x, a) (((((unsigned long long) x) + a) & ~(a - 1)) - a)
 #define BITS_PER_LONG (sizeof(unsigned long) * 8)
 #define HPAGE_SIZE (2 << 20)
 
-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [ndctl PATCH v2 4/6] ndctl/test: add a new unit test for inject-error
  2017-10-13  0:09 [ndctl PATCH v2 0/6] add an inject-error command to ndctl Vishal Verma
                   ` (2 preceding siblings ...)
  2017-10-13  0:10 ` [ndctl PATCH v2 3/6] ndctl: add an inject-error command Vishal Verma
@ 2017-10-13  0:10 ` Vishal Verma
  2017-10-13 23:09   ` Dan Williams
  2017-10-13  0:10 ` [ndctl PATCH v2 5/6] ndctl/test: update existing unit tests to use inject-error Vishal Verma
  2017-10-13  0:10 ` [ndctl PATCH v2 6/6] ndctl/test: add a new unit test for BTT error clearing Vishal Verma
  5 siblings, 1 reply; 13+ messages in thread
From: Vishal Verma @ 2017-10-13  0:10 UTC (permalink / raw)
  To: linux-nvdimm

Add a new unit test to test all the features of the inject-error
command.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 test/Makefile.am     |   3 +-
 test/inject-error.sh | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+), 1 deletion(-)
 create mode 100755 test/inject-error.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index ac3547e..8cec451 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -13,7 +13,8 @@ TESTS =\
 	multi-dax.sh \
 	btt-check.sh \
 	label-compat.sh \
-	blk-exhaust.sh
+	blk-exhaust.sh \
+	inject-error.sh
 
 check_PROGRAMS =\
 	libndctl \
diff --git a/test/inject-error.sh b/test/inject-error.sh
new file mode 100755
index 0000000..976be5e
--- /dev/null
+++ b/test/inject-error.sh
@@ -0,0 +1,120 @@
+#!/bin/bash -Ex
+
+# Copyright(c) 2015-2017 Intel Corporation. All rights reserved.
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of version 2 of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+
+[ -f "../ndctl/ndctl" ] && [ -x "../ndctl/ndctl" ] && ndctl="../ndctl/ndctl"
+[ -f "./ndctl/ndctl" ] && [ -x "./ndctl/ndctl" ] && ndctl="./ndctl/ndctl"
+[ -z "$ndctl" ] && echo "Couldn't find an ndctl binary" && exit 1
+bus="nfit_test.0"
+json2var="s/[{}\",]//g; s/:/=/g"
+dev=""
+size=""
+blockdev=""
+rc=77
+err_block=42
+err_count=8
+
+trap 'err $LINENO' ERR
+
+# sample json:
+#{
+#  "dev":"namespace7.0",
+#  "mode":"memory",
+#  "size":"60.00 MiB (62.92 MB)",
+#  "uuid":"f1baa71a-d165-4da4-bb6a-083a2b0e6469",
+#  "blockdev":"pmem7",
+#}
+
+# $1: Line number
+# $2: exit code
+err()
+{
+	[ -n "$2" ] && rc="$2"
+	echo "test/inject-error.sh: failed at line $1"
+	exit "$rc"
+}
+
+check_min_kver()
+{
+	local ver="$1"
+	: "${KVER:=$(uname -r)}"
+
+	[ -n "$ver" ] || return 1
+	[[ "$ver" == "$(echo -e "$ver\n$KVER" | sort -V | head -1)" ]]
+}
+
+check_min_kver "4.15" || { echo "kernel $KVER may not support error injection"; exit "$rc"; }
+
+create()
+{
+	json=$($ndctl create-namespace -b "$bus" -t pmem --align=4k)
+	eval "$(echo "$json" | sed -e "$json2var")"
+	[ -n "$dev" ] || err "$LINENO" 2
+	[ -n "$size" ] || err "$LINENO" 2
+	[ -n "$blockdev" ] || err "$LINENO" 2
+	[ $size -gt 0 ] || err "$LINENO" 2
+}
+
+reset()
+{
+	$ndctl disable-region -b "$bus" all
+	$ndctl zero-labels -b "$bus" all
+	$ndctl enable-region -b "$bus" all
+}
+
+check_status()
+{
+	local sector="$1"
+	local count="$2"
+
+	json="$($ndctl inject-error --status $dev)"
+	[[ "$sector" == "$(jq ".badblocks[0].block" <<< "$json")" ]]
+	[[ "$count" == "$(jq ".badblocks[0].count" <<< "$json")" ]]
+}
+
+do_tests()
+{
+	# inject without notification
+	$ndctl inject-error --block=$err_block --count=$err_count --no-notify $dev
+	check_status "$err_block" "$err_count"
+	if read -r sector len < /sys/block/$blockdev/badblocks; then
+		# fail if reading badblocks returns data
+		echo "fail: $LINENO" && exit 1
+	fi
+
+	# clear via err-inj-clear
+	$ndctl inject-error --block=$err_block --count=$err_count --uninject $dev
+	check_status
+
+	# inject normally
+	$ndctl inject-error --block=$err_block --count=$err_count $dev
+	check_status "$err_block" "$err_count"
+	if read -r sector len < /sys/block/$blockdev/badblocks; then
+		test "$sector" -eq "$err_block"
+		test "$len" -eq "$err_count"
+	fi
+
+	# clear via write
+	dd if=/dev/zero of=/dev/$blockdev bs=512 count=$err_count seek=$err_block oflag=direct
+	if read -r sector len < /sys/block/$blockdev/badblocks; then
+		# fail if reading badblocks returns data
+		echo "fail: $LINENO" && exit 1
+	fi
+	check_status
+}
+
+modprobe nfit_test
+rc=1
+reset && create
+do_tests
+reset
+exit 0
-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [ndctl PATCH v2 5/6] ndctl/test: update existing unit tests to use inject-error
  2017-10-13  0:09 [ndctl PATCH v2 0/6] add an inject-error command to ndctl Vishal Verma
                   ` (3 preceding siblings ...)
  2017-10-13  0:10 ` [ndctl PATCH v2 4/6] ndctl/test: add a new unit test for inject-error Vishal Verma
@ 2017-10-13  0:10 ` Vishal Verma
  2017-10-13  0:10 ` [ndctl PATCH v2 6/6] ndctl/test: add a new unit test for BTT error clearing Vishal Verma
  5 siblings, 0 replies; 13+ messages in thread
From: Vishal Verma @ 2017-10-13  0:10 UTC (permalink / raw)
  To: linux-nvdimm

Until now, various unit tests related to error handling used to expect
'canned' errors to be present in the middle of every nfit_test
namespace. With the ACPI error injection patches for nfit_test, this is
no longer the case. Update the existing unit tests it inject any errors
they need for testing themselves using inject-error, rather than
expecting canned errors.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 test/clear.sh         |  5 ++++-
 test/dax-errors.sh    |  5 ++++-
 test/daxdev-errors.sh | 17 ++++++++++++++---
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/test/clear.sh b/test/clear.sh
index a60c3d9..c22ff3b 100755
--- a/test/clear.sh
+++ b/test/clear.sh
@@ -54,7 +54,10 @@ eval $(echo $json | sed -e "$json2var")
 [ $dev = "x" ] && echo "fail: $LINENO" && exit 1
 [ $mode != "raw" ] && echo "fail: $LINENO" && exit 1
 
-# check for expected errors in the middle of the namespace
+# inject errors in the middle of the namespace, verify that reading fails
+err_sector="$(((size/512) / 2))"
+err_count=8
+$NDCTL inject-error --block="$err_sector" --count=$err_count $dev
 read sector len < /sys/block/$blockdev/badblocks
 [ $((sector * 2)) -ne $((size /512)) ] && echo "fail: $LINENO" && exit 1
 if dd if=/dev/$blockdev of=/dev/null iflag=direct bs=512 skip=$sector count=$len; then
diff --git a/test/dax-errors.sh b/test/dax-errors.sh
index 5af3859..02bbca8 100755
--- a/test/dax-errors.sh
+++ b/test/dax-errors.sh
@@ -64,7 +64,10 @@ eval $(echo $json | sed -e "$json2var")
 [ $dev = "x" ] && echo "fail: $LINENO" && exit 1
 [ $mode != "raw" ] && echo "fail: $LINENO" && exit 1
 
-# check for expected errors in the middle of the namespace
+# inject errors in the middle of the namespace, verify that reading fails
+err_sector="$(((size/512) / 2))"
+err_count=8
+$NDCTL inject-error --block="$err_sector" --count=$err_count $dev
 read sector len < /sys/block/$blockdev/badblocks
 [ $((sector * 2)) -ne $((size /512)) ] && echo "fail: $LINENO" && exit 1
 if dd if=/dev/$blockdev of=/dev/null iflag=direct bs=512 skip=$sector count=$len; then
diff --git a/test/daxdev-errors.sh b/test/daxdev-errors.sh
index 8115087..e9d9d57 100755
--- a/test/daxdev-errors.sh
+++ b/test/daxdev-errors.sh
@@ -37,6 +37,7 @@ check_min_kver "4.12" || { echo "kernel $KVER lacks dax dev error handling"; exi
 
 set -e
 trap 'err $LINENO' ERR
+rc=1
 
 # setup (reset nfit_test dimms)
 modprobe nfit_test
@@ -68,18 +69,28 @@ chardev=$(echo $json | jq ". | select(.mode == \"dax\") | .daxregion.devices[0].
 #  }
 #}
 
+json1=$($NDCTL list $BUS --mode=dax --namespaces)
+eval $(echo $json1 | sed -e "$json2var")
+nsdev=$dev
+
 json1=$($NDCTL list $BUS)
 eval $(echo $json1 | sed -e "$json2var")
+busdev=$dev
+
+# inject errors in the middle of the namespace
+err_sector="$(((size/512) / 2))"
+err_count=8
+$NDCTL inject-error --block="$err_sector" --count=$err_count $nsdev
 
-read sector len < /sys/bus/platform/devices/nfit_test.0/$dev/$region/badblocks
+read sector len < /sys/bus/nd/devices/$region/badblocks
 echo "sector: $sector len: $len"
 
 # run the daxdev-errors test
 test -x ./daxdev-errors
-./daxdev-errors $dev $region
+./daxdev-errors $busdev $region
 
 # check badblocks, should be empty
-if read sector len < /sys/bus/platform/devices/nfit_test.0/$dev/$region/badblocks; then
+if read sector len < /sys/bus/platform/devices/nfit_test.0/$busdev/$region/badblocks; then
 	echo "badblocks empty, expected"
 fi
 [ -n "$sector" ] && echo "fail: $LINENO" && exit 1
-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [ndctl PATCH v2 6/6] ndctl/test: add a new unit test for BTT error clearing
  2017-10-13  0:09 [ndctl PATCH v2 0/6] add an inject-error command to ndctl Vishal Verma
                   ` (4 preceding siblings ...)
  2017-10-13  0:10 ` [ndctl PATCH v2 5/6] ndctl/test: update existing unit tests to use inject-error Vishal Verma
@ 2017-10-13  0:10 ` Vishal Verma
  2017-10-13 23:12   ` Dan Williams
  5 siblings, 1 reply; 13+ messages in thread
From: Vishal Verma @ 2017-10-13  0:10 UTC (permalink / raw)
  To: linux-nvdimm

The new error injection command allows us to inject errors that persist
through changing the mode of a BTT namespace to 'raw' and back. This
allows us to test error clearing with a BTT by adding a selective error
block to the raw namespace, enabling the BTT, and then clearing it via a
write.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 test/Makefile.am   |   3 +-
 test/btt-errors.sh | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 151 insertions(+), 1 deletion(-)
 create mode 100755 test/btt-errors.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index 8cec451..881fcea 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -14,7 +14,8 @@ TESTS =\
 	btt-check.sh \
 	label-compat.sh \
 	blk-exhaust.sh \
-	inject-error.sh
+	inject-error.sh \
+	btt-errors.sh
 
 check_PROGRAMS =\
 	libndctl \
diff --git a/test/btt-errors.sh b/test/btt-errors.sh
new file mode 100755
index 0000000..aec0b9d
--- /dev/null
+++ b/test/btt-errors.sh
@@ -0,0 +1,149 @@
+#!/bin/bash -x
+
+# Copyright(c) 2015-2017 Intel Corporation. All rights reserved.
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of version 2 of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+
+NDCTL="../ndctl/ndctl"
+BUS="nfit_test.0"
+MNT=test_btt_mnt
+FILE=image
+json2var="s/[{}\",]//g; s/:/=/g"
+blockdev=""
+rc=77
+
+err() {
+	rc=1
+	echo "test/btt-errors: failed at line $1"
+
+	rm -f $FILE
+	rm -f $MNT/$FILE
+	if [ -n "$blockdev" ]; then
+		umount "/dev/$blockdev"
+	else
+		rc=77
+	fi
+	rmdir $MNT
+	exit $rc
+}
+
+check_min_kver()
+{
+	local ver="$1"
+	: "${KVER:=$(uname -r)}"
+
+	[ -n "$ver" ] || return 1
+	[[ "$ver" == "$(echo -e "$ver\n$KVER" | sort -V | head -1)" ]]
+}
+
+force_raw()
+{
+	raw="$1"
+	if grep -q "$MNT" /proc/mounts; then umount $MNT; fi
+	ndctl disable-namespace "$dev"
+	echo "$raw" > "/sys/bus/nd/devices/$dev/force_raw"
+	ndctl enable-namespace "$dev"
+	echo "Set $dev to raw mode: $raw"
+	if [[ "$raw" == "1" ]]; then
+		raw_bdev=${blockdev%s}
+		test -b "/dev/$raw_bdev"
+	else
+		raw_bdev=""
+	fi
+}
+
+check_min_kver "4.14" || { echo "kernel $KVER may lack BTT error handling"; exit $rc; }
+
+set -e
+mkdir -p $MNT
+trap 'err $LINENO' ERR
+
+# setup (reset nfit_test dimms)
+modprobe nfit_test
+$NDCTL disable-region -b "$BUS" all
+$NDCTL zero-labels -b "$BUS" all
+$NDCTL enable-region -b "$BUS" all
+
+rc=1
+
+# create a btt namespace and clear errors (if any)
+dev="x"
+json=$($NDCTL create-namespace -b "$BUS" -t pmem -m sector)
+eval "$(echo "$json" | sed -e "$json2var")"
+[ $dev = "x" ] && echo "fail: $LINENO" && exit 1
+
+force_raw 1
+if read -r sector len < "/sys/block/$raw_bdev/badblocks"; then
+	dd of=/dev/$raw_bdev if=/dev/zero oflag=direct bs=512 seek="$sector" count="$len"
+fi
+force_raw 0
+
+mkfs.ext4 "/dev/$blockdev" -b 4096
+mount -o nodelalloc "/dev/$blockdev" $MNT
+
+# prepare an image file with random data
+dd if=/dev/urandom of=$FILE bs=4096 count=1
+test -s $FILE
+
+# copy it to the file system
+cp $FILE $MNT/$FILE
+
+# Get the start sector for the file
+start_sect=$(filefrag -v -b512 $MNT/$FILE | grep -E "^[ ]+[0-9]+.*" | head -1 | awk '{ print $4 }' | cut -d. -f1)
+start_4k=$((start_sect/8))
+test -n "$start_sect"
+echo "start sector of the file is: $start_sect (512B) or $start_4k (4096B)"
+
+# figure out the btt offset
+
+force_raw 1
+
+# calculate start of the map
+map=$(hexdump -s 96 -n 4 "/dev/$raw_bdev" | head -1 | cut -d' ' -f2-)
+map=$(tr -d ' ' <<< "0x${map#* }${map%% *}")
+printf "btt map starts at: %x\n" "$map"
+
+# calculate map entry byte offset for the file's block
+map_idx=$((map + (4 * start_4k)))
+printf "btt map entry location for sector %x: %x\n" "$start_4k" "$map_idx"
+
+# read the map entry
+map_ent=$(hexdump -s $map_idx -n 4 "/dev/$raw_bdev" | head -1 | cut -d' ' -f2-)
+map_ent=$(tr -d ' ' <<< "0x${map_ent#* }${map_ent%% *}")
+map_ent=$((map_ent & 0x3fffffff))
+printf "btt map entry: 0x%x\n" "$map_ent"
+
+# calculate the data offset
+dataoff=$(((map_ent * 4096) + 4096))
+printf "dataoff: 0x%x\n" "$dataoff"
+
+bb_inj=$((dataoff/512))
+
+# inject badblocks for one page at the start of the file
+$NDCTL inject-error --block="$bb_inj" --count=8 $dev
+
+force_raw 0
+mount -o nodelalloc "/dev/$blockdev" $MNT
+
+# make sure reading the first block of the file fails as expected
+: The following 'dd' is expected to hit an I/O Error
+dd if=$MNT/$FILE of=/dev/null iflag=direct bs=4096 count=1 && err $LINENO || true
+
+# write via btt to clear the error
+dd if=/dev/zero of=$MNT/$FILE oflag=direct bs=4096 count=1
+
+# read again and that should succeed
+dd if=$MNT/$FILE of=/dev/null iflag=direct bs=4096 count=1
+
+# done, exit
+$NDCTL disable-region -b "$BUS" all
+$NDCTL zero-labels -b "$BUS" all
+$NDCTL enable-region -b "$BUS" all
+exit 0
-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [ndctl PATCH v2 3/6] ndctl: add an inject-error command
  2017-10-13  0:10 ` [ndctl PATCH v2 3/6] ndctl: add an inject-error command Vishal Verma
@ 2017-10-13 22:24   ` Dan Williams
  0 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2017-10-13 22:24 UTC (permalink / raw)
  To: Vishal Verma; +Cc: linux-nvdimm

On Thu, Oct 12, 2017 at 5:10 PM, Vishal Verma <vishal.l.verma@intel.com> wrote:
> Add an inject-error command to ndctl. This uses the error injection DSMs
> in ACPI6.2 to provide a generic error injection and management
> interface. Once can inject errors, and view as well as clear injected
> errors using these commands.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
[..]
> diff --git a/ndctl/inject-error.c b/ndctl/inject-error.c
> new file mode 100644
> index 0000000..4c45902
> --- /dev/null
> +++ b/ndctl/inject-error.c
[..]
> +static int uninject_error(struct ndctl_namespace *ndns, u64 offset, u64 length)
> +{
> +       int rc;
> +
> +       rc = ndctl_namespace_uninject_error(ndns, offset, length);
> +       if (rc) {
> +               fprintf(stderr, "Unable to uninject error: %d\n", rc);
> +               return rc;
> +       }
> +
> +       printf("Warning: Un-injecting previously injected errors here will\n");
> +       printf("not cause the kernel to 'forget' its badblock entries. Those\n");
> +       printf("have to be cleared through the normal process of writing\n");
> +       printf("the affected blocks\n\n");

Rest of the patch looks good, but when I apply it I'll move this to go
to stderr since we only expect json on stdout.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [ndctl PATCH v2 1/6] libndctl: add APIs to get scrub count and to wait for a scrub
  2017-10-13  0:09 ` [ndctl PATCH v2 1/6] libndctl: add APIs to get scrub count and to wait for a scrub Vishal Verma
@ 2017-10-13 23:01   ` Dan Williams
  0 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2017-10-13 23:01 UTC (permalink / raw)
  To: Vishal Verma; +Cc: linux-nvdimm

On Thu, Oct 12, 2017 at 5:09 PM, Vishal Verma <vishal.l.verma@intel.com> wrote:
> The kernel uses sysfs to notify userspace of the number of completed
> Address Range Scrubs, as well as any ongoing scrubs. Add libndctl
> helpers to get the scrub count, and to wait for an in-progress scrub to
> complete.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

Looks good.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [ndctl PATCH v2 2/6] libndctl: add error injection related interfaces
  2017-10-13  0:10 ` [ndctl PATCH v2 2/6] libndctl: add error injection related interfaces Vishal Verma
@ 2017-10-13 23:07   ` Dan Williams
  2017-10-15  3:35     ` Vishal Verma
  0 siblings, 1 reply; 13+ messages in thread
From: Dan Williams @ 2017-10-13 23:07 UTC (permalink / raw)
  To: Vishal Verma; +Cc: linux-nvdimm

On Thu, Oct 12, 2017 at 5:10 PM, Vishal Verma <vishal.l.verma@intel.com> wrote:
> Add interfaces to enable error injection commands. Add nfit specific
> error injection helpers in ndctl/lib/nfit.c, and generic wrappers for
> them in libndctl.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  ndctl/lib/Makefile.am  |   1 +
>  ndctl/lib/inject.c     | 391 +++++++++++++++++++++++++++++++++++++++++++++++++
>  ndctl/lib/libndctl.c   |  47 +-----
>  ndctl/lib/libndctl.sym |   9 ++
>  ndctl/lib/nfit.c       |  89 +++++++++++
>  ndctl/lib/private.h    |  53 +++++++
>  ndctl/libndctl-nfit.h  |   2 +
>  ndctl/libndctl.h.in    |  23 +++
>  8 files changed, 573 insertions(+), 42 deletions(-)
>  create mode 100644 ndctl/lib/inject.c
>
> diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
> index 9a7734d..3a6024e 100644
> --- a/ndctl/lib/Makefile.am
> +++ b/ndctl/lib/Makefile.am
> @@ -18,6 +18,7 @@ libndctl_la_SOURCES =\
>         ../../util/sysfs.c \
>         ../../util/sysfs.h \
>         dimm.c \
> +       inject.c \
>         libndctl.c
>
>  libndctl_la_LIBADD =\
> diff --git a/ndctl/lib/inject.c b/ndctl/lib/inject.c
> new file mode 100644
> index 0000000..2660384
> --- /dev/null
> +++ b/ndctl/lib/inject.c
> @@ -0,0 +1,391 @@
> +/*
> + * Copyright (c) 2014-2017, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU Lesser General Public License,
> + * version 2.1, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT ANY
> + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
> + * more details.
> + */
> +#include <stdlib.h>
> +#include <limits.h>
> +#include <util/list.h>
> +#include <util/size.h>
> +#include <ndctl/libndctl.h>
> +#include <ccan/list/list.h>
> +#include <ndctl/libndctl-nfit.h>
> +#include <ccan/short_types/short_types.h>
> +#include "private.h"
> +
> +NDCTL_EXPORT int ndctl_bus_has_error_injection(struct ndctl_bus *bus)
> +{
> +       /* Currently, only nfit buses have error injection */
> +       if (!bus || !ndctl_bus_has_nfit(bus))
> +               return 0;
> +
> +       if (ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_ARS_INJECT_SET) &&
> +               ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_ARS_INJECT_GET) &&
> +               ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_ARS_INJECT_CLEAR))
> +               return 1;
> +
> +       return 0;
> +}
> +
> +NDCTL_EXPORT void ndctl_namespace_get_injection_bounds(
> +               struct ndctl_namespace *ndns, unsigned long long *ns_offset,
> +               unsigned long long *ns_size)
> +{

This routine should have an error code because the resource values are
root-only, but I think a better solution is to just not NDCTL_EXPORT
it since it's only used internally in the library. In fact, if you
didn't find a use case for a new library call in the error injection
command then we should make it internal-only until an external user
shows up.

Are there any other commands like this?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [ndctl PATCH v2 4/6] ndctl/test: add a new unit test for inject-error
  2017-10-13  0:10 ` [ndctl PATCH v2 4/6] ndctl/test: add a new unit test for inject-error Vishal Verma
@ 2017-10-13 23:09   ` Dan Williams
  0 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2017-10-13 23:09 UTC (permalink / raw)
  To: Vishal Verma; +Cc: linux-nvdimm

On Thu, Oct 12, 2017 at 5:10 PM, Vishal Verma <vishal.l.verma@intel.com> wrote:
> Add a new unit test to test all the features of the inject-error
> command.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

Looks good.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [ndctl PATCH v2 6/6] ndctl/test: add a new unit test for BTT error clearing
  2017-10-13  0:10 ` [ndctl PATCH v2 6/6] ndctl/test: add a new unit test for BTT error clearing Vishal Verma
@ 2017-10-13 23:12   ` Dan Williams
  0 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2017-10-13 23:12 UTC (permalink / raw)
  To: Vishal Verma; +Cc: linux-nvdimm

On Thu, Oct 12, 2017 at 5:10 PM, Vishal Verma <vishal.l.verma@intel.com> wrote:
> The new error injection command allows us to inject errors that persist
> through changing the mode of a BTT namespace to 'raw' and back. This
> allows us to test error clearing with a BTT by adding a selective error
> block to the raw namespace, enabling the BTT, and then clearing it via a
> write.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

Nice to see this. As for the BTT details I'm going to trust you as the
BTT maintainer, but if any of those are wrong we can handle it later
with incremental fixes.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [ndctl PATCH v2 2/6] libndctl: add error injection related interfaces
  2017-10-13 23:07   ` Dan Williams
@ 2017-10-15  3:35     ` Vishal Verma
  0 siblings, 0 replies; 13+ messages in thread
From: Vishal Verma @ 2017-10-15  3:35 UTC (permalink / raw)
  To: Dan Williams, Vishal Verma; +Cc: linux-nvdimm

On Fri, 2017-10-13 at 16:07 -0700, Dan Williams wrote:
> On Thu, Oct 12, 2017 at 5:10 PM, Vishal Verma <vishal.l.verma@intel.c
> om> wrote:
> > Add interfaces to enable error injection commands. Add nfit
> > specific
> > error injection helpers in ndctl/lib/nfit.c, and generic wrappers
> > for
> > them in libndctl.
> > 
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> >  ndctl/lib/Makefile.am  |   1 +
> >  ndctl/lib/inject.c     | 391
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  ndctl/lib/libndctl.c   |  47 +-----
> >  ndctl/lib/libndctl.sym |   9 ++
> >  ndctl/lib/nfit.c       |  89 +++++++++++
> >  ndctl/lib/private.h    |  53 +++++++
> >  ndctl/libndctl-nfit.h  |   2 +
> >  ndctl/libndctl.h.in    |  23 +++
> >  8 files changed, 573 insertions(+), 42 deletions(-)
> >  create mode 100644 ndctl/lib/inject.c
> > 
> > diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
> > index 9a7734d..3a6024e 100644
> > --- a/ndctl/lib/Makefile.am
> > +++ b/ndctl/lib/Makefile.am
> > @@ -18,6 +18,7 @@ libndctl_la_SOURCES =\
> >         ../../util/sysfs.c \
> >         ../../util/sysfs.h \
> >         dimm.c \
> > +       inject.c \
> >         libndctl.c
> > 
> >  libndctl_la_LIBADD =\
> > diff --git a/ndctl/lib/inject.c b/ndctl/lib/inject.c
> > new file mode 100644
> > index 0000000..2660384
> > --- /dev/null
> > +++ b/ndctl/lib/inject.c
> > @@ -0,0 +1,391 @@
> > +/*
> > + * Copyright (c) 2014-2017, Intel Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > modify it
> > + * under the terms and conditions of the GNU Lesser General Public
> > License,
> > + * version 2.1, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
> > WITHOUT ANY
> > + * WARRANTY; without even the implied warranty of MERCHANTABILITY
> > or FITNESS
> > + * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public
> > License for
> > + * more details.
> > + */
> > +#include <stdlib.h>
> > +#include <limits.h>
> > +#include <util/list.h>
> > +#include <util/size.h>
> > +#include <ndctl/libndctl.h>
> > +#include <ccan/list/list.h>
> > +#include <ndctl/libndctl-nfit.h>
> > +#include <ccan/short_types/short_types.h>
> > +#include "private.h"
> > +
> > +NDCTL_EXPORT int ndctl_bus_has_error_injection(struct ndctl_bus
> > *bus)
> > +{
> > +       /* Currently, only nfit buses have error injection */
> > +       if (!bus || !ndctl_bus_has_nfit(bus))
> > +               return 0;
> > +
> > +       if (ndctl_bus_is_nfit_cmd_supported(bus,
> > NFIT_CMD_ARS_INJECT_SET) &&
> > +               ndctl_bus_is_nfit_cmd_supported(bus,
> > NFIT_CMD_ARS_INJECT_GET) &&
> > +               ndctl_bus_is_nfit_cmd_supported(bus,
> > NFIT_CMD_ARS_INJECT_CLEAR))
> > +               return 1;
> > +
> > +       return 0;
> > +}
> > +
> > +NDCTL_EXPORT void ndctl_namespace_get_injection_bounds(
> > +               struct ndctl_namespace *ndns, unsigned long long
> > *ns_offset,
> > +               unsigned long long *ns_size)
> > +{
> 
> This routine should have an error code because the resource values
> are
> root-only, but I think a better solution is to just not NDCTL_EXPORT
> it since it's only used internally in the library. In fact, if you
> didn't find a use case for a new library call in the error injection
> command then we should make it internal-only until an external user
> shows up.
> 
> Are there any other commands like this?

Agreed that we should just remove the NDCTL_EXPORT for this. I was
using it from the inject-error code, but in the last round of
refactoring, that usage disappeared, and the export lingered..
Can you fix it while applying or shall I resend?

I don't think there are other exported functions like this that aren't
used outside the library.

> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-10-15  3:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-13  0:09 [ndctl PATCH v2 0/6] add an inject-error command to ndctl Vishal Verma
2017-10-13  0:09 ` [ndctl PATCH v2 1/6] libndctl: add APIs to get scrub count and to wait for a scrub Vishal Verma
2017-10-13 23:01   ` Dan Williams
2017-10-13  0:10 ` [ndctl PATCH v2 2/6] libndctl: add error injection related interfaces Vishal Verma
2017-10-13 23:07   ` Dan Williams
2017-10-15  3:35     ` Vishal Verma
2017-10-13  0:10 ` [ndctl PATCH v2 3/6] ndctl: add an inject-error command Vishal Verma
2017-10-13 22:24   ` Dan Williams
2017-10-13  0:10 ` [ndctl PATCH v2 4/6] ndctl/test: add a new unit test for inject-error Vishal Verma
2017-10-13 23:09   ` Dan Williams
2017-10-13  0:10 ` [ndctl PATCH v2 5/6] ndctl/test: update existing unit tests to use inject-error Vishal Verma
2017-10-13  0:10 ` [ndctl PATCH v2 6/6] ndctl/test: add a new unit test for BTT error clearing Vishal Verma
2017-10-13 23:12   ` Dan Williams

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).