nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] add error injection commands to nfit_test
@ 2017-10-06  1:53 Vishal Verma
  2017-10-06  1:53 ` [PATCH 1/4] libnvdimm: move poison list functions to a new 'badrange' file Vishal Verma
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Vishal Verma @ 2017-10-06  1:53 UTC (permalink / raw)
  To: linux-nvdimm

These patches add error injection support to nfit_test by emulating the
ACPI6.2 ARS error injection commands. The commands are sent via the
ND_CMD_CALL interface, so only nfit_test knows of the various
definitions related to this.

Note that this patch set will break ndctl unit tests unless the ndctl
patches for error injection are also applied.

Dave Jiang (2):
  libnvdimm: move poison list functions to a new 'badrange' file
  nfit_test: add error injection DSMs

Vishal Verma (2):
  libnvdimm, badrange: remove a WARN for list_empty
  nfit_test: when clearing poison, also remove badrange entries

 drivers/acpi/nfit/core.c              |   2 +-
 drivers/acpi/nfit/mce.c               |   2 +-
 drivers/acpi/nfit/nfit.h              |   1 +
 drivers/nvdimm/Makefile               |   1 +
 drivers/nvdimm/badrange.c             | 293 ++++++++++++++++++++++++++++++++++
 drivers/nvdimm/bus.c                  |  24 +--
 drivers/nvdimm/core.c                 | 260 +-----------------------------
 drivers/nvdimm/nd-core.h              |   3 +-
 drivers/nvdimm/nd.h                   |   6 -
 include/linux/libnvdimm.h             |  21 ++-
 tools/testing/nvdimm/Kbuild           |   1 +
 tools/testing/nvdimm/test/nfit.c      | 199 +++++++++++++++++++----
 tools/testing/nvdimm/test/nfit_test.h |   4 +
 13 files changed, 503 insertions(+), 314 deletions(-)
 create mode 100644 drivers/nvdimm/badrange.c

-- 
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] 12+ messages in thread

* [PATCH 1/4] libnvdimm: move poison list functions to a new 'badrange' file
  2017-10-06  1:53 [PATCH 0/4] add error injection commands to nfit_test Vishal Verma
@ 2017-10-06  1:53 ` Vishal Verma
  2017-10-06 18:52   ` Dan Williams
  2017-10-06  1:53 ` [PATCH 2/4] nfit_test: add error injection DSMs Vishal Verma
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Vishal Verma @ 2017-10-06  1:53 UTC (permalink / raw)
  To: linux-nvdimm

From: Dave Jiang <dave.jiang@intel.com>

From: Dave Jiang <dave.jiang@intel.com>

nfit_test needs to use the poison list manipulation code as well. Make
it more generic and in the process rename poison to badrange, and move
all the related helpers to a new file.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
[vishal: add a missed include in bus.c for the new badrange functions]
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 drivers/acpi/nfit/core.c  |   2 +-
 drivers/acpi/nfit/mce.c   |   2 +-
 drivers/nvdimm/Makefile   |   1 +
 drivers/nvdimm/badrange.c | 294 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvdimm/bus.c      |  24 ++--
 drivers/nvdimm/core.c     | 260 +---------------------------------------
 drivers/nvdimm/nd-core.h  |   3 +-
 drivers/nvdimm/nd.h       |   6 -
 include/linux/libnvdimm.h |  21 +++-
 9 files changed, 331 insertions(+), 282 deletions(-)
 create mode 100644 drivers/nvdimm/badrange.c

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index a3ecd5e..4b157f8 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2240,7 +2240,7 @@ static int ars_status_process_records(struct acpi_nfit_desc *acpi_desc,
 		if (ars_status->out_length
 				< 44 + sizeof(struct nd_ars_record) * (i + 1))
 			break;
-		rc = nvdimm_bus_add_poison(nvdimm_bus,
+		rc = nvdimm_bus_add_badrange(nvdimm_bus,
 				ars_status->records[i].err_address,
 				ars_status->records[i].length);
 		if (rc)
diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c
index feeb95d..b929214 100644
--- a/drivers/acpi/nfit/mce.c
+++ b/drivers/acpi/nfit/mce.c
@@ -67,7 +67,7 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val,
 			continue;
 
 		/* If this fails due to an -ENOMEM, there is little we can do */
-		nvdimm_bus_add_poison(acpi_desc->nvdimm_bus,
+		nvdimm_bus_add_badrange(acpi_desc->nvdimm_bus,
 				ALIGN(mce->addr, L1_CACHE_BYTES),
 				L1_CACHE_BYTES);
 		nvdimm_region_notify(nfit_spa->nd_region,
diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
index 909554c..ca6d325 100644
--- a/drivers/nvdimm/Makefile
+++ b/drivers/nvdimm/Makefile
@@ -20,6 +20,7 @@ libnvdimm-y += region_devs.o
 libnvdimm-y += region.o
 libnvdimm-y += namespace_devs.o
 libnvdimm-y += label.o
+libnvdimm-y += badrange.o
 libnvdimm-$(CONFIG_ND_CLAIM) += claim.o
 libnvdimm-$(CONFIG_BTT) += btt_devs.o
 libnvdimm-$(CONFIG_NVDIMM_PFN) += pfn_devs.o
diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c
new file mode 100644
index 0000000..6ad782f
--- /dev/null
+++ b/drivers/nvdimm/badrange.c
@@ -0,0 +1,294 @@
+/*
+ * Copyright(c) 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 <linux/libnvdimm.h>
+#include <linux/badblocks.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/blkdev.h>
+#include <linux/device.h>
+#include <linux/ctype.h>
+#include <linux/ndctl.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include "nd-core.h"
+#include "nd.h"
+
+void badrange_init(struct badrange *badrange)
+{
+	INIT_LIST_HEAD(&badrange->list);
+	spin_lock_init(&badrange->lock);
+}
+EXPORT_SYMBOL_GPL(badrange_init);
+
+static void append_badrange_entry(struct badrange *badrange,
+		struct badrange_entry *be, u64 addr, u64 length)
+{
+	lockdep_assert_held(&badrange->lock);
+	be->start = addr;
+	be->length = length;
+	list_add_tail(&be->list, &badrange->list);
+}
+
+static int alloc_and_append_badrange_entry(struct badrange *badrange,
+		u64 addr, u64 length, gfp_t flags)
+{
+	struct badrange_entry *be;
+
+	be = kzalloc(sizeof(*be), flags);
+	if (!be)
+		return -ENOMEM;
+
+	append_badrange_entry(badrange, be, addr, length);
+	return 0;
+}
+
+static int add_badrange(struct badrange *badrange, u64 addr, u64 length)
+{
+	struct badrange_entry *be, *be_new;
+
+	spin_unlock(&badrange->lock);
+	be_new = kzalloc(sizeof(*be_new), GFP_KERNEL);
+	spin_lock(&badrange->lock);
+
+	if (list_empty(&badrange->list)) {
+		if (!be_new)
+			return -ENOMEM;
+		append_badrange_entry(badrange, be_new, addr, length);
+		return 0;
+	}
+
+	/*
+	 * There is a chance this is a duplicate, check for those first.
+	 * This will be the common case as ARS_STATUS returns all known
+	 * errors in the SPA space, and we can't query it per region
+	 */
+	list_for_each_entry(be, &badrange->list, list)
+		if (be->start == addr) {
+			/* If length has changed, update this list entry */
+			if (be->length != length)
+				be->length = length;
+			kfree(be_new);
+			return 0;
+		}
+
+	/*
+	 * If not a duplicate or a simple length update, add the entry as is,
+	 * as any overlapping ranges will get resolved when the list is consumed
+	 * and converted to badblocks
+	 */
+	if (!be_new)
+		return -ENOMEM;
+	append_badrange_entry(badrange, be_new, addr, length);
+
+	return 0;
+}
+
+int badrange_add(struct badrange *badrange, u64 addr, u64 length)
+{
+	int rc;
+
+	spin_lock(&badrange->lock);
+	rc = add_badrange(badrange, addr, length);
+	spin_unlock(&badrange->lock);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(badrange_add);
+
+void badrange_forget(struct badrange *badrange, phys_addr_t start,
+		unsigned int len)
+{
+	struct list_head *badrange_list = &badrange->list;
+	u64 clr_end = start + len - 1;
+	struct badrange_entry *be, *next;
+
+	spin_lock(&badrange->lock);
+	WARN_ON_ONCE(list_empty(badrange_list));
+
+	/*
+	 * [start, clr_end] is the badrange interval being cleared.
+	 * [be->start, be_end] is the badrange_list entry we're comparing
+	 * the above interval against. The badrange list entry may need
+	 * to be modified (update either start or length), deleted, or
+	 * split into two based on the overlap characteristics
+	 */
+
+	list_for_each_entry_safe(be, next, badrange_list, list) {
+		u64 be_end = be->start + be->length - 1;
+
+		/* Skip intervals with no intersection */
+		if (be_end < start)
+			continue;
+		if (be->start >  clr_end)
+			continue;
+		/* Delete combeetely overlapped badrange entries */
+		if ((be->start >= start) && (be_end <= clr_end)) {
+			list_del(&be->list);
+			kfree(be);
+			continue;
+		}
+		/* Adjust start point of partially cleared entries */
+		if ((start <= be->start) && (clr_end > be->start)) {
+			be->length -= clr_end - be->start + 1;
+			be->start = clr_end + 1;
+			continue;
+		}
+		/* Adjust be->length for partial clearing at the tail end */
+		if ((be->start < start) && (be_end <= clr_end)) {
+			/* be->start remains the same */
+			be->length = start - be->start;
+			continue;
+		}
+		/*
+		 * If clearing in the middle of an entry, we split it into
+		 * two by modifying the current entry to represent one half of
+		 * the split, and adding a new entry for the second half.
+		 */
+		if ((be->start < start) && (be_end > clr_end)) {
+			u64 new_start = clr_end + 1;
+			u64 new_len = be_end - new_start + 1;
+
+			/* Add new entry covering the right half */
+			alloc_and_append_badrange_entry(badrange, new_start,
+					new_len, GFP_NOWAIT);
+			/* Adjust this entry to cover the left half */
+			be->length = start - be->start;
+			continue;
+		}
+	}
+	spin_unlock(&badrange->lock);
+}
+EXPORT_SYMBOL_GPL(badrange_forget);
+
+static void set_badblock(struct badblocks *bb, sector_t s, int num)
+{
+	dev_dbg(bb->dev, "Found a bad range (0x%llx, 0x%llx)\n",
+			(u64) s * 512, (u64) num * 512);
+	/* this isn't an error as the hardware will still throw an exception */
+	if (badblocks_set(bb, s, num, 1))
+		dev_info_once(bb->dev, "%s: failed for sector %llx\n",
+				__func__, (u64) s);
+}
+
+/**
+ * __add_badblock_range() - Convert a physical address range to bad sectors
+ * @bb:		badblocks instance to populate
+ * @ns_offset:	namespace offset where the error range begins (in bytes)
+ * @len:	number of bytes of badrange to be added
+ *
+ * This assumes that the range provided with (ns_offset, len) is within
+ * the bounds of physical addresses for this namespace, i.e. lies in the
+ * interval [ns_start, ns_start + ns_size)
+ */
+static void __add_badblock_range(struct badblocks *bb, u64 ns_offset, u64 len)
+{
+	const unsigned int sector_size = 512;
+	sector_t start_sector, end_sector;
+	u64 num_sectors;
+	u32 rem;
+
+	start_sector = div_u64(ns_offset, sector_size);
+	end_sector = div_u64_rem(ns_offset + len, sector_size, &rem);
+	if (rem)
+		end_sector++;
+	num_sectors = end_sector - start_sector;
+
+	if (unlikely(num_sectors > (u64)INT_MAX)) {
+		u64 remaining = num_sectors;
+		sector_t s = start_sector;
+
+		while (remaining) {
+			int done = min_t(u64, remaining, INT_MAX);
+
+			set_badblock(bb, s, done);
+			remaining -= done;
+			s += done;
+		}
+	} else
+		set_badblock(bb, start_sector, num_sectors);
+}
+
+static void badblocks_populate(struct badrange *badrange,
+		struct badblocks *bb, const struct resource *res)
+{
+	struct badrange_entry *be;
+
+	if (list_empty(&badrange->list))
+		return;
+
+	list_for_each_entry(be, &badrange->list, list) {
+		u64 be_end = be->start + be->length - 1;
+
+		/* Discard intervals with no intersection */
+		if (be_end < res->start)
+			continue;
+		if (be->start >  res->end)
+			continue;
+		/* Deal with any overlap after start of the namespace */
+		if (be->start >= res->start) {
+			u64 start = be->start;
+			u64 len;
+
+			if (be_end <= res->end)
+				len = be->length;
+			else
+				len = res->start + resource_size(res)
+					- be->start;
+			__add_badblock_range(bb, start - res->start, len);
+			continue;
+		}
+		/*
+		 * Deal with overlap for badrange starting before
+		 * the namespace.
+		 */
+		if (be->start < res->start) {
+			u64 len;
+
+			if (be_end < res->end)
+				len = be->start + be->length - res->start;
+			else
+				len = resource_size(res);
+			__add_badblock_range(bb, 0, len);
+		}
+	}
+}
+
+/**
+ * nvdimm_badblocks_populate() - Convert a list of badranges to badblocks
+ * @region: parent region of the range to interrogate
+ * @bb: badblocks instance to populate
+ * @res: resource range to consider
+ *
+ * The badrange list generated during bus initialization may contain
+ * multiple, possibly overlapping physical address ranges.  Compare each
+ * of these ranges to the resource range currently being initialized,
+ * and add badblocks entries for all matching sub-ranges
+ */
+void nvdimm_badblocks_populate(struct nd_region *nd_region,
+		struct badblocks *bb, const struct resource *res)
+{
+	struct nvdimm_bus *nvdimm_bus;
+
+	if (!is_memory(&nd_region->dev)) {
+		dev_WARN_ONCE(&nd_region->dev, 1,
+				"%s only valid for pmem regions\n", __func__);
+		return;
+	}
+	nvdimm_bus = walk_to_nvdimm_bus(&nd_region->dev);
+
+	nvdimm_bus_lock(&nvdimm_bus->dev);
+	badblocks_populate(&nvdimm_bus->badrange, bb, res);
+	nvdimm_bus_unlock(&nvdimm_bus->dev);
+}
+EXPORT_SYMBOL_GPL(nvdimm_badblocks_populate);
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index baf2839..0d4d0f4 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -11,6 +11,7 @@
  * General Public License for more details.
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/libnvdimm.h>
 #include <linux/sched/mm.h>
 #include <linux/vmalloc.h>
 #include <linux/uaccess.h>
@@ -221,7 +222,7 @@ static void nvdimm_account_cleared_poison(struct nvdimm_bus *nvdimm_bus,
 		phys_addr_t phys, u64 cleared)
 {
 	if (cleared > 0)
-		nvdimm_forget_poison(nvdimm_bus, phys, cleared);
+		badrange_forget(&nvdimm_bus->badrange, phys, cleared);
 
 	if (cleared > 0 && cleared / 512)
 		nvdimm_clear_badblocks_regions(nvdimm_bus, phys, cleared);
@@ -344,11 +345,10 @@ struct nvdimm_bus *nvdimm_bus_register(struct device *parent,
 		return NULL;
 	INIT_LIST_HEAD(&nvdimm_bus->list);
 	INIT_LIST_HEAD(&nvdimm_bus->mapping_list);
-	INIT_LIST_HEAD(&nvdimm_bus->poison_list);
 	init_waitqueue_head(&nvdimm_bus->probe_wait);
 	nvdimm_bus->id = ida_simple_get(&nd_ida, 0, 0, GFP_KERNEL);
 	mutex_init(&nvdimm_bus->reconfig_mutex);
-	spin_lock_init(&nvdimm_bus->poison_lock);
+	badrange_init(&nvdimm_bus->badrange);
 	if (nvdimm_bus->id < 0) {
 		kfree(nvdimm_bus);
 		return NULL;
@@ -395,15 +395,15 @@ static int child_unregister(struct device *dev, void *data)
 	return 0;
 }
 
-static void free_poison_list(struct list_head *poison_list)
+static void free_badrange_list(struct list_head *badrange_list)
 {
-	struct nd_poison *pl, *next;
+	struct badrange_entry *be, *next;
 
-	list_for_each_entry_safe(pl, next, poison_list, list) {
-		list_del(&pl->list);
-		kfree(pl);
+	list_for_each_entry_safe(be, next, badrange_list, list) {
+		list_del(&be->list);
+		kfree(be);
 	}
-	list_del_init(poison_list);
+	list_del_init(badrange_list);
 }
 
 static int nd_bus_remove(struct device *dev)
@@ -417,9 +417,9 @@ static int nd_bus_remove(struct device *dev)
 	nd_synchronize();
 	device_for_each_child(&nvdimm_bus->dev, NULL, child_unregister);
 
-	spin_lock(&nvdimm_bus->poison_lock);
-	free_poison_list(&nvdimm_bus->poison_list);
-	spin_unlock(&nvdimm_bus->poison_lock);
+	spin_lock(&nvdimm_bus->badrange.lock);
+	free_badrange_list(&nvdimm_bus->badrange.list);
+	spin_unlock(&nvdimm_bus->badrange.lock);
 
 	nvdimm_bus_destroy_ndctl(nvdimm_bus);
 
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index bb71f0c..1dc5276 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -398,265 +398,11 @@ struct attribute_group nvdimm_bus_attribute_group = {
 };
 EXPORT_SYMBOL_GPL(nvdimm_bus_attribute_group);
 
-static void set_badblock(struct badblocks *bb, sector_t s, int num)
+int nvdimm_bus_add_badrange(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length)
 {
-	dev_dbg(bb->dev, "Found a poison range (0x%llx, 0x%llx)\n",
-			(u64) s * 512, (u64) num * 512);
-	/* this isn't an error as the hardware will still throw an exception */
-	if (badblocks_set(bb, s, num, 1))
-		dev_info_once(bb->dev, "%s: failed for sector %llx\n",
-				__func__, (u64) s);
+	return badrange_add(&nvdimm_bus->badrange, addr, length);
 }
-
-/**
- * __add_badblock_range() - Convert a physical address range to bad sectors
- * @bb:		badblocks instance to populate
- * @ns_offset:	namespace offset where the error range begins (in bytes)
- * @len:	number of bytes of poison to be added
- *
- * This assumes that the range provided with (ns_offset, len) is within
- * the bounds of physical addresses for this namespace, i.e. lies in the
- * interval [ns_start, ns_start + ns_size)
- */
-static void __add_badblock_range(struct badblocks *bb, u64 ns_offset, u64 len)
-{
-	const unsigned int sector_size = 512;
-	sector_t start_sector, end_sector;
-	u64 num_sectors;
-	u32 rem;
-
-	start_sector = div_u64(ns_offset, sector_size);
-	end_sector = div_u64_rem(ns_offset + len, sector_size, &rem);
-	if (rem)
-		end_sector++;
-	num_sectors = end_sector - start_sector;
-
-	if (unlikely(num_sectors > (u64)INT_MAX)) {
-		u64 remaining = num_sectors;
-		sector_t s = start_sector;
-
-		while (remaining) {
-			int done = min_t(u64, remaining, INT_MAX);
-
-			set_badblock(bb, s, done);
-			remaining -= done;
-			s += done;
-		}
-	} else
-		set_badblock(bb, start_sector, num_sectors);
-}
-
-static void badblocks_populate(struct list_head *poison_list,
-		struct badblocks *bb, const struct resource *res)
-{
-	struct nd_poison *pl;
-
-	if (list_empty(poison_list))
-		return;
-
-	list_for_each_entry(pl, poison_list, list) {
-		u64 pl_end = pl->start + pl->length - 1;
-
-		/* Discard intervals with no intersection */
-		if (pl_end < res->start)
-			continue;
-		if (pl->start >  res->end)
-			continue;
-		/* Deal with any overlap after start of the namespace */
-		if (pl->start >= res->start) {
-			u64 start = pl->start;
-			u64 len;
-
-			if (pl_end <= res->end)
-				len = pl->length;
-			else
-				len = res->start + resource_size(res)
-					- pl->start;
-			__add_badblock_range(bb, start - res->start, len);
-			continue;
-		}
-		/* Deal with overlap for poison starting before the namespace */
-		if (pl->start < res->start) {
-			u64 len;
-
-			if (pl_end < res->end)
-				len = pl->start + pl->length - res->start;
-			else
-				len = resource_size(res);
-			__add_badblock_range(bb, 0, len);
-		}
-	}
-}
-
-/**
- * nvdimm_badblocks_populate() - Convert a list of poison ranges to badblocks
- * @region: parent region of the range to interrogate
- * @bb: badblocks instance to populate
- * @res: resource range to consider
- *
- * The poison list generated during bus initialization may contain
- * multiple, possibly overlapping physical address ranges.  Compare each
- * of these ranges to the resource range currently being initialized,
- * and add badblocks entries for all matching sub-ranges
- */
-void nvdimm_badblocks_populate(struct nd_region *nd_region,
-		struct badblocks *bb, const struct resource *res)
-{
-	struct nvdimm_bus *nvdimm_bus;
-	struct list_head *poison_list;
-
-	if (!is_memory(&nd_region->dev)) {
-		dev_WARN_ONCE(&nd_region->dev, 1,
-				"%s only valid for pmem regions\n", __func__);
-		return;
-	}
-	nvdimm_bus = walk_to_nvdimm_bus(&nd_region->dev);
-	poison_list = &nvdimm_bus->poison_list;
-
-	nvdimm_bus_lock(&nvdimm_bus->dev);
-	badblocks_populate(poison_list, bb, res);
-	nvdimm_bus_unlock(&nvdimm_bus->dev);
-}
-EXPORT_SYMBOL_GPL(nvdimm_badblocks_populate);
-
-static void append_poison_entry(struct nvdimm_bus *nvdimm_bus,
-		struct nd_poison *pl, u64 addr, u64 length)
-{
-	lockdep_assert_held(&nvdimm_bus->poison_lock);
-	pl->start = addr;
-	pl->length = length;
-	list_add_tail(&pl->list, &nvdimm_bus->poison_list);
-}
-
-static int add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length,
-			gfp_t flags)
-{
-	struct nd_poison *pl;
-
-	pl = kzalloc(sizeof(*pl), flags);
-	if (!pl)
-		return -ENOMEM;
-
-	append_poison_entry(nvdimm_bus, pl, addr, length);
-	return 0;
-}
-
-static int bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length)
-{
-	struct nd_poison *pl, *pl_new;
-
-	spin_unlock(&nvdimm_bus->poison_lock);
-	pl_new = kzalloc(sizeof(*pl_new), GFP_KERNEL);
-	spin_lock(&nvdimm_bus->poison_lock);
-
-	if (list_empty(&nvdimm_bus->poison_list)) {
-		if (!pl_new)
-			return -ENOMEM;
-		append_poison_entry(nvdimm_bus, pl_new, addr, length);
-		return 0;
-	}
-
-	/*
-	 * There is a chance this is a duplicate, check for those first.
-	 * This will be the common case as ARS_STATUS returns all known
-	 * errors in the SPA space, and we can't query it per region
-	 */
-	list_for_each_entry(pl, &nvdimm_bus->poison_list, list)
-		if (pl->start == addr) {
-			/* If length has changed, update this list entry */
-			if (pl->length != length)
-				pl->length = length;
-			kfree(pl_new);
-			return 0;
-		}
-
-	/*
-	 * If not a duplicate or a simple length update, add the entry as is,
-	 * as any overlapping ranges will get resolved when the list is consumed
-	 * and converted to badblocks
-	 */
-	if (!pl_new)
-		return -ENOMEM;
-	append_poison_entry(nvdimm_bus, pl_new, addr, length);
-
-	return 0;
-}
-
-int nvdimm_bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length)
-{
-	int rc;
-
-	spin_lock(&nvdimm_bus->poison_lock);
-	rc = bus_add_poison(nvdimm_bus, addr, length);
-	spin_unlock(&nvdimm_bus->poison_lock);
-
-	return rc;
-}
-EXPORT_SYMBOL_GPL(nvdimm_bus_add_poison);
-
-void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus, phys_addr_t start,
-		unsigned int len)
-{
-	struct list_head *poison_list = &nvdimm_bus->poison_list;
-	u64 clr_end = start + len - 1;
-	struct nd_poison *pl, *next;
-
-	spin_lock(&nvdimm_bus->poison_lock);
-	WARN_ON_ONCE(list_empty(poison_list));
-
-	/*
-	 * [start, clr_end] is the poison interval being cleared.
-	 * [pl->start, pl_end] is the poison_list entry we're comparing
-	 * the above interval against. The poison list entry may need
-	 * to be modified (update either start or length), deleted, or
-	 * split into two based on the overlap characteristics
-	 */
-
-	list_for_each_entry_safe(pl, next, poison_list, list) {
-		u64 pl_end = pl->start + pl->length - 1;
-
-		/* Skip intervals with no intersection */
-		if (pl_end < start)
-			continue;
-		if (pl->start >  clr_end)
-			continue;
-		/* Delete completely overlapped poison entries */
-		if ((pl->start >= start) && (pl_end <= clr_end)) {
-			list_del(&pl->list);
-			kfree(pl);
-			continue;
-		}
-		/* Adjust start point of partially cleared entries */
-		if ((start <= pl->start) && (clr_end > pl->start)) {
-			pl->length -= clr_end - pl->start + 1;
-			pl->start = clr_end + 1;
-			continue;
-		}
-		/* Adjust pl->length for partial clearing at the tail end */
-		if ((pl->start < start) && (pl_end <= clr_end)) {
-			/* pl->start remains the same */
-			pl->length = start - pl->start;
-			continue;
-		}
-		/*
-		 * If clearing in the middle of an entry, we split it into
-		 * two by modifying the current entry to represent one half of
-		 * the split, and adding a new entry for the second half.
-		 */
-		if ((pl->start < start) && (pl_end > clr_end)) {
-			u64 new_start = clr_end + 1;
-			u64 new_len = pl_end - new_start + 1;
-
-			/* Add new entry covering the right half */
-			add_poison(nvdimm_bus, new_start, new_len, GFP_NOWAIT);
-			/* Adjust this entry to cover the left half */
-			pl->length = start - pl->start;
-			continue;
-		}
-	}
-	spin_unlock(&nvdimm_bus->poison_lock);
-}
-EXPORT_SYMBOL_GPL(nvdimm_forget_poison);
+EXPORT_SYMBOL_GPL(nvdimm_bus_add_badrange);
 
 #ifdef CONFIG_BLK_DEV_INTEGRITY
 int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 86bc19a..79274ea 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -29,10 +29,9 @@ struct nvdimm_bus {
 	struct list_head list;
 	struct device dev;
 	int id, probe_active;
-	struct list_head poison_list;
 	struct list_head mapping_list;
 	struct mutex reconfig_mutex;
-	spinlock_t poison_lock;
+	struct badrange badrange;
 };
 
 struct nvdimm {
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index 023fc93..377c800 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -34,12 +34,6 @@ enum {
 	NVDIMM_IO_ATOMIC = 1,
 };
 
-struct nd_poison {
-	u64 start;
-	u64 length;
-	struct list_head list;
-};
-
 struct nvdimm_drvdata {
 	struct device *dev;
 	int nslabel_size;
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 3eaad2f..f8109dd 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -18,6 +18,18 @@
 #include <linux/sizes.h>
 #include <linux/types.h>
 #include <linux/uuid.h>
+#include <linux/spinlock.h>
+
+struct badrange_entry {
+	u64 start;
+	u64 length;
+	struct list_head list;
+};
+
+struct badrange {
+	struct list_head list;
+	spinlock_t lock;
+};
 
 enum {
 	/* when a dimm supports both PMEM and BLK access a label is required */
@@ -129,9 +141,12 @@ static inline struct nd_blk_region_desc *to_blk_region_desc(
 
 }
 
-int nvdimm_bus_add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length);
-void nvdimm_forget_poison(struct nvdimm_bus *nvdimm_bus,
-		phys_addr_t start, unsigned int len);
+void badrange_init(struct badrange *badrange);
+int badrange_add(struct badrange *badrange, u64 addr, u64 length);
+void badrange_forget(struct badrange *badrange, phys_addr_t start,
+		unsigned int len);
+int nvdimm_bus_add_badrange(struct nvdimm_bus *nvdimm_bus, u64 addr,
+		u64 length);
 struct nvdimm_bus *nvdimm_bus_register(struct device *parent,
 		struct nvdimm_bus_descriptor *nfit_desc);
 void nvdimm_bus_unregister(struct nvdimm_bus *nvdimm_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] 12+ messages in thread

* [PATCH 2/4] nfit_test: add error injection DSMs
  2017-10-06  1:53 [PATCH 0/4] add error injection commands to nfit_test Vishal Verma
  2017-10-06  1:53 ` [PATCH 1/4] libnvdimm: move poison list functions to a new 'badrange' file Vishal Verma
@ 2017-10-06  1:53 ` Vishal Verma
  2017-10-07 17:07   ` Dan Williams
  2017-10-06  1:53 ` [PATCH 3/4] libnvdimm, badrange: remove a WARN for list_empty Vishal Verma
  2017-10-06  1:53 ` [PATCH 4/4] nfit_test: when clearing poison, also remove badrange entries Vishal Verma
  3 siblings, 1 reply; 12+ messages in thread
From: Vishal Verma @ 2017-10-06  1:53 UTC (permalink / raw)
  To: linux-nvdimm

From: Dave Jiang <dave.jiang@intel.com>

From: Dave Jiang <dave.jiang@intel.com>

Add nfit_test emulation for the new ACPI 6.2 error injectino DSMs.
This will allow unit tests to selectively inject the errors they wish to
test for.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
[vishal: Add badrange.o to nfit_test's Kbuild]
[vishal: Move injection functions to ND_CMD_CALL]
[vishal: Add support for the notification option]
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 drivers/acpi/nfit/nfit.h              |   1 +
 tools/testing/nvdimm/Kbuild           |   1 +
 tools/testing/nvdimm/test/nfit.c      | 187 +++++++++++++++++++++++++++++-----
 tools/testing/nvdimm/test/nfit_test.h |   4 +
 4 files changed, 169 insertions(+), 24 deletions(-)

diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
index 7093bd5..7ddf48f 100644
--- a/drivers/acpi/nfit/nfit.h
+++ b/drivers/acpi/nfit/nfit.h
@@ -75,6 +75,7 @@ enum {
 	NFIT_ARS_CAP_NONE = 1,
 	NFIT_ARS_F_OVERFLOW = 1,
 	NFIT_ARS_TIMEOUT = 90,
+	NFIT_ARS_INJECT_INVALID = 2,
 };
 
 enum nfit_root_notifiers {
diff --git a/tools/testing/nvdimm/Kbuild b/tools/testing/nvdimm/Kbuild
index d870520..3272ab5 100644
--- a/tools/testing/nvdimm/Kbuild
+++ b/tools/testing/nvdimm/Kbuild
@@ -69,6 +69,7 @@ libnvdimm-y += $(NVDIMM_SRC)/region_devs.o
 libnvdimm-y += $(NVDIMM_SRC)/region.o
 libnvdimm-y += $(NVDIMM_SRC)/namespace_devs.o
 libnvdimm-y += $(NVDIMM_SRC)/label.o
+libnvdimm-y += $(NVDIMM_SRC)/badrange.o
 libnvdimm-$(CONFIG_ND_CLAIM) += $(NVDIMM_SRC)/claim.o
 libnvdimm-$(CONFIG_BTT) += $(NVDIMM_SRC)/btt_devs.o
 libnvdimm-$(CONFIG_NVDIMM_PFN) += $(NVDIMM_SRC)/pfn_devs.o
diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index 435d298..43f948e 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -168,8 +168,12 @@ struct nfit_test {
 		spinlock_t lock;
 	} ars_state;
 	struct device *dimm_dev[NUM_DCR];
+	struct badrange badrange;
+	struct work_struct work;
 };
 
+static struct workqueue_struct *nfit_wq;
+
 static struct nfit_test *to_nfit_test(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -234,48 +238,68 @@ static int nfit_test_cmd_set_config_data(struct nd_cmd_set_config_hdr *nd_cmd,
 	return rc;
 }
 
-#define NFIT_TEST_ARS_RECORDS 4
 #define NFIT_TEST_CLEAR_ERR_UNIT 256
 
 static int nfit_test_cmd_ars_cap(struct nd_cmd_ars_cap *nd_cmd,
 		unsigned int buf_len)
 {
+	int ars_recs;
+
 	if (buf_len < sizeof(*nd_cmd))
 		return -EINVAL;
 
+	/* for testing, only store up to n records fit withint 4k */
+	ars_recs = SZ_4K / sizeof(struct nd_ars_record);
+
 	nd_cmd->max_ars_out = sizeof(struct nd_cmd_ars_status)
-		+ NFIT_TEST_ARS_RECORDS * sizeof(struct nd_ars_record);
+		+ ars_recs * sizeof(struct nd_ars_record);
 	nd_cmd->status = (ND_ARS_PERSISTENT | ND_ARS_VOLATILE) << 16;
 	nd_cmd->clear_err_unit = NFIT_TEST_CLEAR_ERR_UNIT;
 
 	return 0;
 }
 
-/*
- * Initialize the ars_state to return an ars_result 1 second in the future with
- * a 4K error range in the middle of the requested address range.
- */
-static void post_ars_status(struct ars_state *ars_state, u64 addr, u64 len)
+static void post_ars_status(struct ars_state *ars_state,
+		struct badrange *badrange, u64 addr, u64 len)
 {
 	struct nd_cmd_ars_status *ars_status;
 	struct nd_ars_record *ars_record;
+	struct badrange_entry *be;
+	u64 end = addr + len - 1;
+	int i = 0;
 
 	ars_state->deadline = jiffies + 1*HZ;
 	ars_status = ars_state->ars_status;
 	ars_status->status = 0;
-	ars_status->out_length = sizeof(struct nd_cmd_ars_status)
-		+ sizeof(struct nd_ars_record);
 	ars_status->address = addr;
 	ars_status->length = len;
 	ars_status->type = ND_ARS_PERSISTENT;
-	ars_status->num_records = 1;
-	ars_record = &ars_status->records[0];
-	ars_record->handle = 0;
-	ars_record->err_address = addr + len / 2;
-	ars_record->length = SZ_4K;
+
+	spin_lock(&badrange->lock);
+	list_for_each_entry(be, &badrange->list, list) {
+		u64 be_end = be->start + be->length - 1;
+		u64 rstart, rend;
+
+		/* skip entries outside the range */
+		if (be_end < addr || be->start > end)
+			continue;
+
+		rstart = (be->start < addr) ? addr : be->start;
+		rend = (be_end < end) ? be_end : end;
+		ars_record = &ars_status->records[i];
+		ars_record->handle = 0;
+		ars_record->err_address = rstart;
+		ars_record->length = rend - rstart + 1;
+		i++;
+	}
+	spin_unlock(&badrange->lock);
+	ars_status->num_records = i;
+	ars_status->out_length = sizeof(struct nd_cmd_ars_status)
+		+ i * sizeof(struct nd_ars_record);
 }
 
-static int nfit_test_cmd_ars_start(struct ars_state *ars_state,
+static int nfit_test_cmd_ars_start(struct nfit_test *t,
+		struct ars_state *ars_state,
 		struct nd_cmd_ars_start *ars_start, unsigned int buf_len,
 		int *cmd_rc)
 {
@@ -289,7 +313,7 @@ static int nfit_test_cmd_ars_start(struct ars_state *ars_state,
 	} else {
 		ars_start->status = 0;
 		ars_start->scrub_time = 1;
-		post_ars_status(ars_state, ars_start->address,
+		post_ars_status(ars_state, &t->badrange, ars_start->address,
 				ars_start->length);
 		*cmd_rc = 0;
 	}
@@ -456,6 +480,93 @@ static int nfit_test_cmd_smart_threshold(struct nd_cmd_smart_threshold *smart_t,
 	return 0;
 }
 
+static void uc_error_notify(struct work_struct *work)
+{
+	struct nfit_test *t = container_of(work, typeof(*t), work);
+
+	__acpi_nfit_notify(&t->pdev.dev, t, NFIT_NOTIFY_UC_MEMORY_ERROR);
+}
+
+static int nfit_test_cmd_ars_error_inject(struct nfit_test *t,
+		struct nd_cmd_ars_err_inj *err_inj, unsigned int buf_len)
+{
+	int rc;
+
+	if (buf_len < sizeof(*err_inj)) {
+		rc = -EINVAL;
+		goto err;
+	}
+
+	if (err_inj->err_inj_spa_range_length <= 0) {
+		rc = -EINVAL;
+		goto err;
+	}
+
+	rc =  badrange_add(&t->badrange, err_inj->err_inj_spa_range_base,
+			err_inj->err_inj_spa_range_length);
+	if (rc < 0)
+		goto err;
+
+	if (err_inj->err_inj_options & (1 << ND_ARS_ERR_INJ_OPT_NOTIFY))
+		queue_work(nfit_wq, &t->work);
+
+	err_inj->status = 0;
+	return 0;
+
+err:
+	err_inj->status = NFIT_ARS_INJECT_INVALID;
+	return rc;
+}
+
+static int nfit_test_cmd_ars_inject_clear(struct nfit_test *t,
+		struct nd_cmd_ars_err_inj_clr *err_clr, unsigned int buf_len)
+{
+	int rc;
+
+	if (buf_len < sizeof(*err_clr)) {
+		rc = -EINVAL;
+		goto err;
+	}
+
+	if (err_clr->err_inj_clr_spa_range_length <= 0) {
+		rc = -EINVAL;
+		goto err;
+	}
+
+	badrange_forget(&t->badrange, err_clr->err_inj_clr_spa_range_base,
+			err_clr->err_inj_clr_spa_range_length);
+
+	err_clr->status = 0;
+	return 0;
+
+err:
+	err_clr->status = NFIT_ARS_INJECT_INVALID;
+	return rc;
+}
+
+static int nfit_test_cmd_ars_inject_status(struct nfit_test *t,
+		struct nd_cmd_ars_err_inj_stat *err_stat,
+		unsigned int buf_len)
+{
+	struct badrange_entry *be;
+	int max = SZ_4K / sizeof(struct nd_error_stat_query_record);
+	int i = 0;
+
+	err_stat->status = 0;
+	spin_lock(&t->badrange.lock);
+	list_for_each_entry(be, &t->badrange.list, list) {
+		err_stat->record[i].err_inj_stat_spa_range_base = be->start;
+		err_stat->record[i].err_inj_stat_spa_range_length = be->length;
+		i++;
+		if (i > max)
+			break;
+	}
+	spin_unlock(&t->badrange.lock);
+	err_stat->inj_err_rec_count = i;
+
+	return 0;
+}
+
 static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 		struct nvdimm *nvdimm, unsigned int cmd, void *buf,
 		unsigned int buf_len, int *cmd_rc)
@@ -543,6 +654,18 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 				rc = nfit_test_cmd_translate_spa(acpi_desc->nvdimm_bus,
 							buf, buf_len);
 				return rc;
+			case NFIT_CMD_ARS_INJECT_SET:
+				rc = nfit_test_cmd_ars_error_inject(t, buf,
+					buf_len);
+				return rc;
+			case NFIT_CMD_ARS_INJECT_CLEAR:
+				rc = nfit_test_cmd_ars_inject_clear(t, buf,
+					buf_len);
+				return rc;
+			case NFIT_CMD_ARS_INJECT_GET:
+				rc = nfit_test_cmd_ars_inject_status(t, buf,
+					buf_len);
+				return rc;
 			default:
 				return -ENOTTY;
 			}
@@ -556,8 +679,8 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 			rc = nfit_test_cmd_ars_cap(buf, buf_len);
 			break;
 		case ND_CMD_ARS_START:
-			rc = nfit_test_cmd_ars_start(ars_state, buf, buf_len,
-					cmd_rc);
+			rc = nfit_test_cmd_ars_start(t, ars_state, buf,
+					buf_len, cmd_rc);
 			break;
 		case ND_CMD_ARS_STATUS:
 			rc = nfit_test_cmd_ars_status(ars_state, buf, buf_len,
@@ -664,10 +787,9 @@ static struct nfit_test_resource *nfit_test_lookup(resource_size_t addr)
 
 static int ars_state_init(struct device *dev, struct ars_state *ars_state)
 {
+	/* for testing, only store up to n records fit withint 4k */
 	ars_state->ars_status = devm_kzalloc(dev,
-			sizeof(struct nd_cmd_ars_status)
-			+ sizeof(struct nd_ars_record) * NFIT_TEST_ARS_RECORDS,
-			GFP_KERNEL);
+			sizeof(struct nd_cmd_ars_status) + SZ_4K, GFP_KERNEL);
 	if (!ars_state->ars_status)
 		return -ENOMEM;
 	spin_lock_init(&ars_state->lock);
@@ -1517,7 +1639,8 @@ static void nfit_test0_setup(struct nfit_test *t)
 				+ i * sizeof(u64);
 	}
 
-	post_ars_status(&t->ars_state, t->spa_set_dma[0], SPA0_SIZE);
+	post_ars_status(&t->ars_state, &t->badrange, t->spa_set_dma[0],
+			SPA0_SIZE);
 
 	acpi_desc = &t->acpi_desc;
 	set_bit(ND_CMD_GET_CONFIG_SIZE, &acpi_desc->dimm_cmd_force_en);
@@ -1531,6 +1654,9 @@ static void nfit_test0_setup(struct nfit_test *t)
 	set_bit(ND_CMD_CALL, &acpi_desc->bus_cmd_force_en);
 	set_bit(ND_CMD_SMART_THRESHOLD, &acpi_desc->dimm_cmd_force_en);
 	set_bit(NFIT_CMD_TRANSLATE_SPA, &acpi_desc->bus_passthru_cmd_force_en);
+	set_bit(NFIT_CMD_ARS_INJECT_SET, &acpi_desc->bus_passthru_cmd_force_en);
+	set_bit(NFIT_CMD_ARS_INJECT_CLEAR, &acpi_desc->bus_passthru_cmd_force_en);
+	set_bit(NFIT_CMD_ARS_INJECT_GET, &acpi_desc->bus_passthru_cmd_force_en);
 }
 
 static void nfit_test1_setup(struct nfit_test *t)
@@ -1620,7 +1746,8 @@ static void nfit_test1_setup(struct nfit_test *t)
 	dcr->code = NFIT_FIC_BYTE;
 	dcr->windows = 0;
 
-	post_ars_status(&t->ars_state, t->spa_set_dma[0], SPA2_SIZE);
+	post_ars_status(&t->ars_state, &t->badrange, t->spa_set_dma[0],
+			SPA2_SIZE);
 
 	acpi_desc = &t->acpi_desc;
 	set_bit(ND_CMD_ARS_CAP, &acpi_desc->bus_cmd_force_en);
@@ -1721,7 +1848,10 @@ static int nfit_ctl_test(struct device *dev)
 			.module = THIS_MODULE,
 			.provider_name = "ACPI.NFIT",
 			.ndctl = acpi_nfit_ctl,
-			.bus_dsm_mask = 1UL << NFIT_CMD_TRANSLATE_SPA,
+			.bus_dsm_mask = 1UL << NFIT_CMD_TRANSLATE_SPA
+				| 1UL << NFIT_CMD_ARS_INJECT_SET
+				| 1UL << NFIT_CMD_ARS_INJECT_CLEAR
+				| 1UL << NFIT_CMD_ARS_INJECT_GET,
 		},
 		.dev = &adev->dev,
 	};
@@ -2020,6 +2150,10 @@ static __init int nfit_test_init(void)
 
 	nfit_test_setup(nfit_test_lookup, nfit_test_evaluate_dsm);
 
+	nfit_wq = create_singlethread_workqueue("nfit");
+	if (!nfit_wq)
+		return -ENOMEM;
+
 	nfit_test_dimm = class_create(THIS_MODULE, "nfit_test_dimm");
 	if (IS_ERR(nfit_test_dimm)) {
 		rc = PTR_ERR(nfit_test_dimm);
@@ -2036,6 +2170,7 @@ static __init int nfit_test_init(void)
 			goto err_register;
 		}
 		INIT_LIST_HEAD(&nfit_test->resources);
+		badrange_init(&nfit_test->badrange);
 		switch (i) {
 		case 0:
 			nfit_test->num_pm = NUM_PM;
@@ -2071,6 +2206,7 @@ static __init int nfit_test_init(void)
 			goto err_register;
 
 		instances[i] = nfit_test;
+		INIT_WORK(&nfit_test->work, uc_error_notify);
 	}
 
 	rc = platform_driver_register(&nfit_test_driver);
@@ -2079,6 +2215,7 @@ static __init int nfit_test_init(void)
 	return 0;
 
  err_register:
+	destroy_workqueue(nfit_wq);
 	for (i = 0; i < NUM_NFITS; i++)
 		if (instances[i])
 			platform_device_unregister(&instances[i]->pdev);
@@ -2094,6 +2231,8 @@ static __exit void nfit_test_exit(void)
 {
 	int i;
 
+	flush_workqueue(nfit_wq);
+	destroy_workqueue(nfit_wq);
 	for (i = 0; i < NUM_NFITS; i++)
 		platform_device_unregister(&instances[i]->pdev);
 	platform_driver_unregister(&nfit_test_driver);
diff --git a/tools/testing/nvdimm/test/nfit_test.h b/tools/testing/nvdimm/test/nfit_test.h
index 735a452..b44f927 100644
--- a/tools/testing/nvdimm/test/nfit_test.h
+++ b/tools/testing/nvdimm/test/nfit_test.h
@@ -34,6 +34,10 @@ struct nfit_test_resource {
 
 #define ND_TRANSLATE_SPA_STATUS_INVALID_SPA  2
 
+enum err_inj_options {
+	ND_ARS_ERR_INJ_OPT_NOTIFY = 0,
+};
+
 /* bus passthru commands */
 enum nfit_test_passthru_cmd {
 	NFIT_CMD_TRANSLATE_SPA = 5,
-- 
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] 12+ messages in thread

* [PATCH 3/4] libnvdimm, badrange: remove a WARN for list_empty
  2017-10-06  1:53 [PATCH 0/4] add error injection commands to nfit_test Vishal Verma
  2017-10-06  1:53 ` [PATCH 1/4] libnvdimm: move poison list functions to a new 'badrange' file Vishal Verma
  2017-10-06  1:53 ` [PATCH 2/4] nfit_test: add error injection DSMs Vishal Verma
@ 2017-10-06  1:53 ` Vishal Verma
  2017-10-06  1:53 ` [PATCH 4/4] nfit_test: when clearing poison, also remove badrange entries Vishal Verma
  3 siblings, 0 replies; 12+ messages in thread
From: Vishal Verma @ 2017-10-06  1:53 UTC (permalink / raw)
  To: linux-nvdimm

Now that we're reusing the badrange functions for nfit_test, and that
exposes badrange injection/clearing to userspace via the DSM paths, it
is plausible that a user may call the clear DSM multiple times. Since it
is harmless to do so, we can remove the WARN in badrange_forget.

Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 drivers/nvdimm/badrange.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c
index 6ad782f..bfdec70 100644
--- a/drivers/nvdimm/badrange.c
+++ b/drivers/nvdimm/badrange.c
@@ -114,7 +114,6 @@ void badrange_forget(struct badrange *badrange, phys_addr_t start,
 	struct badrange_entry *be, *next;
 
 	spin_lock(&badrange->lock);
-	WARN_ON_ONCE(list_empty(badrange_list));
 
 	/*
 	 * [start, clr_end] is the badrange interval being cleared.
-- 
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] 12+ messages in thread

* [PATCH 4/4] nfit_test: when clearing poison, also remove badrange entries
  2017-10-06  1:53 [PATCH 0/4] add error injection commands to nfit_test Vishal Verma
                   ` (2 preceding siblings ...)
  2017-10-06  1:53 ` [PATCH 3/4] libnvdimm, badrange: remove a WARN for list_empty Vishal Verma
@ 2017-10-06  1:53 ` Vishal Verma
  2017-10-09 16:12   ` Dave Jiang
  3 siblings, 1 reply; 12+ messages in thread
From: Vishal Verma @ 2017-10-06  1:53 UTC (permalink / raw)
  To: linux-nvdimm

The injected badrange entries can only be cleared from the kernel's
accounting by writing to the affected blocks, so when such a write sends
the clear error DSM to nfit_test, also clear the ranges from
nfit_test's badrange list. This lets an 'ARS Inject error status' DSM to
return the correct status, omitting the cleared ranges.

Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 tools/testing/nvdimm/test/nfit.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index 43f948e..e400418 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -344,7 +344,8 @@ static int nfit_test_cmd_ars_status(struct ars_state *ars_state,
 	return 0;
 }
 
-static int nfit_test_cmd_clear_error(struct nd_cmd_clear_error *clear_err,
+static int nfit_test_cmd_clear_error(struct nfit_test *t,
+		struct nd_cmd_clear_error *clear_err,
 		unsigned int buf_len, int *cmd_rc)
 {
 	const u64 mask = NFIT_TEST_CLEAR_ERR_UNIT - 1;
@@ -354,12 +355,7 @@ static int nfit_test_cmd_clear_error(struct nd_cmd_clear_error *clear_err,
 	if ((clear_err->address & mask) || (clear_err->length & mask))
 		return -EINVAL;
 
-	/*
-	 * Report 'all clear' success for all commands even though a new
-	 * scrub will find errors again.  This is enough to have the
-	 * error removed from the 'badblocks' tracking in the pmem
-	 * driver.
-	 */
+	badrange_forget(&t->badrange, clear_err->address, clear_err->length);
 	clear_err->status = 0;
 	clear_err->cleared = clear_err->length;
 	*cmd_rc = 0;
@@ -687,7 +683,7 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 					cmd_rc);
 			break;
 		case ND_CMD_CLEAR_ERROR:
-			rc = nfit_test_cmd_clear_error(buf, buf_len, cmd_rc);
+			rc = nfit_test_cmd_clear_error(t, buf, buf_len, cmd_rc);
 			break;
 		default:
 			return -ENOTTY;
-- 
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] 12+ messages in thread

* Re: [PATCH 1/4] libnvdimm: move poison list functions to a new 'badrange' file
  2017-10-06  1:53 ` [PATCH 1/4] libnvdimm: move poison list functions to a new 'badrange' file Vishal Verma
@ 2017-10-06 18:52   ` Dan Williams
  2017-10-09 20:23     ` Verma, Vishal L
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2017-10-06 18:52 UTC (permalink / raw)
  To: Vishal Verma; +Cc: linux-nvdimm

On Thu, Oct 5, 2017 at 6:53 PM, Vishal Verma <vishal.l.verma@intel.com> wrote:
> From: Dave Jiang <dave.jiang@intel.com>
>
> From: Dave Jiang <dave.jiang@intel.com>
>
> nfit_test needs to use the poison list manipulation code as well. Make
> it more generic and in the process rename poison to badrange, and move
> all the related helpers to a new file.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> [vishal: add a missed include in bus.c for the new badrange functions]
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  drivers/acpi/nfit/core.c  |   2 +-
>  drivers/acpi/nfit/mce.c   |   2 +-
>  drivers/nvdimm/Makefile   |   1 +
>  drivers/nvdimm/badrange.c | 294 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/nvdimm/bus.c      |  24 ++--
>  drivers/nvdimm/core.c     | 260 +---------------------------------------
>  drivers/nvdimm/nd-core.h  |   3 +-
>  drivers/nvdimm/nd.h       |   6 -
>  include/linux/libnvdimm.h |  21 +++-
>  9 files changed, 331 insertions(+), 282 deletions(-)
>  create mode 100644 drivers/nvdimm/badrange.c
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index a3ecd5e..4b157f8 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -2240,7 +2240,7 @@ static int ars_status_process_records(struct acpi_nfit_desc *acpi_desc,
>                 if (ars_status->out_length
>                                 < 44 + sizeof(struct nd_ars_record) * (i + 1))
>                         break;
> -               rc = nvdimm_bus_add_poison(nvdimm_bus,
> +               rc = nvdimm_bus_add_badrange(nvdimm_bus,
>                                 ars_status->records[i].err_address,
>                                 ars_status->records[i].length);
>                 if (rc)
> diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c
> index feeb95d..b929214 100644
> --- a/drivers/acpi/nfit/mce.c
> +++ b/drivers/acpi/nfit/mce.c
> @@ -67,7 +67,7 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val,
>                         continue;
>
>                 /* If this fails due to an -ENOMEM, there is little we can do */
> -               nvdimm_bus_add_poison(acpi_desc->nvdimm_bus,
> +               nvdimm_bus_add_badrange(acpi_desc->nvdimm_bus,
>                                 ALIGN(mce->addr, L1_CACHE_BYTES),
>                                 L1_CACHE_BYTES);
>                 nvdimm_region_notify(nfit_spa->nd_region,
> diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
> index 909554c..ca6d325 100644
> --- a/drivers/nvdimm/Makefile
> +++ b/drivers/nvdimm/Makefile
> @@ -20,6 +20,7 @@ libnvdimm-y += region_devs.o
>  libnvdimm-y += region.o
>  libnvdimm-y += namespace_devs.o
>  libnvdimm-y += label.o
> +libnvdimm-y += badrange.o
>  libnvdimm-$(CONFIG_ND_CLAIM) += claim.o
>  libnvdimm-$(CONFIG_BTT) += btt_devs.o
>  libnvdimm-$(CONFIG_NVDIMM_PFN) += pfn_devs.o
> diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c
> new file mode 100644
> index 0000000..6ad782f
> --- /dev/null
> +++ b/drivers/nvdimm/badrange.c
> @@ -0,0 +1,294 @@
> +/*
> + * Copyright(c) 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 <linux/libnvdimm.h>
> +#include <linux/badblocks.h>
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/blkdev.h>
> +#include <linux/device.h>
> +#include <linux/ctype.h>
> +#include <linux/ndctl.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +#include "nd-core.h"
> +#include "nd.h"
> +
> +void badrange_init(struct badrange *badrange)
> +{
> +       INIT_LIST_HEAD(&badrange->list);
> +       spin_lock_init(&badrange->lock);
> +}
> +EXPORT_SYMBOL_GPL(badrange_init);
> +
> +static void append_badrange_entry(struct badrange *badrange,
> +               struct badrange_entry *be, u64 addr, u64 length)
> +{
> +       lockdep_assert_held(&badrange->lock);
> +       be->start = addr;
> +       be->length = length;
> +       list_add_tail(&be->list, &badrange->list);
> +}

Small nit, can we rename the instance variable from 'be' to 'bre'?
'be' triggers all my 'big endian' neurons to fire. Other than that,
looks good.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 2/4] nfit_test: add error injection DSMs
  2017-10-06  1:53 ` [PATCH 2/4] nfit_test: add error injection DSMs Vishal Verma
@ 2017-10-07 17:07   ` Dan Williams
  2017-10-09 20:25     ` Verma, Vishal L
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2017-10-07 17:07 UTC (permalink / raw)
  To: Vishal Verma; +Cc: linux-nvdimm

On Thu, Oct 5, 2017 at 6:53 PM, Vishal Verma <vishal.l.verma@intel.com> wrote:
> From: Dave Jiang <dave.jiang@intel.com>
>
> From: Dave Jiang <dave.jiang@intel.com>
>
> Add nfit_test emulation for the new ACPI 6.2 error injectino DSMs.
> This will allow unit tests to selectively inject the errors they wish to
> test for.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> [vishal: Add badrange.o to nfit_test's Kbuild]
> [vishal: Move injection functions to ND_CMD_CALL]
> [vishal: Add support for the notification option]
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  drivers/acpi/nfit/nfit.h              |   1 +
>  tools/testing/nvdimm/Kbuild           |   1 +
>  tools/testing/nvdimm/test/nfit.c      | 187 +++++++++++++++++++++++++++++-----
>  tools/testing/nvdimm/test/nfit_test.h |   4 +
>  4 files changed, 169 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
> index 7093bd5..7ddf48f 100644
> --- a/drivers/acpi/nfit/nfit.h
> +++ b/drivers/acpi/nfit/nfit.h
> @@ -75,6 +75,7 @@ enum {
>         NFIT_ARS_CAP_NONE = 1,
>         NFIT_ARS_F_OVERFLOW = 1,
>         NFIT_ARS_TIMEOUT = 90,
> +       NFIT_ARS_INJECT_INVALID = 2,

If there is no usage of this define outside of nfit_test then it
should be defined in tools/testing/nvdimm/test/nfit.h.

>  };
>
>  enum nfit_root_notifiers {
> diff --git a/tools/testing/nvdimm/Kbuild b/tools/testing/nvdimm/Kbuild
> index d870520..3272ab5 100644
> --- a/tools/testing/nvdimm/Kbuild
> +++ b/tools/testing/nvdimm/Kbuild
> @@ -69,6 +69,7 @@ libnvdimm-y += $(NVDIMM_SRC)/region_devs.o
>  libnvdimm-y += $(NVDIMM_SRC)/region.o
>  libnvdimm-y += $(NVDIMM_SRC)/namespace_devs.o
>  libnvdimm-y += $(NVDIMM_SRC)/label.o
> +libnvdimm-y += $(NVDIMM_SRC)/badrange.o

This hunk should go in the same patch that changes
drivers/nvdimm/Makefile to add badrange.o so we don't have an
nfit_test compile breakage gap between commits.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 4/4] nfit_test: when clearing poison, also remove badrange entries
  2017-10-06  1:53 ` [PATCH 4/4] nfit_test: when clearing poison, also remove badrange entries Vishal Verma
@ 2017-10-09 16:12   ` Dave Jiang
  0 siblings, 0 replies; 12+ messages in thread
From: Dave Jiang @ 2017-10-09 16:12 UTC (permalink / raw)
  To: Vishal Verma, linux-nvdimm



On 10/05/2017 06:53 PM, Vishal Verma wrote:
> The injected badrange entries can only be cleared from the kernel's
> accounting by writing to the affected blocks, so when such a write sends
> the clear error DSM to nfit_test, also clear the ranges from
> nfit_test's badrange list. This lets an 'ARS Inject error status' DSM to
> return the correct status, omitting the cleared ranges.
> 
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  tools/testing/nvdimm/test/nfit.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
> index 43f948e..e400418 100644
> --- a/tools/testing/nvdimm/test/nfit.c
> +++ b/tools/testing/nvdimm/test/nfit.c
> @@ -344,7 +344,8 @@ static int nfit_test_cmd_ars_status(struct ars_state *ars_state,
>  	return 0;
>  }
>  
> -static int nfit_test_cmd_clear_error(struct nd_cmd_clear_error *clear_err,
> +static int nfit_test_cmd_clear_error(struct nfit_test *t,
> +		struct nd_cmd_clear_error *clear_err,
>  		unsigned int buf_len, int *cmd_rc)
>  {
>  	const u64 mask = NFIT_TEST_CLEAR_ERR_UNIT - 1;
> @@ -354,12 +355,7 @@ static int nfit_test_cmd_clear_error(struct nd_cmd_clear_error *clear_err,
>  	if ((clear_err->address & mask) || (clear_err->length & mask))
>  		return -EINVAL;
>  
> -	/*
> -	 * Report 'all clear' success for all commands even though a new
> -	 * scrub will find errors again.  This is enough to have the
> -	 * error removed from the 'badblocks' tracking in the pmem
> -	 * driver.
> -	 */
> +	badrange_forget(&t->badrange, clear_err->address, clear_err->length);
>  	clear_err->status = 0;
>  	clear_err->cleared = clear_err->length;
>  	*cmd_rc = 0;
> @@ -687,7 +683,7 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
>  					cmd_rc);
>  			break;
>  		case ND_CMD_CLEAR_ERROR:
> -			rc = nfit_test_cmd_clear_error(buf, buf_len, cmd_rc);
> +			rc = nfit_test_cmd_clear_error(t, buf, buf_len, cmd_rc);
>  			break;
>  		default:
>  			return -ENOTTY;
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 1/4] libnvdimm: move poison list functions to a new 'badrange' file
  2017-10-06 18:52   ` Dan Williams
@ 2017-10-09 20:23     ` Verma, Vishal L
  0 siblings, 0 replies; 12+ messages in thread
From: Verma, Vishal L @ 2017-10-09 20:23 UTC (permalink / raw)
  To: Williams, Dan J; +Cc: linux-nvdimm

On Fri, 2017-10-06 at 11:52 -0700, Dan Williams wrote:
> On Thu, Oct 5, 2017 at 6:53 PM, Vishal Verma <vishal.l.verma@intel.com
> > wrote:
> > From: Dave Jiang <dave.jiang@intel.com>
> > 
> > From: Dave Jiang <dave.jiang@intel.com>
> > 
> > nfit_test needs to use the poison list manipulation code as well.
> > Make
> > it more generic and in the process rename poison to badrange, and
> > move
> > all the related helpers to a new file.
> > 
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > [vishal: add a missed include in bus.c for the new badrange
> > functions]
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> >  drivers/acpi/nfit/core.c  |   2 +-
> >  drivers/acpi/nfit/mce.c   |   2 +-
> >  drivers/nvdimm/Makefile   |   1 +
> >  drivers/nvdimm/badrange.c | 294
> > ++++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/nvdimm/bus.c      |  24 ++--
> >  drivers/nvdimm/core.c     | 260 +--------------------------------
> > -------
> >  drivers/nvdimm/nd-core.h  |   3 +-
> >  drivers/nvdimm/nd.h       |   6 -
> >  include/linux/libnvdimm.h |  21 +++-
> >  9 files changed, 331 insertions(+), 282 deletions(-)
> >  create mode 100644 drivers/nvdimm/badrange.c
> > 
> > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> > index a3ecd5e..4b157f8 100644
> > --- a/drivers/acpi/nfit/core.c
> > +++ b/drivers/acpi/nfit/core.c
> > @@ -2240,7 +2240,7 @@ static int ars_status_process_records(struct
> > acpi_nfit_desc *acpi_desc,
> >                 if (ars_status->out_length
> >                                 < 44 + sizeof(struct nd_ars_record)
> > * (i + 1))
> >                         break;
> > -               rc = nvdimm_bus_add_poison(nvdimm_bus,
> > +               rc = nvdimm_bus_add_badrange(nvdimm_bus,
> >                                 ars_status->records[i].err_address,
> >                                 ars_status->records[i].length);
> >                 if (rc)
> > diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c
> > index feeb95d..b929214 100644
> > --- a/drivers/acpi/nfit/mce.c
> > +++ b/drivers/acpi/nfit/mce.c
> > @@ -67,7 +67,7 @@ static int nfit_handle_mce(struct notifier_block
> > *nb, unsigned long val,
> >                         continue;
> > 
> >                 /* If this fails due to an -ENOMEM, there is little
> > we can do */
> > -               nvdimm_bus_add_poison(acpi_desc->nvdimm_bus,
> > +               nvdimm_bus_add_badrange(acpi_desc->nvdimm_bus,
> >                                 ALIGN(mce->addr, L1_CACHE_BYTES),
> >                                 L1_CACHE_BYTES);
> >                 nvdimm_region_notify(nfit_spa->nd_region,
> > diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
> > index 909554c..ca6d325 100644
> > --- a/drivers/nvdimm/Makefile
> > +++ b/drivers/nvdimm/Makefile
> > @@ -20,6 +20,7 @@ libnvdimm-y += region_devs.o
> >  libnvdimm-y += region.o
> >  libnvdimm-y += namespace_devs.o
> >  libnvdimm-y += label.o
> > +libnvdimm-y += badrange.o
> >  libnvdimm-$(CONFIG_ND_CLAIM) += claim.o
> >  libnvdimm-$(CONFIG_BTT) += btt_devs.o
> >  libnvdimm-$(CONFIG_NVDIMM_PFN) += pfn_devs.o
> > diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c
> > new file mode 100644
> > index 0000000..6ad782f
> > --- /dev/null
> > +++ b/drivers/nvdimm/badrange.c
> > @@ -0,0 +1,294 @@
> > +/*
> > + * Copyright(c) 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 <linux/libnvdimm.h>
> > +#include <linux/badblocks.h>
> > +#include <linux/export.h>
> > +#include <linux/module.h>
> > +#include <linux/blkdev.h>
> > +#include <linux/device.h>
> > +#include <linux/ctype.h>
> > +#include <linux/ndctl.h>
> > +#include <linux/mutex.h>
> > +#include <linux/slab.h>
> > +#include <linux/io.h>
> > +#include "nd-core.h"
> > +#include "nd.h"
> > +
> > +void badrange_init(struct badrange *badrange)
> > +{
> > +       INIT_LIST_HEAD(&badrange->list);
> > +       spin_lock_init(&badrange->lock);
> > +}
> > +EXPORT_SYMBOL_GPL(badrange_init);
> > +
> > +static void append_badrange_entry(struct badrange *badrange,
> > +               struct badrange_entry *be, u64 addr, u64 length)
> > +{
> > +       lockdep_assert_held(&badrange->lock);
> > +       be->start = addr;
> > +       be->length = length;
> > +       list_add_tail(&be->list, &badrange->list);
> > +}
> 
> Small nit, can we rename the instance variable from 'be' to 'bre'?
> 'be' triggers all my 'big endian' neurons to fire. Other than that,
> looks good.

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

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

* Re: [PATCH 2/4] nfit_test: add error injection DSMs
  2017-10-07 17:07   ` Dan Williams
@ 2017-10-09 20:25     ` Verma, Vishal L
  2017-10-09 20:40       ` Dan Williams
  0 siblings, 1 reply; 12+ messages in thread
From: Verma, Vishal L @ 2017-10-09 20:25 UTC (permalink / raw)
  To: Williams, Dan J; +Cc: linux-nvdimm

On Sat, 2017-10-07 at 10:07 -0700, Dan Williams wrote:
> On Thu, Oct 5, 2017 at 6:53 PM, Vishal Verma <vishal.l.verma@intel.com
> > wrote:

[]

> >         NFIT_ARS_TIMEOUT = 90,
> > +       NFIT_ARS_INJECT_INVALID = 2,
> 
> If there is no usage of this define outside of nfit_test then it
> should be defined in tools/testing/nvdimm/test/nfit.h.

Yes moved.

> 
> >  };
> > 
> >  enum nfit_root_notifiers {
> > diff --git a/tools/testing/nvdimm/Kbuild
> > b/tools/testing/nvdimm/Kbuild
> > index d870520..3272ab5 100644
> > --- a/tools/testing/nvdimm/Kbuild
> > +++ b/tools/testing/nvdimm/Kbuild
> > @@ -69,6 +69,7 @@ libnvdimm-y += $(NVDIMM_SRC)/region_devs.o
> >  libnvdimm-y += $(NVDIMM_SRC)/region.o
> >  libnvdimm-y += $(NVDIMM_SRC)/namespace_devs.o
> >  libnvdimm-y += $(NVDIMM_SRC)/label.o
> > +libnvdimm-y += $(NVDIMM_SRC)/badrange.o
> 
> This hunk should go in the same patch that changes
> drivers/nvdimm/Makefile to add badrange.o so we don't have an
> nfit_test compile breakage gap between commits.

The kernel's normal usage already has the kbuild change in the right
place (patch 1). This adds the use of badrange functions to nfit_test
for the first time, and hence the kbuild addition here.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 2/4] nfit_test: add error injection DSMs
  2017-10-09 20:25     ` Verma, Vishal L
@ 2017-10-09 20:40       ` Dan Williams
  2017-10-09 20:53         ` Verma, Vishal L
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2017-10-09 20:40 UTC (permalink / raw)
  To: Verma, Vishal L; +Cc: linux-nvdimm

On Mon, Oct 9, 2017 at 1:25 PM, Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
> On Sat, 2017-10-07 at 10:07 -0700, Dan Williams wrote:
>> On Thu, Oct 5, 2017 at 6:53 PM, Vishal Verma <vishal.l.verma@intel.com
>> > wrote:
>
> []
>
>> >         NFIT_ARS_TIMEOUT = 90,
>> > +       NFIT_ARS_INJECT_INVALID = 2,
>>
>> If there is no usage of this define outside of nfit_test then it
>> should be defined in tools/testing/nvdimm/test/nfit.h.
>
> Yes moved.
>
>>
>> >  };
>> >
>> >  enum nfit_root_notifiers {
>> > diff --git a/tools/testing/nvdimm/Kbuild
>> > b/tools/testing/nvdimm/Kbuild
>> > index d870520..3272ab5 100644
>> > --- a/tools/testing/nvdimm/Kbuild
>> > +++ b/tools/testing/nvdimm/Kbuild
>> > @@ -69,6 +69,7 @@ libnvdimm-y += $(NVDIMM_SRC)/region_devs.o
>> >  libnvdimm-y += $(NVDIMM_SRC)/region.o
>> >  libnvdimm-y += $(NVDIMM_SRC)/namespace_devs.o
>> >  libnvdimm-y += $(NVDIMM_SRC)/label.o
>> > +libnvdimm-y += $(NVDIMM_SRC)/badrange.o
>>
>> This hunk should go in the same patch that changes
>> drivers/nvdimm/Makefile to add badrange.o so we don't have an
>> nfit_test compile breakage gap between commits.
>
> The kernel's normal usage already has the kbuild change in the right
> place (patch 1). This adds the use of badrange functions to nfit_test
> for the first time, and hence the kbuild addition here.

It's not the nfit_test usages I'm worried about. The internal calls in
libnvdimm will break. Try building tools/testing/nvdimm/libnvdimm.ko
with only patch1 applied.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 2/4] nfit_test: add error injection DSMs
  2017-10-09 20:40       ` Dan Williams
@ 2017-10-09 20:53         ` Verma, Vishal L
  0 siblings, 0 replies; 12+ messages in thread
From: Verma, Vishal L @ 2017-10-09 20:53 UTC (permalink / raw)
  To: Williams, Dan J; +Cc: linux-nvdimm

On Mon, 2017-10-09 at 13:40 -0700, Dan Williams wrote:
> On Mon, Oct 9, 2017 at 1:25 PM, Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
> > On Sat, 2017-10-07 at 10:07 -0700, Dan Williams wrote:
> > > On Thu, Oct 5, 2017 at 6:53 PM, Vishal Verma <vishal.l.verma@intel
> > > .com
> > > > wrote:
> > 
> > []
> > 
> > > >         NFIT_ARS_TIMEOUT = 90,
> > > > +       NFIT_ARS_INJECT_INVALID = 2,
> > > 
> > > If there is no usage of this define outside of nfit_test then it
> > > should be defined in tools/testing/nvdimm/test/nfit.h.
> > 
> > Yes moved.
> > 
> > > 
> > > >  };
> > > > 
> > > >  enum nfit_root_notifiers {
> > > > diff --git a/tools/testing/nvdimm/Kbuild
> > > > b/tools/testing/nvdimm/Kbuild
> > > > index d870520..3272ab5 100644
> > > > --- a/tools/testing/nvdimm/Kbuild
> > > > +++ b/tools/testing/nvdimm/Kbuild
> > > > @@ -69,6 +69,7 @@ libnvdimm-y += $(NVDIMM_SRC)/region_devs.o
> > > >  libnvdimm-y += $(NVDIMM_SRC)/region.o
> > > >  libnvdimm-y += $(NVDIMM_SRC)/namespace_devs.o
> > > >  libnvdimm-y += $(NVDIMM_SRC)/label.o
> > > > +libnvdimm-y += $(NVDIMM_SRC)/badrange.o
> > > 
> > > This hunk should go in the same patch that changes
> > > drivers/nvdimm/Makefile to add badrange.o so we don't have an
> > > nfit_test compile breakage gap between commits.
> > 
> > The kernel's normal usage already has the kbuild change in the right
> > place (patch 1). This adds the use of badrange functions to
> > nfit_test
> > for the first time, and hence the kbuild addition here.
> 
> It's not the nfit_test usages I'm worried about. The internal calls in
> libnvdimm will break. Try building tools/testing/nvdimm/libnvdimm.ko
> with only patch1 applied.

Ah I see what the problem is, I'll send a v3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

end of thread, other threads:[~2017-10-09 20:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-06  1:53 [PATCH 0/4] add error injection commands to nfit_test Vishal Verma
2017-10-06  1:53 ` [PATCH 1/4] libnvdimm: move poison list functions to a new 'badrange' file Vishal Verma
2017-10-06 18:52   ` Dan Williams
2017-10-09 20:23     ` Verma, Vishal L
2017-10-06  1:53 ` [PATCH 2/4] nfit_test: add error injection DSMs Vishal Verma
2017-10-07 17:07   ` Dan Williams
2017-10-09 20:25     ` Verma, Vishal L
2017-10-09 20:40       ` Dan Williams
2017-10-09 20:53         ` Verma, Vishal L
2017-10-06  1:53 ` [PATCH 3/4] libnvdimm, badrange: remove a WARN for list_empty Vishal Verma
2017-10-06  1:53 ` [PATCH 4/4] nfit_test: when clearing poison, also remove badrange entries Vishal Verma
2017-10-09 16:12   ` Dave Jiang

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).