[ndctl PATCH v5 0/5] ndctl: show broken dimm info with translate SPA feature.

[ndctl PATCH v5 0/5] ndctl: show broken dimm info with translate SPA feature.

[Yasunori Goto]

Hi,

I wrote v5 patch set to show broken NVDIMM for ndctl list command.
If possible, please merge it.

---
Change log since v4 [1]:
  - Renamed to ndctl_bus_get_(region/dimm)_by_physical_address()
  - Check ndctl_bus_has_nfit() in ndctl_bus_get_dimm_by_physical_address()
  - No export ndctl_bus_nfit_translate_spa()
  - Move ndctl_bus to private.h
  - attribute packed is added again.


Change log since v3 [2]:
  - Return dimm info when dimm is not interleaved.
    Translate SPA is not necessary for its case.
  - Since ndctl_region is needed to get # of ways of interleave,
    ndctl_region_get_by_physical_address() is created,
    and sanity check (is_valid_spa) use it.
  - rename libndctl-nfit.c to nfit.c
  - move libndctl-nfit.h from ndctl/lib to ndctl/ .
  - not to make libndctl-nfit.la
  - rename to ndctl_bus_is_nfit_cmd_supported()
  - add annotations.

Change log since v2 [3]:
 - Make libndctl-nfit.h and libndctl-nfit.c and define new interfaces
   which use translate spa on them.
 - Add sanity checks for new interfaces.
 - Fix some names
      o trans_spa -> translate_spa
      o sub_cmd -> passthru_cmd
   

Change log since v1 [4]:
 - Use ND_CMD_CALL to call translate SPA feature.
 - Separate patch set of ndctl from kernel patch set.
 - Add a interface to check what feature can call via ND_CMD_CALL by reading
   /device/nfit/dsm_mask 
 - Get only one nvdimm handle and DPA via ioctl() for the time being.
 - Bug fix which i found
     fix calculation of SPA from bad block at dev_badblocks_to_json().


[1] https://www.mail-archive.com/linux-nvdimm@lists.01.org/msg05927.html
[2] https://www.mail-archive.com/linux-nvdimm@lists.01.org/msg05857.html
[3] https://www.mail-archive.com/linux-nvdimm@lists.01.org/msg05577.html
[4] https://www.mail-archive.com/linux-nvdimm@lists.01.org/msg05287.html


----
This patch set is to show information of broken NVDIMM on ndctl.

When a region has a broken block, user needs to replace
the NVDIMM which includes the block.
However there is no information to find which DIMM module have the block.
Not only ndctl does not have such information, nvdimm driver can not
find it if the region is interleaved.


Fortunately, ACPI 6.2 has new specification of _DSM.
It is "translate spa" which can get NVDIMM handle
and DPA(Dimm Physical Address) from SPA(system Physical Addreess).
It helps for ndctl command to find broken NVDIMM when the region is
interleaved.

This patch set includes followings.
 - introduce libndctl-nfit.h
 - some preparations to call Translate SPA via ND_CMD_CALL.
 - make a function to get dimm by System physical address, and
   other helper functions.
 - ndctl list command show bad DIMM.


Thanks,
---
Yasunori Goto




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

[ndctl PATCH v5 1/5] ndctl: Introduce libndctl-nfit.h

[Yasunori Goto]

This patch introduces libndctl-nfit.h.

Since these command can be executed via ND_CMD_CALL,
libndctl.h which is shared between ndctl command and kernel does not
have to include these defintions.

So, libndctl-nfit.h, which is defined for only ndctl, is created instead.
---
 ndctl/libndctl-nfit.h | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/ndctl/libndctl-nfit.h b/ndctl/libndctl-nfit.h
new file mode 100644
index 0000000..8eeeb10
--- /dev/null
+++ b/ndctl/libndctl-nfit.h
@@ -0,0 +1,76 @@
+/*
+ * 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.
+ */
+
+#ifndef __LIBNDCTL_NFIT_H__
+#define __LIBNDCTL_NFIT_H__
+
+/*
+ * libndctl-nfit.h : definitions for NFIT related commands/functions.
+ */
+
+/* nfit command numbers which are called via ND_CMD_CALL */
+enum {
+	NFIT_CMD_TRANSLATE_SPA = 5,
+	NFIT_CMD_ARS_INJECT_SET = 7,
+	NFIT_CMD_ARS_INJECT_CLEAR = 8,
+	NFIT_CMD_ARS_INJECT_GET = 9,
+};
+
+/* error number of Translate SPA by firmware  */
+#define ND_TRANSLATE_SPA_STATUS_INVALID_SPA  2
+
+/*
+ * The following structures are command packages which are
+ * defined by ACPI 6.2 (or later).
+ */
+
+/* For Translate SPA */
+struct nd_cmd_translate_spa {
+	__u64 spa;
+	__u32 status;
+	__u8  flags;
+	__u8  _reserved[3];
+	__u64 translate_length;
+	__u32 num_nvdimms;
+	struct nd_nvdimm_device {
+		__u32 nfit_device_handle;
+		__u32 _reserved;
+		__u64 dpa;
+	} __attribute__((packed)) devices[0];
+
+} __attribute__((packed));
+
+/* For ARS Error Inject */
+struct nd_cmd_ars_err_inj {
+	__u64 err_inj_spa_range_base;
+	__u64 err_inj_spa_range_length;
+	__u8  err_inj_options;
+	__u32 status;
+} __attribute__((packed));
+
+/* For ARS Error Inject Clear */
+struct nd_cmd_ars_err_inj_clr {
+	__u64 err_inj_clr_spa_range_base;
+	__u64 err_inj_clr_spa_range_length;
+	__u32 status;
+} __attribute__((packed));
+
+/* For ARS Error Inject Status Query */
+struct nd_cmd_ars_err_inj_stat {
+	__u32 status;
+	__u32 inj_err_rec_count;
+	struct nd_error_stat_query_record {
+		__u64 err_inj_stat_spa_range_base;
+		__u64 err_inj_stat_spa_range_length;
+	} __attribute__((packed)) record[0];
+} __attribute__((packed));
+
+#endif /* __LIBNDCTL_NFIT_H__ */



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

[ndctl PATCH v5 2/5] ndctl: read device/nfit/dsm_mask

[Yasunori Goto]

To check what feature can be called via ND_CMD_CALL, ndctl needs to read
device/nfit/dsm_mask.

To read it from ndctl/lib/nfit.c, ndctl_bus is moved to ndctl/lib/private.h.
This is used later patch.

Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 ndctl/lib/libndctl.c | 34 ++++++----------------------------
 ndctl/lib/private.h  | 30 ++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 4361cd8..6852137 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -77,34 +77,6 @@ NDCTL_EXPORT size_t ndctl_sizeof_namespace_label(void)
 }
 
 struct ndctl_ctx;
-/**
- * struct ndctl_bus - a nfit table instance
- * @major: control character device major number
- * @minor: control character device minor number
- * @revision: NFIT table revision number
- * @provider: identifier for the source of the NFIT table
- *
- * The expectation is one NFIT/nd bus per system provided by platform
- * firmware (for example @provider == "ACPI.NFIT").  However, the
- * nfit_test module provides multiple test busses with provider names of
- * the format "nfit_test.N"
- */
-struct ndctl_bus {
-	struct ndctl_ctx *ctx;
-	unsigned int id, major, minor, revision;
-	char *provider;
-	struct list_head dimms;
-	struct list_head regions;
-	struct list_node list;
-	int dimms_init;
-	int regions_init;
-	int has_nfit;
-	char *bus_path;
-	char *bus_buf;
-	size_t buf_len;
-	char *wait_probe_path;
-	unsigned long dsm_mask;
-};
 
 /**
  * struct ndctl_mapping - dimm extent relative to a region
@@ -793,6 +765,12 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
 		bus->revision = strtoul(buf, NULL, 0);
 	}
 
+	sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		bus->nfit_dsm_mask = 0;
+	else
+		bus->nfit_dsm_mask = strtoul(buf, NULL, 0);
+
 	sprintf(path, "%s/device/provider", ctl_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		goto err_read;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 83a8067..aeedc03 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -127,6 +127,36 @@ struct ndctl_ctx {
 };
 
 /**
+ * struct ndctl_bus - a nfit table instance
+ * @major: control character device major number
+ * @minor: control character device minor number
+ * @revision: NFIT table revision number
+ * @provider: identifier for the source of the NFIT table
+ *
+ * The expectation is one NFIT/nd bus per system provided by platform
+ * firmware (for example @provider == "ACPI.NFIT").  However, the
+ * nfit_test module provides multiple test busses with provider names of
+ * the format "nfit_test.N"
+ */
+struct ndctl_bus {
+	struct ndctl_ctx *ctx;
+	unsigned int id, major, minor, revision;
+	char *provider;
+	struct list_head dimms;
+	struct list_head regions;
+	struct list_node list;
+	int dimms_init;
+	int regions_init;
+	int has_nfit;
+	char *bus_path;
+	char *bus_buf;
+	size_t buf_len;
+	char *wait_probe_path;
+	unsigned long dsm_mask;
+	unsigned long nfit_dsm_mask;
+};
+
+/**
  * 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



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

[ndctl PATCH v5 3/5] ndctl: allow ND_CMD_CALL for bus

[Yasunori Goto]

Currently ndctl supports ND_CMD_CALL only for DIMM,
but Translate SPA is the feature of bus.
So ND_CMD_CALL must be allowed bus's ioctl().

Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 ndctl/lib/libndctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 6852137..84b71e9 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2190,6 +2190,7 @@ static int to_ioctl_cmd(int cmd, int dimm)
 #ifdef HAVE_NDCTL_CLEAR_ERROR
 		case ND_CMD_CLEAR_ERROR:     return ND_IOCTL_CLEAR_ERROR;
 #endif
+		case ND_CMD_CALL:            return ND_IOCTL_CALL;
 		default:
 						       return 0;
 		};



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

[ndctl PATCH v5 4/5] ndctl: Make new interfaces to get information by Physical Address

[Yasunori Goto]

This patch add features to get information by physical address.
Here is new interfaces :
  - Confirm NFIT command is supported or not. (nfit.c)
  - Find region by System Physical Address (libndctl.c)
  - Find DIMM by System Physical Address. (libndctl.c)

Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 ndctl/lib/Makefile.am  |   2 +
 ndctl/lib/libndctl.c   |  74 +++++++++++++++++++++++++
 ndctl/lib/libndctl.sym |   2 +
 ndctl/lib/nfit.c       | 145 +++++++++++++++++++++++++++++++++++++++++++++++++
 ndctl/libndctl-nfit.h  |   4 ++
 ndctl/libndctl.h.in    |   4 ++
 6 files changed, 231 insertions(+)

diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
index d8df87d..9a7734d 100644
--- a/ndctl/lib/Makefile.am
+++ b/ndctl/lib/Makefile.am
@@ -36,6 +36,8 @@ libndctl_la_SOURCES += hpe1.c
 libndctl_la_SOURCES += msft.c
 endif
 
+libndctl_la_SOURCES += nfit.c
+
 EXTRA_DIST += libndctl.sym
 
 libndctl_la_LDFLAGS = $(AM_LDFLAGS) \
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 84b71e9..e3e0421 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -38,6 +38,7 @@
 #include <ndctl/libndctl.h>
 #include <ndctl/namespace.h>
 #include <daxctl/libdaxctl.h>
+#include <ndctl/libndctl-nfit.h>
 #include "private.h"
 
 static uuid_t null_uuid;
@@ -1532,6 +1533,79 @@ static struct ndctl_dimm *ndctl_dimm_get_by_id(struct ndctl_bus *bus, unsigned i
 	return NULL;
 }
 
+/**
+ * ndctl_bus_get_region_by_physical_address - get region by physical address
+ * @bus: ndctl_bus instance
+ * @address: (System) Physical Address
+ *
+ * If @bus and @address is valid, returns a region address, which
+ * physical address belongs to.
+ */
+NDCTL_EXPORT struct ndctl_region *ndctl_bus_get_region_by_physical_address(struct ndctl_bus *bus,
+		unsigned long long address)
+{
+	unsigned long long region_start, region_end;
+	struct ndctl_region *region;
+
+	ndctl_region_foreach(bus, region) {
+		region_start = ndctl_region_get_resource(region);
+		region_end = region_start + ndctl_region_get_size(region);
+		if (region_start <= address && address < region_end)
+			return region;
+	}
+
+	return NULL;
+}
+
+/**
+ * ndctl_bus_get_dimm_by_physical_address - get ndctl_dimm pointer by physical address
+ * @bus: ndctl_bus instance
+ * @address: (System) Physical Address
+ *
+ * Returns address of ndctl_dimm on success.
+ */
+NDCTL_EXPORT struct ndctl_dimm *ndctl_bus_get_dimm_by_physical_address(struct ndctl_bus *bus,
+	        unsigned long long address)
+{
+        unsigned int handle;
+	unsigned long long dpa;
+	struct ndctl_region *region;
+
+	if (!bus)
+		return NULL;
+
+	region = ndctl_bus_get_region_by_physical_address(bus, address);
+	if (!region)
+		return NULL;
+
+	if (ndctl_region_get_interleave_ways(region) == 1) {
+		/*
+		 * No need to ask firmware. The first dimm is iff.
+		 */
+		struct ndctl_mapping *mapping = ndctl_mapping_get_first(region);
+		if (!mapping)
+			return NULL;
+		return ndctl_mapping_get_dimm(mapping);
+	}
+
+	/*
+	 * Since the region is interleaved, we need to ask firmware about it.
+	 * If it supports Translate SPA, the dimm is returned.
+	 */
+	if (ndctl_bus_has_nfit(bus)) {
+		int rc;
+
+		rc = ndctl_bus_nfit_translate_spa(bus, address, &handle, &dpa);
+		if (rc)
+			return NULL;
+
+		return ndctl_dimm_get_by_handle(bus, handle);
+	}
+	/* No way to get dimm info */
+	return NULL;
+}
+
+
 static int region_set_type(struct ndctl_region *region, char *path)
 {
 	struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 4c1773d..5a73a42 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -36,6 +36,8 @@ global:
 	ndctl_bus_get_provider;
 	ndctl_bus_get_ctx;
 	ndctl_bus_wait_probe;
+	ndctl_bus_get_region_by_physical_address;
+	ndctl_bus_get_dimm_by_physical_address;
 	ndctl_dimm_get_first;
 	ndctl_dimm_get_next;
 	ndctl_dimm_get_handle;
diff --git a/ndctl/lib/nfit.c b/ndctl/lib/nfit.c
new file mode 100644
index 0000000..f3806a4
--- /dev/null
+++ b/ndctl/lib/nfit.c
@@ -0,0 +1,145 @@
+/*
+ * Copyright (c) 2017, FUJITSU LIMITED. All rights reserved.
+ *
+ * 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 <ndctl/libndctl.h>
+#include "private.h"
+#include <ndctl/libndctl-nfit.h>
+
+/**
+ * ndctl_bus_is_nfit_cmd_supported - ask nfit command is supported on @bus.
+ * @bus: ndctl_bus instance
+ * @cmd: nfit command number (defined as NFIT_CMD_XXX in libndctl-nfit.h)
+ *
+ * Return 1: command is supported. Return 0: command is not supported.
+ *
+ */
+NDCTL_EXPORT int ndctl_bus_is_nfit_cmd_supported(struct ndctl_bus *bus,
+                int cmd)
+{
+        return !!(bus->nfit_dsm_mask & (1ULL << cmd));
+}
+
+static int bus_has_translate_spa(struct ndctl_bus *bus)
+{
+	if (!ndctl_bus_has_nfit(bus))
+		return 0;
+
+	return ndctl_bus_is_nfit_cmd_supported(bus, NFIT_CMD_TRANSLATE_SPA);
+}
+
+static struct ndctl_cmd *ndctl_bus_cmd_new_translate_spa(struct ndctl_bus *bus)
+{
+	struct ndctl_cmd *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nd_cmd_translate_spa *translate_spa;
+	size_t size, spa_length;
+
+	spa_length = sizeof(struct nd_cmd_translate_spa)
+		+ sizeof(struct nd_nvdimm_device);
+	size = sizeof(*cmd) + sizeof(*pkg) + spa_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_TRANSLATE_SPA;
+	pkg->nd_size_in = sizeof(unsigned long long);
+	pkg->nd_size_out = spa_length;
+	pkg->nd_fw_size = spa_length;
+	translate_spa = (struct nd_cmd_translate_spa *)&pkg->nd_payload[0];
+	cmd->firmware_status = &translate_spa->status;
+	translate_spa->translate_length = spa_length;
+
+	return cmd;
+}
+
+static int ndctl_bus_cmd_get_translate_spa(struct ndctl_cmd *cmd,
+					unsigned int *handle, unsigned long long *dpa)
+{
+	struct nd_cmd_pkg *pkg;
+	struct nd_cmd_translate_spa *translate_spa;
+
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	translate_spa = (struct nd_cmd_translate_spa *)&pkg->nd_payload[0];
+
+	if (translate_spa->status == ND_TRANSLATE_SPA_STATUS_INVALID_SPA)
+		return -EINVAL;
+
+	/*
+	 * XXX: Currently NVDIMM mirroring is not supported.
+	 * Even if ACPI returned plural dimms due to mirroring,
+	 * this function returns just the first dimm.
+	 */
+
+	*handle = translate_spa->devices[0].nfit_device_handle;
+	*dpa = translate_spa->devices[0].dpa;
+
+	return 0;
+}
+
+static int is_valid_spa(struct ndctl_bus *bus, unsigned long long spa)
+{
+	return !!ndctl_bus_get_region_by_physical_address(bus, spa);
+}
+
+/**
+ * ndctl_bus_nfit_translate_spa - call translate spa.
+ * @bus: bus which belongs to.
+ * @address: address (System Physical Address)
+ * @handle: pointer to return dimm handle
+ * @dpa: pointer to return Dimm Physical address
+ *
+ * If success, returns zero, store dimm's @handle, and @dpa.
+ */
+int ndctl_bus_nfit_translate_spa(struct ndctl_bus *bus,
+	unsigned long long address, unsigned int *handle, unsigned long long *dpa)
+{
+
+	struct ndctl_cmd *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nd_cmd_translate_spa *translate_spa;
+	int rc;
+
+	if (!bus || !handle || !dpa)
+		return -EINVAL;
+
+	if (!bus_has_translate_spa(bus))
+		return -ENOTTY;
+
+	if (!is_valid_spa(bus, address))
+		return -EINVAL;
+
+	cmd = ndctl_bus_cmd_new_translate_spa(bus);
+	if (!cmd)
+		return -ENOMEM;
+
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	translate_spa = (struct nd_cmd_translate_spa *)&pkg->nd_payload[0];
+	translate_spa->spa = address;
+
+	rc = ndctl_cmd_submit(cmd);
+	if (rc) {
+		ndctl_cmd_unref(cmd);
+		return rc;
+	}
+
+	rc = ndctl_bus_cmd_get_translate_spa(cmd, handle, dpa);
+	ndctl_cmd_unref(cmd);
+
+	return rc;
+}
diff --git a/ndctl/libndctl-nfit.h b/ndctl/libndctl-nfit.h
index 8eeeb10..78ac43e 100644
--- a/ndctl/libndctl-nfit.h
+++ b/ndctl/libndctl-nfit.h
@@ -73,4 +73,8 @@ struct nd_cmd_ars_err_inj_stat {
 	} __attribute__((packed)) record[0];
 } __attribute__((packed));
 
+int ndctl_bus_is_nfit_cmd_supported(struct ndctl_bus *bus, int cmd);
+int ndctl_bus_nfit_translate_spa(struct ndctl_bus *bus,
+	unsigned long long addr, unsigned int *handle, unsigned long long *dpa);
+
 #endif /* __LIBNDCTL_NFIT_H__ */
diff --git a/ndctl/libndctl.h.in b/ndctl/libndctl.h.in
index 2bbda04..42e53c6 100644
--- a/ndctl/libndctl.h.in
+++ b/ndctl/libndctl.h.in
@@ -161,6 +161,8 @@ struct ndctl_smart_ops *ndctl_dimm_get_smart_ops(struct ndctl_dimm *dimm);
 struct ndctl_ctx *ndctl_dimm_get_ctx(struct ndctl_dimm *dimm);
 struct ndctl_dimm *ndctl_dimm_get_by_handle(struct ndctl_bus *bus,
 		unsigned int handle);
+struct ndctl_dimm *ndctl_bus_get_dimm_by_physical_address(struct ndctl_bus *bus,
+		unsigned long long address);
 int ndctl_dimm_is_active(struct ndctl_dimm *dimm);
 int ndctl_dimm_is_enabled(struct ndctl_dimm *dimm);
 int ndctl_dimm_disable(struct ndctl_dimm *dimm);
@@ -421,6 +423,8 @@ struct ndctl_ctx *ndctl_region_get_ctx(struct ndctl_region *region);
 struct ndctl_dimm *ndctl_region_get_first_dimm(struct ndctl_region *region);
 struct ndctl_dimm *ndctl_region_get_next_dimm(struct ndctl_region *region,
 		struct ndctl_dimm *dimm);
+struct ndctl_region *ndctl_bus_get_region_by_physical_address(struct ndctl_bus *bus,
+		unsigned long long address);
 #define ndctl_dimm_foreach_in_region(region, dimm) \
         for (dimm = ndctl_region_get_first_dimm(region); \
              dimm != NULL; \



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

[ndctl PATCH v5 5/5] ndctl: show bad dimm's name by ndctl list command.

[Yasunori Goto]

Show dimm's name which has badblock by ndctl list command.
This patch uses translate SPA interface to get bad dimm info.

Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 util/json.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/util/json.c b/util/json.c
index 98165b7..639f0ff 100644
--- a/util/json.c
+++ b/util/json.c
@@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
 	return NULL;
 }
 
+static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
+		unsigned long long spa)
+{
+	struct ndctl_bus *bus;
+
+	bus = ndctl_region_get_bus(region);
+	return ndctl_bus_get_dimm_by_physical_address(bus, spa);
+}
+
 struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 		unsigned int *bb_count, unsigned long flags)
 {
@@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 
 	ndctl_region_badblock_foreach(region, bb) {
 		if (flags & UTIL_JSON_MEDIA_ERRORS) {
+			struct ndctl_dimm *dimm = NULL;
+			unsigned long long spa;
+
+			/* get start address of region */
+			spa = ndctl_region_get_resource(region);
+			if (spa == ULONG_MAX)
+				goto err_array;
+
+			/* get address of bad block */
+			spa += bb->offset << 9;
+			dimm = badblock_to_dimm(region, spa);
+
 			jbb = json_object_new_object();
 			if (!jbb)
 				goto err_array;
@@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 				goto err;
 			json_object_object_add(jbb, "length", jobj);
 
+			if (dimm) {
+				jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
+				if (!jobj)
+					goto err;
+				json_object_object_add(jbb, "dimm", jobj);
+			}
 			json_object_array_add(jbbs, jbb);
 		}
 
@@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 
 	ndctl_region_badblock_foreach(region, bb) {
 		unsigned long long bb_begin, bb_end, begin, end;
+		struct ndctl_dimm *dimm = NULL;
 
 		bb_begin = region_begin + (bb->offset << 9);
 		bb_end = bb_begin + (bb->len << 9) - 1;
@@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 		offset = (begin - dev_begin) >> 9;
 		len = (end - begin + 1) >> 9;
 
+		dimm = badblock_to_dimm(region, begin);
+
 		if (flags & UTIL_JSON_MEDIA_ERRORS) {
 			/* add to json */
 			jbb = json_object_new_object();
@@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 				goto err;
 			json_object_object_add(jbb, "length", jobj);
 
+			if (dimm) {
+				jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
+				if (!jobj)
+					goto err;
+				json_object_object_add(jbb, "dimm", jobj);
+			}
+
 			json_object_array_add(jbbs, jbb);
 		}
 		bbs += len;



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

Re: [ndctl PATCH v5 5/5] ndctl: show bad dimm's name by ndctl list command.

[Dan Williams]

On Wed, Sep 6, 2017 at 10:08 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>
> Show dimm's name which has badblock by ndctl list command.
> This patch uses translate SPA interface to get bad dimm info.
>
> Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> ---
>  util/json.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/util/json.c b/util/json.c
> index 98165b7..639f0ff 100644
> --- a/util/json.c
> +++ b/util/json.c
> @@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
>         return NULL;
>  }
>
> +static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
> +               unsigned long long spa)
> +{
> +       struct ndctl_bus *bus;
> +
> +       bus = ndctl_region_get_bus(region);
> +       return ndctl_bus_get_dimm_by_physical_address(bus, spa);
> +}
> +
>  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>                 unsigned int *bb_count, unsigned long flags)
>  {
> @@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>
>         ndctl_region_badblock_foreach(region, bb) {
>                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
> +                       struct ndctl_dimm *dimm = NULL;
> +                       unsigned long long spa;
> +
> +                       /* get start address of region */
> +                       spa = ndctl_region_get_resource(region);
> +                       if (spa == ULONG_MAX)
> +                               goto err_array;
> +
> +                       /* get address of bad block */
> +                       spa += bb->offset << 9;
> +                       dimm = badblock_to_dimm(region, spa);

Ok the libndctl pieces are looking good, but I have question about
this. We only return the first DIMM that has the error... what if the
length of the record causes us to span more than one dimm?

> +
>                         jbb = json_object_new_object();
>                         if (!jbb)
>                                 goto err_array;
> @@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>                                 goto err;
>                         json_object_object_add(jbb, "length", jobj);
>
> +                       if (dimm) {
> +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));

We probably need to make this an array of DIMMs and call badblock() to
DIMM for every 512 byte block in the range.

> +                               if (!jobj)
> +                                       goto err;
> +                               json_object_object_add(jbb, "dimm", jobj);
> +                       }
>                         json_object_array_add(jbbs, jbb);
>                 }
>
> @@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>
>         ndctl_region_badblock_foreach(region, bb) {
>                 unsigned long long bb_begin, bb_end, begin, end;
> +               struct ndctl_dimm *dimm = NULL;
>
>                 bb_begin = region_begin + (bb->offset << 9);
>                 bb_end = bb_begin + (bb->len << 9) - 1;
> @@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>                 offset = (begin - dev_begin) >> 9;
>                 len = (end - begin + 1) >> 9;
>
> +               dimm = badblock_to_dimm(region, begin);
> +
>                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
>                         /* add to json */
>                         jbb = json_object_new_object();
> @@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>                                 goto err;
>                         json_object_object_add(jbb, "length", jobj);
>
> +                       if (dimm) {
> +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));

Since this will eventually be the same code as above lets turn it into
a common badblocks_to_jdimms() helper that returns a json array.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH v5 5/5] ndctl: show bad dimm's name by ndctl list command.

[Yasunori Goto]

> On Wed, Sep 6, 2017 at 10:08 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> >
> > Show dimm's name which has badblock by ndctl list command.
> > This patch uses translate SPA interface to get bad dimm info.
> >
> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> > ---
> >  util/json.c | 37 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git a/util/json.c b/util/json.c
> > index 98165b7..639f0ff 100644
> > --- a/util/json.c
> > +++ b/util/json.c
> > @@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
> >         return NULL;
> >  }
> >
> > +static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
> > +               unsigned long long spa)
> > +{
> > +       struct ndctl_bus *bus;
> > +
> > +       bus = ndctl_region_get_bus(region);
> > +       return ndctl_bus_get_dimm_by_physical_address(bus, spa);
> > +}
> > +
> >  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >                 unsigned int *bb_count, unsigned long flags)
> >  {
> > @@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >
> >         ndctl_region_badblock_foreach(region, bb) {
> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
> > +                       struct ndctl_dimm *dimm = NULL;
> > +                       unsigned long long spa;
> > +
> > +                       /* get start address of region */
> > +                       spa = ndctl_region_get_resource(region);
> > +                       if (spa == ULONG_MAX)
> > +                               goto err_array;
> > +
> > +                       /* get address of bad block */
> > +                       spa += bb->offset << 9;
> > +                       dimm = badblock_to_dimm(region, spa);
> 
> Ok the libndctl pieces are looking good, but I have question about
> this. We only return the first DIMM that has the error... what if the
> length of the record causes us to span more than one dimm?
> 
> > +
> >                         jbb = json_object_new_object();
> >                         if (!jbb)
> >                                 goto err_array;
> > @@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >                                 goto err;
> >                         json_object_object_add(jbb, "length", jobj);
> >
> > +                       if (dimm) {
> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
> 
> We probably need to make this an array of DIMMs and call badblock() to
> DIMM for every 512 byte block in the range.

Hmmmmm, Ok.


> 
> > +                               if (!jobj)
> > +                                       goto err;
> > +                               json_object_object_add(jbb, "dimm", jobj);
> > +                       }
> >                         json_object_array_add(jbbs, jbb);
> >                 }
> >
> > @@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >
> >         ndctl_region_badblock_foreach(region, bb) {
> >                 unsigned long long bb_begin, bb_end, begin, end;
> > +               struct ndctl_dimm *dimm = NULL;
> >
> >                 bb_begin = region_begin + (bb->offset << 9);
> >                 bb_end = bb_begin + (bb->len << 9) - 1;
> > @@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >                 offset = (begin - dev_begin) >> 9;
> >                 len = (end - begin + 1) >> 9;
> >
> > +               dimm = badblock_to_dimm(region, begin);
> > +
> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
> >                         /* add to json */
> >                         jbb = json_object_new_object();
> > @@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >                                 goto err;
> >                         json_object_object_add(jbb, "length", jobj);
> >
> > +                       if (dimm) {
> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
> 
> Since this will eventually be the same code as above lets turn it into
> a common badblocks_to_jdimms() helper that returns a json array.
> 

Ok. I'll make next version.

BTW, should I send this No.5 patch for v6?
Otherwise, should I re-send all of this patch set again?





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

Re: [ndctl PATCH v5 5/5] ndctl: show bad dimm's name by ndctl list command.

[Dan Williams]

On Wed, Sep 6, 2017 at 11:21 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>> On Wed, Sep 6, 2017 at 10:08 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>> >
>> > Show dimm's name which has badblock by ndctl list command.
>> > This patch uses translate SPA interface to get bad dimm info.
>> >
>> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
>> > ---
>> >  util/json.c | 37 +++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 37 insertions(+)
>> >
>> > diff --git a/util/json.c b/util/json.c
>> > index 98165b7..639f0ff 100644
>> > --- a/util/json.c
>> > +++ b/util/json.c
>> > @@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
>> >         return NULL;
>> >  }
>> >
>> > +static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
>> > +               unsigned long long spa)
>> > +{
>> > +       struct ndctl_bus *bus;
>> > +
>> > +       bus = ndctl_region_get_bus(region);
>> > +       return ndctl_bus_get_dimm_by_physical_address(bus, spa);
>> > +}
>> > +
>> >  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>> >                 unsigned int *bb_count, unsigned long flags)
>> >  {
>> > @@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>> >
>> >         ndctl_region_badblock_foreach(region, bb) {
>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
>> > +                       struct ndctl_dimm *dimm = NULL;
>> > +                       unsigned long long spa;
>> > +
>> > +                       /* get start address of region */
>> > +                       spa = ndctl_region_get_resource(region);
>> > +                       if (spa == ULONG_MAX)
>> > +                               goto err_array;
>> > +
>> > +                       /* get address of bad block */
>> > +                       spa += bb->offset << 9;
>> > +                       dimm = badblock_to_dimm(region, spa);
>>
>> Ok the libndctl pieces are looking good, but I have question about
>> this. We only return the first DIMM that has the error... what if the
>> length of the record causes us to span more than one dimm?
>>
>> > +
>> >                         jbb = json_object_new_object();
>> >                         if (!jbb)
>> >                                 goto err_array;
>> > @@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>> >                                 goto err;
>> >                         json_object_object_add(jbb, "length", jobj);
>> >
>> > +                       if (dimm) {
>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
>>
>> We probably need to make this an array of DIMMs and call badblock() to
>> DIMM for every 512 byte block in the range.
>
> Hmmmmm, Ok.
>
>
>>
>> > +                               if (!jobj)
>> > +                                       goto err;
>> > +                               json_object_object_add(jbb, "dimm", jobj);
>> > +                       }
>> >                         json_object_array_add(jbbs, jbb);
>> >                 }
>> >
>> > @@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>> >
>> >         ndctl_region_badblock_foreach(region, bb) {
>> >                 unsigned long long bb_begin, bb_end, begin, end;
>> > +               struct ndctl_dimm *dimm = NULL;
>> >
>> >                 bb_begin = region_begin + (bb->offset << 9);
>> >                 bb_end = bb_begin + (bb->len << 9) - 1;
>> > @@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>> >                 offset = (begin - dev_begin) >> 9;
>> >                 len = (end - begin + 1) >> 9;
>> >
>> > +               dimm = badblock_to_dimm(region, begin);
>> > +
>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
>> >                         /* add to json */
>> >                         jbb = json_object_new_object();
>> > @@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>> >                                 goto err;
>> >                         json_object_object_add(jbb, "length", jobj);
>> >
>> > +                       if (dimm) {
>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
>>
>> Since this will eventually be the same code as above lets turn it into
>> a common badblocks_to_jdimms() helper that returns a json array.
>>
>
> Ok. I'll make next version.
>
> BTW, should I send this No.5 patch for v6?
> Otherwise, should I re-send all of this patch set again?

Just this one is fine, the others I'll get applied and pushed out tomorrow.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH v5 0/5] ndctl: show broken dimm info with translate SPA feature.

[Dan Williams]

On Wed, Sep 6, 2017 at 10:00 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> Hi,
>
> I wrote v5 patch set to show broken NVDIMM for ndctl list command.
> If possible, please merge it.

Ok, I merged patches 1 - 4 with some minor fixups. I moved
ndctl_bus_nfit_translate_spa() from libndctl-nfit.h into private.h
since the interface end applications should be using is the generic
ndctl_dimm_get_by_physical_address().
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH v5 5/5] ndctl: show bad dimm's name by ndctl list command.

[Dan Williams]

On Wed, Sep 6, 2017 at 11:25 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Wed, Sep 6, 2017 at 11:21 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>>> On Wed, Sep 6, 2017 at 10:08 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>>> >
>>> > Show dimm's name which has badblock by ndctl list command.
>>> > This patch uses translate SPA interface to get bad dimm info.
>>> >
>>> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
>>> > ---
>>> >  util/json.c | 37 +++++++++++++++++++++++++++++++++++++
>>> >  1 file changed, 37 insertions(+)
>>> >
>>> > diff --git a/util/json.c b/util/json.c
>>> > index 98165b7..639f0ff 100644
>>> > --- a/util/json.c
>>> > +++ b/util/json.c
>>> > @@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
>>> >         return NULL;
>>> >  }
>>> >
>>> > +static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
>>> > +               unsigned long long spa)
>>> > +{
>>> > +       struct ndctl_bus *bus;
>>> > +
>>> > +       bus = ndctl_region_get_bus(region);
>>> > +       return ndctl_bus_get_dimm_by_physical_address(bus, spa);
>>> > +}
>>> > +
>>> >  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>>> >                 unsigned int *bb_count, unsigned long flags)
>>> >  {
>>> > @@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>>> >
>>> >         ndctl_region_badblock_foreach(region, bb) {
>>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
>>> > +                       struct ndctl_dimm *dimm = NULL;
>>> > +                       unsigned long long spa;
>>> > +
>>> > +                       /* get start address of region */
>>> > +                       spa = ndctl_region_get_resource(region);
>>> > +                       if (spa == ULONG_MAX)
>>> > +                               goto err_array;
>>> > +
>>> > +                       /* get address of bad block */
>>> > +                       spa += bb->offset << 9;
>>> > +                       dimm = badblock_to_dimm(region, spa);
>>>
>>> Ok the libndctl pieces are looking good, but I have question about
>>> this. We only return the first DIMM that has the error... what if the
>>> length of the record causes us to span more than one dimm?
>>>
>>> > +
>>> >                         jbb = json_object_new_object();
>>> >                         if (!jbb)
>>> >                                 goto err_array;
>>> > @@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>>> >                                 goto err;
>>> >                         json_object_object_add(jbb, "length", jobj);
>>> >
>>> > +                       if (dimm) {
>>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
>>>
>>> We probably need to make this an array of DIMMs and call badblock() to
>>> DIMM for every 512 byte block in the range.
>>
>> Hmmmmm, Ok.
>>
>>
>>>
>>> > +                               if (!jobj)
>>> > +                                       goto err;
>>> > +                               json_object_object_add(jbb, "dimm", jobj);
>>> > +                       }
>>> >                         json_object_array_add(jbbs, jbb);
>>> >                 }
>>> >
>>> > @@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>>> >
>>> >         ndctl_region_badblock_foreach(region, bb) {
>>> >                 unsigned long long bb_begin, bb_end, begin, end;
>>> > +               struct ndctl_dimm *dimm = NULL;
>>> >
>>> >                 bb_begin = region_begin + (bb->offset << 9);
>>> >                 bb_end = bb_begin + (bb->len << 9) - 1;
>>> > @@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>>> >                 offset = (begin - dev_begin) >> 9;
>>> >                 len = (end - begin + 1) >> 9;
>>> >
>>> > +               dimm = badblock_to_dimm(region, begin);
>>> > +
>>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
>>> >                         /* add to json */
>>> >                         jbb = json_object_new_object();
>>> > @@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>>> >                                 goto err;
>>> >                         json_object_object_add(jbb, "length", jobj);
>>> >
>>> > +                       if (dimm) {
>>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
>>>
>>> Since this will eventually be the same code as above lets turn it into
>>> a common badblocks_to_jdimms() helper that returns a json array.
>>>
>>
>> Ok. I'll make next version.
>>
>> BTW, should I send this No.5 patch for v6?
>> Otherwise, should I re-send all of this patch set again?
>
> Just this one is fine, the others I'll get applied and pushed out tomorrow.

Since this is the last feature I'm waiting for to cut the v58 release,
I'll go ahead and send incremental patches to fix this up.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH v5 5/5] ndctl: show bad dimm's name by ndctl list command.

[Yasunori Goto]

> On Wed, Sep 6, 2017 at 11:25 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> > On Wed, Sep 6, 2017 at 11:21 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> >>> On Wed, Sep 6, 2017 at 10:08 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> >>> >
> >>> > Show dimm's name which has badblock by ndctl list command.
> >>> > This patch uses translate SPA interface to get bad dimm info.
> >>> >
> >>> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> >>> > ---
> >>> >  util/json.c | 37 +++++++++++++++++++++++++++++++++++++
> >>> >  1 file changed, 37 insertions(+)
> >>> >
> >>> > diff --git a/util/json.c b/util/json.c
> >>> > index 98165b7..639f0ff 100644
> >>> > --- a/util/json.c
> >>> > +++ b/util/json.c
> >>> > @@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
> >>> >         return NULL;
> >>> >  }
> >>> >
> >>> > +static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
> >>> > +               unsigned long long spa)
> >>> > +{
> >>> > +       struct ndctl_bus *bus;
> >>> > +
> >>> > +       bus = ndctl_region_get_bus(region);
> >>> > +       return ndctl_bus_get_dimm_by_physical_address(bus, spa);
> >>> > +}
> >>> > +
> >>> >  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >>> >                 unsigned int *bb_count, unsigned long flags)
> >>> >  {
> >>> > @@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >>> >
> >>> >         ndctl_region_badblock_foreach(region, bb) {
> >>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
> >>> > +                       struct ndctl_dimm *dimm = NULL;
> >>> > +                       unsigned long long spa;
> >>> > +
> >>> > +                       /* get start address of region */
> >>> > +                       spa = ndctl_region_get_resource(region);
> >>> > +                       if (spa == ULONG_MAX)
> >>> > +                               goto err_array;
> >>> > +
> >>> > +                       /* get address of bad block */
> >>> > +                       spa += bb->offset << 9;
> >>> > +                       dimm = badblock_to_dimm(region, spa);
> >>>
> >>> Ok the libndctl pieces are looking good, but I have question about
> >>> this. We only return the first DIMM that has the error... what if the
> >>> length of the record causes us to span more than one dimm?
> >>>
> >>> > +
> >>> >                         jbb = json_object_new_object();
> >>> >                         if (!jbb)
> >>> >                                 goto err_array;
> >>> > @@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >>> >                                 goto err;
> >>> >                         json_object_object_add(jbb, "length", jobj);
> >>> >
> >>> > +                       if (dimm) {
> >>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
> >>>
> >>> We probably need to make this an array of DIMMs and call badblock() to
> >>> DIMM for every 512 byte block in the range.
> >>
> >> Hmmmmm, Ok.
> >>
> >>
> >>>
> >>> > +                               if (!jobj)
> >>> > +                                       goto err;
> >>> > +                               json_object_object_add(jbb, "dimm", jobj);
> >>> > +                       }
> >>> >                         json_object_array_add(jbbs, jbb);
> >>> >                 }
> >>> >
> >>> > @@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >>> >
> >>> >         ndctl_region_badblock_foreach(region, bb) {
> >>> >                 unsigned long long bb_begin, bb_end, begin, end;
> >>> > +               struct ndctl_dimm *dimm = NULL;
> >>> >
> >>> >                 bb_begin = region_begin + (bb->offset << 9);
> >>> >                 bb_end = bb_begin + (bb->len << 9) - 1;
> >>> > @@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >>> >                 offset = (begin - dev_begin) >> 9;
> >>> >                 len = (end - begin + 1) >> 9;
> >>> >
> >>> > +               dimm = badblock_to_dimm(region, begin);
> >>> > +
> >>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
> >>> >                         /* add to json */
> >>> >                         jbb = json_object_new_object();
> >>> > @@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >>> >                                 goto err;
> >>> >                         json_object_object_add(jbb, "length", jobj);
> >>> >
> >>> > +                       if (dimm) {
> >>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
> >>>
> >>> Since this will eventually be the same code as above lets turn it into
> >>> a common badblocks_to_jdimms() helper that returns a json array.
> >>>
> >>
> >> Ok. I'll make next version.
> >>
> >> BTW, should I send this No.5 patch for v6?
> >> Otherwise, should I re-send all of this patch set again?
> >
> > Just this one is fine, the others I'll get applied and pushed out tomorrow.
> 
> Since this is the last feature I'm waiting for to cut the v58 release,
> I'll go ahead and send incremental patches to fix this up.
> 


Wow, thanks!

Since I was off from office at Friday,
This is great help for me.

Anyway, I'll review your patches.





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