linux-nvdimm.lists.01.org archive mirror
 help / color / mirror / Atom feed
* [ndctl PATCH 00/36] Multiple topics / backlog for v68
@ 2020-02-29 20:20 Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 01/36] ndctl/list: Add 'target_node' to region and namespace verbose listings Dan Williams
                   ` (36 more replies)
  0 siblings, 37 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Michal Biesek, Auke Kok, Jan Kara, Aneesh Kumar K.V

Changes from review:
- Add NDCTL_LIST_LINT to not regress list output (Jeff)
- Add kernel-doc description for ndctl_region_set_align() (Jeff)

---

About half of these have been posted previously, but have been reworked
and revised as they have percolated in my tree relative to other
arriving features. Yes, it is quite a lot to ingest at once, but given
the interdependencies and need to catch up I decided to post it all
together.

The recommendation on how to review is to start with the new tests,
those introduce some new commands, and those new commands introduce some
new library routines. The rest are miscellaneous updates, fixes, and
cleanups.

New tests:
----------
dm.sh: When touching the kernel's core dax infrastructure there are
simple bugs that can be caught by simply firing up a device-mapper dax
configuration.

track-uuid.sh: The kernel bug that lead to this condition of the kernel
garbage collecting the wrong label required a specific namespace
manipulation sequence. Record and replay that sequence against future
kernel updates.

sub-section.sh: Sub-section support adds machinery to add/remove
namespaces on sub-128M boundaries. Force these section collisions and
validate the kernel tracks the mappings.

align.sh: The kernel has grown the ability to align namespace capacity
provisioning at the region level, since the sub-section changes it has
stopped creating namespace with non-zero start_pad, and now it also
needs to enforce up to a 16M minimum alignment on PowerPC. These
alignment requirements / checks need to be balanced against not
regressing currently defined namespaces. Given the kernel will stop
generating problematic configurations by default a tool was needed to
regression test old configs. The new read-infoblock and write-infoblock
commands are added to support that testing and debug.

New commands:
-------------
ndctl read-infoblock: A compliment to read-labels, it can validate and
dump the known infoblock formats in json format. It can also dump the
raw binary infoblock image from a namespace for backup purposes.

ndctl write-infoblock: Generate an fsdax, or devdax infoblock from
passed in parameters. It currently does not support generating btt
infoblocks since that support would also want btt-arena generation
support... maybe in a future update.

New library apis:
-----------------
ndctl_namespace_get_target_node:
ndctl_region_get_target_node: The target_node attribute indicates the
numa node that this memory range would instantiate / join if it were
hot-added via the dax_kmem driver. The target_node is also used to query
platform firmware performance data.

ndctl_region_get_align
ndctl_region_set_align: Support the new alignment settings at the region
level. This augments namespace creation to provision aligned spans of dpa
(device-physical-address).

New command options:
--------------------
ndctl create-namespace: --force: By default ndctl will disallow sub-16M
aligned dax-mode namespace creation to encourage cross-arch
compatibility. However, non-x86 supports 2M aligned namespaces, and
--force will bypass the 16M alignment check.

ndctl create-namespace: --no-autorecover: For debug cases the automatic
cleanup of namespace creation failures destroys some forensic details.
Exit without cleanup when this option is specified.

ndctl list: --configured: The --idle option can be used to enumerate 
namespaces that failed to initialize, but the output is cluttered with
seed devices. The --configured option limits the listing to any
namespace that has capacity allocated.

ndctl list: NDCTL_LIST_LINT (environment variable): Allow environments
to opt-in to 'ndctl list' fixes that structurally change the output
format. This prevents ndctl invalidating scripts that are dependent on
the buggy output.

Fixes:
------
- Multiple fixups to create-namespace error reporting 
- Require 16M minimum size for any non-raw namespace mode
- Fix destruction of tpm.handle in test/security.sh
- Fix ndctl_namespace_get_resource() to return the updated resource base
  after ndctl_namespace_set_size()
- Fix the warning spew from taking the address of a packed structure
  member.


---

Dan Williams (36):
      ndctl/list: Add 'target_node' to region and namespace verbose listings
      ndctl/docs: Fix mailing list sign-up link
      ndctl/list: Drop named list objects from verbose listing
      daxctl/list: Avoid memory operations without resource data
      ndctl/build: Fix distcheck
      ndctl/namespace: Fix destroy-namespace accounting relative to seed devices
      ndctl/region: Support ndctl_region_{get,set}_align()
      ndctl/namespace: Emit better errors on failure
      ndctl/namespace: Check for region alignment violations
      ndctl/util: Up-level is_power_of_2() and introduce IS_ALIGNED
      ndctl/namespace: Validate resource alignment for dax-mode namespaces
      ndctl/namespace: Add read-infoblock command
      ndctl/test: Update dax-dev to handle multiple e820 ranges
      ndctl/namespace: Always zero info-blocks
      ndctl/namespace: Disable autorecovery of create-namespace failures
      ndctl/build: Fix EXTRA_DIST already defined errors
      ndctl/test: Checkout device-mapper + dax operation
      ndctl/test: Exercise sub-section sized namespace creation/deletion
      ndctl/namespace: Kill off the legacy mode names
      ndctl/namespace: Introduce mode-to-name and name-to-mode helpers
      ndctl/namespace: Validate namespace size within validate_namespace_options()
      ndctl/namespace: Clarify 16M minimum size requirement
      ndctl/test: Regression test 'failed to track'
      ndctl/dimm: Rework dimm command status reporting
      ndctl/dimm: Rework iteration to drop unaligned pointers
      ndctl/test: Fix typos / loss of tpm.handle in security test
      ndctl/test: Relax dax_pmem_compat requirement
      ndctl/namespace: Fix namespace-action vs namespace-mode confusion
      ndctl/namespace: Update 'pfn' infoblock definition
      ndctl/util: Return 0 for NULL arguments to parse_size64()
      ndctl/namespace: Fix read-info-block vs read-infoblock
      ndctl/namespace: Parse infoblocks from stdin
      ndctl/namespace: Add write-infoblock command
      ndctl/list: Add option to list configured + disabled namespaces
      ndctl/lib/namespace: Fix resource retrieval after size change
      ndctl/test: Regression test misaligned namespaces


 CONTRIBUTING.md                                |    2 
 Documentation/ndctl/Makefile.am                |    4 
 Documentation/ndctl/ndctl-create-namespace.txt |   17 
 Documentation/ndctl/ndctl-list.txt             |   64 +
 Documentation/ndctl/ndctl-read-infoblock.txt   |   94 ++
 Documentation/ndctl/ndctl-write-infoblock.txt  |  132 +++
 ndctl/action.h                                 |    2 
 ndctl/builtin.h                                |    2 
 ndctl/check.c                                  |   20 
 ndctl/lib/ars.c                                |   34 +
 ndctl/lib/hpe1.c                               |   17 
 ndctl/lib/hyperv.c                             |    7 
 ndctl/lib/intel.c                              |   56 +
 ndctl/lib/libndctl.c                           |  191 ++++
 ndctl/lib/libndctl.sym                         |    4 
 ndctl/lib/msft.c                               |    8 
 ndctl/lib/nfit.c                               |   36 +
 ndctl/lib/private.h                            |   16 
 ndctl/libndctl-nfit.h                          |   11 
 ndctl/libndctl.h                               |    5 
 ndctl/list.c                                   |   59 +
 ndctl/namespace.c                              | 1050 +++++++++++++++++++++---
 ndctl/namespace.h                              |   61 +
 ndctl/ndctl.c                                  |    2 
 test/Makefile.am                               |   14 
 test/align.sh                                  |  118 +++
 test/blk-exhaust.sh                            |    2 
 test/blk_namespaces.c                          |    1 
 test/btt-check.sh                              |    2 
 test/btt-errors.sh                             |    7 
 test/btt-pad-compat.sh                         |    5 
 test/clear.sh                                  |    2 
 test/core.c                                    |    8 
 test/create.sh                                 |    2 
 test/dax-dev.c                                 |   17 
 test/dax.sh                                    |    2 
 test/daxctl-devices.sh                         |    2 
 test/daxdev-errors.sh                          |    2 
 test/device-dax-fio.sh                         |    2 
 test/dm.sh                                     |   75 ++
 test/dpa-alloc.c                               |   10 
 test/dsm-fail.c                                |    5 
 test/firmware-update.sh                        |    2 
 test/inject-error.sh                           |    2 
 test/inject-smart.sh                           |    2 
 test/label-compat.sh                           |    5 
 test/libndctl.c                                |   16 
 test/max_available_extent_ns.sh                |    2 
 test/mmap.sh                                   |    2 
 test/monitor.sh                                |    2 
 test/multi-dax.sh                              |    2 
 test/parent-uuid.c                             |    1 
 test/pfn-meta-errors.sh                        |    2 
 test/pmem-errors.sh                            |   11 
 test/rescan-partitions.sh                      |    2 
 test/sector-mode.sh                            |    2 
 test/security.sh                               |    6 
 test/sub-section.sh                            |   77 ++
 test/track-uuid.sh                             |   40 +
 util/filter.c                                  |   46 +
 util/filter.h                                  |    3 
 util/fletcher.h                                |    1 
 util/json.c                                    |   18 
 util/json.h                                    |    1 
 util/size.c                                    |    2 
 util/size.h                                    |    8 
 66 files changed, 2112 insertions(+), 313 deletions(-)
 create mode 100644 Documentation/ndctl/ndctl-read-infoblock.txt
 create mode 100644 Documentation/ndctl/ndctl-write-infoblock.txt
 create mode 100755 test/align.sh
 create mode 100755 test/dm.sh
 create mode 100755 test/sub-section.sh
 create mode 100755 test/track-uuid.sh

base-commit: 56f4a91b51b532fcdb8b44ace422dce48ed27c7d
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 01/36] ndctl/list: Add 'target_node' to region and namespace verbose listings
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
@ 2020-02-29 20:20 ` Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 02/36] ndctl/docs: Fix mailing list sign-up link Dan Williams
                   ` (35 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Aneesh Kumar K.V

Historically the 'numa_node' attribute of a device has been the local,
or closest cpu numa node that can access the device. With the ACPI HMAT
and other platform descriptions of performance differentiated memory,
memory device targets may have their own numa identifier. The
target_node property indicates that target information and the effective
online numa node the memory range would receive if it were onlined.

While this property has been available to device-dax instances since
kernel commit 21c75763a3ae "device-dax: Add a 'target_node' attribute",
recent kernels have also started exporting for regions and namespaces.
Add it to the verbose listing.

Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/libndctl.c   |   25 ++++++++++++++++++++++++-
 ndctl/lib/libndctl.sym |    2 ++
 ndctl/lib/private.h    |    3 ++-
 ndctl/libndctl.h       |    2 ++
 ndctl/list.c           |    9 ++++++++-
 util/json.c            |    9 ++++++++-
 6 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 2d23dbb3caf7..889a83720c0d 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -135,6 +135,7 @@ struct ndctl_mapping {
  * @generation: incremented everytime the region is disabled
  * @nstype: the resulting type of namespace this region produces
  * @numa_node: numa node attribute
+ * @target_node: target node were this region to be onlined
  *
  * A region may alias between pmem and block-window access methods.  The
  * region driver is tasked with parsing the label (if their is one) and
@@ -160,7 +161,7 @@ struct ndctl_region {
 	char *region_buf;
 	int buf_len;
 	int generation;
-	int numa_node;
+	int numa_node, target_node;
 	struct list_head btts;
 	struct list_head pfns;
 	struct list_head daxs;
@@ -2151,6 +2152,12 @@ static void *add_region(void *parent, int id, const char *region_base)
 	else
 		region->numa_node = -1;
 
+	sprintf(path, "%s/target_node", region_base);
+	if (sysfs_read_attr(ctx, path, buf) == 0)
+		region->target_node = strtol(buf, NULL, 0);
+	else
+		region->target_node = -1;
+
 	if (region_set_type(region, path) < 0)
 		goto err_read;
 
@@ -2424,6 +2431,11 @@ NDCTL_EXPORT int ndctl_region_get_numa_node(struct ndctl_region *region)
 	return region->numa_node;
 }
 
+NDCTL_EXPORT int ndctl_region_get_target_node(struct ndctl_region *region)
+{
+	return region->target_node;
+}
+
 NDCTL_EXPORT struct badblock *ndctl_region_get_next_badblock(struct ndctl_region *region)
 {
 	return badblocks_iter_next(&region->bb_iter);
@@ -3477,6 +3489,12 @@ static void *add_namespace(void *parent, int id, const char *ndns_base)
 	else
 		ndns->numa_node = -1;
 
+	sprintf(path, "%s/target_node", ndns_base);
+	if (sysfs_read_attr(ctx, path, buf) == 0)
+		ndns->target_node = strtol(buf, NULL, 0);
+	else
+		ndns->target_node = -1;
+
 	sprintf(path, "%s/holder_class", ndns_base);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		ndns->enforce_mode = enforce_name_to_id(buf);
@@ -4398,6 +4416,11 @@ NDCTL_EXPORT int ndctl_namespace_get_numa_node(struct ndctl_namespace *ndns)
     return ndns->numa_node;
 }
 
+NDCTL_EXPORT int ndctl_namespace_get_target_node(struct ndctl_namespace *ndns)
+{
+	return ndns->target_node;
+}
+
 static int __ndctl_namespace_set_write_cache(struct ndctl_namespace *ndns,
 		int state)
 {
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 4e767789dfe1..bf049af1393a 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -426,4 +426,6 @@ LIBNDCTL_22 {
 
 LIBNDCTL_23 {
 	ndctl_namespace_is_configuration_idle;
+	ndctl_namespace_get_target_node;
+	ndctl_region_get_target_node;
 } LIBNDCTL_22;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 2b8fee2d9e5a..16bf8f953828 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -201,6 +201,7 @@ struct badblocks_iter {
  * @bdev: associated block_device of a namespace
  * @size: unsigned
  * @numa_node: numa node attribute
+ * @target_node: target node were this region to be onlined
  *
  * A 'namespace' is the resulting device after region-aliasing and
  * label-parsing is resolved.
@@ -220,7 +221,7 @@ struct ndctl_namespace {
 	char *alt_name;
 	uuid_t uuid;
 	struct ndctl_lbasize lbasize;
-	int numa_node;
+	int numa_node, target_node;
 	struct list_head injected_bb;
 };
 
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 9a53049e7f61..208240b20aee 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -383,6 +383,7 @@ 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);
 int ndctl_region_get_numa_node(struct ndctl_region *region);
+int ndctl_region_get_target_node(struct ndctl_region *region);
 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) \
@@ -511,6 +512,7 @@ int ndctl_namespace_set_sector_size(struct ndctl_namespace *ndns,
 int ndctl_namespace_get_raw_mode(struct ndctl_namespace *ndns);
 int ndctl_namespace_set_raw_mode(struct ndctl_namespace *ndns, int raw_mode);
 int ndctl_namespace_get_numa_node(struct ndctl_namespace *ndns);
+int ndctl_namespace_get_target_node(struct ndctl_namespace *ndns);
 int ndctl_namespace_inject_error(struct ndctl_namespace *ndns,
 		unsigned long long block, unsigned long long count,
 		bool notify);
diff --git a/ndctl/list.c b/ndctl/list.c
index 8f3e9ad4efd6..86ffbcfe8560 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -80,7 +80,7 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 	unsigned int bb_count = 0;
 	unsigned long long extent;
 	enum ndctl_persistence_domain pd;
-	int numa;
+	int numa, target;
 
 	if (!jregion)
 		return NULL;
@@ -130,6 +130,13 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 			json_object_object_add(jregion, "numa_node", jobj);
 	}
 
+	target = ndctl_region_get_target_node(region);
+	if (target >= 0 && flags & UTIL_JSON_VERBOSE) {
+		jobj = json_object_new_int(target);
+		if (jobj)
+			json_object_object_add(jregion, "target_node", jobj);
+	}
+
 	iset = ndctl_region_get_interleave_set(region);
 	if (iset) {
 		jobj = util_json_object_hex(
diff --git a/util/json.c b/util/json.c
index 497c52ba1a00..0abaf3a5b9c2 100644
--- a/util/json.c
+++ b/util/json.c
@@ -912,7 +912,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 	unsigned long align = 0;
 	char buf[40];
 	uuid_t uuid;
-	int numa;
+	int numa, target;
 
 	if (!jndns)
 		return NULL;
@@ -1092,6 +1092,13 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 			json_object_object_add(jndns, "numa_node", jobj);
 	}
 
+	target = ndctl_namespace_get_target_node(ndns);
+	if (target >= 0 && flags & UTIL_JSON_VERBOSE) {
+		jobj = json_object_new_int(target);
+		if (jobj)
+			json_object_object_add(jndns, "target_node", jobj);
+	}
+
 	if (pfn)
 		jbbs = util_pfn_badblocks_to_json(pfn, &bb_count, flags);
 	else if (dax)
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 02/36] ndctl/docs: Fix mailing list sign-up link
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 01/36] ndctl/list: Add 'target_node' to region and namespace verbose listings Dan Williams
@ 2020-02-29 20:20 ` Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 03/36] ndctl/list: Drop named list objects from verbose listing Dan Williams
                   ` (34 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

At some point 01.org switched its mailing list management software from
Mailman to Postorious. Update the stale mailman link.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 CONTRIBUTING.md |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 811a8c0ad7da..4c29d31e49e9 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -6,7 +6,7 @@ The following is a set of guidelines that we adhere to, and request that
 contributors follow.
 
 1. The libnvdimm (kernel subsystem) and ndctl developers primarily use
-   the [linux-nvdimm](https://lists.01.org/mailman/listinfo/linux-nvdimm)
+   the [linux-nvdimm](https://lists.01.org/postorius/lists/linux-nvdimm.lists.01.org/)
    mailing list for everything. It is recommended to send patches to
    **```linux-nvdimm@lists.01.org```**
 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 03/36] ndctl/list: Drop named list objects from verbose listing
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 01/36] ndctl/list: Add 'target_node' to region and namespace verbose listings Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 02/36] ndctl/docs: Fix mailing list sign-up link Dan Williams
@ 2020-02-29 20:20 ` Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 04/36] daxctl/list: Avoid memory operations without resource data Dan Williams
                   ` (33 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

The only expected difference between "ndctl list -R" and "ndctl list
-Rv" is some additional output fields. Instead it currently results in
the region array being contained in a named "regions" list object.

# ndctl list -R -r 0
[
  {
    "dev":"region0",
    "size":4294967296,
    "available_size":0,
    "max_available_extent":0,
    "type":"pmem",
    "persistence_domain":"unknown"
  }
]

# ndctl list -Rv -r 0
{
  "regions":[
    {
      "dev":"region0",
      "size":4294967296,
      "available_size":0,
      "max_available_extent":0,
      "type":"pmem",
      "numa_node":0,
      "target_node":2,
      "persistence_domain":"unknown",
      "namespaces":[
        {
          "dev":"namespace0.0",
          "mode":"fsdax",
          "map":"mem",
          "size":4294967296,
          "sector_size":512,
          "blockdev":"pmem0",
          "numa_node":0,
          "target_node":2
        }
      ]
    }
  ]
}

Drop the named list, by not including namespaces in the listing. Extra
objects only appear at the -vv level. "ndctl list -v" and "ndctl list
-Nv" are synonyms and behave as expected.

# export NDCTL_LIST_LINT=1
# ndctl list -Rv -r 0
[
  {
    "dev":"region0",
    "size":4294967296,
    "available_size":0,
    "max_available_extent":0,
    "type":"pmem",
    "numa_node":0,
    "target_node":2,
    "persistence_domain":"unknown"
  }
]

Another side effect of this change is that it allows for:

    ndctl list -Rvvv

...to only show the verbose region details vs assuming that namespaces
and dimms etc also need to be added.

Note that a new NDCTL_LIST_LINT environment variable is added to limit
this output cleanup to environments that were not dependent on the
legacy behavior.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl/ndctl-list.txt |   58 ++++++++++++++++++++++++++++++++++++
 ndctl/list.c                       |   17 ++++++++---
 2 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index f9c7434d3b0b..bc725baa6656 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -234,6 +234,52 @@ include::xable-bus-options.txt[]
 	- *-vvv*
 	  Everything '-vv' provides, plus --health, --capabilities,
 	  --idle, and --firmware.
+::
+	The verbosity can also be scoped by the object type. For example
+	to just list regions with capabilities and media error info.
+----
+# ndctl list -Ru -vvv -r 0
+{
+  "dev":"region0",
+  "size":"4.00 GiB (4.29 GB)",
+  "available_size":0,
+  "max_available_extent":0,
+  "type":"pmem",
+  "numa_node":0,
+  "target_node":2,
+  "capabilities":[
+    {
+      "mode":"sector",
+      "sector_sizes":[
+        512,
+        520,
+        528,
+        4096,
+        4104,
+        4160,
+        4224
+      ]
+    },
+    {
+      "mode":"fsdax",
+      "alignments":[
+        4096,
+        2097152,
+        1073741824
+      ]
+    },
+    {
+      "mode":"devdax",
+      "alignments":[
+        4096,
+        2097152,
+        1073741824
+      ]
+    }
+  ],
+  "persistence_domain":"unknown"
+}
+----
 
 include::human-option.txt[]
 
@@ -261,6 +307,18 @@ include::human-option.txt[]
 }
 ----
 
+ENVIRONMENT VARIABLES
+---------------------
+'NDCTL_LIST_LINT'::
+	A bug in the "ndctl list" output needs to be fixed with care for
+	other tooling that may have developed a dependency on the buggy
+	behavior. The NDCTL_LIST_LINT variable is an opt-in to apply
+	fixes, and not regress previously shipped behavior by default.
+	This environment variable applies the following fixups:
+	- Fix "ndctl list -Rv" to only show region objects and not include
+	  namespace objects.
+::
+
 include::../copyright.txt[]
 
 SEE ALSO
diff --git a/ndctl/list.c b/ndctl/list.c
index 86ffbcfe8560..7d7835247005 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -481,6 +481,7 @@ int cmd_list(int argc, const char **argv, struct ndctl_ctx *ctx)
 		"ndctl list [<options>]",
 		NULL
 	};
+	bool lint = !!secure_getenv("NDCTL_LIST_LINT");
 	struct util_filter_ctx fctx = { 0 };
 	struct list_filter_arg lfa = { 0 };
 	int i, rc;
@@ -507,12 +508,20 @@ int cmd_list(int argc, const char **argv, struct ndctl_ctx *ctx)
 		list.health = true;
 		list.capabilities = true;
 	case 2:
-		list.dimms = true;
-		list.buses = true;
-		list.regions = true;
+		if (!lint) {
+			list.dimms = true;
+			list.buses = true;
+			list.regions = true;
+		} else if (num_list_flags() == 0) {
+			list.dimms = true;
+			list.buses = true;
+			list.regions = true;
+			list.namespaces = true;
+		}
 	case 1:
 		list.media_errors = true;
-		list.namespaces = true;
+		if (!lint)
+			list.namespaces = true;
 		list.dax = true;
 	case 0:
 		break;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 04/36] daxctl/list: Avoid memory operations without resource data
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (2 preceding siblings ...)
  2020-02-29 20:20 ` [ndctl PATCH 03/36] ndctl/list: Drop named list objects from verbose listing Dan Williams
@ 2020-02-29 20:20 ` Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 05/36] ndctl/build: Fix distcheck Dan Williams
                   ` (32 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Michal Biesek

Resource data is restricted to root-only, and "daxctl list" is likely to
be run by non-root. Skip listing memory-object details when the resource
is not available.

Otherwise, "daxctl list" from non-root emits:

   libdaxctl: memblock_in_dev: dax1.0: Unable to determine resource

Reported-by: Michal Biesek <michal.biesek@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 util/json.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/json.c b/util/json.c
index 0abaf3a5b9c2..6745bcc19058 100644
--- a/util/json.c
+++ b/util/json.c
@@ -306,7 +306,7 @@ struct json_object *util_daxctl_dev_to_json(struct daxctl_dev *dev,
 	if (jobj)
 		json_object_object_add(jdev, "mode", jobj);
 
-	if (mem) {
+	if (mem && daxctl_dev_get_resource(dev) != 0) {
 		movable = daxctl_memory_is_movable(mem);
 		if (movable == 1)
 			jobj = json_object_new_boolean(true);
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 05/36] ndctl/build: Fix distcheck
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (3 preceding siblings ...)
  2020-02-29 20:20 ` [ndctl PATCH 04/36] daxctl/list: Avoid memory operations without resource data Dan Williams
@ 2020-02-29 20:20 ` Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 06/36] ndctl/namespace: Fix destroy-namespace accounting relative to seed devices Dan Williams
                   ` (31 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Auke Kok

- Add missing dependencies to EXTRA_DIST

- Fix up relative path names

- Fix up test cleanup to not leave straggling file behind

Reported-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/Makefile.am                |    4 +++-
 test/blk-exhaust.sh             |    2 +-
 test/btt-check.sh               |    2 +-
 test/btt-errors.sh              |    7 ++++---
 test/btt-pad-compat.sh          |    5 +++--
 test/clear.sh                   |    2 +-
 test/create.sh                  |    2 +-
 test/dax.sh                     |    2 +-
 test/daxctl-devices.sh          |    2 +-
 test/daxdev-errors.sh           |    2 +-
 test/device-dax-fio.sh          |    2 +-
 test/firmware-update.sh         |    2 +-
 test/inject-error.sh            |    2 +-
 test/inject-smart.sh            |    2 +-
 test/label-compat.sh            |    5 +++--
 test/max_available_extent_ns.sh |    2 +-
 test/mmap.sh                    |    2 +-
 test/monitor.sh                 |    2 +-
 test/multi-dax.sh               |    2 +-
 test/pfn-meta-errors.sh         |    2 +-
 test/pmem-errors.sh             |   11 ++---------
 test/rescan-partitions.sh       |    2 +-
 test/sector-mode.sh             |    2 +-
 test/security.sh                |    2 +-
 24 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/test/Makefile.am b/test/Makefile.am
index 5e795a9e7fdc..58873a6bc341 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -27,7 +27,9 @@ TESTS =\
 	max_available_extent_ns.sh \
 	pfn-meta-errors.sh
 
-EXTRA_DIST = $(TESTS)
+EXTRA_DIST = $(TESTS) common \
+		btt-pad-compat.xxd \
+		nmem1.bin nmem2.bin nmem3.bin nmem4.bin
 
 check_PROGRAMS =\
 	libndctl \
diff --git a/test/blk-exhaust.sh b/test/blk-exhaust.sh
index 326ce73ade6c..db7dc25aecbd 100755
--- a/test/blk-exhaust.sh
+++ b/test/blk-exhaust.sh
@@ -15,7 +15,7 @@ set -e
 
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 check_min_kver "4.11" || do_skip "may lack blk-exhaustion fix"
 
diff --git a/test/btt-check.sh b/test/btt-check.sh
index ceabee52b868..bd782f477728 100755
--- a/test/btt-check.sh
+++ b/test/btt-check.sh
@@ -19,7 +19,7 @@ blockdev=""
 bs=4096
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 trap 'err $LINENO' ERR
 
diff --git a/test/btt-errors.sh b/test/btt-errors.sh
index 00c07960ae7b..a56069789d4b 100755
--- a/test/btt-errors.sh
+++ b/test/btt-errors.sh
@@ -16,14 +16,14 @@ FILE=image
 blockdev=""
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 cleanup()
 {
 	rm -f $FILE
 	rm -f $MNT/$FILE
-	if [ -n "$blockdev" ]; then
-		umount "/dev/$blockdev"
+	if grep -q "$MNT" /proc/mounts; then
+		umount $MNT
 	else
 		rc=77
 	fi
@@ -160,5 +160,6 @@ dd if=/dev/$blockdev of=/dev/null iflag=direct bs=4096 count=1 && err $LINENO ||
 $NDCTL disable-region -b $NFIT_TEST_BUS0 all
 $NDCTL zero-labels -b $NFIT_TEST_BUS0 all
 $NDCTL enable-region -b $NFIT_TEST_BUS0 all
+cleanup
 _cleanup
 exit 0
diff --git a/test/btt-pad-compat.sh b/test/btt-pad-compat.sh
index 2c1f2718c701..b1a46edeaf9d 100755
--- a/test/btt-pad-compat.sh
+++ b/test/btt-pad-compat.sh
@@ -16,7 +16,8 @@ size=""
 blockdev=""
 rc=77
 
-. ./common
+BASE=$(dirname $0)
+. $BASE/common
 
 trap 'err $LINENO' ERR
 
@@ -107,7 +108,7 @@ force_raw()
 copy_xxd_img()
 {
 	local bdev="$1"
-	local xxd_patch="btt-pad-compat.xxd"
+	local xxd_patch="$BASE/btt-pad-compat.xxd"
 
 	test -s "$xxd_patch"
 	test -b "$bdev"
diff --git a/test/clear.sh b/test/clear.sh
index 1bd12da1675a..1fffd166504a 100755
--- a/test/clear.sh
+++ b/test/clear.sh
@@ -15,7 +15,7 @@ set -e
 
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 check_min_kver "4.6" || do_skip "lacks clear poison support"
 
diff --git a/test/create.sh b/test/create.sh
index 8d787976a06f..520f3a9c1dc1 100755
--- a/test/create.sh
+++ b/test/create.sh
@@ -16,7 +16,7 @@ set -e
 SECTOR_SIZE="4096"
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 check_min_kver "4.5" || do_skip "may lack namespace mode attribute"
 
diff --git a/test/dax.sh b/test/dax.sh
index 3933107920a9..5383c433283f 100755
--- a/test/dax.sh
+++ b/test/dax.sh
@@ -11,7 +11,7 @@
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 # General Public License for more details.
 
-. ./common
+. $(dirname $0)/common
 
 MNT=test_dax_mnt
 FILE=image
diff --git a/test/daxctl-devices.sh b/test/daxctl-devices.sh
index 00f4715d74dd..ff2bcd212294 100755
--- a/test/daxctl-devices.sh
+++ b/test/daxctl-devices.sh
@@ -3,7 +3,7 @@
 # Copyright(c) 2019 Intel Corporation. All rights reserved.
 
 rc=77
-. ./common
+. $(dirname $0)/common
 
 trap 'cleanup $LINENO' ERR
 
diff --git a/test/daxdev-errors.sh b/test/daxdev-errors.sh
index c5adb7260527..15d3e67d1166 100755
--- a/test/daxdev-errors.sh
+++ b/test/daxdev-errors.sh
@@ -15,7 +15,7 @@ set -e
 
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 check_min_kver "4.12" || do_skip "lacks dax dev error handling"
 
diff --git a/test/device-dax-fio.sh b/test/device-dax-fio.sh
index b6d5e0eb1170..d4ca7ab238e4 100755
--- a/test/device-dax-fio.sh
+++ b/test/device-dax-fio.sh
@@ -11,7 +11,7 @@
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 # General Public License for more details.
 
-. ./common
+. $(dirname $0)/common
 
 rc=77
 
diff --git a/test/firmware-update.sh b/test/firmware-update.sh
index 7852e58e915e..ed7d7e53772c 100755
--- a/test/firmware-update.sh
+++ b/test/firmware-update.sh
@@ -6,7 +6,7 @@ rc=77
 dev=""
 image="update-fw.img"
 
-. ./common
+. $(dirname $0)/common
 
 trap 'err $LINENO' ERR
 
diff --git a/test/inject-error.sh b/test/inject-error.sh
index 825bf181f24c..5667b5131c7a 100755
--- a/test/inject-error.sh
+++ b/test/inject-error.sh
@@ -18,7 +18,7 @@ rc=77
 err_block=42
 err_count=8
 
-. ./common
+. $(dirname $0)/common
 
 trap 'err $LINENO' ERR
 
diff --git a/test/inject-smart.sh b/test/inject-smart.sh
index 18655bab2291..5e563df546b8 100755
--- a/test/inject-smart.sh
+++ b/test/inject-smart.sh
@@ -3,7 +3,7 @@
 # Copyright(c) 2018 Intel Corporation. All rights reserved.
 
 rc=77
-. ./common
+. $(dirname $0)/common
 bus="$NFIT_TEST_BUS0"
 inj_val="42"
 
diff --git a/test/label-compat.sh b/test/label-compat.sh
index dc6226d5e94e..a29dd1367233 100755
--- a/test/label-compat.sh
+++ b/test/label-compat.sh
@@ -15,7 +15,8 @@ set -e
 
 rc=77
 
-. ./common
+BASE=$(dirname $0)
+. $BASE/common
 
 check_min_kver "4.11" || do_skip "may not provide reliable isetcookie values"
 
@@ -36,7 +37,7 @@ dimms=$($NDCTL list -DRi -r $region | jq -r "$query" | xargs)
 i=1
 for d in $dimms
 do
-	$NDCTL write-labels $d -i nmem${i}.bin
+	$NDCTL write-labels $d -i $BASE/nmem${i}.bin
 	i=$((i+1))
 done
 
diff --git a/test/max_available_extent_ns.sh b/test/max_available_extent_ns.sh
index 1c7e7bfa1b83..c6acdabf7fd7 100755
--- a/test/max_available_extent_ns.sh
+++ b/test/max_available_extent_ns.sh
@@ -5,7 +5,7 @@
 
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 trap 'err $LINENO' ERR
 
diff --git a/test/mmap.sh b/test/mmap.sh
index d072ea289f31..0bcc35f619f5 100755
--- a/test/mmap.sh
+++ b/test/mmap.sh
@@ -11,7 +11,7 @@
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 # General Public License for more details.
 
-. ./common
+. $(dirname $0)/common
 
 MNT=test_mmap_mnt
 FILE=image
diff --git a/test/monitor.sh b/test/monitor.sh
index 29d4ea13d0b7..261fbfa3d461 100755
--- a/test/monitor.sh
+++ b/test/monitor.sh
@@ -12,7 +12,7 @@ monitor_regions=""
 monitor_namespace=""
 smart_supported_bus=""
 
-. ./common
+. $(dirname $0)/common
 
 trap 'err $LINENO' ERR
 
diff --git a/test/multi-dax.sh b/test/multi-dax.sh
index 0829bf2462c4..110ba3d80339 100755
--- a/test/multi-dax.sh
+++ b/test/multi-dax.sh
@@ -15,7 +15,7 @@ set -e
 
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 check_min_kver "4.13" || do_skip "may lack multi-dax support"
 
diff --git a/test/pfn-meta-errors.sh b/test/pfn-meta-errors.sh
index 14a15aed98dd..4ac33d86b8d3 100755
--- a/test/pfn-meta-errors.sh
+++ b/test/pfn-meta-errors.sh
@@ -6,7 +6,7 @@
 blockdev=""
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 force_raw()
 {
diff --git a/test/pmem-errors.sh b/test/pmem-errors.sh
index 3d905082c8e6..f9c8eb213df0 100755
--- a/test/pmem-errors.sh
+++ b/test/pmem-errors.sh
@@ -6,7 +6,7 @@ MNT=test_dax_mnt
 FILE=image
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 cleanup()
 {
@@ -113,14 +113,7 @@ echo $((start_sect + 1)) 1 > /sys/block/$blockdev/badblocks
 : The following 'dd' is expected to hit an I/O Error
 dd if=$MNT/$FILE of=/dev/null iflag=direct bs=4096 count=1 && err $LINENO || true
 
-# cleanup
-rm -f $FILE
-rm -f $MNT/$FILE
-if [ -n "$blockdev" ]; then
-	umount /dev/$blockdev
-fi
-rmdir $MNT
-
+cleanup
 _cleanup
 
 exit 0
diff --git a/test/rescan-partitions.sh b/test/rescan-partitions.sh
index 9c7b7a0151df..b3f2b167053f 100755
--- a/test/rescan-partitions.sh
+++ b/test/rescan-partitions.sh
@@ -7,7 +7,7 @@ size=""
 blockdev=""
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 trap 'err $LINENO' ERR
 
diff --git a/test/sector-mode.sh b/test/sector-mode.sh
index 4b964c5eebc7..eef8dc6d6a3e 100755
--- a/test/sector-mode.sh
+++ b/test/sector-mode.sh
@@ -13,7 +13,7 @@
 
 rc=77
 
-. ./common
+. $(dirname $0)/common
 
 set -e
 trap 'err $LINENO' ERR
diff --git a/test/security.sh b/test/security.sh
index 183e3fea22dd..771135b7ab18 100755
--- a/test/security.sh
+++ b/test/security.sh
@@ -11,7 +11,7 @@ masterpath="$keypath/$masterkey.blob"
 backup_key=0
 backup_handle=0
 
-. ./common
+. $(dirname $0)/common
 
 trap 'err $LINENO' ERR
 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 06/36] ndctl/namespace: Fix destroy-namespace accounting relative to seed devices
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (4 preceding siblings ...)
  2020-02-29 20:20 ` [ndctl PATCH 05/36] ndctl/build: Fix distcheck Dan Williams
@ 2020-02-29 20:20 ` Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 07/36] ndctl/region: Support ndctl_region_{get, set}_align() Dan Williams
                   ` (30 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Seed namespaces are included in "ndctl destroy-namespace all". However
since the user never "creates" them it is surprising to see
"destroy-namespace" report 1 more namespace relative to the number that
have been created. Catch attempts to destroy a zero-sized namespace:

Before:
# ndctl create-namespace -s 500M
{
  "dev":"namespace1.0",
  "size":"492.00 MiB (515.90 MB)",
  "blockdev":"pmem1"
}
# ndctl create-namespace -s 500M
{
  "dev":"namespace1.1",
  "size":"492.00 MiB (515.90 MB)",
  "blockdev":"pmem1.1"
}
# ndctl create-namespace -s 500M
{
  "dev":"namespace1.2",
  "size":"492.00 MiB (515.90 MB)",
  "blockdev":"pmem1.2"
}
# ndctl destroy-namespace -r 1 all -f
destroyed 4 namespaces

After:
# ndctl create-namespace -s 500M
{
  "dev":"namespace1.0",
  "size":"492.00 MiB (515.90 MB)",
  "blockdev":"pmem1"
}
# ndctl create-namespace -s 500M
{
  "dev":"namespace1.3",
  "size":"492.00 MiB (515.90 MB)",
  "blockdev":"pmem1.3"
}
# ndctl create-namespace -s 500M
{
  "dev":"namespace1.1",
  "size":"492.00 MiB (515.90 MB)",
  "blockdev":"pmem1.1"
}
# ndctl destroy-namespace -r 1 all -f
destroyed 3 namespaces

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 2f463509f8ca..994b4e8791ea 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -907,6 +907,7 @@ static int namespace_destroy(struct ndctl_region *region,
 	struct ndctl_pfn *pfn = ndctl_namespace_get_pfn(ndns);
 	struct ndctl_dax *dax = ndctl_namespace_get_dax(ndns);
 	struct ndctl_btt *btt = ndctl_namespace_get_btt(ndns);
+	unsigned long long size;
 	bool did_zero = false;
 	int rc;
 
@@ -953,10 +954,19 @@ static int namespace_destroy(struct ndctl_region *region,
 		goto out;
 	}
 
+	size = ndctl_namespace_get_size(ndns);
+
 	rc = ndctl_namespace_delete(ndns);
 	if (rc)
 		debug("%s: failed to reclaim\n", devname);
 
+	/*
+	 * Don't report a destroyed namespace when no capacity was
+	 * allocated.
+	 */
+	if (size == 0 && rc == 0)
+		rc = 1;
+
 out:
 	return rc;
 }
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 07/36] ndctl/region: Support ndctl_region_{get, set}_align()
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (5 preceding siblings ...)
  2020-02-29 20:20 ` [ndctl PATCH 06/36] ndctl/namespace: Fix destroy-namespace accounting relative to seed devices Dan Williams
@ 2020-02-29 20:20 ` Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 08/36] ndctl/namespace: Emit better errors on failure Dan Williams
                   ` (29 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Aneesh Kumar K.V

Add support for the new kernel facility to set space alignment
constraints at the region level. Update the unit tests to bypass the
default 16MiB alignment constraint. Add the new parameter to the default
region listing given how central it is to understanding the valid values
for "create-namespace --size=...".

Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/libndctl.c   |   45 +++++++++++++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym |    2 ++
 ndctl/libndctl.h       |    2 ++
 ndctl/list.c           |   10 +++++++++-
 test/blk_namespaces.c  |    1 +
 test/dpa-alloc.c       |   10 ++++++++--
 test/dsm-fail.c        |    5 ++++-
 test/libndctl.c        |   10 ++++++++--
 test/parent-uuid.c     |    1 +
 9 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 889a83720c0d..d0c1236f0698 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -150,6 +150,7 @@ struct ndctl_region {
 	struct kmod_module *module;
 	struct ndctl_bus *bus;
 	int id, num_mappings, nstype, range_index, ro;
+	unsigned long align;
 	int mappings_init;
 	int namespaces_init;
 	int btts_init;
@@ -1109,6 +1110,44 @@ NDCTL_EXPORT int ndctl_region_set_ro(struct ndctl_region *region, int ro)
 	return ro;
 }
 
+NDCTL_EXPORT unsigned long ndctl_region_get_align(struct ndctl_region *region)
+{
+	return region->align;
+}
+
+/**
+ * ndctl_region_set_align() - Align namespace dpa allocations to @align
+ * @region: region to modify
+ * @align: alignment that must be a power-of-2 and >= the platform minimum
+ *
+ * WARNING: setting the region align value to anything less than the
+ * kernel default (16M) may result in namespaces that are not cross-arch
+ * (PowerPC) compatible. The minimum alignment for raw mode namespaces
+ * is PAGE_SIZE.
+ */
+NDCTL_EXPORT int ndctl_region_set_align(struct ndctl_region *region,
+		unsigned long align)
+{
+	struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
+	char *path = region->region_buf;
+	int len = region->buf_len, rc;
+	char buf[SYSFS_ATTR_SIZE];
+
+	if (snprintf(path, len, "%s/align", region->region_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				ndctl_region_get_devname(region));
+		return -ENXIO;
+	}
+
+	sprintf(buf, "%#lx\n", align);
+	rc = sysfs_write_attr(ctx, path, buf);
+	if (rc < 0)
+		return rc;
+
+	region->align = align;
+	return 0;
+}
+
 NDCTL_EXPORT unsigned long long ndctl_region_get_resource(struct ndctl_region *region)
 {
 	struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
@@ -2158,6 +2197,12 @@ static void *add_region(void *parent, int id, const char *region_base)
 	else
 		region->target_node = -1;
 
+	sprintf(path, "%s/align", region_base);
+	if (sysfs_read_attr(ctx, path, buf) == 0)
+		region->align = strtoul(buf, NULL, 0);
+	else
+		region->align = ULONG_MAX;
+
 	if (region_set_type(region, path) < 0)
 		goto err_read;
 
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index bf049af1393a..ac575a23d035 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -428,4 +428,6 @@ LIBNDCTL_23 {
 	ndctl_namespace_is_configuration_idle;
 	ndctl_namespace_get_target_node;
 	ndctl_region_get_target_node;
+	ndctl_region_get_align;
+	ndctl_region_set_align;
 } LIBNDCTL_22;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 208240b20aee..076c34583b7d 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -372,6 +372,8 @@ struct ndctl_namespace *ndctl_region_get_namespace_seed(
 		struct ndctl_region *region);
 int ndctl_region_get_ro(struct ndctl_region *region);
 int ndctl_region_set_ro(struct ndctl_region *region, int ro);
+unsigned long ndctl_region_get_align(struct ndctl_region *region);
+int ndctl_region_set_align(struct ndctl_region *region, unsigned long align);
 unsigned long long ndctl_region_get_resource(struct ndctl_region *region);
 struct ndctl_btt *ndctl_region_get_btt_seed(struct ndctl_region *region);
 struct ndctl_pfn *ndctl_region_get_pfn_seed(struct ndctl_region *region);
diff --git a/ndctl/list.c b/ndctl/list.c
index 7d7835247005..aedccfe8fe75 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -78,7 +78,7 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 	struct ndctl_interleave_set *iset;
 	struct ndctl_mapping *mapping;
 	unsigned int bb_count = 0;
-	unsigned long long extent;
+	unsigned long long extent, align;
 	enum ndctl_persistence_domain pd;
 	int numa, target;
 
@@ -95,6 +95,14 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 		goto err;
 	json_object_object_add(jregion, "size", jobj);
 
+	align = ndctl_region_get_align(region);
+	if (align < ULLONG_MAX) {
+		jobj = util_json_object_size(align, flags);
+		if (!jobj)
+			goto err;
+		json_object_object_add(jregion, "align", jobj);
+	}
+
 	jobj = util_json_object_size(ndctl_region_get_available_size(region),
 			flags);
 	if (!jobj)
diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
index b587ab93fbb8..437fcad0a8f5 100644
--- a/test/blk_namespaces.c
+++ b/test/blk_namespaces.c
@@ -54,6 +54,7 @@ static struct ndctl_namespace *create_blk_namespace(int region_fraction,
 	unsigned long long size;
 	uuid_t uuid;
 
+	ndctl_region_set_align(region, sysconf(_SC_PAGESIZE));
 	ndctl_namespace_foreach(region, ndns)
 		if (ndctl_namespace_get_size(ndns) == 0) {
 			seed_ns = ndns;
diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
index 9a9c6b64c504..b757b9ad9c2c 100644
--- a/test/dpa-alloc.c
+++ b/test/dpa-alloc.c
@@ -58,15 +58,21 @@ static int do_test(struct ndctl_ctx *ctx, struct ndctl_test *test)
 	bus = ndctl_bus_get_by_provider(ctx, NFIT_PROVIDER1);
 	if (!bus)
 		return -ENXIO;
-	ndctl_region_foreach(bus, region)
+	ndctl_region_foreach(bus, region) {
 		ndctl_region_disable_invalidate(region);
+		ndctl_region_set_align(region, sysconf(_SC_PAGESIZE)
+				* ndctl_region_get_interleave_ways(region));
+	}
 
 	/* init nfit_test.0 */
 	bus = ndctl_bus_get_by_provider(ctx, NFIT_PROVIDER0);
 	if (!bus)
 		return -ENXIO;
-	ndctl_region_foreach(bus, region)
+	ndctl_region_foreach(bus, region) {
 		ndctl_region_disable_invalidate(region);
+		ndctl_region_set_align(region, sysconf(_SC_PAGESIZE)
+				* ndctl_region_get_interleave_ways(region));
+	}
 
 	ndctl_dimm_foreach(bus, dimm) {
 		rc = ndctl_dimm_zero_labels(dimm);
diff --git a/test/dsm-fail.c b/test/dsm-fail.c
index 6e812aec008f..b2c51db4aa3a 100644
--- a/test/dsm-fail.c
+++ b/test/dsm-fail.c
@@ -48,8 +48,11 @@ static int reset_bus(struct ndctl_bus *bus)
 	}
 
 	/* set regions back to their default state */
-	ndctl_region_foreach(bus, region)
+	ndctl_region_foreach(bus, region) {
 		ndctl_region_enable(region);
+		ndctl_region_set_align(region, sysconf(_SC_PAGESIZE)
+				* ndctl_region_get_interleave_ways(region));
+	}
 	return 0;
 }
 
diff --git a/test/libndctl.c b/test/libndctl.c
index 02bb9ccaa465..9ad8f87b92dc 100644
--- a/test/libndctl.c
+++ b/test/libndctl.c
@@ -2622,9 +2622,15 @@ static int do_test0(struct ndctl_ctx *ctx, struct ndctl_test *test)
 		}
 	}
 
-	/* set regions back to their default state */
-	ndctl_region_foreach(bus, region)
+	/*
+	 * Enable regions and adjust the space-align to drop the default
+	 * alignment constraints
+	 */
+	ndctl_region_foreach(bus, region) {
 		ndctl_region_enable(region);
+		ndctl_region_set_align(region, sysconf(_SC_PAGESIZE)
+				* ndctl_region_get_interleave_ways(region));
+	}
 
 	/* pfn and dax tests require vmalloc-enabled nfit_test */
 	if (ndctl_test_attempt(test, KERNEL_VERSION(4, 8, 0))) {
diff --git a/test/parent-uuid.c b/test/parent-uuid.c
index 3a63f7244e21..f41ca2c7bd75 100644
--- a/test/parent-uuid.c
+++ b/test/parent-uuid.c
@@ -61,6 +61,7 @@ static struct ndctl_namespace *create_blk_namespace(int region_fraction,
 	struct ndctl_namespace *ndns, *seed_ns = NULL;
 	unsigned long long size;
 
+	ndctl_region_set_align(region, sysconf(_SC_PAGESIZE));
 	ndctl_namespace_foreach(region, ndns)
 		if (ndctl_namespace_get_size(ndns) == 0) {
 			seed_ns = ndns;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 08/36] ndctl/namespace: Emit better errors on failure
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (6 preceding siblings ...)
  2020-02-29 20:20 ` [ndctl PATCH 07/36] ndctl/region: Support ndctl_region_{get, set}_align() Dan Williams
@ 2020-02-29 20:20 ` Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 09/36] ndctl/namespace: Check for region alignment violations Dan Williams
                   ` (28 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Aneesh Kumar K.V

When namespace creation fails, useful information about the reason is
not available unless the command happened to be run with the verbose. It
is needlessly confusing and user-unfriendly.

Introduce a new err() facility and use it to replace the debug() prints
that are actually error reasons. The err() helper suppresses the cryptic
strerror() summary message at the end of the call, but if no error
message was printed then the summary message is the fallback.

Some existing error() calls are also move to err().

Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   74 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 47 insertions(+), 27 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 994b4e8791ea..b967e9be578f 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -57,6 +57,8 @@ static struct parameters {
 	.autolabel = true,
 };
 
+const char *cmd_name = "namespace";
+
 void builtin_xaction_namespace_reset(void)
 {
 	/*
@@ -88,6 +90,10 @@ struct parsed_parameters {
 		do { } while (0); \
 	}})
 
+static int err_count;
+#define err(fmt, ...) \
+	({ err_count++; error("%s: " fmt, cmd_name, ##__VA_ARGS__); })
+
 #define BASE_OPTIONS() \
 OPT_STRING('b', "bus", &param.bus, "bus-id", \
 	"limit namespace to a bus with an id or provider of <bus-id>"), \
@@ -325,7 +331,7 @@ static const char *parse_namespace_options(int argc, const char **argv,
 do { \
 	int __rc = prefix##_##op(dev, p); \
 	if (__rc) { \
-		debug("%s: " #op " failed: %s\n", \
+		err("%s: " #op " failed: %s\n", \
 				prefix##_get_devname(dev), \
 				strerror(abs(__rc))); \
 		return __rc; \
@@ -475,6 +481,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 	unsigned long long size_align, units = 1, resource;
 	struct ndctl_pfn *pfn = NULL;
 	struct ndctl_dax *dax = NULL;
+	unsigned long region_align;
 	unsigned int ways;
 	int rc = 0;
 
@@ -492,7 +499,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 
 	if (param.uuid) {
 		if (uuid_parse(param.uuid, p->uuid) != 0) {
-			debug("%s: invalid uuid\n", __func__);
+			err("%s: invalid uuid\n", __func__);
 			return -EINVAL;
 		}
 	} else
@@ -504,7 +511,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		rc = snprintf(p->name, sizeof(p->name), "%s",
 				ndctl_namespace_get_alt_name(ndns));
 	if (rc >= (int) sizeof(p->name)) {
-		debug("%s: alt name overflow\n", __func__);
+		err("%s: alt name overflow\n", __func__);
 		return -EINVAL;
 	}
 
@@ -523,7 +530,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		if (ndctl_region_get_type(region) != ND_DEVICE_REGION_PMEM
 				&& (p->mode == NDCTL_NS_MODE_MEMORY
 					|| p->mode == NDCTL_NS_MODE_DAX)) {
-			debug("blk %s does not support %s mode\n", region_name,
+			err("blk %s does not support %s mode\n", region_name,
 					p->mode == NDCTL_NS_MODE_MEMORY
 					? "fsdax" : "devdax");
 			return -EAGAIN;
@@ -534,7 +541,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 	if (p->mode == NDCTL_NS_MODE_MEMORY) {
 		pfn = ndctl_region_get_pfn_seed(region);
 		if (!pfn && param.mode_default) {
-			debug("%s fsdax mode not available\n", region_name);
+			err("%s fsdax mode not available\n", region_name);
 			p->mode = NDCTL_NS_MODE_RAW;
 		}
 		/*
@@ -671,7 +678,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		p->size++;
 		p->size *= size_align;
 		p->size /= units;
-		error("'--size=' must align to interleave-width: %d and alignment: %ld\n"
+		err("'--size=' must align to interleave-width: %d and alignment: %ld\n"
 				"did you intend --size=%lld%s?\n",
 				ways, p->align, p->size, suffix);
 		return -EINVAL;
@@ -685,7 +692,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		btt = ndctl_region_get_btt_seed(region);
 		if (p->mode == NDCTL_NS_MODE_SAFE) {
 			if (!btt) {
-				debug("%s: does not support 'sector' mode\n",
+				err("%s: does not support 'sector' mode\n",
 						region_name);
 				return -EINVAL;
 			}
@@ -695,7 +702,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 						== p->sector_size)
 					break;
 			if (i >= num) {
-				debug("%s: does not support btt sector_size %lu\n",
+				err("%s: does not support btt sector_size %lu\n",
 						region_name, p->sector_size);
 				return -EINVAL;
 			}
@@ -710,7 +717,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 						== p->sector_size)
 					break;
 			if (i >= num) {
-				debug("%s: does not support namespace sector_size %lu\n",
+				err("%s: does not support namespace sector_size %lu\n",
 						region_name, p->sector_size);
 				return -EINVAL;
 			}
@@ -752,7 +759,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 
 		if (ndns && p->mode != NDCTL_NS_MODE_MEMORY
 			&& p->mode != NDCTL_NS_MODE_DAX) {
-			debug("%s: --map= only valid for fsdax mode namespace\n",
+			err("%s: --map= only valid for fsdax mode namespace\n",
 				ndctl_namespace_get_devname(ndns));
 			return -EINVAL;
 		}
@@ -850,7 +857,7 @@ static int zero_info_block(struct ndctl_namespace *ndns)
 	ndctl_namespace_set_raw_mode(ndns, 1);
 	rc = ndctl_namespace_enable(ndns);
 	if (rc < 0) {
-		debug("%s failed to enable for zeroing, continuing\n", devname);
+		err("%s failed to enable for zeroing, continuing\n", devname);
 		rc = 1;
 		goto out;
 	}
@@ -867,7 +874,7 @@ static int zero_info_block(struct ndctl_namespace *ndns)
 	sprintf(path, "/dev/%s", ndctl_namespace_get_block_device(ndns));
 	fd = open(path, O_RDWR|O_DIRECT|O_EXCL);
 	if (fd < 0) {
-		debug("%s: failed to open %s to zero info block\n",
+		err("%s: failed to open %s to zero info block\n",
 				devname, path);
 		goto out;
 	}
@@ -875,7 +882,7 @@ static int zero_info_block(struct ndctl_namespace *ndns)
 	memset(buf, 0, info_size);
 	rc = pread(fd, read_buf, info_size, 0);
 	if (rc < info_size) {
-		debug("%s: failed to read info block, continuing\n",
+		err("%s: failed to read info block, continuing\n",
 			devname);
 	}
 	if (memcmp(buf, read_buf, info_size) == 0) {
@@ -885,7 +892,7 @@ static int zero_info_block(struct ndctl_namespace *ndns)
 
 	rc = pwrite(fd, buf, info_size, 0);
 	if (rc < info_size) {
-		debug("%s: failed to zero info block %s\n",
+		err("%s: failed to zero info block %s\n",
 				devname, path);
 		rc = -ENXIO;
 	} else
@@ -1046,7 +1053,7 @@ retry:
 out:
 	ndctl_region_enable(region);
 	if (ndctl_region_get_nstype(region) != ND_DEVICE_NAMESPACE_PMEM) {
-		debug("%s: failed to initialize labels\n",
+		err("%s: failed to initialize labels\n",
 				ndctl_region_get_devname(region));
 		return -EBUSY;
 	}
@@ -1101,19 +1108,19 @@ static int bus_send_clear(struct ndctl_bus *bus, unsigned long long start,
 	/* get ars_cap */
 	cmd_cap = ndctl_bus_cmd_new_ars_cap(bus, start, size);
 	if (!cmd_cap) {
-		debug("bus: %s failed to create cmd\n", busname);
+		err("bus: %s failed to create cmd\n", busname);
 		return -ENOTTY;
 	}
 
 	rc = ndctl_cmd_submit_xlat(cmd_cap);
 	if (rc < 0) {
-		debug("bus: %s failed to submit cmd: %d\n", busname, rc);
+		err("bus: %s failed to submit cmd: %d\n", busname, rc);
 		goto out_cap;
 	}
 
 	/* send clear_error */
 	if (ndctl_cmd_ars_cap_get_range(cmd_cap, &range)) {
-		debug("bus: %s failed to get ars_cap range\n", busname);
+		err("bus: %s failed to get ars_cap range\n", busname);
 		rc = -ENXIO;
 		goto out_cap;
 	}
@@ -1121,20 +1128,20 @@ static int bus_send_clear(struct ndctl_bus *bus, unsigned long long start,
 	cmd_clear = ndctl_bus_cmd_new_clear_error(range.address,
 					range.length, cmd_cap);
 	if (!cmd_clear) {
-		debug("bus: %s failed to create cmd\n", busname);
+		err("bus: %s failed to create cmd\n", busname);
 		rc = -ENOTTY;
 		goto out_cap;
 	}
 
 	rc = ndctl_cmd_submit_xlat(cmd_clear);
 	if (rc < 0) {
-		debug("bus: %s failed to submit cmd: %d\n", busname, rc);
+		err("bus: %s failed to submit cmd: %d\n", busname, rc);
 		goto out_clr;
 	}
 
 	cleared = ndctl_cmd_clear_error_get_cleared(cmd_clear);
 	if (cleared != range.length) {
-		debug("bus: %s expected to clear: %lld actual: %lld\n",
+		err("bus: %s expected to clear: %lld actual: %lld\n",
 				busname, range.length, cleared);
 		rc = -ENXIO;
 	}
@@ -1356,6 +1363,19 @@ static int do_xaction_namespace(const char *namespace,
 	if (verbose)
 		ndctl_set_log_priority(ctx, LOG_DEBUG);
 
+	if (action == ACTION_ENABLE)
+		cmd_name = "enable namespace";
+	else if (action == ACTION_DISABLE)
+		cmd_name = "disable namespace";
+	else if (action == ACTION_CREATE)
+		cmd_name = "create namespace";
+	else if (action == ACTION_DESTROY)
+		cmd_name = "destroy namespace";
+	else if (action == ACTION_CHECK)
+		cmd_name = "check namespace";
+	else if (action == ACTION_CLEAR)
+		cmd_name = "clear errors namespace";
+
         ndctl_bus_foreach(ctx, bus) {
 		bool do_scrub;
 
@@ -1478,7 +1498,7 @@ int cmd_disable_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
 	int disabled, rc;
 
 	rc = do_xaction_namespace(namespace, ACTION_DISABLE, ctx, &disabled);
-	if (rc < 0)
+	if (rc < 0 && !err_count)
 		fprintf(stderr, "error disabling namespaces: %s\n",
 				strerror(-rc));
 
@@ -1495,7 +1515,7 @@ int cmd_enable_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
 	int enabled, rc;
 
 	rc = do_xaction_namespace(namespace, ACTION_ENABLE, ctx, &enabled);
-	if (rc < 0)
+	if (rc < 0 && !err_count)
 		fprintf(stderr, "error enabling namespaces: %s\n",
 				strerror(-rc));
 	fprintf(stderr, "enabled %d namespace%s\n", enabled,
@@ -1525,7 +1545,7 @@ int cmd_create_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
 	if (param.greedy)
 		fprintf(stderr, "created %d namespace%s\n", created,
 			created == 1 ? "" : "s");
-	if (rc < 0 || (!namespace && created < 1)) {
+	if ((rc < 0 || (!namespace && created < 1)) && !err_count) {
 		fprintf(stderr, "failed to %s namespace: %s\n", namespace
 				? "reconfigure" : "create", strerror(-rc));
 		if (!namespace)
@@ -1543,7 +1563,7 @@ int cmd_destroy_namespace(int argc , const char **argv, struct ndctl_ctx *ctx)
 	int destroyed, rc;
 
 	rc = do_xaction_namespace(namespace, ACTION_DESTROY, ctx, &destroyed);
-	if (rc < 0)
+	if (rc < 0 && !err_count)
 		fprintf(stderr, "error destroying namespaces: %s\n",
 				strerror(-rc));
 	fprintf(stderr, "destroyed %d namespace%s\n", destroyed,
@@ -1559,7 +1579,7 @@ int cmd_check_namespace(int argc , const char **argv, struct ndctl_ctx *ctx)
 	int checked, rc;
 
 	rc = do_xaction_namespace(namespace, ACTION_CHECK, ctx, &checked);
-	if (rc < 0)
+	if (rc < 0 && !err_count)
 		fprintf(stderr, "error checking namespaces: %s\n",
 				strerror(-rc));
 	fprintf(stderr, "checked %d namespace%s\n", checked,
@@ -1575,7 +1595,7 @@ int cmd_clear_errors(int argc , const char **argv, struct ndctl_ctx *ctx)
 	int cleared, rc;
 
 	rc = do_xaction_namespace(namespace, ACTION_CLEAR, ctx, &cleared);
-	if (rc < 0)
+	if (rc < 0 && !err_count)
 		fprintf(stderr, "error clearing namespaces: %s\n",
 				strerror(-rc));
 	fprintf(stderr, "cleared %d namespace%s\n", cleared,
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 09/36] ndctl/namespace: Check for region alignment violations
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (7 preceding siblings ...)
  2020-02-29 20:20 ` [ndctl PATCH 08/36] ndctl/namespace: Emit better errors on failure Dan Williams
@ 2020-02-29 20:20 ` Dan Williams
  2020-02-29 20:20 ` [ndctl PATCH 10/36] ndctl/util: Up-level is_power_of_2() and introduce IS_ALIGNED Dan Williams
                   ` (27 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Aneesh Kumar K.V

With the new kernel updates to enforce wider alignment constraints by
default ndctl has the ability to validate alignments problems before the
kernel fails the namespace instantiation. Teach create-namespace to
check the size argument against the region alignment rather than waiting
for the kernel to fail the operation:

Before:
#  ndctl create-namespace -m fsdax -s 1073750016 -a 4k
failed to create namespace: Invalid argument

After:
#  ndctl create-namespace -m fsdax -s 1073750016 -a 4k
  Error: create namespace: region2: align setting is 0x1000000 size 0x40002000 is misaligned

Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index b967e9be578f..c4aab94abcd4 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -652,6 +652,14 @@ static int validate_namespace_options(struct ndctl_region *region,
 		}
 	}
 
+	region_align = ndctl_region_get_align(region);
+	if (region_align < ULONG_MAX && p->size % region_align) {
+		err("%s: align setting is %#lx size %#llx is misaligned\n",
+				ndctl_region_get_devname(region), region_align,
+				p->size);
+		return -EINVAL;
+	}
+
 	size_align = p->align;
 
 	/* (re-)validate that the size satisfies the alignment */
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 10/36] ndctl/util: Up-level is_power_of_2() and introduce IS_ALIGNED
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (8 preceding siblings ...)
  2020-02-29 20:20 ` [ndctl PATCH 09/36] ndctl/namespace: Check for region alignment violations Dan Williams
@ 2020-02-29 20:20 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 11/36] ndctl/namespace: Validate resource alignment for dax-mode namespaces Dan Williams
                   ` (26 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:20 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Add helpers for checking alignment to util/size.h.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/ars.c |    6 +-----
 util/size.h     |    7 +++++++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/ndctl/lib/ars.c b/ndctl/lib/ars.c
index bd75131081ba..d91a99d00d10 100644
--- a/ndctl/lib/ars.c
+++ b/ndctl/lib/ars.c
@@ -11,6 +11,7 @@
  * more details.
  */
 #include <stdlib.h>
+#include <util/size.h>
 #include <ndctl/libndctl.h>
 #include "private.h"
 
@@ -43,11 +44,6 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_bus_cmd_new_ars_cap(struct ndctl_bus *bus,
 	return cmd;
 }
 
-static bool is_power_of_2(unsigned int v)
-{
-	return v && ((v & (v - 1)) == 0);
-}
-
 static bool validate_clear_error(struct ndctl_cmd *ars_cap)
 {
 	if (!is_power_of_2(ars_cap->ars_cap->clear_err_unit))
diff --git a/util/size.h b/util/size.h
index 34fac58d6945..2f36c2c85ca7 100644
--- a/util/size.h
+++ b/util/size.h
@@ -13,6 +13,7 @@
 
 #ifndef _NDCTL_SIZE_H_
 #define _NDCTL_SIZE_H_
+#include <stdbool.h>
 
 #define SZ_1K     0x00000400
 #define SZ_4K     0x00001000
@@ -27,8 +28,14 @@
 unsigned long long parse_size64(const char *str);
 unsigned long long __parse_size64(const char *str, unsigned long long *units);
 
+static inline bool is_power_of_2(unsigned long long v)
+{
+	return v && ((v & (v - 1)) == 0);
+}
+
 #define ALIGN(x, a) ((((unsigned long long) x) + (a - 1)) & ~(a - 1))
 #define ALIGN_DOWN(x, a) (((((unsigned long long) x) + a) & ~(a - 1)) - a)
+#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
 #define BITS_PER_LONG (sizeof(unsigned long) * 8)
 #define HPAGE_SIZE (2 << 20)
 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 11/36] ndctl/namespace: Validate resource alignment for dax-mode namespaces
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (9 preceding siblings ...)
  2020-02-29 20:20 ` [ndctl PATCH 10/36] ndctl/util: Up-level is_power_of_2() and introduce IS_ALIGNED Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 12/36] ndctl/namespace: Add read-infoblock command Dan Williams
                   ` (25 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Aneesh Kumar K.V

The kernel sets the default region alignment to 16M to promote
cross-arch compatible namespace creation. While ndctl never touches the
region alignment the end user might have changed it from its default.
Enforce 16MiB alignment for the namespace resource base by default for
dax-mode namespaces.

It is still possible to use a 2MiB region-align for dax-mode namespaces
on x86, but that requires --force to bypass this default alignment
check.

I chose a hard coded default value in ndctl with a --force to bypass the
check rather than having a new sysfs attribute to probe for this detail.
I.e. the kernel could export the minimum alignment for dax namespaces,
but since the minimum compat value is already known, no need for a trip
to the kernel.

Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl/ndctl-create-namespace.txt |    8 ++++++++
 ndctl/namespace.c                              |   24 ++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/Documentation/ndctl/ndctl-create-namespace.txt b/Documentation/ndctl/ndctl-create-namespace.txt
index 8cd80fa789c1..7637e2403132 100644
--- a/Documentation/ndctl/ndctl-create-namespace.txt
+++ b/Documentation/ndctl/ndctl-create-namespace.txt
@@ -88,6 +88,14 @@ OPTIONS
 	    fault scenarios are supported. I.e. if a device is
 	    configured with a 2M alignment an attempt to fault a 4K
 	    aligned offset will result in SIGBUS.
+::
+	Note both 'fsdax' and 'devdax' mode require 16MiB physical
+	alignment to be cross-arch compatible. By default ndctl will
+	block attempts to create namespaces in these modes when the
+	physical starting address of the namespace is not 16MiB aligned.
+	The --force option tries to override this constraint if the
+	platform supports a smaller alignment, but this is not
+	recommended.
 
 -s::
 --size=::
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index c4aab94abcd4..96d318166300 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -356,6 +356,24 @@ static bool do_setup_pfn(struct ndctl_namespace *ndns,
 	return false;
 }
 
+static int check_dax_align(struct ndctl_namespace *ndns)
+{
+	unsigned long long resource = ndctl_namespace_get_resource(ndns);
+	const char *devname = ndctl_namespace_get_devname(ndns);
+
+	if (resource == ULLONG_MAX) {
+		warning("%s unable to validate alignment\n", devname);
+		return 0;
+	}
+
+	if (IS_ALIGNED(resource, SZ_16M) || force)
+		return 0;
+
+	error("%s misaligned to 16M, adjust region alignment and retry\n",
+			devname);
+	return -EINVAL;
+}
+
 static int setup_namespace(struct ndctl_region *region,
 		struct ndctl_namespace *ndns, struct parsed_parameters *p)
 {
@@ -406,6 +424,9 @@ static int setup_namespace(struct ndctl_region *region,
 	if (do_setup_pfn(ndns, p)) {
 		struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
 
+		rc = check_dax_align(ndns);
+		if (rc)
+			return rc;
 		try(ndctl_pfn, set_uuid, pfn, uuid);
 		try(ndctl_pfn, set_location, pfn, p->loc);
 		if (ndctl_pfn_has_align(pfn))
@@ -417,6 +438,9 @@ static int setup_namespace(struct ndctl_region *region,
 	} else if (p->mode == NDCTL_NS_MODE_DAX) {
 		struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
 
+		rc = check_dax_align(ndns);
+		if (rc)
+			return rc;
 		try(ndctl_dax, set_uuid, dax, uuid);
 		try(ndctl_dax, set_location, dax, p->loc);
 		/* device-dax assumes 'align' attribute present */
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 12/36] ndctl/namespace: Add read-infoblock command
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (10 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 11/36] ndctl/namespace: Validate resource alignment for dax-mode namespaces Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 13/36] ndctl/test: Update dax-dev to handle multiple e820 ranges Dan Williams
                   ` (24 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Namespaces may contain an info-block that correlates with the possible
modes of a namespace: 'sector', 'fsdax', or 'devdax'. Provide a command
that can temporarily convert the namespace into 'raw' mode to read the
info-block.

Also, similar to 'read-labels' provide a facility to decode the
info-block into json.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl/Makefile.am              |    3 
 Documentation/ndctl/ndctl-read-infoblock.txt |   94 ++++++
 ndctl/action.h                               |    1 
 ndctl/builtin.h                              |    1 
 ndctl/check.c                                |   20 -
 ndctl/namespace.c                            |  404 ++++++++++++++++++++++++++
 ndctl/namespace.h                            |   51 +++
 ndctl/ndctl.c                                |    1 
 util/fletcher.h                              |    1 
 util/size.h                                  |    1 
 10 files changed, 555 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/ndctl/ndctl-read-infoblock.txt

diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index 659cb32f0878..a5b20715fa9b 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -55,7 +55,8 @@ man1_MANS = \
 	ndctl-freeze-security.1 \
 	ndctl-sanitize-dimm.1 \
 	ndctl-load-keys.1 \
-	ndctl-wait-overwrite.1
+	ndctl-wait-overwrite.1 \
+	ndctl-read-infoblock.1
 
 EXTRA_DIST = $(man1_MANS)
 
diff --git a/Documentation/ndctl/ndctl-read-infoblock.txt b/Documentation/ndctl/ndctl-read-infoblock.txt
new file mode 100644
index 000000000000..603cd6b223d3
--- /dev/null
+++ b/Documentation/ndctl/ndctl-read-infoblock.txt
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-read-infoblock(1)
+=======================
+
+NAME
+----
+ndctl-read-infoblock - read and optionally parse the info-block a namespace
+
+SYNOPSIS
+--------
+[verse]
+'ndctl read-infoblock' <namespace0.0> [<namespace1.0>..<namespaceN.Y>] [<options>]
+
+DESCRIPTION
+-----------
+As described in the theory of operation section of
+linkndctl:ndctl-create-namespace[1], the raw capacity of a namespace may
+encapsulate a personality, or mode of operation. Specifically, the mode
+may be set to one of "sector", "fsdax", and "devdax". Each of those
+modes is defined by an info-block format that uniquely identifies the
+mode of operation. The read-infoblock command knows the location
+(relative to the start of the namespace) and field definition of those
+data structures.
+
+Note that unlike a partition table info-block is not exposed by default,
+so the namespace needs to be disabled before the info-block can be
+accessed.
+
+EXAMPLE
+-------
+
+[verse]
+ndctl disable-namespace namespace0.0
+disabled 1 namespace
+ndctl read-infoblock -j namespace0.0
+[
+  {
+    "dev":"namespace0.0",
+    "signature":"NVDIMM_PFN_INFO",
+    "uuid":"56b11990-66b1-4d91-9cac-ca084c051259",
+    "parent_uuid":"00000000-0000-0000-0000-000000000000",
+    "flags":0,
+    "version":"1.3",
+    "dataoff":69206016,
+    "npfns":1031680,
+    "mode":2,
+    "start_pad":0,
+    "end_trunc":0,
+    "align":2097152
+  }
+]
+
+
+OPTIONS
+-------
+<namespace(s)>::
+	One or more 'namespaceX.Y' device names. The keyword 'all' can be specified to
+	operate on every namespace in the system, optionally filtered by bus id (see
+        --bus= option), or region id (see --region= option).
+
+-V::
+--verify::
+	Attempt to validate that the info-block is self consistent by
+	validating the embedded checksum, and that info-block formats
+	that contain a 'parent-uuid' attribute also match the base-uuid
+	of the namespace.
+
+-o::
+--output::
+	Output file
+
+-j::
+--json::
+	Parse the info-block data into json.
+-u::
+--human::
+	Enable json output and convert number formats to human readable
+	strings, for example show the size in terms of "KB", "MB", "GB", etc
+	instead of a signed 64-bit numbers per the JSON interchange
+	format (implies --json).
+
+-r::
+--region=::
+include::xable-region-options.txt[]
+
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkndctl:ndctl-create-namespace[1],
+http://www.uefi.org/sites/default/files/resources/UEFI_Spec_2_7.pdf[UEFI NVDIMM Label Protocol]
+
diff --git a/ndctl/action.h b/ndctl/action.h
index 50da010ae826..636524c01f20 100644
--- a/ndctl/action.h
+++ b/ndctl/action.h
@@ -14,5 +14,6 @@ enum device_action {
 	ACTION_WAIT,
 	ACTION_START,
 	ACTION_CLEAR,
+	ACTION_READ_INFOBLOCK,
 };
 #endif /* __NDCTL_ACTION_H__ */
diff --git a/ndctl/builtin.h b/ndctl/builtin.h
index 94ab3177e9b6..aa41ad01a84c 100644
--- a/ndctl/builtin.h
+++ b/ndctl/builtin.h
@@ -8,6 +8,7 @@ int cmd_create_nfit(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_enable_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_create_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_destroy_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_read_infoblock(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_disable_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_check_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_clear_errors(int argc, const char **argv, struct ndctl_ctx *ctx);
diff --git a/ndctl/check.c b/ndctl/check.c
index 365abafc12d1..cdb3d0bb5ae7 100644
--- a/ndctl/check.c
+++ b/ndctl/check.c
@@ -297,24 +297,6 @@ static int btt_log_read(struct arena_info *a, u32 lane, struct log_entry *ent)
 	return 0;
 }
 
-static int btt_checksum_verify(struct btt_sb *btt_sb)
-{
-	uint64_t sum;
-	le64 sum_save;
-
-	BUILD_BUG_ON(sizeof(struct btt_sb) != SZ_4K);
-
-	sum_save = btt_sb->checksum;
-	btt_sb->checksum = 0;
-	sum = fletcher64(btt_sb, sizeof(*btt_sb), 1);
-	if (sum != sum_save)
-		return 1;
-	/* restore the checksum in the buffer */
-	btt_sb->checksum = sum_save;
-
-	return 0;
-}
-
 /*
  * Never pass a mmapped buffer to this as it will attempt to write to
  * the buffer, and we want writes to only happened in a controlled fashion.
@@ -330,7 +312,7 @@ static int btt_info_verify(struct btt_chk *bttc, struct btt_sb *btt_sb)
 		if (uuid_compare(bttc->parent_uuid, btt_sb->parent_uuid) != 0)
 			return -ENXIO;
 
-	if (btt_checksum_verify(btt_sb))
+	if (!verify_infoblock_checksum((union info_block *) btt_sb))
 		return -ENXIO;
 
 	return 0;
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 96d318166300..9bc54abfd437 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -21,6 +21,7 @@
 
 #include <ndctl.h>
 #include "action.h"
+#include "namespace.h"
 #include <sys/stat.h>
 #include <uuid/uuid.h>
 #include <sys/types.h>
@@ -42,6 +43,9 @@ static struct parameters {
 	bool mode_default;
 	bool autolabel;
 	bool greedy;
+	bool verify;
+	bool human;
+	bool json;
 	const char *bus;
 	const char *map;
 	const char *type;
@@ -53,6 +57,8 @@ static struct parameters {
 	const char *reconfig;
 	const char *sector_size;
 	const char *align;
+	const char *outfile;
+	const char *infile;
 } param = {
 	.autolabel = true,
 };
@@ -83,6 +89,13 @@ struct parsed_parameters {
 	bool autolabel;
 };
 
+#define pr_verbose(fmt, ...) \
+	({if (verbose) { \
+		fprintf(stderr, fmt, ##__VA_ARGS__); \
+	} else { \
+		do { } while (0); \
+	}})
+
 #define debug(fmt, ...) \
 	({if (verbose) { \
 		fprintf(stderr, "%s:%d: " fmt, __func__, __LINE__, ##__VA_ARGS__); \
@@ -133,6 +146,16 @@ OPT_BOOLEAN('f', "force", &force, "check namespace even if currently active")
 #define CLEAR_OPTIONS() \
 OPT_BOOLEAN('s', "scrub", &scrub, "run a scrub to find latent errors")
 
+#define READ_INFOBLOCK_OPTIONS() \
+OPT_FILENAME('o', "output", &param.outfile, "output-file", \
+	"filename to write info-block contents"), \
+OPT_FILENAME('i', "input", &param.infile, "input-file", \
+	"filename to read info-block instead of a namespace"), \
+OPT_BOOLEAN('V', "verify", &param.verify, \
+	"validate parent uuid, and info-block checksum"), \
+OPT_BOOLEAN('j', "json", &param.json, "parse label data into json"), \
+OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats (implies --json)")
+
 static const struct option base_options[] = {
 	BASE_OPTIONS(),
 	OPT_END(),
@@ -163,6 +186,12 @@ static const struct option clear_options[] = {
 	OPT_END(),
 };
 
+static const struct option read_infoblock_options[] = {
+	BASE_OPTIONS(),
+	READ_INFOBLOCK_OPTIONS(),
+	OPT_END(),
+};
+
 static int set_defaults(enum device_action mode)
 {
 	int rc = 0;
@@ -307,18 +336,30 @@ static const char *parse_namespace_options(int argc, const char **argv,
 			case ACTION_CLEAR:
 				action_string = "clear errors for";
 				break;
+			case ACTION_READ_INFOBLOCK:
+				action_string = "read-infoblock";
+				break;
 			default:
 				action_string = "<>";
 				break;
 		}
-		error("specify a namespace to %s, or \"all\"\n", action_string);
-		rc = -EINVAL;
+
+		if ((mode == ACTION_READ_INFOBLOCK && !param.infile)
+				|| mode != ACTION_READ_INFOBLOCK) {
+			error("specify a namespace to %s, or \"all\"\n", action_string);
+			rc = -EINVAL;
+		}
 	}
 	for (i = mode == ACTION_CREATE ? 0 : 1; i < argc; i++) {
 		error("unknown extra parameter \"%s\"\n", argv[i]);
 		rc = -EINVAL;
 	}
 
+	if (mode == ACTION_READ_INFOBLOCK && param.infile && argc) {
+		error("specify a namespace, or --input, not both\n");
+		rc = -EINVAL;
+	}
+
 	if (rc) {
 		usage_with_options(u, options);
 		return NULL; /* we won't return from usage_with_options() */
@@ -1377,10 +1418,322 @@ static int namespace_clear_bb(struct ndctl_namespace *ndns, bool do_scrub)
 	return 0;
 }
 
+struct read_infoblock_ctx {
+	struct json_object *jblocks;
+	FILE *f_out;
+};
+
+#define parse_field(sb, field)						\
+do {									\
+	jobj = json_object_new_int(le32_to_cpu((sb)->field));		\
+	if (!jobj)							\
+		goto err;						\
+	json_object_object_add(jblock, #field, jobj);			\
+} while (0)
+
+#define parse_hex(sb, field, sz)						\
+do {										\
+	jobj = util_json_object_hex(le##sz##_to_cpu((sb)->field), flags);	\
+	if (!jobj)								\
+		goto err;							\
+	json_object_object_add(jblock, #field, jobj);				\
+} while (0)
+
+static json_object *btt_parse(struct btt_sb *btt_sb, struct ndctl_namespace *ndns,
+		const char *path, unsigned long flags)
+{
+	uuid_t uuid;
+	char str[40];
+	struct json_object *jblock, *jobj;
+	const char *cmd = "read-info-block";
+	const bool verify = param.verify;
+
+	if (verify && !verify_infoblock_checksum((union info_block *) btt_sb)) {
+		pr_verbose("%s: %s checksum verification failed\n", cmd, __func__);
+		return NULL;
+	}
+
+	if (ndns) {
+		ndctl_namespace_get_uuid(ndns, uuid);
+		if (verify && !uuid_is_null(uuid) && memcmp(uuid, btt_sb->parent_uuid,
+					sizeof(uuid) != 0)) {
+			pr_verbose("%s: %s uuid verification failed\n", cmd, __func__);
+			return NULL;
+		}
+	}
+
+	jblock = json_object_new_object();
+	if (!jblock)
+		return NULL;
+
+	if (ndns) {
+		jobj = json_object_new_string(ndctl_namespace_get_devname(ndns));
+		if (!jobj)
+			goto err;
+		json_object_object_add(jblock, "dev", jobj);
+	} else {
+		jobj = json_object_new_string(path);
+		if (!jobj)
+			goto err;
+		json_object_object_add(jblock, "file", jobj);
+	}
+
+	jobj = json_object_new_string((char *) btt_sb->signature);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "signature", jobj);
+
+	uuid_unparse((void *) btt_sb->uuid, str);
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "uuid", jobj);
+
+	uuid_unparse((void *) btt_sb->parent_uuid, str);
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "parent_uuid", jobj);
+
+	jobj = util_json_object_hex(le32_to_cpu(btt_sb->flags), flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "flags", jobj);
+
+	if (snprintf(str, 4, "%d.%d", btt_sb->version_major,
+				btt_sb->version_minor) >= 4)
+		goto err;
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "version", jobj);
+
+	parse_field(btt_sb, external_lbasize);
+	parse_field(btt_sb, external_nlba);
+	parse_field(btt_sb, internal_lbasize);
+	parse_field(btt_sb, internal_nlba);
+	parse_field(btt_sb, nfree);
+	parse_field(btt_sb, infosize);
+	parse_hex(btt_sb, nextoff, 64);
+	parse_hex(btt_sb, dataoff, 64);
+	parse_hex(btt_sb, mapoff, 64);
+	parse_hex(btt_sb, logoff, 64);
+	parse_hex(btt_sb, info2off, 64);
+
+	return jblock;
+err:
+	pr_verbose("%s: failed to create json representation\n", cmd);
+	json_object_put(jblock);
+	return NULL;
+}
+
+static json_object *pfn_parse(struct pfn_sb *pfn_sb, struct ndctl_namespace *ndns,
+		const char *path, unsigned long flags)
+{
+	uuid_t uuid;
+	char str[40];
+	struct json_object *jblock, *jobj;
+	const char *cmd = "read-info-block";
+	const bool verify = param.verify;
+
+	if (verify && !verify_infoblock_checksum((union info_block *) pfn_sb)) {
+		pr_verbose("%s: %s checksum verification failed\n", cmd, __func__);
+		return NULL;
+	}
+
+	if (ndns) {
+		ndctl_namespace_get_uuid(ndns, uuid);
+		if (verify && !uuid_is_null(uuid) && memcmp(uuid, pfn_sb->parent_uuid,
+					sizeof(uuid) != 0)) {
+			pr_verbose("%s: %s uuid verification failed\n", cmd, __func__);
+			return NULL;
+		}
+	}
+
+	jblock = json_object_new_object();
+	if (!jblock)
+		return NULL;
+
+	if (ndns) {
+		jobj = json_object_new_string(ndctl_namespace_get_devname(ndns));
+		if (!jobj)
+			goto err;
+		json_object_object_add(jblock, "dev", jobj);
+	} else {
+		jobj = json_object_new_string(path);
+		if (!jobj)
+			goto err;
+		json_object_object_add(jblock, "file", jobj);
+	}
+
+	jobj = json_object_new_string((char *) pfn_sb->signature);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "signature", jobj);
+
+	uuid_unparse((void *) pfn_sb->uuid, str);
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "uuid", jobj);
+
+	uuid_unparse((void *) pfn_sb->parent_uuid, str);
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "parent_uuid", jobj);
+
+	jobj = util_json_object_hex(le32_to_cpu(pfn_sb->flags), flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "flags", jobj);
+
+	if (snprintf(str, 4, "%d.%d", pfn_sb->version_major,
+				pfn_sb->version_minor) >= 4)
+		goto err;
+	jobj = json_object_new_string(str);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jblock, "version", jobj);
+
+	parse_hex(pfn_sb, dataoff, 64);
+	parse_hex(pfn_sb, npfns, 64);
+	parse_field(pfn_sb, mode);
+	parse_hex(pfn_sb, start_pad, 32);
+	parse_hex(pfn_sb, end_trunc, 32);
+	parse_hex(pfn_sb, align, 32);
+
+	return jblock;
+err:
+	pr_verbose("%s: failed to create json representation\n", cmd);
+	json_object_put(jblock);
+	return NULL;
+}
+
+#define INFOBLOCK_SZ SZ_8K
+
+static int parse_namespace_infoblock(char *_buf, struct ndctl_namespace *ndns,
+		const char *path, struct read_infoblock_ctx *ri_ctx)
+{
+	int rc;
+	void *buf = _buf;
+	unsigned long flags = param.human ? UTIL_JSON_HUMAN : 0;
+	struct btt_sb *btt1_sb = buf + SZ_4K, *btt2_sb = buf;
+	struct json_object *jblock = NULL, *jblocks = ri_ctx->jblocks;
+	struct pfn_sb *pfn_sb = buf + SZ_4K, *dax_sb = buf + SZ_4K;
+
+	if (!param.json && !param.human) {
+		rc = fwrite(buf, 1, INFOBLOCK_SZ, ri_ctx->f_out);
+		if (rc != INFOBLOCK_SZ)
+			return -EIO;
+		fflush(ri_ctx->f_out);
+		return 0;
+	}
+
+	if (!jblocks) {
+		jblocks = json_object_new_array();
+		if (!jblocks)
+			return -ENOMEM;
+		ri_ctx->jblocks = jblocks;
+	}
+
+	if (memcmp(btt1_sb->signature, BTT_SIG, BTT_SIG_LEN) == 0) {
+		jblock = btt_parse(btt1_sb, ndns, path, flags);
+		if (jblock)
+			json_object_array_add(jblocks, jblock);
+	}
+
+	if (memcmp(btt2_sb->signature, BTT_SIG, BTT_SIG_LEN) == 0) {
+		jblock = btt_parse(btt2_sb, ndns, path, flags);
+		if (jblock)
+			json_object_array_add(jblocks, jblock);
+	}
+
+	if (memcmp(pfn_sb->signature, PFN_SIG, PFN_SIG_LEN) == 0) {
+		jblock = pfn_parse(pfn_sb, ndns, path, flags);
+		if (jblock)
+			json_object_array_add(jblocks, jblock);
+	}
+
+	if (memcmp(dax_sb->signature, DAX_SIG, PFN_SIG_LEN) == 0) {
+		jblock = pfn_parse(dax_sb, ndns, path, flags);
+		if (jblock)
+			json_object_array_add(jblocks, jblock);
+	}
+
+	return 0;
+}
+
+static int file_read_infoblock(const char *path, struct ndctl_namespace *ndns,
+		struct read_infoblock_ctx *ri_ctx)
+{
+	const char *devname = ndns ? ndctl_namespace_get_devname(ndns) : "";
+	const char *cmd = "read-info-block";
+	void *buf = NULL;
+	int fd = -1, rc;
+
+	buf = calloc(1, INFOBLOCK_SZ);
+	if (!buf)
+		return -ENOMEM;
+
+	fd = open(path, O_RDONLY|O_EXCL);
+	if (fd < 0) {
+		pr_verbose("%s: %s failed to open %s: %s\n",
+				cmd, devname, path, strerror(errno));
+		rc = -errno;
+		goto out;
+	}
+
+	rc = read(fd, buf, INFOBLOCK_SZ);
+	if (rc < 0) {
+		pr_verbose("%s: %s failed to read %s: %s\n",
+				cmd, devname, path, strerror(errno));
+		rc = -errno;
+		goto out;
+	}
+
+	rc = parse_namespace_infoblock(buf, ndns, path, ri_ctx);
+out:
+	free(buf);
+	if (fd >= 0)
+		close(fd);
+	return rc;
+}
+
+static int namespace_read_infoblock(struct ndctl_namespace *ndns,
+		struct read_infoblock_ctx *ri_ctx)
+{
+	int rc;
+	char path[50];
+	const char *cmd = "read-info-block";
+	const char *devname = ndctl_namespace_get_devname(ndns);
+
+	if (ndctl_namespace_is_active(ndns)) {
+		pr_verbose("%s: %s enabled, must be disabled\n", cmd, devname);
+		return -EBUSY;
+	}
+
+	ndctl_namespace_set_raw_mode(ndns, 1);
+	rc = ndctl_namespace_enable(ndns);
+	if (rc < 0) {
+		pr_verbose("%s: %s failed to enable\n", cmd, devname);
+		goto out;
+	}
+
+	sprintf(path, "/dev/%s", ndctl_namespace_get_block_device(ndns));
+	rc = file_read_infoblock(path, ndns, ri_ctx);
+
+out:
+	ndctl_namespace_set_raw_mode(ndns, 0);
+	ndctl_namespace_disable_invalidate(ndns);
+	return rc;
+}
+
 static int do_xaction_namespace(const char *namespace,
 		enum device_action action, struct ndctl_ctx *ctx,
 		int *processed)
 {
+	struct read_infoblock_ctx ri_ctx = { 0 };
 	struct ndctl_namespace *ndns, *_n;
 	int rc = -ENXIO, saved_rc = 0;
 	struct ndctl_region *region;
@@ -1389,6 +1742,26 @@ static int do_xaction_namespace(const char *namespace,
 
 	*processed = 0;
 
+	if (action == ACTION_READ_INFOBLOCK) {
+		if (!param.outfile)
+			ri_ctx.f_out = stdout;
+		else {
+			ri_ctx.f_out = fopen(param.outfile, "w+");
+			if (!ri_ctx.f_out) {
+				fprintf(stderr, "failed to open: %s: (%s)\n",
+						param.outfile, strerror(errno));
+				return -errno;
+			}
+		}
+
+		if (param.infile) {
+			rc = file_read_infoblock(param.infile, NULL, &ri_ctx);
+			if (ri_ctx.jblocks)
+				util_display_json_array(ri_ctx.f_out, ri_ctx.jblocks, 0);
+			return rc;
+		}
+	}
+
 	if (!namespace && action != ACTION_CREATE)
 		return rc;
 
@@ -1494,6 +1867,11 @@ static int do_xaction_namespace(const char *namespace,
 					if (rc == 0)
 						*processed = 1;
 					return rc;
+				case ACTION_READ_INFOBLOCK:
+					rc = namespace_read_infoblock(ndns, &ri_ctx);
+					if (rc == 0)
+						(*processed)++;
+					break;
 				default:
 					rc = -EINVAL;
 					break;
@@ -1502,6 +1880,12 @@ static int do_xaction_namespace(const char *namespace,
 		}
 	}
 
+	if (ri_ctx.jblocks)
+		util_display_json_array(ri_ctx.f_out, ri_ctx.jblocks, 0);
+
+	if (ri_ctx.f_out && ri_ctx.f_out != stdout)
+		fclose(ri_ctx.f_out);
+
 	if (action == ACTION_CREATE && rc == -EAGAIN) {
 		/*
 		 * Namespace creation searched through all candidate
@@ -1634,3 +2018,19 @@ int cmd_clear_errors(int argc , const char **argv, struct ndctl_ctx *ctx)
 			cleared == 1 ? "" : "s");
 	return rc;
 }
+
+int cmd_read_infoblock(int argc, const char **argv, struct ndctl_ctx *ctx)
+{
+	char *xable_usage = "ndctl read-info-block <namespace> [<options>]";
+	const char *namespace = parse_namespace_options(argc, argv,
+			ACTION_READ_INFOBLOCK, read_infoblock_options,
+			xable_usage);
+	int read, rc;
+
+	rc = do_xaction_namespace(namespace, ACTION_READ_INFOBLOCK, ctx, &read);
+	if (rc < 0)
+		fprintf(stderr, "error checking namespaces: %s\n",
+				strerror(-rc));
+	fprintf(stderr, "read %d namespace%s\n", read, read == 1 ? "" : "s");
+	return rc;
+}
diff --git a/ndctl/namespace.h b/ndctl/namespace.h
index bc210857642f..861dfbfa5127 100644
--- a/ndctl/namespace.h
+++ b/ndctl/namespace.h
@@ -13,7 +13,9 @@
 #ifndef __NDCTL_NAMESPACE_H__
 #define __NDCTL_NAMESPACE_H__
 #include <sys/types.h>
+#include <util/util.h>
 #include <util/size.h>
+#include <util/fletcher.h>
 #include <ccan/endian/endian.h>
 #include <ccan/short_types/short_types.h>
 
@@ -202,4 +204,53 @@ struct arena_map {
 	struct btt_sb *info2;
 	size_t info2_len;
 };
+
+#define PFN_SIG_LEN 16
+#define PFN_SIG "NVDIMM_PFN_INFO\0"
+#define DAX_SIG "NVDIMM_DAX_INFO\0"
+
+struct pfn_sb {
+	u8 signature[PFN_SIG_LEN];
+	u8 uuid[16];
+	u8 parent_uuid[16];
+	le32 flags;
+	le16 version_major;
+	le16 version_minor;
+	le64 dataoff; /* relative to namespace_base + start_pad */
+	le64 npfns;
+	le32 mode;
+	/* minor-version-1 additions for section alignment */
+	le32 start_pad;
+	le32 end_trunc;
+	/* minor-version-2 record the base alignment of the mapping */
+	le32 align;
+	u8 padding[4000];
+	le64 checksum;
+};
+
+union info_block {
+	struct pfn_sb pfn_sb;
+	struct btt_sb btt_sb;
+};
+
+static inline bool verify_infoblock_checksum(union info_block *sb)
+{
+	uint64_t sum;
+	le64 sum_save;
+
+	BUILD_BUG_ON(sizeof(union info_block) != SZ_4K);
+
+	/* all infoblocks share the btt_sb layout for checksum */
+	sum_save = sb->btt_sb.checksum;
+	sb->btt_sb.checksum = 0;
+	sum = fletcher64(&sb->btt_sb, sizeof(*sb), 1);
+	if (sum != sum_save)
+		return false;
+	/* restore the checksum in the buffer */
+	sb->btt_sb.checksum = sum_save;
+
+	return true;
+}
+
+
 #endif /* __NDCTL_NAMESPACE_H__ */
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 6c4975c9f841..5c9c424dcae6 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -73,6 +73,7 @@ static struct cmd_struct commands[] = {
 	{ "disable-namespace", { cmd_disable_namespace } },
 	{ "create-namespace", { cmd_create_namespace } },
 	{ "destroy-namespace", { cmd_destroy_namespace } },
+	{ "read-infoblock",  { cmd_read_infoblock } },
 	{ "check-namespace", { cmd_check_namespace } },
 	{ "clear-errors", { cmd_clear_errors } },
 	{ "enable-region", { cmd_enable_region } },
diff --git a/util/fletcher.h b/util/fletcher.h
index 54e2ecf5d6ed..8fccac4ec758 100644
--- a/util/fletcher.h
+++ b/util/fletcher.h
@@ -1,6 +1,7 @@
 #ifndef _NDCTL_FLETCHER_H_
 #define _NDCTL_FLETCHER_H_
 
+#include <stdbool.h>
 #include <ccan/endian/endian.h>
 #include <ccan/short_types/short_types.h>
 
diff --git a/util/size.h b/util/size.h
index 2f36c2c85ca7..2138989b42ac 100644
--- a/util/size.h
+++ b/util/size.h
@@ -17,6 +17,7 @@
 
 #define SZ_1K     0x00000400
 #define SZ_4K     0x00001000
+#define SZ_8K     0x00002000
 #define SZ_1M     0x00100000
 #define SZ_2M     0x00200000
 #define SZ_4M     0x00400000
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 13/36] ndctl/test: Update dax-dev to handle multiple e820 ranges
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (11 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 12/36] ndctl/namespace: Add read-infoblock command Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 14/36] ndctl/namespace: Always zero info-blocks Dan Williams
                   ` (23 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Establish a convention that the first, lowest-index, region on the e820
bus always enables in fsdax mode without need for an nd_pfn instance.
This is in preparation for a defining a collision test that requires
multiple section-mis-aligned regions defined by memmap=nn!ss.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/dax-dev.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/test/dax-dev.c b/test/dax-dev.c
index 0183b7af2052..49ccaa334e31 100644
--- a/test/dax-dev.c
+++ b/test/dax-dev.c
@@ -33,17 +33,28 @@ struct ndctl_namespace *ndctl_get_test_dev(struct ndctl_ctx *ctx)
 	struct ndctl_bus *bus;
 	struct ndctl_dax *dax;
 	struct ndctl_pfn *pfn;
-	struct ndctl_region *region;
 	struct ndctl_namespace *ndns;
 	enum ndctl_namespace_mode mode;
+	struct ndctl_region *region, *min = NULL;
 
 	bus = ndctl_bus_get_by_provider(ctx, "e820");
 	if (!bus)
 		goto out;
 
-	region = ndctl_region_get_first(bus);
-	if (!region)
+	ndctl_region_foreach(bus, region) {
+		if (!min) {
+			min = region;
+			continue;
+		}
+		if (ndctl_region_get_id(region) < ndctl_region_get_id(min))
+			min = region;
+	}
+	if (!min)
 		goto out;
+	region = min;
+
+	/* attempt to re-enable the region if a previous test disabled it */
+	ndctl_region_enable(region);
 
 	ndns = ndctl_namespace_get_first(region);
 	if (!ndns)
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 14/36] ndctl/namespace: Always zero info-blocks
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (12 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 13/36] ndctl/test: Update dax-dev to handle multiple e820 ranges Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 15/36] ndctl/namespace: Disable autorecovery of create-namespace failures Dan Williams
                   ` (22 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Do not gate zeroing on whether a namespace is claimed by a personality.
The namespace might not have been able to be claimed due to info-block
corruption.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 9bc54abfd437..91e25044145b 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -984,9 +984,6 @@ static int namespace_destroy(struct ndctl_region *region,
 		struct ndctl_namespace *ndns)
 {
 	const char *devname = ndctl_namespace_get_devname(ndns);
-	struct ndctl_pfn *pfn = ndctl_namespace_get_pfn(ndns);
-	struct ndctl_dax *dax = ndctl_namespace_get_dax(ndns);
-	struct ndctl_btt *btt = ndctl_namespace_get_btt(ndns);
 	unsigned long long size;
 	bool did_zero = false;
 	int rc;
@@ -1009,13 +1006,11 @@ static int namespace_destroy(struct ndctl_region *region,
 
 	ndctl_namespace_set_enforce_mode(ndns, NDCTL_NS_MODE_RAW);
 
-	if (pfn || btt || dax) {
-		rc = zero_info_block(ndns);
-		if (rc < 0)
-			return rc;
-		if (rc == 0)
-			did_zero = true;
-	}
+	rc = zero_info_block(ndns);
+	if (rc < 0)
+		return rc;
+	if (rc == 0)
+		did_zero = true;
 
 	switch (ndctl_namespace_get_type(ndns)) {
         case ND_DEVICE_NAMESPACE_PMEM:
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 15/36] ndctl/namespace: Disable autorecovery of create-namespace failures
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (13 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 14/36] ndctl/namespace: Always zero info-blocks Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 16/36] ndctl/build: Fix EXTRA_DIST already defined errors Dan Williams
                   ` (21 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Add an option to disable the behavior of cleaning up namespaces that
failed creation. This is useful for doing forensics on the label and
info-block state after the failure with assurances that the kernel has
not made further modifications.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl/ndctl-create-namespace.txt |    9 +++++++++
 ndctl/namespace.c                              |   13 +++++++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/Documentation/ndctl/ndctl-create-namespace.txt b/Documentation/ndctl/ndctl-create-namespace.txt
index 7637e2403132..92a89dd71e82 100644
--- a/Documentation/ndctl/ndctl-create-namespace.txt
+++ b/Documentation/ndctl/ndctl-create-namespace.txt
@@ -229,6 +229,15 @@ OPTIONS
 	ndctl init-labels all
 	ndctl enable-region all
 
+-R::
+--autorecover::
+--no-autorecover::
+	By default, if a namespace creation attempt fails, ndctl will
+	cleanup the partially initialized namespace. Use
+	--no-autorecover to disable this behavior for debug and
+	development scenarios where it useful to have the label and
+	info-block state preserved after a failure.
+
 -v::
 --verbose::
 	Emit debug messages for the namespace creation process
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 91e25044145b..cd75038f5ae3 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -44,6 +44,7 @@ static struct parameters {
 	bool autolabel;
 	bool greedy;
 	bool verify;
+	bool autorecover;
 	bool human;
 	bool json;
 	const char *bus;
@@ -61,6 +62,7 @@ static struct parameters {
 	const char *infile;
 } param = {
 	.autolabel = true,
+	.autorecover = true,
 };
 
 const char *cmd_name = "namespace";
@@ -87,6 +89,7 @@ struct parsed_parameters {
 	unsigned long sector_size;
 	unsigned long align;
 	bool autolabel;
+	bool autorecover;
 };
 
 #define pr_verbose(fmt, ...) \
@@ -136,7 +139,8 @@ OPT_STRING('a', "align", &param.align, "align", \
 OPT_BOOLEAN('f', "force", &force, "reconfigure namespace even if currently active"), \
 OPT_BOOLEAN('L', "autolabel", &param.autolabel, "automatically initialize labels"), \
 OPT_BOOLEAN('c', "continue", &param.greedy, \
-	"continue creating namespaces as long as the filter criteria are met")
+	"continue creating namespaces as long as the filter criteria are met"), \
+OPT_BOOLEAN('R', "autorecover", &param.autorecover, "automatically cleanup on failure")
 
 #define CHECK_OPTIONS() \
 OPT_BOOLEAN('R', "repair", &repair, "perform metadata repairs"), \
@@ -474,7 +478,7 @@ static int setup_namespace(struct ndctl_region *region,
 			try(ndctl_pfn, set_align, pfn, p->align);
 		try(ndctl_pfn, set_namespace, pfn, ndns);
 		rc = ndctl_pfn_enable(pfn);
-		if (rc)
+		if (rc && p->autorecover)
 			ndctl_pfn_set_namespace(pfn, NULL);
 	} else if (p->mode == NDCTL_NS_MODE_DAX) {
 		struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
@@ -488,7 +492,7 @@ static int setup_namespace(struct ndctl_region *region,
 		try(ndctl_dax, set_align, dax, p->align);
 		try(ndctl_dax, set_namespace, dax, ndns);
 		rc = ndctl_dax_enable(dax);
-		if (rc)
+		if (rc && p->autorecover)
 			ndctl_dax_set_namespace(dax, NULL);
 	} else if (p->mode == NDCTL_NS_MODE_SAFE) {
 		struct ndctl_btt *btt = ndctl_region_get_btt_seed(region);
@@ -848,6 +852,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 
 
 	p->autolabel = param.autolabel;
+	p->autorecover = param.autorecover;
 
 	return 0;
 }
@@ -906,7 +911,7 @@ static int namespace_create(struct ndctl_region *region)
 	}
 
 	rc = setup_namespace(region, ndns, &p);
-	if (rc) {
+	if (rc && p.autorecover) {
 		ndctl_namespace_set_enforce_mode(ndns, NDCTL_NS_MODE_RAW);
 		ndctl_namespace_delete(ndns);
 	}
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 16/36] ndctl/build: Fix EXTRA_DIST already defined errors
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (14 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 15/36] ndctl/namespace: Disable autorecovery of create-namespace failures Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 17/36] ndctl/test: Checkout device-mapper + dax operation Dan Williams
                   ` (20 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Append rather than assign files to EXTRA_DIST in test/Makefile.am to fix
warnings like:

test/Makefile.am:30: warning: EXTRA_DIST multiply defined in condition TRUE ...
Makefile.am.in:1: ... 'EXTRA_DIST' previously defined here
test/Makefile.am:1:   'Makefile.am.in' included from here

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/Makefile.am |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/Makefile.am b/test/Makefile.am
index 58873a6bc341..bde491247a47 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -27,7 +27,7 @@ TESTS =\
 	max_available_extent_ns.sh \
 	pfn-meta-errors.sh
 
-EXTRA_DIST = $(TESTS) common \
+EXTRA_DIST += $(TESTS) common \
 		btt-pad-compat.xxd \
 		nmem1.bin nmem2.bin nmem3.bin nmem4.bin
 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 17/36] ndctl/test: Checkout device-mapper + dax operation
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (15 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 16/36] ndctl/build: Fix EXTRA_DIST already defined errors Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 18/36] ndctl/test: Exercise sub-section sized namespace creation/deletion Dan Williams
                   ` (19 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Given recent kernel changes broke the device-mapper use case, introduce
a basic unit test to prevent this from regressing in the future.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/Makefile.am |    5 ++--
 test/dm.sh       |   75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 2 deletions(-)
 create mode 100755 test/dm.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index bde491247a47..13a0419226a6 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -54,8 +54,9 @@ TESTS +=\
 	dax-xfs.sh \
 	device-dax \
 	device-dax-fio.sh \
-	mmap.sh \
-	daxctl-devices.sh
+	daxctl-devices.sh \
+	dm.sh \
+	mmap.sh
 
 if ENABLE_KEYUTILS
 TESTS += security.sh
diff --git a/test/dm.sh b/test/dm.sh
new file mode 100755
index 000000000000..fb498c95a29b
--- /dev/null
+++ b/test/dm.sh
@@ -0,0 +1,75 @@
+#!/bin/bash -x
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2015-2019 Intel Corporation. All rights reserved.
+
+set -e
+
+SKIP=77
+FAIL=1
+SUCCESS=0
+
+. ./common
+
+MNT=test_dax_mnt
+TEST_DM_PMEM=/dev/mapper/test_pmem
+NAME=$(basename $TEST_DM_PMEM)
+
+mkdir -p $MNT
+
+TEST_SIZE=$((1<<30))
+
+rc=$FAIL
+cleanup() {
+	if [ $rc -ne $SUCCESS ]; then
+		echo "test/dm.sh: failed at line $1"
+	fi
+	if mountpoint -q $MNT; then
+		umount $MNT
+	fi
+
+	if [ -L $TEST_DM_PMEM ]; then
+		dmsetup remove $TEST_DM_PMEM
+	fi
+	rmdir $MNT
+	# opportunistic cleanup, not fatal if these fail
+	namespaces=$($NDCTL list -N | jq -r ".[] | select(.name==\"$NAME\") | .dev")
+	for i in $namespaces
+	do
+		if ! $NDCTL destroy-namespace -f $i; then
+			echo "test/sub-section.sh: cleanup() failed to destroy $i"
+		fi
+	done
+	exit $rc
+}
+
+trap 'err $LINENO cleanup' ERR
+
+dev="x"
+json=$($NDCTL create-namespace -b ACPI.NFIT -s $TEST_SIZE -t pmem -m fsdax -n "$NAME")
+eval $(echo $json | json2var )
+[ $dev = "x" ] && echo "fail: $LINENO" && exit 1
+[ $mode != "fsdax" ] && echo "fail: $LINENO" &&  exit 1
+
+pmem0=/dev/$blockdev
+size0=$((size/512))
+
+json=$($NDCTL create-namespace -b ACPI.NFIT -s $TEST_SIZE -t pmem -m fsdax -n "$NAME")
+eval $(echo $json | json2var )
+[ $dev = "x" ] && echo "fail: $LINENO" && exit 1
+[ $mode != "fsdax" ] && echo "fail: $LINENO" &&  exit 1
+
+pmem1=/dev/$blockdev
+size1=$((size/512))
+
+cat <<EOF |
+0 $size0 linear $pmem0 0
+$size0 $size1 linear $pmem1 0
+EOF
+dmsetup create $(basename $NAME)
+
+mkfs.ext4 -b 4096 $TEST_DM_PMEM
+mount -o dax $TEST_DM_PMEM $MNT
+umount $MNT
+
+rc=$SUCCESS
+cleanup $LINENO
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 18/36] ndctl/test: Exercise sub-section sized namespace creation/deletion
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (16 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 17/36] ndctl/test: Checkout device-mapper + dax operation Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 19/36] ndctl/namespace: Kill off the legacy mode names Dan Williams
                   ` (18 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

For kernels that have removed the section-aligned namespace constraint
validate that multiple namespaces can be created / deleted that collide
within a given section.

While this test acts on the ACPI.NFIT bus it is not marked "destructive"
because it only operates in available capacity and marks each namespace
created with a unique volume name ("subsection-test").

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/Makefile.am    |    1 +
 test/sub-section.sh |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)
 create mode 100755 test/sub-section.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index 13a0419226a6..bbe58a06d9d0 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -49,6 +49,7 @@ if ENABLE_DESTRUCTIVE
 TESTS +=\
 	blk-ns \
 	pmem-ns \
+	sub-section.sh \
 	dax-dev \
 	dax-ext4.sh \
 	dax-xfs.sh \
diff --git a/test/sub-section.sh b/test/sub-section.sh
new file mode 100755
index 000000000000..91aa5c8e4834
--- /dev/null
+++ b/test/sub-section.sh
@@ -0,0 +1,77 @@
+#!/bin/bash -x
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2015-2019 Intel Corporation. All rights reserved.
+
+set -e
+
+SKIP=77
+FAIL=1
+SUCCESS=0
+
+. ./common
+
+check_min_kver "5.3" || do_skip "may lack align sub-section hotplug support"
+
+MNT=test_dax_mnt
+mkdir -p $MNT
+
+TEST_SIZE=$((16<<20))
+MIN_AVAIL=$((TEST_SIZE*4))
+MAX_NS=10
+NAME="subsection-test"
+
+ndctl list -N | jq -r ".[] | select(.name==\"subsection-test\") | .dev"
+
+rc=$FAIL
+cleanup() {
+	if [ $rc -ne $SUCCESS ]; then
+		echo "test/sub-section.sh: failed at line $1"
+	fi
+	if mountpoint -q $MNT; then
+		umount $MNT
+	fi
+	rmdir $MNT
+	# opportunistic cleanup, not fatal if these fail
+	namespaces=$($NDCTL list -N | jq -r ".[] | select(.name==\"$NAME\") | .dev")
+	for i in $namespaces
+	do
+		if ! $NDCTL destroy-namespace -f $i; then
+			echo "test/sub-section.sh: cleanup() failed to destroy $i"
+		fi
+	done
+	exit $rc
+}
+
+trap 'err $LINENO cleanup' ERR
+
+json=$($NDCTL list -R -b ACPI.NFIT)
+region=$(echo $json | jq -r "[.[] | select(.available_size >= $MIN_AVAIL)][0].dev")
+avail=$(echo $json | jq -r "[.[] | select(.available_size >= $MIN_AVAIL)][0].available_size")
+if [ -z $region ]; then
+	exit $SKIP
+fi
+
+iter=$((avail/TEST_SIZE))
+if [ $iter -gt $MAX_NS ]; then
+	iter=$MAX_NS;
+fi
+
+for i in $(seq 1 $iter)
+do
+	json=$($NDCTL create-namespace -s $TEST_SIZE --no-autorecover -r $region -n "$NAME")
+	dev=$(echo $json | jq -r ".blockdev")
+	mkfs.ext4 -b 4096 /dev/$dev
+	mount -o dax /dev/$dev $MNT
+	umount $MNT
+done
+
+namespaces=$($NDCTL list -N | jq -r ".[] | select(.name==\"$NAME\") | .dev")
+for i in $namespaces
+do
+	$NDCTL disable-namespace $i
+	$NDCTL enable-namespace $i
+	$NDCTL destroy-namespace $i -f
+done
+
+rc=$SUCCESS
+cleanup $LINENO
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 19/36] ndctl/namespace: Kill off the legacy mode names
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (17 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 18/36] ndctl/test: Exercise sub-section sized namespace creation/deletion Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 20/36] ndctl/namespace: Introduce mode-to-name and name-to-mode helpers Dan Williams
                   ` (17 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

The command line interface switched to "fsdax", "devdax", and "sector"
for the mode names. Kill off the remaining instances of "memory", "dax",
and "safe".

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/libndctl.c |    4 ++-
 ndctl/libndctl.h     |    1 +
 ndctl/namespace.c    |   66 +++++++++++++++++++++++++-------------------------
 test/libndctl.c      |    6 ++---
 util/filter.c        |    4 ++-
 util/json.c          |    4 ++-
 6 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index d0c1236f0698..9ad1b7091dc0 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -3465,7 +3465,7 @@ static const char *enforce_id_to_name(enum ndctl_namespace_mode mode)
 {
 	static const char *id_to_name[] = {
 		[NDCTL_NS_MODE_MEMORY] = "pfn",
-		[NDCTL_NS_MODE_SAFE] = "btt", /* TODO: convert to btt2 */
+		[NDCTL_NS_MODE_SECTOR] = "btt", /* TODO: convert to btt2 */
 		[NDCTL_NS_MODE_RAW] = "",
 		[NDCTL_NS_MODE_DAX] = "dax",
 		[NDCTL_NS_MODE_UNKNOWN] = "<unknown>",
@@ -3794,7 +3794,7 @@ NDCTL_EXPORT enum ndctl_namespace_mode ndctl_namespace_get_mode(
 	if (strcmp("raw", buf) == 0)
 		return NDCTL_NS_MODE_RAW;
 	if (strcmp("safe", buf) == 0)
-		return NDCTL_NS_MODE_SAFE;
+		return NDCTL_NS_MODE_SECTOR;
 	return -ENXIO;
 }
 
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 076c34583b7d..2580f433ade8 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -475,6 +475,7 @@ enum ndctl_namespace_mode {
 	NDCTL_NS_MODE_MEMORY,
 	NDCTL_NS_MODE_FSDAX = NDCTL_NS_MODE_MEMORY,
 	NDCTL_NS_MODE_SAFE,
+	NDCTL_NS_MODE_SECTOR = NDCTL_NS_MODE_SAFE,
 	NDCTL_NS_MODE_RAW,
 	NDCTL_NS_MODE_DAX,
 	NDCTL_NS_MODE_DEVDAX = NDCTL_NS_MODE_DAX,
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index cd75038f5ae3..770caa5ebab0 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -234,9 +234,9 @@ static int set_defaults(enum device_action mode)
 		}
 	} else if (!param.reconfig && param.type) {
 		if (strcmp(param.type, "pmem") == 0)
-			param.mode = "memory";
+			param.mode = "fsdax";
 		else
-			param.mode = "safe";
+			param.mode = "sector";
 		param.mode_default = true;
 	}
 
@@ -251,9 +251,9 @@ static int set_defaults(enum device_action mode)
 		}
 
 		if (!param.reconfig && param.mode
-				&& strcmp(param.mode, "memory") != 0
-				&& strcmp(param.mode, "dax") != 0) {
-			error("--map only valid for an dax mode pmem namespace\n");
+				&& strcmp(param.mode, "fsdax") != 0
+				&& strcmp(param.mode, "devdax") != 0) {
+			error("--map only valid for an devdax mode pmem namespace\n");
 			rc = -EINVAL;
 		}
 	} else if (!param.reconfig)
@@ -261,8 +261,8 @@ static int set_defaults(enum device_action mode)
 
 	/* check for incompatible mode and type combinations */
 	if (param.type && param.mode && strcmp(param.type, "blk") == 0
-			&& (strcmp(param.mode, "memory") == 0
-				|| strcmp(param.mode, "dax") == 0)) {
+			&& (strcmp(param.mode, "fsdax") == 0
+				|| strcmp(param.mode, "devdax") == 0)) {
 		error("only 'pmem' namespaces support dax operation\n");
 		rc = -ENXIO;
 	}
@@ -386,7 +386,7 @@ do { \
 static bool do_setup_pfn(struct ndctl_namespace *ndns,
 		struct parsed_parameters *p)
 {
-	if (p->mode != NDCTL_NS_MODE_MEMORY)
+	if (p->mode != NDCTL_NS_MODE_FSDAX)
 		return false;
 
 	/*
@@ -394,7 +394,7 @@ static bool do_setup_pfn(struct ndctl_namespace *ndns,
 	 * instance, and a pfn device is required to place the memmap
 	 * array in device memory.
 	 */
-	if (!ndns || ndctl_namespace_get_mode(ndns) != NDCTL_NS_MODE_MEMORY
+	if (!ndns || ndctl_namespace_get_mode(ndns) != NDCTL_NS_MODE_FSDAX
 			|| p->loc == NDCTL_PFN_LOC_PMEM)
 		return true;
 
@@ -446,7 +446,7 @@ static int setup_namespace(struct ndctl_region *region,
 		if (i < num)
 			try(ndctl_namespace, set_sector_size, ndns,
 					p->sector_size);
-		else if (p->mode == NDCTL_NS_MODE_SAFE)
+		else if (p->mode == NDCTL_NS_MODE_SECTOR)
 			/* pass, the btt sector_size will override */;
 		else if (p->sector_size != 512) {
 			error("%s: sector_size: %ld not supported\n",
@@ -480,7 +480,7 @@ static int setup_namespace(struct ndctl_region *region,
 		rc = ndctl_pfn_enable(pfn);
 		if (rc && p->autorecover)
 			ndctl_pfn_set_namespace(pfn, NULL);
-	} else if (p->mode == NDCTL_NS_MODE_DAX) {
+	} else if (p->mode == NDCTL_NS_MODE_DEVDAX) {
 		struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
 
 		rc = check_dax_align(ndns);
@@ -494,7 +494,7 @@ static int setup_namespace(struct ndctl_region *region,
 		rc = ndctl_dax_enable(dax);
 		if (rc && p->autorecover)
 			ndctl_dax_set_namespace(dax, NULL);
-	} else if (p->mode == NDCTL_NS_MODE_SAFE) {
+	} else if (p->mode == NDCTL_NS_MODE_SECTOR) {
 		struct ndctl_btt *btt = ndctl_region_get_btt_seed(region);
 
 		/*
@@ -586,28 +586,28 @@ static int validate_namespace_options(struct ndctl_region *region,
 
 	if (param.mode) {
 		if (strcmp(param.mode, "memory") == 0)
-			p->mode = NDCTL_NS_MODE_MEMORY;
+			p->mode = NDCTL_NS_MODE_FSDAX;
 		else if (strcmp(param.mode, "sector") == 0)
-			p->mode = NDCTL_NS_MODE_SAFE;
+			p->mode = NDCTL_NS_MODE_SECTOR;
 		else if (strcmp(param.mode, "safe") == 0)
-			p->mode = NDCTL_NS_MODE_SAFE;
+			p->mode = NDCTL_NS_MODE_SECTOR;
 		else if (strcmp(param.mode, "dax") == 0)
-			p->mode = NDCTL_NS_MODE_DAX;
+			p->mode = NDCTL_NS_MODE_DEVDAX;
 		else
 			p->mode = NDCTL_NS_MODE_RAW;
 
 		if (ndctl_region_get_type(region) != ND_DEVICE_REGION_PMEM
-				&& (p->mode == NDCTL_NS_MODE_MEMORY
-					|| p->mode == NDCTL_NS_MODE_DAX)) {
+				&& (p->mode == NDCTL_NS_MODE_FSDAX
+					|| p->mode == NDCTL_NS_MODE_DEVDAX)) {
 			err("blk %s does not support %s mode\n", region_name,
-					p->mode == NDCTL_NS_MODE_MEMORY
+					p->mode == NDCTL_NS_MODE_FSDAX
 					? "fsdax" : "devdax");
 			return -EAGAIN;
 		}
 	} else if (ndns)
 		p->mode = ndctl_namespace_get_mode(ndns);
 
-	if (p->mode == NDCTL_NS_MODE_MEMORY) {
+	if (p->mode == NDCTL_NS_MODE_FSDAX) {
 		pfn = ndctl_region_get_pfn_seed(region);
 		if (!pfn && param.mode_default) {
 			err("%s fsdax mode not available\n", region_name);
@@ -616,7 +616,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		/*
 		 * NB: We only fail validation if a pfn-specific option is used
 		 */
-	} else if (p->mode == NDCTL_NS_MODE_DAX) {
+	} else if (p->mode == NDCTL_NS_MODE_DEVDAX) {
 		dax = ndctl_region_get_dax_seed(region);
 		if (!dax) {
 			error("Kernel does not support devdax mode\n");
@@ -628,7 +628,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		int i, alignments;
 
 		switch (p->mode) {
-		case NDCTL_NS_MODE_MEMORY:
+		case NDCTL_NS_MODE_FSDAX:
 			if (!pfn) {
 				error("Kernel does not support setting an alignment in fsdax mode\n");
 				return -EINVAL;
@@ -637,13 +637,13 @@ static int validate_namespace_options(struct ndctl_region *region,
 			alignments = ndctl_pfn_get_num_alignments(pfn);
 			break;
 
-		case NDCTL_NS_MODE_DAX:
+		case NDCTL_NS_MODE_DEVDAX:
 			alignments = ndctl_dax_get_num_alignments(dax);
 			break;
 
 		default:
 			error("%s mode does not support setting an alignment\n",
-					p->mode == NDCTL_NS_MODE_SAFE
+					p->mode == NDCTL_NS_MODE_SECTOR
 					? "sector" : "raw");
 			return -ENXIO;
 		}
@@ -652,7 +652,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		for (i = 0; i < alignments; i++) {
 			uint64_t a;
 
-			if (p->mode == NDCTL_NS_MODE_MEMORY)
+			if (p->mode == NDCTL_NS_MODE_FSDAX)
 				a = ndctl_pfn_get_supported_alignment(pfn, i);
 			else
 				a = ndctl_dax_get_supported_alignment(dax, i);
@@ -687,7 +687,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		 * one. If we don't then use PAGE_SIZE so the size_align
 		 * checking works.
 		 */
-		if (p->mode == NDCTL_NS_MODE_MEMORY) {
+		if (p->mode == NDCTL_NS_MODE_FSDAX) {
 			/*
 			 * The initial pfn device support in the kernel didn't
 			 * have the 'align' sysfs attribute and assumed a 2MB
@@ -698,7 +698,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 				p->align = ndctl_pfn_get_align(pfn);
 			else
 				p->align = SZ_2M;
-		} else if (p->mode == NDCTL_NS_MODE_DAX) {
+		} else if (p->mode == NDCTL_NS_MODE_DEVDAX) {
 			/*
 			 * device dax mode was added after the align attribute
 			 * so checking for it is unnecessary.
@@ -767,7 +767,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 
 		p->sector_size = parse_size64(param.sector_size);
 		btt = ndctl_region_get_btt_seed(region);
-		if (p->mode == NDCTL_NS_MODE_SAFE) {
+		if (p->mode == NDCTL_NS_MODE_SECTOR) {
 			if (!btt) {
 				err("%s: does not support 'sector' mode\n",
 						region_name);
@@ -807,7 +807,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		 * sector size, otherwise fall back to what the
 		 * namespace supports.
 		 */
-		if (btt && p->mode == NDCTL_NS_MODE_SAFE)
+		if (btt && p->mode == NDCTL_NS_MODE_SECTOR)
 			p->sector_size = ndctl_btt_get_sector_size(btt);
 		else
 			p->sector_size = ndctl_namespace_get_sector_size(ndns);
@@ -834,14 +834,14 @@ static int validate_namespace_options(struct ndctl_region *region,
 		else
 			p->loc = NDCTL_PFN_LOC_PMEM;
 
-		if (ndns && p->mode != NDCTL_NS_MODE_MEMORY
-			&& p->mode != NDCTL_NS_MODE_DAX) {
+		if (ndns && p->mode != NDCTL_NS_MODE_FSDAX
+			&& p->mode != NDCTL_NS_MODE_DEVDAX) {
 			err("%s: --map= only valid for fsdax mode namespace\n",
 				ndctl_namespace_get_devname(ndns));
 			return -EINVAL;
 		}
-	} else if (p->mode == NDCTL_NS_MODE_MEMORY
-			|| p->mode == NDCTL_NS_MODE_DAX)
+	} else if (p->mode == NDCTL_NS_MODE_FSDAX
+			|| p->mode == NDCTL_NS_MODE_DEVDAX)
 		p->loc = NDCTL_PFN_LOC_PMEM;
 
 	if (!pfn && do_setup_pfn(ndns, p)) {
diff --git a/test/libndctl.c b/test/libndctl.c
index 9ad8f87b92dc..994e0fadf4f7 100644
--- a/test/libndctl.c
+++ b/test/libndctl.c
@@ -1080,7 +1080,7 @@ static int check_btt_create(struct ndctl_region *region, struct ndctl_namespace
 		devname = ndctl_btt_get_devname(btt);
 		ndctl_btt_set_uuid(btt, btt_s->uuid);
 		ndctl_btt_set_sector_size(btt, btt_s->sector_sizes[i]);
-		rc = ndctl_namespace_set_enforce_mode(ndns, NDCTL_NS_MODE_SAFE);
+		rc = ndctl_namespace_set_enforce_mode(ndns, NDCTL_NS_MODE_SECTOR);
 		if (ndctl_test_attempt(test, KERNEL_VERSION(4, 13, 0)) && rc < 0) {
 			fprintf(stderr, "%s: failed to enforce btt mode\n", devname);
 			goto err;
@@ -1100,7 +1100,7 @@ static int check_btt_create(struct ndctl_region *region, struct ndctl_namespace
 		/* prior to v4.5 the mode attribute did not exist */
 		if (ndctl_test_attempt(test, KERNEL_VERSION(4, 5, 0))) {
 			mode = ndctl_namespace_get_mode(ndns);
-			if (mode >= 0 && mode != NDCTL_NS_MODE_SAFE)
+			if (mode >= 0 && mode != NDCTL_NS_MODE_SECTOR)
 				fprintf(stderr, "%s: expected safe mode got: %d\n",
 						devname, mode);
 		}
@@ -1474,7 +1474,7 @@ static int check_btt_autodetect(struct ndctl_bus *bus,
 
 	mode = ndctl_namespace_get_enforce_mode(ndns);
 	if (ndctl_test_attempt(test, KERNEL_VERSION(4, 13, 0))
-			&& mode != NDCTL_NS_MODE_SAFE) {
+			&& mode != NDCTL_NS_MODE_SECTOR) {
 		fprintf(stderr, "%s expected enforce_mode btt\n", devname);
 		return -ENXIO;
 	}
diff --git a/util/filter.c b/util/filter.c
index 877d6c74ff18..9f6b65a70117 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -361,9 +361,9 @@ static enum ndctl_namespace_mode mode_to_type(const char *mode)
 	else if (strcasecmp(mode, "fsdax") == 0)
 		return NDCTL_NS_MODE_MEMORY;
 	else if (strcasecmp(mode, "sector") == 0)
-		return NDCTL_NS_MODE_SAFE;
+		return NDCTL_NS_MODE_SECTOR;
 	else if (strcasecmp(mode, "safe") == 0)
-		return NDCTL_NS_MODE_SAFE;
+		return NDCTL_NS_MODE_SECTOR;
 	else if (strcasecmp(mode, "dax") == 0)
 		return NDCTL_NS_MODE_DAX;
 	else if (strcasecmp(mode, "devdax") == 0)
diff --git a/util/json.c b/util/json.c
index 6745bcc19058..50346c5bcbab 100644
--- a/util/json.c
+++ b/util/json.c
@@ -944,7 +944,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 		jobj = json_object_new_string("devdax");
 		loc = ndctl_dax_get_location(dax);
 		break;
-	case NDCTL_NS_MODE_SAFE:
+	case NDCTL_NS_MODE_SECTOR:
 		if (!btt)
 			goto err;
 		jobj = json_object_new_string("sector");
@@ -960,7 +960,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 	if (jobj)
 		json_object_object_add(jndns, "mode", jobj);
 
-	if ((mode != NDCTL_NS_MODE_SAFE) && (mode != NDCTL_NS_MODE_RAW)) {
+	if ((mode != NDCTL_NS_MODE_SECTOR) && (mode != NDCTL_NS_MODE_RAW)) {
 		jobj = json_object_new_string(locations[loc]);
 		if (jobj)
 			json_object_object_add(jndns, "map", jobj);
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 20/36] ndctl/namespace: Introduce mode-to-name and name-to-mode helpers
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (18 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 19/36] ndctl/namespace: Kill off the legacy mode names Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:21 ` [ndctl PATCH 21/36] ndctl/namespace: Validate namespace size within validate_namespace_options() Dan Williams
                   ` (16 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Before open coding yet another translation between modes and their text
name, introduce a util_nsmode() and util_nsmode_name() helpers.

Consolidate the existing mode_to_type() helper into the new common /
public util.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   39 +++++++--------------------------------
 util/filter.c     |   42 +++++++++++++++++++++++++++---------------
 util/filter.h     |    3 +++
 3 files changed, 37 insertions(+), 47 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 770caa5ebab0..1747a061d5b7 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -214,21 +214,7 @@ static int set_defaults(enum device_action mode)
 		param.type = "pmem";
 
 	if (param.mode) {
-		if (strcmp(param.mode, "safe") == 0)
-			/* pass */;
-		else if (strcmp(param.mode, "sector") == 0)
-		      param.mode = "safe"; /* pass */
-		else if (strcmp(param.mode, "memory") == 0)
-		      /* pass */;
-		else if (strcmp(param.mode, "fsdax") == 0)
-			param.mode = "memory"; /* pass */
-		else if (strcmp(param.mode, "raw") == 0)
-		      /* pass */;
-		else if (strcmp(param.mode, "dax") == 0)
-		      /* pass */;
-		else if (strcmp(param.mode, "devdax") == 0)
-			param.mode = "dax"; /* pass */
-		else {
+		if (util_nsmode(param.mode) == NDCTL_NS_MODE_UNKNOWN) {
 			error("invalid mode '%s'\n", param.mode);
 			rc = -EINVAL;
 		}
@@ -294,7 +280,7 @@ static int set_defaults(enum device_action mode)
 			rc = -EINVAL;
 		}
 	} else if (((param.type && strcmp(param.type, "blk") == 0)
-			|| (param.mode && strcmp(param.mode, "safe") == 0))) {
+			|| util_nsmode(param.mode) == NDCTL_NS_MODE_SECTOR)) {
 		/* default sector size for blk-type or safe-mode */
 		param.sector_size = "4096";
 	}
@@ -585,23 +571,12 @@ static int validate_namespace_options(struct ndctl_region *region,
 	}
 
 	if (param.mode) {
-		if (strcmp(param.mode, "memory") == 0)
-			p->mode = NDCTL_NS_MODE_FSDAX;
-		else if (strcmp(param.mode, "sector") == 0)
-			p->mode = NDCTL_NS_MODE_SECTOR;
-		else if (strcmp(param.mode, "safe") == 0)
-			p->mode = NDCTL_NS_MODE_SECTOR;
-		else if (strcmp(param.mode, "dax") == 0)
-			p->mode = NDCTL_NS_MODE_DEVDAX;
-		else
-			p->mode = NDCTL_NS_MODE_RAW;
-
+		p->mode = util_nsmode(param.mode);
 		if (ndctl_region_get_type(region) != ND_DEVICE_REGION_PMEM
 				&& (p->mode == NDCTL_NS_MODE_FSDAX
 					|| p->mode == NDCTL_NS_MODE_DEVDAX)) {
 			err("blk %s does not support %s mode\n", region_name,
-					p->mode == NDCTL_NS_MODE_FSDAX
-					? "fsdax" : "devdax");
+					util_nsmode_name(p->mode));
 			return -EAGAIN;
 		}
 	} else if (ndns)
@@ -619,7 +594,8 @@ static int validate_namespace_options(struct ndctl_region *region,
 	} else if (p->mode == NDCTL_NS_MODE_DEVDAX) {
 		dax = ndctl_region_get_dax_seed(region);
 		if (!dax) {
-			error("Kernel does not support devdax mode\n");
+			error("Kernel does not support %s mode\n",
+					util_nsmode_name(p->mode));
 			return -ENODEV;
 		}
 	}
@@ -643,8 +619,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 
 		default:
 			error("%s mode does not support setting an alignment\n",
-					p->mode == NDCTL_NS_MODE_SECTOR
-					? "sector" : "raw");
+					util_nsmode_name(p->mode));
 			return -ENXIO;
 		}
 
diff --git a/util/filter.c b/util/filter.c
index 9f6b65a70117..af72793929e2 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -351,29 +351,41 @@ struct daxctl_region *util_daxctl_region_filter(struct daxctl_region *region,
 	return NULL;
 }
 
-static enum ndctl_namespace_mode mode_to_type(const char *mode)
+enum ndctl_namespace_mode util_nsmode(const char *mode)
 {
 	if (!mode)
-		return -ENXIO;
-
+		return NDCTL_NS_MODE_UNKNOWN;
 	if (strcasecmp(mode, "memory") == 0)
-		return NDCTL_NS_MODE_MEMORY;
-	else if (strcasecmp(mode, "fsdax") == 0)
-		return NDCTL_NS_MODE_MEMORY;
-	else if (strcasecmp(mode, "sector") == 0)
+		return NDCTL_NS_MODE_FSDAX;
+	if (strcasecmp(mode, "fsdax") == 0)
+		return NDCTL_NS_MODE_FSDAX;
+	if (strcasecmp(mode, "sector") == 0)
 		return NDCTL_NS_MODE_SECTOR;
-	else if (strcasecmp(mode, "safe") == 0)
+	if (strcasecmp(mode, "safe") == 0)
 		return NDCTL_NS_MODE_SECTOR;
-	else if (strcasecmp(mode, "dax") == 0)
-		return NDCTL_NS_MODE_DAX;
-	else if (strcasecmp(mode, "devdax") == 0)
-		return NDCTL_NS_MODE_DAX;
-	else if (strcasecmp(mode, "raw") == 0)
+	if (strcasecmp(mode, "dax") == 0)
+		return NDCTL_NS_MODE_DEVDAX;
+	if (strcasecmp(mode, "devdax") == 0)
+		return NDCTL_NS_MODE_DEVDAX;
+	if (strcasecmp(mode, "raw") == 0)
 		return NDCTL_NS_MODE_RAW;
 
 	return NDCTL_NS_MODE_UNKNOWN;
 }
 
+const char *util_nsmode_name(enum ndctl_namespace_mode mode)
+{
+	static const char * const modes[] = {
+		[NDCTL_NS_MODE_FSDAX] = "fsdax",
+		[NDCTL_NS_MODE_DEVDAX] = "devdax",
+		[NDCTL_NS_MODE_RAW] = "raw",
+		[NDCTL_NS_MODE_SECTOR] = "sector",
+		[NDCTL_NS_MODE_UNKNOWN] = "unknown",
+	};
+
+	return modes[mode];
+}
+
 int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 		struct util_filter_params *param)
 {
@@ -396,7 +408,7 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 			type = ND_DEVICE_REGION_BLK;
 	}
 
-	if (mode_to_type(param->mode) == NDCTL_NS_MODE_UNKNOWN) {
+	if (param->mode && util_nsmode(param->mode) == NDCTL_NS_MODE_UNKNOWN) {
 		error("invalid mode: '%s'\n", param->mode);
 		return -EINVAL;
 	}
@@ -474,7 +486,7 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
 					continue;
 
 				mode = ndctl_namespace_get_mode(ndns);
-				if (param->mode && mode_to_type(param->mode) != mode)
+				if (param->mode && util_nsmode(param->mode) != mode)
 					continue;
 
 				fctx->filter_namespace(ndns, fctx);
diff --git a/util/filter.h b/util/filter.h
index 0c12b94218d6..ad3441cc3a16 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -40,6 +40,9 @@ struct daxctl_dev *util_daxctl_dev_filter(struct daxctl_dev *dev,
 struct daxctl_region *util_daxctl_region_filter(struct daxctl_region *region,
 		const char *ident);
 
+enum ndctl_namespace_mode util_nsmode(const char *mode);
+const char *util_nsmode_name(enum ndctl_namespace_mode mode);
+
 struct json_object;
 
 /* json object hierarchy for the util_filter_walk() performed by cmd_list() */
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 21/36] ndctl/namespace: Validate namespace size within validate_namespace_options()
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (19 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 20/36] ndctl/namespace: Introduce mode-to-name and name-to-mode helpers Dan Williams
@ 2020-02-29 20:21 ` Dan Williams
  2020-02-29 20:22 ` [ndctl PATCH 22/36] ndctl/namespace: Clarify 16M minimum size requirement Dan Williams
                   ` (15 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:21 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Currently validate_namespace_options() handles default option conversion
for every namespace attribute except size. Move default size validation
internal to that helper in advance of teaching ndctl to require
namespace be at least 16M in size to host a metadata personality /
address abstraction.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   50 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 17 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 1747a061d5b7..6786bbb2e096 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -514,6 +514,29 @@ static int setup_namespace(struct ndctl_region *region,
 	return rc;
 }
 
+static int validate_available_capacity(struct ndctl_region *region,
+		struct parsed_parameters *p)
+{
+	unsigned long long available;
+
+	if (ndctl_region_get_nstype(region) == ND_DEVICE_NAMESPACE_IO)
+		available = ndctl_region_get_size(region);
+	else {
+		available = ndctl_region_get_max_available_extent(region);
+		if (available == ULLONG_MAX)
+			available = ndctl_region_get_available_size(region);
+	}
+	if (!available || p->size > available) {
+		debug("%s: insufficient capacity size: %llx avail: %llx\n",
+			ndctl_region_get_devname(region), p->size, available);
+		return -EAGAIN;
+	}
+
+	if (p->size == 0)
+		p->size = available;
+	return 0;
+}
+
 /*
  * validate_namespace_options - init parameters for setup_namespace
  * @region: parent of the namespace to create / reconfigure
@@ -552,6 +575,16 @@ static int validate_namespace_options(struct ndctl_region *region,
 	else if (ndns)
 		p->size = ndctl_namespace_get_size(ndns);
 
+	/*
+	 * Validate available capacity in the create case, in the
+	 * reconfigure case the capacity is already allocated.
+	 */
+	if (!ndns) {
+		rc = validate_available_capacity(region, p);
+		if (rc)
+			return rc;
+	}
+
 	if (param.uuid) {
 		if (uuid_parse(param.uuid, p->uuid) != 0) {
 			err("%s: invalid uuid\n", __func__);
@@ -847,7 +880,6 @@ static struct ndctl_namespace *region_get_namespace(struct ndctl_region *region)
 static int namespace_create(struct ndctl_region *region)
 {
 	const char *devname = ndctl_region_get_devname(region);
-	unsigned long long available;
 	struct ndctl_namespace *ndns;
 	struct parsed_parameters p;
 	int rc;
@@ -862,22 +894,6 @@ static int namespace_create(struct ndctl_region *region)
 		return -EAGAIN;
 	}
 
-	if (ndctl_region_get_nstype(region) == ND_DEVICE_NAMESPACE_IO)
-		available = ndctl_region_get_size(region);
-	else {
-		available = ndctl_region_get_max_available_extent(region);
-		if (available == ULLONG_MAX)
-			available = ndctl_region_get_available_size(region);
-	}
-	if (!available || p.size > available) {
-		debug("%s: insufficient capacity size: %llx avail: %llx\n",
-			devname, p.size, available);
-		return -EAGAIN;
-	}
-
-	if (p.size == 0)
-		p.size = available;
-
 	ndns = region_get_namespace(region);
 	if (!ndns || !ndctl_namespace_is_configuration_idle(ndns)) {
 		debug("%s: no %s namespace seed\n", devname,
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 22/36] ndctl/namespace: Clarify 16M minimum size requirement
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (20 preceding siblings ...)
  2020-02-29 20:21 ` [ndctl PATCH 21/36] ndctl/namespace: Validate namespace size within validate_namespace_options() Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-02-29 20:22 ` [ndctl PATCH 23/36] ndctl/test: Regression test 'failed to track' Dan Williams
                   ` (14 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Jane Chu, vishal.l.verma

The kernel enforces a minimum size for any "claimed" namespace i.e. any
namespace that is wrapped in an address abstraction like the btt or
devdax. The "no such device or address" default print is confusing, so
replace with an explicit error message.

Reported-by: Jane Chu <jane.chu@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 6786bbb2e096..397bd4acd1d1 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -560,6 +560,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 	struct ndctl_pfn *pfn = NULL;
 	struct ndctl_dax *dax = NULL;
 	unsigned long region_align;
+	bool default_size = false;
 	unsigned int ways;
 	int rc = 0;
 
@@ -574,10 +575,13 @@ static int validate_namespace_options(struct ndctl_region *region,
 		p->size = __parse_size64(param.size, &units);
 	else if (ndns)
 		p->size = ndctl_namespace_get_size(ndns);
+	else
+		default_size = true;
 
 	/*
 	 * Validate available capacity in the create case, in the
-	 * reconfigure case the capacity is already allocated.
+	 * reconfigure case the capacity is already allocated. A default
+	 * size will be established from available capacity.
 	 */
 	if (!ndns) {
 		rc = validate_available_capacity(region, p);
@@ -769,6 +773,21 @@ static int validate_namespace_options(struct ndctl_region *region,
 		return -EINVAL;
 	}
 
+	/*
+	 * Catch attempts to create sub-16M namespaces to match the
+	 * kernel's restriction (see nd_namespace_store())
+	 */
+	if (p->size < SZ_16M && p->mode != NDCTL_NS_MODE_RAW) {
+		if (default_size) {
+			debug("%s: insufficient capacity for mode: %s\n",
+					region_name, util_nsmode_name(p->mode));
+			return -EAGAIN;
+		}
+		error("'--size=' must be >= 16MiB for '%s' mode\n",
+				util_nsmode_name(p->mode));
+		return -EINVAL;
+	}
+
 	if (param.sector_size) {
 		struct ndctl_btt *btt;
 		int num, i;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 23/36] ndctl/test: Regression test 'failed to track'
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (21 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 22/36] ndctl/namespace: Clarify 16M minimum size requirement Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-02-29 20:22 ` [ndctl PATCH 24/36] ndctl/dimm: Rework dimm command status reporting Dan Williams
                   ` (13 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Exercise the failing condition behind kernel commit c4703ce11c23
"libnvdimm/namespace: Fix label tracking error", i.e. rename (change
uuid) allocated namespace capacity.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/Makefile.am   |    3 ++-
 test/track-uuid.sh |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100755 test/track-uuid.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index bbe58a06d9d0..cce60c5221fd 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -25,7 +25,8 @@ TESTS =\
 	inject-smart.sh \
 	monitor.sh \
 	max_available_extent_ns.sh \
-	pfn-meta-errors.sh
+	pfn-meta-errors.sh \
+	track-uuid.sh
 
 EXTRA_DIST += $(TESTS) common \
 		btt-pad-compat.xxd \
diff --git a/test/track-uuid.sh b/test/track-uuid.sh
new file mode 100755
index 000000000000..ab77e52708b6
--- /dev/null
+++ b/test/track-uuid.sh
@@ -0,0 +1,40 @@
+#!/bin/bash -Ex
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2018 Intel Corporation. All rights reserved.
+
+blockdev=""
+rc=77
+
+. ./common
+
+set -e
+trap 'err $LINENO' ERR
+
+# setup (reset nfit_test dimms)
+modprobe nfit_test
+$NDCTL disable-region -b $NFIT_TEST_BUS0 all
+$NDCTL zero-labels -b $NFIT_TEST_BUS0 all
+$NDCTL enable-region -b $NFIT_TEST_BUS0 all
+
+rc=1
+
+# create a fsdax namespace and clear errors (if any)
+dev="x"
+json=$($NDCTL create-namespace -b $NFIT_TEST_BUS0 -t pmem -m raw)
+eval "$(echo "$json" | json2var)"
+[ $dev = "x" ] && echo "fail: $LINENO" && exit 1
+
+$NDCTL disable-namespace $dev
+# On broken kernels this reassignment of capacity triggers a warning
+# with the following signature, and results in ENXIO.
+#     WARNING: CPU: 11 PID: 1378 at drivers/nvdimm/label.c:721 __pmem_label_update+0x55d/0x570 [libnvdimm]
+#     Call Trace:
+#      nd_pmem_namespace_label_update+0xd6/0x160 [libnvdimm]
+#      uuid_store+0x15c/0x170 [libnvdimm]
+#      kernfs_fop_write+0xf0/0x1a0
+#      __vfs_write+0x26/0x150
+uuidgen > /sys/bus/nd/devices/$dev/uuid
+$NDCTL enable-namespace $dev
+
+_cleanup
+exit 0
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 24/36] ndctl/dimm: Rework dimm command status reporting
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (22 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 23/36] ndctl/test: Regression test 'failed to track' Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-02-29 20:22 ` [ndctl PATCH 25/36] ndctl/dimm: Rework iteration to drop unaligned pointers Dan Williams
                   ` (12 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

The build currently spews many errors of the form:

hyperv.c: In function ‘alloc_hyperv_cmd’:
hyperv.c:61:25: warning: taking address of packed member of ‘struct nd_hyperv_health_info’ may result in an unaligned pointer value [-Waddress-of-packed-member]
   61 |  cmd->firmware_status = &hyperv->u.health_info.status;

Move the status reporting from passing an unaligned pointer to a new
->get_firmware_status() operation.

Link: https://github.com/pmem/ndctl/issues/131
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/ars.c       |   28 +++++++++++++++++++------
 ndctl/lib/hpe1.c      |   17 +++++++++++----
 ndctl/lib/hyperv.c    |    7 +++++-
 ndctl/lib/intel.c     |   56 +++++++++++++++++++++++++++++++++++--------------
 ndctl/lib/libndctl.c  |   39 ++++++++++++++++++++++++++--------
 ndctl/lib/msft.c      |    8 +++++--
 ndctl/lib/nfit.c      |   36 +++++++++++++++++++++-----------
 ndctl/lib/private.h   |    7 ++++--
 ndctl/libndctl-nfit.h |   11 ++++++++++
 9 files changed, 157 insertions(+), 52 deletions(-)

diff --git a/ndctl/lib/ars.c b/ndctl/lib/ars.c
index d91a99d00d10..44871b2afde2 100644
--- a/ndctl/lib/ars.c
+++ b/ndctl/lib/ars.c
@@ -15,6 +15,22 @@
 #include <ndctl/libndctl.h>
 #include "private.h"
 
+static u32 get_ars_command_status(struct ndctl_cmd *cmd)
+{
+	switch (cmd->type) {
+	case ND_CMD_ARS_CAP:
+		return cmd->ars_cap->status;
+	case ND_CMD_ARS_START:
+		return cmd->ars_start->status;
+	case ND_CMD_ARS_STATUS:
+		return cmd->ars_status->status;
+	case ND_CMD_CLEAR_ERROR:
+		return cmd->clear_err->status;
+	}
+
+	return -1U;
+}
+
 NDCTL_EXPORT struct ndctl_cmd *ndctl_bus_cmd_new_ars_cap(struct ndctl_bus *bus,
 		unsigned long long address, unsigned long long len)
 {
@@ -35,9 +51,9 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_bus_cmd_new_ars_cap(struct ndctl_bus *bus,
 	cmd->bus = bus;
 	ndctl_cmd_ref(cmd);
 	cmd->type = ND_CMD_ARS_CAP;
+	cmd->get_firmware_status = get_ars_command_status;
 	cmd->size = size;
 	cmd->status = 1;
-	cmd->firmware_status = &cmd->ars_cap->status;
 	cmd->ars_cap->address = address;
 	cmd->ars_cap->length = len;
 
@@ -55,7 +71,7 @@ static bool __validate_ars_cap(struct ndctl_cmd *ars_cap)
 {
 	if (ars_cap->type != ND_CMD_ARS_CAP || ars_cap->status != 0)
 		return false;
-	if ((*ars_cap->firmware_status & ARS_STATUS_MASK) != 0)
+	if ((ars_cap->get_firmware_status(ars_cap) & ARS_STATUS_MASK) != 0)
 		return false;
 	return validate_clear_error(ars_cap);
 }
@@ -84,7 +100,7 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_bus_cmd_new_ars_start(struct ndctl_cmd *ars
 	if (!validate_ars_cap(ctx, ars_cap))
 		return NULL;
 
-	if (!(*ars_cap->firmware_status >> ARS_EXT_STATUS_SHIFT & type)) {
+	if (!(ars_cap->get_firmware_status(ars_cap) >> ARS_EXT_STATUS_SHIFT & type)) {
 		dbg(ctx, "ars_cap does not show requested type as supported\n");
 		return NULL;
 	}
@@ -97,9 +113,9 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_bus_cmd_new_ars_start(struct ndctl_cmd *ars
 	cmd->bus = bus;
 	ndctl_cmd_ref(cmd);
 	cmd->type = ND_CMD_ARS_START;
+	cmd->get_firmware_status = get_ars_command_status;
 	cmd->size = size;
 	cmd->status = 1;
-	cmd->firmware_status = &cmd->ars_start->status;
 	cmd->ars_start->address = ars_cap->ars_cap->address;
 	cmd->ars_start->length = ars_cap->ars_cap->length;
 	cmd->ars_start->type = type;
@@ -145,9 +161,9 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_bus_cmd_new_ars_status(struct ndctl_cmd *ar
 	cmd->bus = bus;
 	ndctl_cmd_ref(cmd);
 	cmd->type = ND_CMD_ARS_STATUS;
+	cmd->get_firmware_status = get_ars_command_status;
 	cmd->size = size;
 	cmd->status = 1;
-	cmd->firmware_status = &cmd->ars_status->status;
 	cmd->ars_status->out_length = ars_cap_cmd->max_ars_out;
 
 	return cmd;
@@ -325,9 +341,9 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_bus_cmd_new_clear_error(
 	ndctl_cmd_ref(clear_err);
 	clear_err->bus = bus;
 	clear_err->type = ND_CMD_CLEAR_ERROR;
+	clear_err->get_firmware_status = get_ars_command_status;
 	clear_err->size = size;
 	clear_err->status = 1;
-	clear_err->firmware_status = &clear_err->clear_err->status;
 	clear_err->clear_err->address = address;
 	clear_err->clear_err->length = len;
 
diff --git a/ndctl/lib/hpe1.c b/ndctl/lib/hpe1.c
index b26120e1d3e0..b5ee02608d31 100644
--- a/ndctl/lib/hpe1.c
+++ b/ndctl/lib/hpe1.c
@@ -23,6 +23,17 @@
 #define CMD_HPE1_SMART(_c) (CMD_HPE1(_c)->u.smart.data)
 #define CMD_HPE1_SMART_THRESH(_c) (CMD_HPE1(_c)->u.thresh.data)
 
+static u32 hpe1_get_firmware_status(struct ndctl_cmd *cmd)
+{
+	switch (cmd->hpe1->gen.nd_command) {
+	case NDN_HPE1_CMD_SMART:
+		return cmd->hpe1->u.smart.status;
+	case NDN_HPE1_CMD_SMART_THRESHOLD:
+		return cmd->hpe1->u.thresh.status;
+	}
+	return -1U;
+}
+
 static struct ndctl_cmd *hpe1_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
 {
 	struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
@@ -60,6 +71,7 @@ static struct ndctl_cmd *hpe1_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
 	hpe1->gen.nd_size_in = offsetof(struct ndn_hpe1_smart, status);
 	hpe1->gen.nd_size_out = sizeof(hpe1->u.smart);
 	hpe1->u.smart.status = 3;
+	cmd->get_firmware_status = hpe1_get_firmware_status;
 
 	hpe1->u.smart.in_valid_flags = 0;
 	hpe1->u.smart.in_valid_flags |= NDN_HPE1_SMART_HEALTH_VALID;
@@ -70,8 +82,6 @@ static struct ndctl_cmd *hpe1_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
 	hpe1->u.smart.in_valid_flags |= NDN_HPE1_SMART_SHUTDOWN_VALID;
 	hpe1->u.smart.in_valid_flags |= NDN_HPE1_SMART_VENDOR_VALID;
 
-	cmd->firmware_status = &hpe1->u.smart.status;
-
 	return cmd;
 }
 
@@ -285,8 +295,7 @@ static struct ndctl_cmd *hpe1_dimm_cmd_new_smart_threshold(struct ndctl_dimm *di
 	hpe1->gen.nd_size_in = offsetof(struct ndn_hpe1_smart_threshold, status);
 	hpe1->gen.nd_size_out = sizeof(hpe1->u.smart);
 	hpe1->u.thresh.status = 3;
-
-	cmd->firmware_status = &hpe1->u.thresh.status;
+	cmd->get_firmware_status = hpe1_get_firmware_status;
 
 	return cmd;
 }
diff --git a/ndctl/lib/hyperv.c b/ndctl/lib/hyperv.c
index 9b4fe122af2c..ba1b12111804 100644
--- a/ndctl/lib/hyperv.c
+++ b/ndctl/lib/hyperv.c
@@ -9,6 +9,11 @@
 #include "private.h"
 #include "hyperv.h"
 
+static u32 hyperv_get_firmware_status(struct ndctl_cmd *cmd)
+{
+	return cmd->hyperv->u.status;
+}
+
 static bool hyperv_cmd_is_supported(struct ndctl_dimm *dimm, int cmd)
 {
 	/*
@@ -50,6 +55,7 @@ static struct ndctl_cmd *alloc_hyperv_cmd(struct ndctl_dimm *dimm,
 
 	cmd->dimm = dimm;
 	cmd->type = ND_CMD_CALL;
+	cmd->get_firmware_status = hyperv_get_firmware_status;
 	cmd->size = size;
 	cmd->status = 1;
 
@@ -58,7 +64,6 @@ static struct ndctl_cmd *alloc_hyperv_cmd(struct ndctl_dimm *dimm,
 	hyperv->gen.nd_command = command;
 	hyperv->gen.nd_size_out = sizeof(hyperv->u.health_info);
 
-	cmd->firmware_status = &hyperv->u.health_info.status;
 	return cmd;
 }
 
diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
index d684bac03fec..ebcefd8b5ad2 100644
--- a/ndctl/lib/intel.c
+++ b/ndctl/lib/intel.c
@@ -16,13 +16,49 @@
 #include <ndctl/libndctl.h>
 #include "private.h"
 
+static unsigned int intel_cmd_get_firmware_status(struct ndctl_cmd *cmd)
+{
+	struct nd_pkg_intel *intel = cmd->intel;
+
+	switch (intel->gen.nd_command) {
+	case ND_INTEL_SMART:
+		return intel->smart.status;
+	case ND_INTEL_SMART_THRESHOLD:
+		return intel->thresh.status;
+	case ND_INTEL_SMART_SET_THRESHOLD:
+		return intel->set_thresh.status;
+	case ND_INTEL_SMART_INJECT:
+		return intel->inject.status;
+	case ND_INTEL_FW_GET_INFO:
+		return intel->info.status;
+	case ND_INTEL_FW_START_UPDATE:
+		return intel->start.status;
+	case ND_INTEL_FW_SEND_DATA: {
+		    struct nd_intel_fw_send_data *send = &intel->send;
+		    u32 status;
+
+		    /* the last dword after the payload is reserved for status */
+		    memcpy(&status, ((void *) send) + sizeof(*send) + send->length,
+				    sizeof(status));
+		    return status;
+	}
+	case ND_INTEL_FW_FINISH_UPDATE:
+		return intel->finish.status;
+	case ND_INTEL_FW_FINISH_STATUS_QUERY:
+		return intel->fquery.status;
+	case ND_INTEL_ENABLE_LSS_STATUS:
+		return intel->lss.status;
+	}
+	return -1U;
+}
+
 static int intel_cmd_xlat_firmware_status(struct ndctl_cmd *cmd)
 {
 	struct nd_pkg_intel *pkg = cmd->intel;
 	unsigned int status, ext_status;
 
-	status = (*cmd->firmware_status) & ND_INTEL_STATUS_MASK;
-	ext_status = (*cmd->firmware_status) & ND_INTEL_STATUS_EXTEND_MASK;
+	status = cmd->get_firmware_status(cmd) & ND_INTEL_STATUS_MASK;
+	ext_status = cmd->get_firmware_status(cmd) & ND_INTEL_STATUS_EXTEND_MASK;
 
 	/* Common statuses */
 	switch (status) {
@@ -91,6 +127,7 @@ static struct ndctl_cmd *alloc_intel_cmd(struct ndctl_dimm *dimm,
 	cmd->type = ND_CMD_CALL;
 	cmd->size = size;
 	cmd->status = 1;
+	cmd->get_firmware_status = intel_cmd_get_firmware_status;
 
 	*(cmd->intel) = (struct nd_pkg_intel) {
 		.gen = {
@@ -114,7 +151,6 @@ static struct ndctl_cmd *intel_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
 			0, sizeof(cmd->intel->smart));
 	if (!cmd)
 		return NULL;
-	cmd->firmware_status = &cmd->intel->smart.status;
 
 	return cmd;
 }
@@ -269,7 +305,6 @@ static struct ndctl_cmd *intel_dimm_cmd_new_smart_threshold(
 			0, sizeof(cmd->intel->thresh));
 	if (!cmd)
 		return NULL;
-	cmd->firmware_status = &cmd->intel->thresh.status;
 
 	return cmd;
 }
@@ -299,7 +334,6 @@ static struct ndctl_cmd *intel_dimm_cmd_new_smart_set_threshold(
 	set_thresh->spares = thresh->spares;
 	set_thresh->media_temperature = thresh->media_temperature;
 	set_thresh->ctrl_temperature = thresh->ctrl_temperature;
-	cmd->firmware_status = &set_thresh->status;
 
 	return cmd;
 }
@@ -360,7 +394,6 @@ static struct ndctl_cmd *intel_new_smart_inject(struct ndctl_dimm *dimm)
 			offsetof(struct nd_intel_smart_inject, status), 4);
 	if (!cmd)
 		return NULL;
-	cmd->firmware_status = &cmd->intel->inject.status;
 
 	return cmd;
 }
@@ -468,7 +501,6 @@ static struct ndctl_cmd *intel_dimm_cmd_new_fw_get_info(struct ndctl_dimm *dimm)
 	if (!cmd)
 		return NULL;
 
-	cmd->firmware_status = &cmd->intel->info.status;
 	return cmd;
 }
 
@@ -540,7 +572,6 @@ static struct ndctl_cmd *intel_dimm_cmd_new_fw_start(struct ndctl_dimm *dimm)
 	if (!cmd)
 		return NULL;
 
-	cmd->firmware_status = &cmd->intel->start.status;
 	return cmd;
 }
 
@@ -583,9 +614,6 @@ static struct ndctl_cmd *intel_dimm_cmd_new_fw_send(struct ndctl_cmd *start,
 	cmd->intel->send.offset = offset;
 	cmd->intel->send.length = len;
 	memcpy(cmd->intel->send.data, data, len);
-	/* the last dword is reserved for status */
-	cmd->firmware_status =
-		(unsigned int *)(&cmd->intel->send.data[0] + len);
 	return cmd;
 }
 
@@ -602,7 +630,6 @@ static struct ndctl_cmd *intel_dimm_cmd_new_fw_finish(struct ndctl_cmd *start)
 
 	cmd->intel->finish.context = start->intel->start.context;
 	cmd->intel->finish.ctrl_flags = 0;
-	cmd->firmware_status = &cmd->intel->finish.status;
 	return cmd;
 }
 
@@ -619,7 +646,6 @@ static struct ndctl_cmd *intel_dimm_cmd_new_fw_abort(struct ndctl_cmd *start)
 
 	cmd->intel->finish.context = start->intel->start.context;
 	cmd->intel->finish.ctrl_flags = 1;
-	cmd->firmware_status = &cmd->intel->finish.status;
 	return cmd;
 }
 
@@ -636,7 +662,6 @@ intel_dimm_cmd_new_fw_finish_query(struct ndctl_cmd *start)
 		return NULL;
 
 	cmd->intel->fquery.context = start->intel->start.context;
-	cmd->firmware_status = &cmd->intel->fquery.status;
 	return cmd;
 }
 
@@ -704,7 +729,7 @@ intel_cmd_fw_xlat_extend_firmware_status(struct ndctl_cmd *cmd,
 static enum ND_FW_STATUS
 intel_cmd_fw_xlat_firmware_status(struct ndctl_cmd *cmd)
 {
-	unsigned int status = *cmd->firmware_status;
+	unsigned int status = intel_cmd_get_firmware_status(cmd);
 
 	switch (status & ND_INTEL_STATUS_MASK) {
 	case ND_INTEL_STATUS_SUCCESS:
@@ -742,7 +767,6 @@ intel_dimm_cmd_new_lss(struct ndctl_dimm *dimm)
 		return NULL;
 
 	cmd->intel->lss.enable = 1;
-	cmd->firmware_status = &cmd->intel->lss.status;
 	return cmd;
 }
 
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 9ad1b7091dc0..97fd98545440 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2506,6 +2506,23 @@ static struct nd_cmd_vendor_tail *to_vendor_tail(struct ndctl_cmd *cmd)
 	return tail;
 }
 
+static u32 cmd_get_firmware_status(struct ndctl_cmd *cmd)
+{
+	switch (cmd->type) {
+	case ND_CMD_VENDOR:
+		return to_vendor_tail(cmd)->status;
+	case ND_CMD_GET_CONFIG_SIZE:
+		return cmd->get_size->status;
+	case ND_CMD_GET_CONFIG_DATA:
+		return cmd->get_data->status;
+	case ND_CMD_SET_CONFIG_DATA:
+		return *(u32 *) (cmd->cmd_buf
+				+ sizeof(struct nd_cmd_set_config_hdr)
+				+ cmd->iter.max_xfer);
+	}
+	return -1U;
+}
+
 NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_vendor_specific(
 		struct ndctl_dimm *dimm, unsigned int opcode, size_t input_size,
 		size_t output_size)
@@ -2535,7 +2552,7 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_vendor_specific(
 	cmd->status = 1;
 	cmd->vendor->opcode = opcode;
 	cmd->vendor->in_length = input_size;
-	cmd->firmware_status = &to_vendor_tail(cmd)->status;
+	cmd->get_firmware_status = cmd_get_firmware_status;
 	to_vendor_tail(cmd)->out_length = output_size;
 
 	return cmd;
@@ -2600,7 +2617,7 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_size(struct ndctl_dimm *di
 	cmd->type = ND_CMD_GET_CONFIG_SIZE;
 	cmd->size = size;
 	cmd->status = 1;
-	cmd->firmware_status = &cmd->get_size->status;
+	cmd->get_firmware_status = cmd_get_firmware_status;
 
 	return cmd;
 }
@@ -2641,7 +2658,7 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_read(struct ndctl_cmd *cfg
 	cmd->status = 1;
 	cmd->get_data->in_offset = 0;
 	cmd->get_data->in_length = cfg_size->get_size->max_xfer;
-	cmd->firmware_status = &cmd->get_data->status;
+	cmd->get_firmware_status = cmd_get_firmware_status;
 	cmd->iter.init_offset = 0;
 	cmd->iter.offset = &cmd->get_data->in_offset;
 	cmd->iter.xfer = &cmd->get_data->in_length;
@@ -2728,8 +2745,7 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_write(struct ndctl_cmd *cf
 	cmd->status = 1;
 	cmd->set_data->in_offset = cfg_read->iter.init_offset;
 	cmd->set_data->in_length = cfg_read->iter.max_xfer;
-	cmd->firmware_status = (u32 *) (cmd->cmd_buf
-		+ sizeof(struct nd_cmd_set_config_hdr) + cfg_read->iter.max_xfer);
+	cmd->get_firmware_status = cmd_get_firmware_status;
 	cmd->iter.init_offset = cfg_read->iter.init_offset;
 	cmd->iter.offset = &cmd->set_data->in_offset;
 	cmd->iter.xfer = &cmd->set_data->in_length;
@@ -2936,7 +2952,7 @@ static int do_cmd(int fd, int ioctl_cmd, struct ndctl_cmd *cmd)
 		dbg(ctx, "bus: %d dimm: %#x cmd: %s%s%s status: %d fw: %d (%s)\n",
 				bus->id, dimm ? ndctl_dimm_get_handle(dimm) : 0,
 				name, sub_name ? ":" : "", sub_name ? sub_name : "",
-				rc, *(cmd->firmware_status), rc < 0 ?
+				rc, cmd->get_firmware_status(cmd), rc < 0 ?
 				strerror(errno) : "success");
 		if (rc < 0)
 			return -errno;
@@ -2960,7 +2976,7 @@ static int do_cmd(int fd, int ioctl_cmd, struct ndctl_cmd *cmd)
 		if (iter->dir == READ)
 			memcpy(iter->total_buf + offset, iter->data,
 					*(cmd->iter.xfer) - rc);
-		if (*(cmd->firmware_status) || rc) {
+		if (cmd->get_firmware_status(cmd) || rc) {
 			rc = offset + *(cmd->iter.xfer) - rc;
 			break;
 		}
@@ -2970,7 +2986,7 @@ static int do_cmd(int fd, int ioctl_cmd, struct ndctl_cmd *cmd)
 			bus->id, dimm ? ndctl_dimm_get_handle(dimm) : 0,
 			name, sub_name ? ":" : "", sub_name ? sub_name : "",
 			iter->total_xfer, iter->max_xfer, rc,
-			*(cmd->firmware_status),
+			cmd->get_firmware_status(cmd),
 			rc < 0 ? strerror(errno) : "success");
 
 	return rc;
@@ -2986,6 +3002,11 @@ NDCTL_EXPORT int ndctl_cmd_submit(struct ndctl_cmd *cmd)
 	struct ndctl_bus *bus = cmd_to_bus(cmd);
 	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
 
+	if (!cmd->get_firmware_status) {
+		err(ctx, "missing status retrieval\n");
+		return -EINVAL;
+	}
+
 	if (ioctl_cmd == 0) {
 		rc = -EINVAL;
 		goto out;
@@ -3054,7 +3075,7 @@ NDCTL_EXPORT int ndctl_cmd_get_status(struct ndctl_cmd *cmd)
 
 NDCTL_EXPORT unsigned int ndctl_cmd_get_firmware_status(struct ndctl_cmd *cmd)
 {
-	return *(cmd->firmware_status);
+	return cmd->get_firmware_status(cmd);
 }
 
 NDCTL_EXPORT const char *ndctl_region_get_devname(struct ndctl_region *region)
diff --git a/ndctl/lib/msft.c b/ndctl/lib/msft.c
index 19453cd86143..c060b1f2609e 100644
--- a/ndctl/lib/msft.c
+++ b/ndctl/lib/msft.c
@@ -22,6 +22,11 @@
 #define CMD_MSFT(_c) ((_c)->msft)
 #define CMD_MSFT_SMART(_c) (CMD_MSFT(_c)->u.smart.data)
 
+static u32 msft_get_firmware_status(struct ndctl_cmd *cmd)
+{
+	return cmd->msft->u.smart.status;
+}
+
 static struct ndctl_cmd *msft_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
 {
 	struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
@@ -58,8 +63,7 @@ static struct ndctl_cmd *msft_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
 	msft->gen.nd_size_in = offsetof(struct ndn_msft_smart, status);
 	msft->gen.nd_size_out = sizeof(msft->u.smart);
 	msft->u.smart.status = 0;
-
-	cmd->firmware_status = &msft->u.smart.status;
+	cmd->get_firmware_status = msft_get_firmware_status;
 
 	return cmd;
 }
diff --git a/ndctl/lib/nfit.c b/ndctl/lib/nfit.c
index b10edb1943e8..f9fbe73f7446 100644
--- a/ndctl/lib/nfit.c
+++ b/ndctl/lib/nfit.c
@@ -15,6 +15,24 @@
 #include "private.h"
 #include <ndctl/libndctl-nfit.h>
 
+static u32 bus_get_firmware_status(struct ndctl_cmd *cmd)
+{
+	struct nd_cmd_bus *cmd_bus = cmd->cmd_bus;
+
+	switch (cmd_bus->gen.nd_command) {
+	case NFIT_CMD_TRANSLATE_SPA:
+		return cmd_bus->xlat_spa.status;
+	case NFIT_CMD_ARS_INJECT_SET:
+		return cmd_bus->err_inj.status;
+	case NFIT_CMD_ARS_INJECT_CLEAR:
+		return cmd_bus->err_inj_clr.status;
+	case NFIT_CMD_ARS_INJECT_GET:
+		return cmd_bus->err_inj_stat.status;
+	}
+
+	return -1U;
+}
+
 /**
  * ndctl_bus_is_nfit_cmd_supported - ask nfit command is supported on @bus.
  * @bus: ndctl_bus instance
@@ -54,15 +72,15 @@ static struct ndctl_cmd *ndctl_bus_cmd_new_translate_spa(struct ndctl_bus *bus)
 	cmd->bus = bus;
 	ndctl_cmd_ref(cmd);
 	cmd->type = ND_CMD_CALL;
+	cmd->get_firmware_status = bus_get_firmware_status;
 	cmd->size = size;
 	cmd->status = 1;
-	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	pkg = &cmd->cmd_bus->gen;
 	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 = &cmd->cmd_bus->xlat_spa;
 	translate_spa->translate_length = spa_length;
 
 	return cmd;
@@ -146,7 +164,6 @@ int ndctl_bus_nfit_translate_spa(struct ndctl_bus *bus,
 
 struct ndctl_cmd *ndctl_bus_cmd_new_err_inj(struct ndctl_bus *bus)
 {
-	struct nd_cmd_ars_err_inj *err_inj;
 	size_t size, cmd_length;
 	struct nd_cmd_pkg *pkg;
 	struct ndctl_cmd *cmd;
@@ -160,6 +177,7 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj(struct ndctl_bus *bus)
 	cmd->bus = bus;
 	ndctl_cmd_ref(cmd);
 	cmd->type = ND_CMD_CALL;
+	cmd->get_firmware_status = bus_get_firmware_status;
 	cmd->size = size;
 	cmd->status = 1;
 	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
@@ -167,15 +185,12 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj(struct ndctl_bus *bus)
 	pkg->nd_size_in = offsetof(struct nd_cmd_ars_err_inj, status);
 	pkg->nd_size_out = cmd_length - pkg->nd_size_in;
 	pkg->nd_fw_size = pkg->nd_size_out;
-	err_inj = (struct nd_cmd_ars_err_inj *)&pkg->nd_payload[0];
-	cmd->firmware_status = &err_inj->status;
 
 	return cmd;
 }
 
 struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_clr(struct ndctl_bus *bus)
 {
-	struct nd_cmd_ars_err_inj_clr *err_inj_clr;
 	size_t size, cmd_length;
 	struct nd_cmd_pkg *pkg;
 	struct ndctl_cmd *cmd;
@@ -189,6 +204,7 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_clr(struct ndctl_bus *bus)
 	cmd->bus = bus;
 	ndctl_cmd_ref(cmd);
 	cmd->type = ND_CMD_CALL;
+	cmd->get_firmware_status = bus_get_firmware_status;
 	cmd->size = size;
 	cmd->status = 1;
 	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
@@ -196,8 +212,6 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_clr(struct ndctl_bus *bus)
 	pkg->nd_size_in = offsetof(struct nd_cmd_ars_err_inj_clr, status);
 	pkg->nd_size_out = cmd_length - pkg->nd_size_in;
 	pkg->nd_fw_size = pkg->nd_size_out;
-	err_inj_clr = (struct nd_cmd_ars_err_inj_clr *)&pkg->nd_payload[0];
-	cmd->firmware_status = &err_inj_clr->status;
 
 	return cmd;
 }
@@ -205,7 +219,6 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_clr(struct ndctl_bus *bus)
 struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_stat(struct ndctl_bus *bus,
 	u32 buf_size)
 {
-	struct nd_cmd_ars_err_inj_stat *err_inj_stat;
 	size_t size, cmd_length;
 	struct nd_cmd_pkg *pkg;
 	struct ndctl_cmd *cmd;
@@ -220,6 +233,7 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_stat(struct ndctl_bus *bus,
 	cmd->bus = bus;
 	ndctl_cmd_ref(cmd);
 	cmd->type = ND_CMD_CALL;
+	cmd->get_firmware_status = bus_get_firmware_status;
 	cmd->size = size;
 	cmd->status = 1;
 	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
@@ -227,8 +241,6 @@ struct ndctl_cmd *ndctl_bus_cmd_new_err_inj_stat(struct ndctl_bus *bus,
 	pkg->nd_size_in = 0;
 	pkg->nd_size_out = cmd_length + buf_size;
 	pkg->nd_fw_size = pkg->nd_size_out;
-	err_inj_stat = (struct nd_cmd_ars_err_inj_stat *)&pkg->nd_payload[0];
-	cmd->firmware_status = &err_inj_stat->status;
 
 	return cmd;
 }
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 16bf8f953828..3c121bd00437 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -32,6 +32,7 @@
 #include "hpe1.h"
 #include "msft.h"
 #include "hyperv.h"
+#include "libndctl-nfit.h"
 
 struct nvdimm_data {
 	struct ndctl_cmd *cmd_read;
@@ -233,7 +234,7 @@ struct ndctl_namespace {
  * @type: cmd number
  * @size: total size of the ndctl_cmd allocation
  * @status: negative if failed, 0 if success, > 0 if never submitted
- * @firmware_status: NFIT command output status code
+ * @get_firmware_status: per command firmware status field retrieval
  * @iter: iterator for multi-xfer commands
  * @source: source cmd of an inherited iter.total_buf
  *
@@ -250,7 +251,7 @@ struct ndctl_cmd {
 	int type;
 	int size;
 	int status;
-	u32 *firmware_status;
+	u32 (*get_firmware_status)(struct ndctl_cmd *cmd);
 	struct ndctl_cmd_iter {
 		u32 init_offset;
 		u32 *offset;
@@ -268,6 +269,7 @@ struct ndctl_cmd {
 		struct nd_cmd_ars_status ars_status[0];
 		struct nd_cmd_clear_error clear_err[0];
 		struct nd_cmd_pkg pkg[0];
+		struct nd_cmd_bus cmd_bus[0];
 		struct ndn_pkg_hpe1 hpe1[0];
 		struct ndn_pkg_msft msft[0];
 		struct nd_pkg_hyperv hyperv[0];
@@ -341,6 +343,7 @@ struct ndctl_dimm_ops {
 	struct ndctl_cmd *(*new_ack_shutdown_count)(struct ndctl_dimm *);
 	int (*fw_update_supported)(struct ndctl_dimm *);
 	int (*xlat_firmware_status)(struct ndctl_cmd *);
+	u32 (*get_firmware_status)(struct ndctl_cmd *);
 };
 
 extern struct ndctl_dimm_ops * const intel_dimm_ops;
diff --git a/ndctl/libndctl-nfit.h b/ndctl/libndctl-nfit.h
index d5335c23d28b..8c4f72dfa7ec 100644
--- a/ndctl/libndctl-nfit.h
+++ b/ndctl/libndctl-nfit.h
@@ -17,6 +17,7 @@
 #define __LIBNDCTL_NFIT_H__
 
 #include <linux/types.h>
+#include <ndctl/ndctl.h>
 
 /*
  * libndctl-nfit.h : definitions for NFIT related commands/functions.
@@ -87,6 +88,16 @@ struct nd_cmd_ars_err_inj_stat {
 	} __attribute__((packed)) record[0];
 } __attribute__((packed));
 
+struct nd_cmd_bus {
+	struct nd_cmd_pkg gen;
+	union {
+		struct nd_cmd_ars_err_inj_stat err_inj_stat;
+		struct nd_cmd_ars_err_inj_clr err_inj_clr;
+		struct nd_cmd_ars_err_inj err_inj;
+		struct nd_cmd_translate_spa xlat_spa;
+	};
+};
+
 int ndctl_bus_is_nfit_cmd_supported(struct ndctl_bus *bus, int cmd);
 
 #endif /* __LIBNDCTL_NFIT_H__ */
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 25/36] ndctl/dimm: Rework iteration to drop unaligned pointers
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (23 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 24/36] ndctl/dimm: Rework dimm command status reporting Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-02-29 20:22 ` [ndctl PATCH 26/36] ndctl/test: Fix typos / loss of tpm.handle in security test Dan Williams
                   ` (11 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Similar to ->get_firmware_status() introduce ->{get,set}_xfer() and
->{get,set}_offset() so that the iteration core can consult/update the
command data in place.

Link: https://github.com/pmem/ndctl/issues/131
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/libndctl.c |   60 ++++++++++++++++++++++++++++++++++++++++----------
 ndctl/lib/private.h  |    6 +++--
 2 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 97fd98545440..a996bff66fb2 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2523,6 +2523,36 @@ static u32 cmd_get_firmware_status(struct ndctl_cmd *cmd)
 	return -1U;
 }
 
+static void cmd_set_xfer(struct ndctl_cmd *cmd, u32 xfer)
+{
+	if (cmd->type == ND_CMD_GET_CONFIG_DATA)
+		cmd->get_data->in_length = xfer;
+	else
+		cmd->set_data->in_length = xfer;
+}
+
+static u32 cmd_get_xfer(struct ndctl_cmd *cmd)
+{
+	if (cmd->type == ND_CMD_GET_CONFIG_DATA)
+		return cmd->get_data->in_length;
+	return cmd->set_data->in_length;
+}
+
+static void cmd_set_offset(struct ndctl_cmd *cmd, u32 offset)
+{
+	if (cmd->type == ND_CMD_GET_CONFIG_DATA)
+		cmd->get_data->in_offset = offset;
+	else
+		cmd->set_data->in_offset = offset;
+}
+
+static u32 cmd_get_offset(struct ndctl_cmd *cmd)
+{
+	if (cmd->type == ND_CMD_GET_CONFIG_DATA)
+		return cmd->get_data->in_offset;
+	return cmd->set_data->in_offset;
+}
+
 NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_vendor_specific(
 		struct ndctl_dimm *dimm, unsigned int opcode, size_t input_size,
 		size_t output_size)
@@ -2659,9 +2689,11 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_read(struct ndctl_cmd *cfg
 	cmd->get_data->in_offset = 0;
 	cmd->get_data->in_length = cfg_size->get_size->max_xfer;
 	cmd->get_firmware_status = cmd_get_firmware_status;
+	cmd->get_xfer = cmd_get_xfer;
+	cmd->set_xfer = cmd_set_xfer;
+	cmd->get_offset = cmd_get_offset;
+	cmd->set_offset = cmd_set_offset;
 	cmd->iter.init_offset = 0;
-	cmd->iter.offset = &cmd->get_data->in_offset;
-	cmd->iter.xfer = &cmd->get_data->in_length;
 	cmd->iter.max_xfer = cfg_size->get_size->max_xfer;
 	cmd->iter.data = cmd->get_data->out_buf;
 	cmd->iter.total_xfer = cfg_size->get_size->config_size;
@@ -2680,9 +2712,11 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_read(struct ndctl_cmd *cfg
 static void iter_set_extent(struct ndctl_cmd_iter *iter, unsigned int len,
 		unsigned int offset)
 {
+	struct ndctl_cmd *cmd = container_of(iter, typeof(*cmd), iter);
+
 	iter->init_offset = offset;
-	*iter->offset = offset;
-	*iter->xfer = min(iter->max_xfer, len);
+	cmd->set_offset(cmd, offset);
+	cmd->set_xfer(cmd, min(cmd->get_xfer(cmd), len));
 	iter->total_xfer = len;
 }
 
@@ -2746,9 +2780,11 @@ NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_cfg_write(struct ndctl_cmd *cf
 	cmd->set_data->in_offset = cfg_read->iter.init_offset;
 	cmd->set_data->in_length = cfg_read->iter.max_xfer;
 	cmd->get_firmware_status = cmd_get_firmware_status;
+	cmd->get_xfer = cmd_get_xfer;
+	cmd->set_xfer = cmd_set_xfer;
+	cmd->get_offset = cmd_get_offset;
+	cmd->set_offset = cmd_set_offset;
 	cmd->iter.init_offset = cfg_read->iter.init_offset;
-	cmd->iter.offset = &cmd->set_data->in_offset;
-	cmd->iter.xfer = &cmd->set_data->in_length;
 	cmd->iter.max_xfer = cfg_read->iter.max_xfer;
 	cmd->iter.data = cmd->set_data->in_buf;
 	cmd->iter.total_xfer = cfg_read->iter.total_xfer;
@@ -2961,12 +2997,12 @@ static int do_cmd(int fd, int ioctl_cmd, struct ndctl_cmd *cmd)
 	}
 
 	for (offset = 0; offset < iter->total_xfer; offset += iter->max_xfer) {
-		*(cmd->iter.xfer) = min(iter->total_xfer - offset,
-				iter->max_xfer);
-		*(cmd->iter.offset) = offset;
+		cmd->set_xfer(cmd, min(iter->total_xfer - offset,
+				iter->max_xfer));
+		cmd->set_offset(cmd, offset);
 		if (iter->dir == WRITE)
 			memcpy(iter->data, iter->total_buf + offset,
-					*(cmd->iter.xfer));
+					cmd->get_xfer(cmd));
 		rc = ioctl(fd, ioctl_cmd, cmd->cmd_buf);
 		if (rc < 0) {
 			rc = -errno;
@@ -2975,9 +3011,9 @@ static int do_cmd(int fd, int ioctl_cmd, struct ndctl_cmd *cmd)
 
 		if (iter->dir == READ)
 			memcpy(iter->total_buf + offset, iter->data,
-					*(cmd->iter.xfer) - rc);
+					cmd->get_xfer(cmd) - rc);
 		if (cmd->get_firmware_status(cmd) || rc) {
-			rc = offset + *(cmd->iter.xfer) - rc;
+			rc = offset + cmd->get_xfer(cmd) - rc;
 			break;
 		}
 	}
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 3c121bd00437..2e537f0a8649 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -252,10 +252,12 @@ struct ndctl_cmd {
 	int size;
 	int status;
 	u32 (*get_firmware_status)(struct ndctl_cmd *cmd);
+	u32 (*get_xfer)(struct ndctl_cmd *cmd);
+	u32 (*get_offset)(struct ndctl_cmd *cmd);
+	void (*set_xfer)(struct ndctl_cmd *cmd, u32 xfer);
+	void (*set_offset)(struct ndctl_cmd *cmd, u32 offset);
 	struct ndctl_cmd_iter {
 		u32 init_offset;
-		u32 *offset;
-		u32 *xfer; /* pointer to xfer length in cmd */
 		u8 *data; /* pointer to the data buffer location in cmd */
 		u32 max_xfer;
 		char *total_buf;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 26/36] ndctl/test: Fix typos / loss of tpm.handle in security test
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (24 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 25/36] ndctl/dimm: Rework iteration to drop unaligned pointers Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-02-29 20:22 ` [ndctl PATCH 27/36] ndctl/test: Relax dax_pmem_compat requirement Dan Williams
                   ` (10 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Dave Jiang, vishal.l.verma

The DIMM security test confuses "tmp" and "tpm" in multiple places. It
saves the current tpm.handle file to tmp.handle.bak and tries to restore
it from tmp.handle.bak to tmp.handle. Replace all occurrences of "tmp"
with "tpm".

Cc: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/security.sh |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/security.sh b/test/security.sh
index 771135b7ab18..8e2d870c0d43 100755
--- a/test/security.sh
+++ b/test/security.sh
@@ -39,7 +39,7 @@ setup_keys()
 		backup_key=1
 	fi
 	if [ -f "$keypath/tpm.handle" ]; then
-		mv "$keypath/tpm.handle" "$keypath/tmp.handle.bak"
+		mv "$keypath/tpm.handle" "$keypath/tpm.handle.bak"
 		backup_handle=1
 	fi
 
@@ -71,7 +71,7 @@ post_cleanup()
 		mv "$masterpath.bak" "$masterpath"
 	fi
 	if [ "$backup_handle" -eq 1 ]; then
-		mv "$keypath/tpm.handle.bak" "$keypath/tmp.handle"
+		mv "$keypath/tpm.handle.bak" "$keypath/tpm.handle"
 	fi
 }
 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 27/36] ndctl/test: Relax dax_pmem_compat requirement
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (25 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 26/36] ndctl/test: Fix typos / loss of tpm.handle in security test Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-03-03 13:28   ` Jan Kara
                     ` (2 more replies)
  2020-02-29 20:22 ` [ndctl PATCH 28/36] ndctl/namespace: Fix namespace-action vs namespace-mode confusion Dan Williams
                   ` (9 subsequent siblings)
  36 siblings, 3 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Jan Kara

While there are some tests that require the new "dax-bus" device model,
none of the tests require compatibility mode. Drop the requirement so
the tests work with DEV_DAX_PMEM_COMPAT=n kernels.

Link: http://lore.kernel.org/r/20200123154720.12097-1-jack@suse.cz
Cc: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/core.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/test/core.c b/test/core.c
index 888f5d8c0e42..dff842a9f378 100644
--- a/test/core.c
+++ b/test/core.c
@@ -180,6 +180,14 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
 
 retry:
 		rc = kmod_module_new_from_name(*ctx, name, mod);
+
+		/*
+		 * dax_pmem_compat is not required, missing is ok,
+		 * present-but-production is not ok.
+		 */
+		if (rc && strstr(name, "dax_pmem_compat"))
+			continue;
+
 		if (rc) {
 			log_err(&log_ctx, "%s.ko: missing\n", name);
 			break;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 28/36] ndctl/namespace: Fix namespace-action vs namespace-mode confusion
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (26 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 27/36] ndctl/test: Relax dax_pmem_compat requirement Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-02-29 20:22 ` [ndctl PATCH 29/36] ndctl/namespace: Update 'pfn' infoblock definition Dan Williams
                   ` (8 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Rename the 'enum device_action' parameter to set_defaults() to 'action'
to remove confusion with the namespace mode parameter.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 397bd4acd1d1..cfa0563f59a1 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -196,7 +196,7 @@ static const struct option read_infoblock_options[] = {
 	OPT_END(),
 };
 
-static int set_defaults(enum device_action mode)
+static int set_defaults(enum device_action action)
 {
 	int rc = 0;
 
@@ -210,7 +210,7 @@ static int set_defaults(enum device_action mode)
 				param.type);
 			rc = -EINVAL;
 		}
-	} else if (!param.reconfig && mode == ACTION_CREATE)
+	} else if (!param.reconfig && action == ACTION_CREATE)
 		param.type = "pmem";
 
 	if (param.mode) {
@@ -293,7 +293,7 @@ static int set_defaults(enum device_action mode)
  * looking at actual namespace devices and available resources.
  */
 static const char *parse_namespace_options(int argc, const char **argv,
-		enum device_action mode, const struct option *options,
+		enum device_action action, const struct option *options,
 		char *xable_usage)
 {
 	const char * const u[] = {
@@ -305,12 +305,12 @@ static const char *parse_namespace_options(int argc, const char **argv,
 	param.do_scan = argc == 1;
         argc = parse_options(argc, argv, options, u, 0);
 
-	rc = set_defaults(mode);
+	rc = set_defaults(action);
 
-	if (argc == 0 && mode != ACTION_CREATE) {
+	if (argc == 0 && action != ACTION_CREATE) {
 		char *action_string;
 
-		switch (mode) {
+		switch (action) {
 			case ACTION_ENABLE:
 				action_string = "enable";
 				break;
@@ -334,18 +334,18 @@ static const char *parse_namespace_options(int argc, const char **argv,
 				break;
 		}
 
-		if ((mode == ACTION_READ_INFOBLOCK && !param.infile)
-				|| mode != ACTION_READ_INFOBLOCK) {
+		if ((action == ACTION_READ_INFOBLOCK && !param.infile)
+				|| action != ACTION_READ_INFOBLOCK) {
 			error("specify a namespace to %s, or \"all\"\n", action_string);
 			rc = -EINVAL;
 		}
 	}
-	for (i = mode == ACTION_CREATE ? 0 : 1; i < argc; i++) {
+	for (i = action == ACTION_CREATE ? 0 : 1; i < argc; i++) {
 		error("unknown extra parameter \"%s\"\n", argv[i]);
 		rc = -EINVAL;
 	}
 
-	if (mode == ACTION_READ_INFOBLOCK && param.infile && argc) {
+	if (action == ACTION_READ_INFOBLOCK && param.infile && argc) {
 		error("specify a namespace, or --input, not both\n");
 		rc = -EINVAL;
 	}
@@ -355,7 +355,7 @@ static const char *parse_namespace_options(int argc, const char **argv,
 		return NULL; /* we won't return from usage_with_options() */
 	}
 
-	return mode == ACTION_CREATE ? param.reconfig : argv[0];
+	return action == ACTION_CREATE ? param.reconfig : argv[0];
 }
 
 #define try(prefix, op, dev, p) \
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 29/36] ndctl/namespace: Update 'pfn' infoblock definition
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (27 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 28/36] ndctl/namespace: Fix namespace-action vs namespace-mode confusion Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-02-29 20:22 ` [ndctl PATCH 30/36] ndctl/util: Return 0 for NULL arguments to parse_size64() Dan Williams
                   ` (7 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Grab the latest definition of the pfn infoblock in preparation for
creating a write-infoblock command, and update read-infoblock parsing.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |    2 ++
 ndctl/namespace.h |   12 +++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index cfa0563f59a1..e38151430436 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -1612,6 +1612,8 @@ static json_object *pfn_parse(struct pfn_sb *pfn_sb, struct ndctl_namespace *ndn
 	parse_hex(pfn_sb, start_pad, 32);
 	parse_hex(pfn_sb, end_trunc, 32);
 	parse_hex(pfn_sb, align, 32);
+	parse_hex(pfn_sb, page_size, 32);
+	parse_hex(pfn_sb, page_struct_size, 16);
 
 	return jblock;
 err:
diff --git a/ndctl/namespace.h b/ndctl/namespace.h
index 861dfbfa5127..0f17df20ca3a 100644
--- a/ndctl/namespace.h
+++ b/ndctl/namespace.h
@@ -209,6 +209,12 @@ struct arena_map {
 #define PFN_SIG "NVDIMM_PFN_INFO\0"
 #define DAX_SIG "NVDIMM_DAX_INFO\0"
 
+enum pfn_mode {
+	PFN_MODE_NONE,
+	PFN_MODE_RAM,
+	PFN_MODE_PMEM,
+};
+
 struct pfn_sb {
 	u8 signature[PFN_SIG_LEN];
 	u8 uuid[16];
@@ -224,7 +230,11 @@ struct pfn_sb {
 	le32 end_trunc;
 	/* minor-version-2 record the base alignment of the mapping */
 	le32 align;
-	u8 padding[4000];
+	/* minor-version-3 guarantee the padding and flags are zero */
+	/* minor-version-4 record the page size and struct page size */
+	le32 page_size;
+	le16 page_struct_size;
+	u8 padding[3994];
 	le64 checksum;
 };
 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 30/36] ndctl/util: Return 0 for NULL arguments to parse_size64()
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (28 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 29/36] ndctl/namespace: Update 'pfn' infoblock definition Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-02-29 20:22 ` [ndctl PATCH 31/36] ndctl/namespace: Fix read-info-block vs read-infoblock Dan Williams
                   ` (6 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Allow callers to skip their own NULL checks and just assume that
parse_size64() handles the NULL case.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 util/size.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/util/size.c b/util/size.c
index f54ae5b870b8..6efa91f6eef9 100644
--- a/util/size.c
+++ b/util/size.c
@@ -65,5 +65,7 @@ unsigned long long __parse_size64(const char *str, unsigned long long *units)
 
 unsigned long long parse_size64(const char *str)
 {
+	if (!str)
+		return 0;
 	return __parse_size64(str, NULL);
 }
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 31/36] ndctl/namespace: Fix read-info-block vs read-infoblock
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (29 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 30/36] ndctl/util: Return 0 for NULL arguments to parse_size64() Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-02-29 20:22 ` [ndctl PATCH 32/36] ndctl/namespace: Parse infoblocks from stdin Dan Williams
                   ` (5 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Correct occurrences that hyphenate infoblock, and fixup use of
"namespace" that should be "infoblock".

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index e38151430436..ef82254a7b18 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -152,11 +152,11 @@ OPT_BOOLEAN('s', "scrub", &scrub, "run a scrub to find latent errors")
 
 #define READ_INFOBLOCK_OPTIONS() \
 OPT_FILENAME('o', "output", &param.outfile, "output-file", \
-	"filename to write info-block contents"), \
+	"filename to write infoblock contents"), \
 OPT_FILENAME('i', "input", &param.infile, "input-file", \
-	"filename to read info-block instead of a namespace"), \
+	"filename to read infoblock instead of a namespace"), \
 OPT_BOOLEAN('V', "verify", &param.verify, \
-	"validate parent uuid, and info-block checksum"), \
+	"validate parent uuid, and infoblock checksum"), \
 OPT_BOOLEAN('j', "json", &param.json, "parse label data into json"), \
 OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats (implies --json)")
 
@@ -1455,7 +1455,7 @@ static json_object *btt_parse(struct btt_sb *btt_sb, struct ndctl_namespace *ndn
 	uuid_t uuid;
 	char str[40];
 	struct json_object *jblock, *jobj;
-	const char *cmd = "read-info-block";
+	const char *cmd = "read-infoblock";
 	const bool verify = param.verify;
 
 	if (verify && !verify_infoblock_checksum((union info_block *) btt_sb)) {
@@ -1543,7 +1543,7 @@ static json_object *pfn_parse(struct pfn_sb *pfn_sb, struct ndctl_namespace *ndn
 	uuid_t uuid;
 	char str[40];
 	struct json_object *jblock, *jobj;
-	const char *cmd = "read-info-block";
+	const char *cmd = "read-infoblock";
 	const bool verify = param.verify;
 
 	if (verify && !verify_infoblock_checksum((union info_block *) pfn_sb)) {
@@ -1680,7 +1680,7 @@ static int file_read_infoblock(const char *path, struct ndctl_namespace *ndns,
 		struct read_infoblock_ctx *ri_ctx)
 {
 	const char *devname = ndns ? ndctl_namespace_get_devname(ndns) : "";
-	const char *cmd = "read-info-block";
+	const char *cmd = "read-infoblock";
 	void *buf = NULL;
 	int fd = -1, rc;
 
@@ -1717,7 +1717,7 @@ static int namespace_read_infoblock(struct ndctl_namespace *ndns,
 {
 	int rc;
 	char path[50];
-	const char *cmd = "read-info-block";
+	const char *cmd = "read-infoblock";
 	const char *devname = ndctl_namespace_get_devname(ndns);
 
 	if (ndctl_namespace_is_active(ndns)) {
@@ -2033,7 +2033,7 @@ int cmd_clear_errors(int argc , const char **argv, struct ndctl_ctx *ctx)
 
 int cmd_read_infoblock(int argc, const char **argv, struct ndctl_ctx *ctx)
 {
-	char *xable_usage = "ndctl read-info-block <namespace> [<options>]";
+	char *xable_usage = "ndctl read-infoblock <namespace> [<options>]";
 	const char *namespace = parse_namespace_options(argc, argv,
 			ACTION_READ_INFOBLOCK, read_infoblock_options,
 			xable_usage);
@@ -2041,8 +2041,8 @@ int cmd_read_infoblock(int argc, const char **argv, struct ndctl_ctx *ctx)
 
 	rc = do_xaction_namespace(namespace, ACTION_READ_INFOBLOCK, ctx, &read);
 	if (rc < 0)
-		fprintf(stderr, "error checking namespaces: %s\n",
+		fprintf(stderr, "error reading infoblock data: %s\n",
 				strerror(-rc));
-	fprintf(stderr, "read %d namespace%s\n", read, read == 1 ? "" : "s");
+	fprintf(stderr, "read %d infoblock%s\n", read, read == 1 ? "" : "s");
 	return rc;
 }
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 32/36] ndctl/namespace: Parse infoblocks from stdin
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (30 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 31/36] ndctl/namespace: Fix read-info-block vs read-infoblock Dan Williams
@ 2020-02-29 20:22 ` Dan Williams
  2020-02-29 20:23 ` [ndctl PATCH 33/36] ndctl/namespace: Add write-infoblock command Dan Williams
                   ` (4 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

In preparation for validating a new write-infoblock command, teach
read-infoblock to parse stdin by default.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index ef82254a7b18..7bfc7d1ab61d 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -334,8 +334,7 @@ static const char *parse_namespace_options(int argc, const char **argv,
 				break;
 		}
 
-		if ((action == ACTION_READ_INFOBLOCK && !param.infile)
-				|| action != ACTION_READ_INFOBLOCK) {
+		if (action != ACTION_READ_INFOBLOCK) {
 			error("specify a namespace to %s, or \"all\"\n", action_string);
 			rc = -EINVAL;
 		}
@@ -355,6 +354,8 @@ static const char *parse_namespace_options(int argc, const char **argv,
 		return NULL; /* we won't return from usage_with_options() */
 	}
 
+	if (action == ACTION_READ_INFOBLOCK && !param.infile && !argc)
+		return NULL;
 	return action == ACTION_CREATE ? param.reconfig : argv[0];
 }
 
@@ -1688,7 +1689,12 @@ static int file_read_infoblock(const char *path, struct ndctl_namespace *ndns,
 	if (!buf)
 		return -ENOMEM;
 
-	fd = open(path, O_RDONLY|O_EXCL);
+	if (!path) {
+		fd = STDIN_FILENO;
+		path = "<stdin>";
+	} else
+		fd = open(path, O_RDONLY|O_EXCL);
+
 	if (fd < 0) {
 		pr_verbose("%s: %s failed to open %s: %s\n",
 				cmd, devname, path, strerror(errno));
@@ -1697,17 +1703,20 @@ static int file_read_infoblock(const char *path, struct ndctl_namespace *ndns,
 	}
 
 	rc = read(fd, buf, INFOBLOCK_SZ);
-	if (rc < 0) {
+	if (rc < INFOBLOCK_SZ) {
 		pr_verbose("%s: %s failed to read %s: %s\n",
 				cmd, devname, path, strerror(errno));
-		rc = -errno;
+		if (rc < 0)
+			rc = -errno;
+		else
+			rc = -EIO;
 		goto out;
 	}
 
 	rc = parse_namespace_infoblock(buf, ndns, path, ri_ctx);
 out:
 	free(buf);
-	if (fd >= 0)
+	if (fd >= 0 && fd != STDIN_FILENO)
 		close(fd);
 	return rc;
 }
@@ -1766,10 +1775,12 @@ static int do_xaction_namespace(const char *namespace,
 			}
 		}
 
-		if (param.infile) {
+		if (param.infile || !namespace) {
 			rc = file_read_infoblock(param.infile, NULL, &ri_ctx);
 			if (ri_ctx.jblocks)
 				util_display_json_array(ri_ctx.f_out, ri_ctx.jblocks, 0);
+			if (rc >= 0)
+				(*processed)++;
 			return rc;
 		}
 	}
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 33/36] ndctl/namespace: Add write-infoblock command
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (31 preceding siblings ...)
  2020-02-29 20:22 ` [ndctl PATCH 32/36] ndctl/namespace: Parse infoblocks from stdin Dan Williams
@ 2020-02-29 20:23 ` Dan Williams
  2020-02-29 20:23 ` [ndctl PATCH 34/36] ndctl/list: Add option to list configured + disabled namespaces Dan Williams
                   ` (3 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:23 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Add a tool that can generate infoblock images for fsdax and devdax mode
namespaces. The motivation for this tool is to be able to test namespace
alignment corner cases, or enable persistent memory namespace images to be
provisioned outside of the driver for a deployment, for example, inside a
virtual-machine guest.

The tool can optionally write to an existing namespace, a regular file,
or stdout.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl/Makefile.am               |    3 
 Documentation/ndctl/ndctl-write-infoblock.txt |  132 ++++++++++
 ndctl/action.h                                |    1 
 ndctl/builtin.h                               |    1 
 ndctl/namespace.c                             |  327 ++++++++++++++++++++++++-
 ndctl/ndctl.c                                 |    1 
 6 files changed, 451 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/ndctl/ndctl-write-infoblock.txt

diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index a5b20715fa9b..b8e239107ff9 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -56,7 +56,8 @@ man1_MANS = \
 	ndctl-sanitize-dimm.1 \
 	ndctl-load-keys.1 \
 	ndctl-wait-overwrite.1 \
-	ndctl-read-infoblock.1
+	ndctl-read-infoblock.1 \
+	ndctl-write-infoblock.1
 
 EXTRA_DIST = $(man1_MANS)
 
diff --git a/Documentation/ndctl/ndctl-write-infoblock.txt b/Documentation/ndctl/ndctl-write-infoblock.txt
new file mode 100644
index 000000000000..8197559f1c49
--- /dev/null
+++ b/Documentation/ndctl/ndctl-write-infoblock.txt
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-write-infoblock(1)
+========================
+
+NAME
+----
+ndctl-write-infoblock - generate and write an infoblock
+
+SYNOPSIS
+--------
+[verse]
+'ndctl write-infoblock' [<namespaceX.Y> | -o <file> | --stdout] [<options>]
+
+DESCRIPTION
+-----------
+As described in the theory of operation section of
+linkndctl:ndctl-create-namespace[1], the raw capacity of a namespace may
+encapsulate a personality, or mode of operation. Specifically, the mode
+may be set to one of "sector", "fsdax", and "devdax". Each of those
+modes is defined by an info-block format that uniquely identifies the
+mode of operation. The write-infoblock command knows how to generate an
+"fsdax" or "devdax" info-block relative to the specified image size.
+
+The generated block can be written to an existing namespace (provided
+that namespace is not presently active), written to a file, or piped to
+standard-out.
+
+WARNING: This command is a debug facility that can generate image files
+with valid infoblocks, but also invalid infoblocks for testing the
+kernel. Use the --offset and --align options with care. Namely --offset
+must match the actual physical address offset of the namespace it is
+applied to, and --align must be one of the architectures supported page
+sizes.
+
+EXAMPLE
+-------
+
+[verse]
+ndctl write-infoblock -s 1T -c | ndctl read-infoblock -j
+wrote 1 infoblock
+[
+  {
+    "file":"<stdin>",
+    "signature":"NVDIMM_PFN_INFO",
+    "uuid":"42e1d574-76ac-402c-9132-5436e31528c0",
+    "parent_uuid":"ef83e49c-4c4a-4fae-b908-72e94675b1b7",
+    "flags":0,
+    "version":"1.4",
+    "dataoff":17196646400,
+    "npfns":264237056,
+    "mode":2,
+    "start_pad":0,
+    "end_trunc":0,
+    "align":16777216,
+    "page_size":4096,
+    "page_struct_size":64
+  }
+]
+read 1 infoblock
+
+OPTIONS
+-------
+<namespace(s)>::
+	One or more 'namespaceX.Y' device names. The keyword 'all' can be specified to
+	operate on every namespace in the system, optionally filtered by bus id (see
+        --bus= option), or region id (see --region= option).
+
+-c::
+--stdout::
+	Write the infoblock to stdout
+
+-o::
+--output=::
+	Write the infoblock to the given file (mutually
+	exclusive with --stdout).
+
+-m::
+--mode=::
+	Select the infoblock mode between 'fsdax' and 'devdax'. See
+	linkndctl:ndctl-create-namespace[1] for details on --mode.
+
+-s::
+--size=::
+	Override the default size determined from the size of the file
+	specified to --output. In the --stdout case, this option is
+	required.
+
+-a::
+--align=::
+	Specify the "align" value in the infoblock. In the --mode=devdax
+	case "align" designates a page mapping size. There is no
+	validation of this value relative to the page mapping
+	capabilities of the platform.
+
+-u::
+--uuid=::
+	Override the default autogenerated UUID with the given
+	value.
+
+-M::
+--map=::
+	Select whether the page map array is allocated from the
+	device or from "System RAM". Defaults to the device. See
+	linkndctl:ndctl-create-namespace[1] for more details.
+
+-p::
+--parent-uuid=::
+	When the infoblock is stored on a labelled namespace the UUID of
+	the namespace must match the "parent uuid" attribute in the
+	infoblock. This option defaults to the UUID of the namespace
+	when --output and --stdout are not used, otherwise it defaults
+	to a NULL UUID (all zeroes).
+
+-O::
+--offset=::
+	By default the assumption is that the infoblock is being written
+	to a namespace or namespace-image that is aligned to its size.
+	Specify this EXPERT/DEBUG option to experiment / test the
+	kernel's handling of namespaces that violate that assumption.
+
+-r::
+--region=::
+include::xable-region-options.txt[]
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkndctl:ndctl-create-namespace[1],
+http://www.uefi.org/sites/default/files/resources/UEFI_Spec_2_7.pdf[UEFI NVDIMM Label Protocol]
+
diff --git a/ndctl/action.h b/ndctl/action.h
index 636524c01f20..bcf6bf3196c6 100644
--- a/ndctl/action.h
+++ b/ndctl/action.h
@@ -15,5 +15,6 @@ enum device_action {
 	ACTION_START,
 	ACTION_CLEAR,
 	ACTION_READ_INFOBLOCK,
+	ACTION_WRITE_INFOBLOCK,
 };
 #endif /* __NDCTL_ACTION_H__ */
diff --git a/ndctl/builtin.h b/ndctl/builtin.h
index aa41ad01a84c..8aeada86c1a7 100644
--- a/ndctl/builtin.h
+++ b/ndctl/builtin.h
@@ -9,6 +9,7 @@ int cmd_enable_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_create_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_destroy_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_read_infoblock(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_write_infoblock(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_disable_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_check_namespace(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_clear_errors(int argc, const char **argv, struct ndctl_ctx *ctx);
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 7bfc7d1ab61d..21252b6afe50 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -23,7 +23,9 @@
 #include "action.h"
 #include "namespace.h"
 #include <sys/stat.h>
+#include <linux/fs.h>
 #include <uuid/uuid.h>
+#include <sys/ioctl.h>
 #include <sys/types.h>
 #include <util/size.h>
 #include <util/json.h>
@@ -47,6 +49,7 @@ static struct parameters {
 	bool autorecover;
 	bool human;
 	bool json;
+	bool std_out;
 	const char *bus;
 	const char *map;
 	const char *type;
@@ -58,8 +61,10 @@ static struct parameters {
 	const char *reconfig;
 	const char *sector_size;
 	const char *align;
+	const char *offset;
 	const char *outfile;
 	const char *infile;
+	const char *parent_uuid;
 } param = {
 	.autolabel = true,
 	.autorecover = true,
@@ -160,6 +165,26 @@ OPT_BOOLEAN('V', "verify", &param.verify, \
 OPT_BOOLEAN('j', "json", &param.json, "parse label data into json"), \
 OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats (implies --json)")
 
+#define WRITE_INFOBLOCK_OPTIONS() \
+OPT_FILENAME('o', "output", &param.outfile, "output-file", \
+	"filename to write infoblock contents"), \
+OPT_BOOLEAN('c', "stdout", &param.std_out, \
+	"write the infoblock data to stdout"), \
+OPT_STRING('m', "mode", &param.mode, "operation-mode", \
+	"specify the infoblock mode, 'fsdax' or 'devdax' (default 'fsdax')"), \
+OPT_STRING('s', "size", &param.size, "size", \
+	"override the image size to instantiate the infoblock"), \
+OPT_STRING('a', "align", &param.align, "align", \
+	"specify the expected physical alignment (default: 2M)"), \
+OPT_STRING('u', "uuid", &param.uuid, "uuid", \
+	"specify the uuid for the infoblock (default: autogenerate)"), \
+OPT_STRING('M', "map", &param.map, "memmap-location", \
+	"specify 'mem' or 'dev' for the location of the memmap"), \
+OPT_STRING('p', "parent-uuid", &param.parent_uuid, "parent-uuid", \
+	"specify the parent namespace uuid for the infoblock (default: 0)"), \
+OPT_STRING('O', "offset", &param.offset, "offset", \
+	"EXPERT/DEBUG only: enable namespace inner alignment padding")
+
 static const struct option base_options[] = {
 	BASE_OPTIONS(),
 	OPT_END(),
@@ -196,8 +221,15 @@ static const struct option read_infoblock_options[] = {
 	OPT_END(),
 };
 
+static const struct option write_infoblock_options[] = {
+	BASE_OPTIONS(),
+	WRITE_INFOBLOCK_OPTIONS(),
+	OPT_END(),
+};
+
 static int set_defaults(enum device_action action)
 {
+	uuid_t uuid;
 	int rc = 0;
 
 	if (param.type) {
@@ -214,10 +246,25 @@ static int set_defaults(enum device_action action)
 		param.type = "pmem";
 
 	if (param.mode) {
-		if (util_nsmode(param.mode) == NDCTL_NS_MODE_UNKNOWN) {
+		enum ndctl_namespace_mode mode = util_nsmode(param.mode);
+
+		switch (mode) {
+		case NDCTL_NS_MODE_UNKNOWN:
 			error("invalid mode '%s'\n", param.mode);
 			rc = -EINVAL;
+			break;
+		case NDCTL_NS_MODE_FSDAX:
+		case NDCTL_NS_MODE_DEVDAX:
+			break;
+		default:
+			if (action == ACTION_WRITE_INFOBLOCK) {
+				error("unsupported mode '%s'\n", param.mode);
+				rc = -EINVAL;
+			}
+			break;
 		}
+	} else if (action == ACTION_WRITE_INFOBLOCK) {
+		param.mode = "fsdax";
 	} else if (!param.reconfig && param.type) {
 		if (strcmp(param.type, "pmem") == 0)
 			param.mode = "fsdax";
@@ -259,21 +306,59 @@ static int set_defaults(enum device_action action)
 		rc = -EINVAL;
 	}
 
-	if (param.align && parse_size64(param.align) == ULLONG_MAX) {
-		error("failed to parse namespace alignment '%s'\n",
-				param.align);
+	if (param.offset && parse_size64(param.offset) == ULLONG_MAX) {
+		error("failed to parse physical offset'%s'\n",
+				param.offset);
 		rc = -EINVAL;
 	}
 
-	if (param.uuid) {
-		uuid_t uuid;
+	if (param.align) {
+		unsigned long long align = parse_size64(param.align);
+
+		if (align == ULLONG_MAX) {
+			error("failed to parse namespace alignment '%s'\n",
+					param.align);
+			rc = -EINVAL;
+		} else if (!is_power_of_2(align)
+			|| align < (unsigned long long) sysconf(_SC_PAGE_SIZE)) {
+			error("align must be a power-of-2 greater than %ld\n",
+					sysconf(_SC_PAGE_SIZE));
+			rc = -EINVAL;
+		}
+	} else if (action == ACTION_WRITE_INFOBLOCK)
+		param.align = "2M";
+
+	if (param.size) {
+		unsigned long long size = parse_size64(param.size);
+		unsigned long long align = parse_size64(param.align);
+
+		if (size == ULLONG_MAX) {
+			error("failed to parse namespace size '%s'\n",
+					param.size);
+			rc = -EINVAL;
+		} else if (action == ACTION_WRITE_INFOBLOCK
+				&& align < ULLONG_MAX
+				&& !IS_ALIGNED(size, align)) {
+			error("--size=%s not aligned to %s\n", param.size,
+					param.align);
+			rc = -EINVAL;
+		}
+	}
 
+	if (param.uuid) {
 		if (uuid_parse(param.uuid, uuid)) {
 			error("failed to parse uuid: '%s'\n", param.uuid);
 			rc = -EINVAL;
 		}
 	}
 
+	if (param.parent_uuid) {
+		if (uuid_parse(param.parent_uuid, uuid)) {
+			error("failed to parse uuid: '%s'\n", param.parent_uuid);
+			rc = -EINVAL;
+		}
+	}
+
 	if (param.sector_size) {
 		if (parse_size64(param.sector_size) == ULLONG_MAX) {
 			error("invalid sector size: %s\n", param.sector_size);
@@ -329,12 +414,18 @@ static const char *parse_namespace_options(int argc, const char **argv,
 			case ACTION_READ_INFOBLOCK:
 				action_string = "read-infoblock";
 				break;
+			case ACTION_WRITE_INFOBLOCK:
+				action_string = "write-infoblock";
+				break;
 			default:
 				action_string = "<>";
 				break;
 		}
 
-		if (action != ACTION_READ_INFOBLOCK) {
+		if ((action != ACTION_READ_INFOBLOCK
+					&& action != ACTION_WRITE_INFOBLOCK)
+				|| (action == ACTION_WRITE_INFOBLOCK
+					&& !param.outfile && !param.std_out)) {
 			error("specify a namespace to %s, or \"all\"\n", action_string);
 			rc = -EINVAL;
 		}
@@ -349,6 +440,17 @@ static const char *parse_namespace_options(int argc, const char **argv,
 		rc = -EINVAL;
 	}
 
+	if (action == ACTION_WRITE_INFOBLOCK && (param.outfile || param.std_out)
+			&& argc) {
+		error("specify only one of a namespace filter, --output, or --stdout\n");
+		rc = -EINVAL;
+	}
+
+	if (action == ACTION_WRITE_INFOBLOCK && param.std_out && !param.size) {
+		error("--size required with --stdout\n");
+		rc = -EINVAL;
+	}
+
 	if (rc) {
 		usage_with_options(u, options);
 		return NULL; /* we won't return from usage_with_options() */
@@ -1721,12 +1823,172 @@ out:
 	return rc;
 }
 
-static int namespace_read_infoblock(struct ndctl_namespace *ndns,
-		struct read_infoblock_ctx *ri_ctx)
+static unsigned long PHYS_PFN(unsigned long long phys)
+{
+	return phys / sysconf(_SC_PAGE_SIZE);
+}
+
+#define SUBSECTION_SHIFT 21
+#define SUBSECTION_SIZE (1UL << SUBSECTION_SHIFT)
+#define MAX_STRUCT_PAGE_SIZE 64
+
+/* Derived from nd_pfn_init() in kernel version v5.5 */
+static int write_pfn_sb(int fd, unsigned long long size, const char *sig,
+		void *buf)
+{
+	unsigned long npfns, align, pfn_align;
+	struct pfn_sb *pfn_sb = buf + SZ_4K;
+	unsigned long long start, offset;
+	uuid_t uuid, parent_uuid;
+	u32 end_trunc, start_pad;
+	enum pfn_mode mode;
+	u64 checksum;
+	int rc;
+
+	start = parse_size64(param.offset);
+	npfns = PHYS_PFN(size - SZ_8K);
+	pfn_align = parse_size64(param.align);
+	align = max(pfn_align, SUBSECTION_SIZE);
+	if (param.uuid)
+		uuid_parse(param.uuid, uuid);
+	else
+		uuid_generate(uuid);
+
+	if (param.parent_uuid)
+		uuid_parse(param.parent_uuid, parent_uuid);
+	else
+		memset(parent_uuid, 0, sizeof(uuid_t));
+
+	if (strcmp(param.map, "dev") == 0)
+		mode = PFN_MODE_PMEM;
+	else
+		mode = PFN_MODE_RAM;
+
+	/*
+	 * Unlike the kernel implementation always set start_pad and
+	 * end_trunc relative to the specified --offset= option to allow
+	 * testing legacy / "buggy" configurations.
+	 */
+	start_pad = ALIGN(start, align) - start;
+	end_trunc = start + size - ALIGN_DOWN(start + size, align);
+	if (mode == PFN_MODE_PMEM) {
+		/*
+		 * The altmap should be padded out to the block size used
+		 * when populating the vmemmap. This *should* be equal to
+		 * PMD_SIZE for most architectures.
+		 *
+		 * Also make sure size of struct page is less than 64. We
+		 * want to make sure we use large enough size here so that
+		 * we don't have a dynamic reserve space depending on
+		 * struct page size. But we also want to make sure we notice
+		 * when we end up adding new elements to struct page.
+		 */
+		offset = ALIGN(start + SZ_8K + MAX_STRUCT_PAGE_SIZE * npfns, align)
+			- start;
+	} else
+		offset = ALIGN(start + SZ_8K, align) - start;
+
+	if (offset >= size) {
+		error("unable to satisfy requested alignment\n");
+		return -ENXIO;
+	}
+
+	npfns = PHYS_PFN(size - offset - end_trunc - start_pad);
+	pfn_sb->mode = cpu_to_le32(mode);
+	pfn_sb->dataoff = cpu_to_le64(offset);
+	pfn_sb->npfns = cpu_to_le64(npfns);
+	memcpy(pfn_sb->signature, sig, PFN_SIG_LEN);
+	memcpy(pfn_sb->uuid, uuid, 16);
+	memcpy(pfn_sb->parent_uuid, parent_uuid, 16);
+	pfn_sb->version_major = cpu_to_le16(1);
+	pfn_sb->version_minor = cpu_to_le16(4);
+	pfn_sb->start_pad = cpu_to_le32(start_pad);
+	pfn_sb->end_trunc = cpu_to_le32(end_trunc);
+	pfn_sb->align = cpu_to_le32(pfn_align);
+	pfn_sb->page_struct_size = cpu_to_le16(MAX_STRUCT_PAGE_SIZE);
+	pfn_sb->page_size = cpu_to_le32(sysconf(_SC_PAGE_SIZE));
+	checksum = fletcher64(pfn_sb, sizeof(*pfn_sb), 0);
+	pfn_sb->checksum = cpu_to_le64(checksum);
+
+	rc = write(fd, buf, INFOBLOCK_SZ);
+	if (rc < INFOBLOCK_SZ)
+		return -EIO;
+	return 0;
+}
+
+static int file_write_infoblock(const char *path)
+{
+	unsigned long long size = parse_size64(param.size);
+	int fd = -1, rc;
+	void *buf;
+
+	if (param.std_out)
+		fd = STDOUT_FILENO;
+	else {
+		fd = open(path, O_CREAT|O_RDWR, 0644);
+		if (fd < 0) {
+			error("failed to open: %s\n", path);
+			return -errno;
+		}
+
+		if (!size) {
+			struct stat st;
+
+			rc = fstat(fd, &st);
+			if (rc < 0) {
+				error("failed to stat: %s\n", path);
+				rc = -errno;
+				goto out;
+			}
+			if (S_ISREG(st.st_mode))
+				size = st.st_size;
+			else if (S_ISBLK(st.st_mode)) {
+				rc = ioctl(fd, BLKGETSIZE64, &size);
+				if (rc < 0) {
+					error("failed to retrieve size: %s\n", path);
+					rc = -errno;
+					goto out;
+				}
+			} else {
+				error("unsupported file type: %s\n", path);
+				rc = -EINVAL;
+				goto out;
+			}
+		}
+	}
+
+	buf = calloc(INFOBLOCK_SZ, 1);
+	if (!buf)
+		return -ENOMEM;
+
+	switch (util_nsmode(param.mode)) {
+	case NDCTL_NS_MODE_FSDAX:
+		rc = write_pfn_sb(fd, size, PFN_SIG, buf);
+		break;
+	case NDCTL_NS_MODE_DEVDAX:
+		rc = write_pfn_sb(fd, size, DAX_SIG, buf);
+		break;
+	default:
+		rc = -EINVAL;
+		break;
+	}
+
+	free(buf);
+out:
+	if (fd > 0 && fd != STDOUT_FILENO)
+		close(fd);
+	return rc;
+}
+
+static int namespace_rw_infoblock(struct ndctl_namespace *ndns,
+		struct read_infoblock_ctx *ri_ctx, int write)
 {
 	int rc;
+	uuid_t uuid;
+	char str[40];
 	char path[50];
-	const char *cmd = "read-infoblock";
+	const char *save;
+	const char *cmd = write ? "write-infoblock" : "read-infoblock";
 	const char *devname = ndctl_namespace_get_devname(ndns);
 
 	if (ndctl_namespace_is_active(ndns)) {
@@ -1741,9 +2003,19 @@ static int namespace_read_infoblock(struct ndctl_namespace *ndns,
 		goto out;
 	}
 
-	sprintf(path, "/dev/%s", ndctl_namespace_get_block_device(ndns));
-	rc = file_read_infoblock(path, ndns, ri_ctx);
+	save = param.parent_uuid;
+	if (!param.parent_uuid) {
+		ndctl_namespace_get_uuid(ndns, uuid);
+		uuid_unparse(uuid, str);
+		param.parent_uuid = str;
+	}
 
+	sprintf(path, "/dev/%s", ndctl_namespace_get_block_device(ndns));
+	if (write)
+		rc = file_write_infoblock(path);
+	else
+		rc = file_read_infoblock(path, ndns, ri_ctx);
+	param.parent_uuid = save;
 out:
 	ndctl_namespace_set_raw_mode(ndns, 0);
 	ndctl_namespace_disable_invalidate(ndns);
@@ -1785,6 +2057,13 @@ static int do_xaction_namespace(const char *namespace,
 		}
 	}
 
+	if (action == ACTION_WRITE_INFOBLOCK && !namespace) {
+		rc = file_write_infoblock(param.outfile);
+		if (rc >= 0)
+			(*processed)++;
+		return rc;
+	}
+
 	if (!namespace && action != ACTION_CREATE)
 		return rc;
 
@@ -1891,7 +2170,12 @@ static int do_xaction_namespace(const char *namespace,
 						*processed = 1;
 					return rc;
 				case ACTION_READ_INFOBLOCK:
-					rc = namespace_read_infoblock(ndns, &ri_ctx);
+					rc = namespace_rw_infoblock(ndns, &ri_ctx, READ);
+					if (rc == 0)
+						(*processed)++;
+					break;
+				case ACTION_WRITE_INFOBLOCK:
+					rc = namespace_rw_infoblock(ndns, NULL, WRITE);
 					if (rc == 0)
 						(*processed)++;
 					break;
@@ -2057,3 +2341,20 @@ int cmd_read_infoblock(int argc, const char **argv, struct ndctl_ctx *ctx)
 	fprintf(stderr, "read %d infoblock%s\n", read, read == 1 ? "" : "s");
 	return rc;
 }
+
+int cmd_write_infoblock(int argc, const char **argv, struct ndctl_ctx *ctx)
+{
+	char *xable_usage = "ndctl write-infoblock <namespace> [<options>]";
+	const char *namespace = parse_namespace_options(argc, argv,
+			ACTION_WRITE_INFOBLOCK, write_infoblock_options,
+			xable_usage);
+	int write, rc;
+
+	rc = do_xaction_namespace(namespace, ACTION_WRITE_INFOBLOCK, ctx,
+			&write);
+	if (rc < 0)
+		fprintf(stderr, "error checking infoblocks: %s\n",
+				strerror(-rc));
+	fprintf(stderr, "wrote %d infoblock%s\n", write, write == 1 ? "" : "s");
+	return rc;
+}
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 5c9c424dcae6..58cc9c7bb07e 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -74,6 +74,7 @@ static struct cmd_struct commands[] = {
 	{ "create-namespace", { cmd_create_namespace } },
 	{ "destroy-namespace", { cmd_destroy_namespace } },
 	{ "read-infoblock",  { cmd_read_infoblock } },
+	{ "write-infoblock",  { cmd_write_infoblock } },
 	{ "check-namespace", { cmd_check_namespace } },
 	{ "clear-errors", { cmd_clear_errors } },
 	{ "enable-region", { cmd_enable_region } },
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 34/36] ndctl/list: Add option to list configured + disabled namespaces
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (32 preceding siblings ...)
  2020-02-29 20:23 ` [ndctl PATCH 33/36] ndctl/namespace: Add write-infoblock command Dan Williams
@ 2020-02-29 20:23 ` Dan Williams
  2020-02-29 20:23 ` [ndctl PATCH 35/36] ndctl/lib/namespace: Fix resource retrieval after size change Dan Williams
                   ` (2 subsequent siblings)
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:23 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

When inspecting namespaces multiple tools require the namespace to be
disabled so that the tool can change modes to inspect namespace
metadata, for example read-infoblock and check-namespace.

While a namespace is disabled it can be listed with "--idle", but that
will also include seed namespaces in the output. Add a --configured
option that lists includes namespaces with non-zero size in the listing.
For regions, dimms, and buses, it is equivalent to --idle.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl/ndctl-list.txt |    6 ++++++
 ndctl/list.c                       |   23 ++++++++++++++++++-----
 util/json.c                        |    3 ++-
 util/json.h                        |    1 +
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index bc725baa6656..7c7e3ac9d05c 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -179,6 +179,12 @@ include::xable-bus-options.txt[]
 --idle::
 	Include idle (not enabled) devices in the listing
 
+-c::
+--configured::
+	Include configured devices (non-zero sized namespaces)
+	regardless of whether they are enabled, or not. Other devices
+	besides namespaces are always considered "configured".
+
 -C::
 --capabilities::
 	Include region capabilities in the listing, i.e. supported
diff --git a/ndctl/list.c b/ndctl/list.c
index aedccfe8fe75..31fb1b9593a2 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -37,6 +37,7 @@ static struct {
 	bool human;
 	bool firmware;
 	bool capabilities;
+	bool configured;
 	int verbose;
 } list;
 
@@ -46,6 +47,8 @@ static unsigned long listopts_to_flags(void)
 
 	if (list.idle)
 		flags |= UTIL_JSON_IDLE;
+	if (list.configured)
+		flags |= UTIL_JSON_CONFIGURED;
 	if (list.media_errors)
 		flags |= UTIL_JSON_MEDIA_ERRORS;
 	if (list.dax)
@@ -165,7 +168,8 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 		if (!util_dimm_filter(dimm, param.dimm))
 			continue;
 
-		if (!list.idle && !ndctl_dimm_is_enabled(dimm))
+		if (!list.configured && !list.idle
+				&& !ndctl_dimm_is_enabled(dimm))
 			continue;
 
 		if (!jmappings) {
@@ -242,8 +246,15 @@ static void filter_namespace(struct ndctl_namespace *ndns,
 	struct json_object *jndns;
 	struct list_filter_arg *lfa = ctx->list;
 	struct json_object *container = lfa->jregion ? lfa->jregion : lfa->jbus;
-
-	if (!list.idle && !ndctl_namespace_is_active(ndns))
+	unsigned long long size = ndctl_namespace_get_size(ndns);
+
+	if (ndctl_namespace_is_active(ndns))
+		/* pass */;
+	else if (list.idle)
+		/* pass */;
+	else if (list.configured && (size > 0 && size < ULLONG_MAX))
+		/* pass */;
+	else
 		return;
 
 	if (!lfa->jnamespaces) {
@@ -277,7 +288,7 @@ static bool filter_region(struct ndctl_region *region,
 	if (!list.regions)
 		return true;
 
-	if (!list.idle && !ndctl_region_is_enabled(region))
+	if (!list.configured && !list.idle && !ndctl_region_is_enabled(region))
 		return true;
 
 	if (!lfa->jregions) {
@@ -319,7 +330,7 @@ static void filter_dimm(struct ndctl_dimm *dimm, struct util_filter_ctx *ctx)
 	struct list_filter_arg *lfa = ctx->list;
 	struct json_object *jdimm;
 
-	if (!list.idle && !ndctl_dimm_is_enabled(dimm))
+	if (!list.configured && !list.idle && !ndctl_dimm_is_enabled(dimm))
 		return;
 
 	if (!lfa->jdimms) {
@@ -477,6 +488,8 @@ int cmd_list(int argc, const char **argv, struct ndctl_ctx *ctx)
 		OPT_BOOLEAN('C', "capabilities", &list.capabilities,
 				"include region capability info"),
 		OPT_BOOLEAN('i', "idle", &list.idle, "include idle devices"),
+		OPT_BOOLEAN('c', "configured", &list.configured,
+				"include configured namespaces, disabled or not"),
 		OPT_BOOLEAN('M', "media-errors", &list.media_errors,
 				"include media errors"),
 		OPT_BOOLEAN('u', "human", &list.human,
diff --git a/util/json.c b/util/json.c
index 50346c5bcbab..21ab25674624 100644
--- a/util/json.c
+++ b/util/json.c
@@ -339,7 +339,8 @@ struct json_object *util_daxctl_devs_to_list(struct daxctl_region *region,
 		if (!util_daxctl_dev_filter(dev, ident))
 			continue;
 
-		if (!(flags & UTIL_JSON_IDLE) && !daxctl_dev_get_size(dev))
+		if (!(flags & (UTIL_JSON_IDLE|UTIL_JSON_CONFIGURED))
+				&& !daxctl_dev_get_size(dev))
 			continue;
 
 		if (!jdevs) {
diff --git a/util/json.h b/util/json.h
index 7c3f64932cec..6d39d3aa4693 100644
--- a/util/json.h
+++ b/util/json.h
@@ -25,6 +25,7 @@ enum util_json_flags {
 	UTIL_JSON_HUMAN = (1 << 4),
 	UTIL_JSON_VERBOSE = (1 << 5),
 	UTIL_JSON_CAPABILITIES = (1 << 6),
+	UTIL_JSON_CONFIGURED = (1 << 7),
 };
 
 struct json_object;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 35/36] ndctl/lib/namespace: Fix resource retrieval after size change
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (33 preceding siblings ...)
  2020-02-29 20:23 ` [ndctl PATCH 34/36] ndctl/list: Add option to list configured + disabled namespaces Dan Williams
@ 2020-02-29 20:23 ` Dan Williams
  2020-02-29 20:23 ` [ndctl PATCH 36/36] ndctl/test: Regression test misaligned namespaces Dan Williams
  2020-03-19  4:13 ` [ndctl PATCH 00/36] Multiple topics / backlog for v68 Verma, Vishal L
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:23 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

The namespaceX.Y/resource attribute returns -1 while the namespace does
not have capacity allocated. While it is valid after setting the size
the library has already cached the error value. Teach
ndctl_namespace_set_size() to refresh ->resource.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/libndctl.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index a996bff66fb2..ee737cbbfe3e 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -4484,6 +4484,24 @@ static int namespace_set_size(struct ndctl_namespace *ndns,
 		return rc;
 
 	ndns->size = size;
+
+	/*
+	 * A size change event invalidates / establishes 'resource', try
+	 * to refresh it.
+	 */
+	if (snprintf(path, len, "%s/resource", ndns->ndns_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				ndctl_namespace_get_devname(ndns));
+		ndns->resource = ULLONG_MAX;
+		return 0;
+	}
+
+	if (sysfs_read_attr(ctx, path, buf) < 0) {
+		ndns->resource = ULLONG_MAX;
+		return 0;
+	}
+
+	ndns->resource = strtoull(buf, NULL, 0);
 	return 0;
 }
 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH 36/36] ndctl/test: Regression test misaligned namespaces
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (34 preceding siblings ...)
  2020-02-29 20:23 ` [ndctl PATCH 35/36] ndctl/lib/namespace: Fix resource retrieval after size change Dan Williams
@ 2020-02-29 20:23 ` Dan Williams
  2020-03-19  4:13 ` [ndctl PATCH 00/36] Multiple topics / backlog for v68 Verma, Vishal L
  36 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-02-29 20:23 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Aneesh Kumar K.V

The kernel is now requiring that namespace creation results in
cross-arch compatible namespaces by default. However, it must also
continue to support previously valid, but misaligned, namespaces. Use
the write-infoblock utility to create "legacy" configurations and
validate that the kernel still accepts it along with other corner case
configurations.

Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/Makefile.am |    1 
 test/align.sh    |  118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)
 create mode 100755 test/align.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index cce60c5221fd..1d24a65ded8b 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -54,6 +54,7 @@ TESTS +=\
 	dax-dev \
 	dax-ext4.sh \
 	dax-xfs.sh \
+	align.sh \
 	device-dax \
 	device-dax-fio.sh \
 	daxctl-devices.sh \
diff --git a/test/align.sh b/test/align.sh
new file mode 100755
index 000000000000..0129255610ab
--- /dev/null
+++ b/test/align.sh
@@ -0,0 +1,118 @@
+#!/bin/bash -x
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2015-2020 Intel Corporation. All rights reserved.
+
+. $(dirname $0)/common
+
+rc=77
+cleanup() {
+	echo "align.sh: failed at line $1"
+	if [ "x$region" != "x" -a x$save_align != "x" ]; then
+		echo $save_align > $region_path/align
+	fi
+
+	if [ "x$ns1" != "x" ]; then
+		$NDCTL destroy-namespace -f $ns1
+	fi
+	if [ "x$ns2" != "x" ]; then
+		$NDCTL destroy-namespace -f $ns2
+	fi
+
+	exit $rc
+}
+
+is_aligned() {
+	val=$1
+	align=$2
+
+	if [ $((val & (align - 1))) -eq 0 ]; then
+		return 0
+	fi
+	return 1
+}
+
+set -e
+trap 'err $LINENO cleanup' ERR
+
+region=$($NDCTL list -R -b ACPI.NFIT | jq -r '.[] | [select(.available_size == .size)] | .[0].dev')
+
+if [ "x$region" = "xnull"  ]; then
+	unset $region
+	echo "unable to find empty region"
+	false
+fi
+
+region_path="/sys/bus/nd/devices/$region"
+save_align=$(cat $region_path/align)
+
+# check that the region is 1G aligned
+resource=$(cat $region_path/resource)
+is_aligned $resource $((1 << 30)) || (echo "need a 1GB aligned namespace to test alignment conditions" && false)
+
+rc=1
+
+# check that start-aligned, but end-misaligned namespaces can be created
+# and probed
+echo 4096 > $region_path/align
+SIZE=$(((1<<30) + (8<<10)))
+json=$($NDCTL create-namespace -r $region -s $SIZE -m fsdax -a 4K)
+eval $(json2var <<< "$json")
+$NDCTL disable-namespace $dev
+$NDCTL enable-namespace $dev
+ns1=$dev
+
+# check that start-misaligned namespaces can't be created until the
+# region alignment is set to a compatible value.
+# Note the namespace capacity alignment requirement in this case is
+# SUBSECTION_SIZE (2M) as the data alignment can be satisfied with
+# metadata padding.
+json=$($NDCTL create-namespace -r $region -s $SIZE -m fsdax -a 4K -f) || status="failed"
+if [ $status != "failed" ]; then
+	echo "expected namespace creation failure"
+	eval $(json2var <<< "$json")
+	$NDCTL destroy-namespace -f $dev
+	false
+fi
+
+# check that start-misaligned namespaces can't be probed. Since the
+# kernel does not support creating this misalignment, force it with a
+# custom info-block
+json=$($NDCTL create-namespace -r $region -s $SIZE -m raw)
+eval $(json2var <<< "$json")
+
+$NDCTL disable-namespace $dev
+$NDCTL write-infoblock $dev -a 4K
+$NDCTL enable-namespace $dev || status="failed"
+
+if [ $status != "failed" ]; then
+	echo "expected namespace enable failure"
+	$NDCTL destroy-namespace -f $dev
+	false
+fi
+ns2=$dev
+
+# check that namespace with proper inner padding can be enabled, even
+# though non-zero start_pad namespaces don't support dax
+$NDCTL write-infoblock $ns2 -a 4K -O 8K
+$NDCTL enable-namespace $ns2
+$NDCTL destroy-namespace $ns2 -f
+unset ns2
+
+# check that all namespace alignments can be created with the region
+# alignment at a compatible value
+SIZE=$((2 << 30))
+echo $((16 << 20)) > $region_path/align
+for i in $((1 << 30)) $((2 << 20)) $((4 << 10))
+do
+	json=$($NDCTL create-namespace -r $region -s $SIZE -m fsdax -a $i)
+	eval $(json2var <<< "$json")
+	ns2=$dev
+	$NDCTL disable-namespace $dev
+	$NDCTL enable-namespace $dev
+	$NDCTL destroy-namespace $dev -f
+	unset ns2
+done
+
+# final cleanup
+$NDCTL destroy-namespace $ns1 -f
+exit 0
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [ndctl PATCH 27/36] ndctl/test: Relax dax_pmem_compat requirement
  2020-02-29 20:22 ` [ndctl PATCH 27/36] ndctl/test: Relax dax_pmem_compat requirement Dan Williams
@ 2020-03-03 13:28   ` Jan Kara
  2020-03-03 21:05     ` Dan Williams
  2020-03-03 22:58   ` [ndctl PATCH v2 1/2] ndctl/test: Cleanup test-vs-production nvdimm module detection Dan Williams
  2020-03-03 22:58   ` [ndctl PATCH v2 2/2] ndctl/test: Relax dax_pmem_compat requirement Dan Williams
  2 siblings, 1 reply; 44+ messages in thread
From: Jan Kara @ 2020-03-03 13:28 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-nvdimm, Jan Kara

On Sat 29-02-20 12:22:28, Dan Williams wrote:
> While there are some tests that require the new "dax-bus" device model,
> none of the tests require compatibility mode. Drop the requirement so
> the tests work with DEV_DAX_PMEM_COMPAT=n kernels.
> 
> Link: http://lore.kernel.org/r/20200123154720.12097-1-jack@suse.cz
> Cc: Jan Kara <jack@suse.cz>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

The patch looks good to me. Thanks for fixing this! I just have to say that
the strstr(3) usage in this function looks rather unusual to me. Why not
just strcmp(3)?

								Honza

> ---
>  test/core.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/test/core.c b/test/core.c
> index 888f5d8c0e42..dff842a9f378 100644
> --- a/test/core.c
> +++ b/test/core.c
> @@ -180,6 +180,14 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>  
>  retry:
>  		rc = kmod_module_new_from_name(*ctx, name, mod);
> +
> +		/*
> +		 * dax_pmem_compat is not required, missing is ok,
> +		 * present-but-production is not ok.
> +		 */
> +		if (rc && strstr(name, "dax_pmem_compat"))
> +			continue;
> +
>  		if (rc) {
>  			log_err(&log_ctx, "%s.ko: missing\n", name);
>  			break;
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [ndctl PATCH 27/36] ndctl/test: Relax dax_pmem_compat requirement
  2020-03-03 13:28   ` Jan Kara
@ 2020-03-03 21:05     ` Dan Williams
  0 siblings, 0 replies; 44+ messages in thread
From: Dan Williams @ 2020-03-03 21:05 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-nvdimm

On Tue, Mar 3, 2020 at 5:29 AM Jan Kara <jack@suse.cz> wrote:
>
> On Sat 29-02-20 12:22:28, Dan Williams wrote:
> > While there are some tests that require the new "dax-bus" device model,
> > none of the tests require compatibility mode. Drop the requirement so
> > the tests work with DEV_DAX_PMEM_COMPAT=n kernels.
> >
> > Link: http://lore.kernel.org/r/20200123154720.12097-1-jack@suse.cz
> > Cc: Jan Kara <jack@suse.cz>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> The patch looks good to me. Thanks for fixing this! I just have to say that
> the strstr(3) usage in this function looks rather unusual to me. Why not
> just strcmp(3)?

Yeah, this is confusing the 'name' check with the 'path' check, the
name check should be fine to use strcmp(). Will fix up.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH v2 1/2] ndctl/test: Cleanup test-vs-production nvdimm module detection
  2020-02-29 20:22 ` [ndctl PATCH 27/36] ndctl/test: Relax dax_pmem_compat requirement Dan Williams
  2020-03-03 13:28   ` Jan Kara
@ 2020-03-03 22:58   ` Dan Williams
  2020-03-04 12:44     ` Jan Kara
  2020-03-03 22:58   ` [ndctl PATCH v2 2/2] ndctl/test: Relax dax_pmem_compat requirement Dan Williams
  2 siblings, 1 reply; 44+ messages in thread
From: Dan Williams @ 2020-03-03 22:58 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Jan Kara

Update nfit_test_init() to use strcmp() instead of strstr() to filter
which modules are probed to come from the out-of-tree unit-test set.

Reported-by: Jan Kara <jack@suse.cz>
Link: http://lore.kernel.org/r/20200303132850.GA21048@quack2.suse.cz
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/core.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/core.c b/test/core.c
index 888f5d8c0e42..3aa746fe6786 100644
--- a/test/core.c
+++ b/test/core.c
@@ -164,7 +164,7 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
 		 * Don't check for device-dax modules on kernels older
 		 * than 4.7.
 		 */
-		if (strstr(name, "dax")
+		if (strcmp(name, "dax") == 0
 				&& !ndctl_test_attempt(test,
 					KERNEL_VERSION(4, 7, 0)))
 			continue;
@@ -172,8 +172,8 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
 		/*
 		 * Skip device-dax bus-model modules on pre-v5.1
 		 */
-		if ((strstr(name, "dax_pmem_core")
-				|| strstr(name, "dax_pmem_compat"))
+		if ((strcmp(name, "dax_pmem_core") == 0
+				|| strcmp(name, "dax_pmem_compat") == 0)
 				&& !ndctl_test_attempt(test,
 					KERNEL_VERSION(5, 1, 0)))
 			continue;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH v2 2/2] ndctl/test: Relax dax_pmem_compat requirement
  2020-02-29 20:22 ` [ndctl PATCH 27/36] ndctl/test: Relax dax_pmem_compat requirement Dan Williams
  2020-03-03 13:28   ` Jan Kara
  2020-03-03 22:58   ` [ndctl PATCH v2 1/2] ndctl/test: Cleanup test-vs-production nvdimm module detection Dan Williams
@ 2020-03-03 22:58   ` Dan Williams
  2020-03-04 12:44     ` Jan Kara
  2 siblings, 1 reply; 44+ messages in thread
From: Dan Williams @ 2020-03-03 22:58 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Jan Kara

While there are some tests that require the new "dax-bus" device model,
none of the tests require compatibility mode. Drop the requirement so
the tests work with DEV_DAX_PMEM_COMPAT=n kernels.

Link: http://lore.kernel.org/r/20200123154720.12097-1-jack@suse.cz
Cc: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/core.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/test/core.c b/test/core.c
index 3aa746fe6786..5118d86483d4 100644
--- a/test/core.c
+++ b/test/core.c
@@ -180,6 +180,14 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
 
 retry:
 		rc = kmod_module_new_from_name(*ctx, name, mod);
+
+		/*
+		 * dax_pmem_compat is not required, missing is ok,
+		 * present-but-production is not ok.
+		 */
+		if (rc && strcmp(name, "dax_pmem_compat") == 0)
+			continue;
+
 		if (rc) {
 			log_err(&log_ctx, "%s.ko: missing\n", name);
 			break;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [ndctl PATCH v2 1/2] ndctl/test: Cleanup test-vs-production nvdimm module detection
  2020-03-03 22:58   ` [ndctl PATCH v2 1/2] ndctl/test: Cleanup test-vs-production nvdimm module detection Dan Williams
@ 2020-03-04 12:44     ` Jan Kara
  0 siblings, 0 replies; 44+ messages in thread
From: Jan Kara @ 2020-03-04 12:44 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-nvdimm, Jan Kara

On Tue 03-03-20 14:58:30, Dan Williams wrote:
> Update nfit_test_init() to use strcmp() instead of strstr() to filter
> which modules are probed to come from the out-of-tree unit-test set.
> 
> Reported-by: Jan Kara <jack@suse.cz>
> Link: http://lore.kernel.org/r/20200303132850.GA21048@quack2.suse.cz
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  test/core.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/test/core.c b/test/core.c
> index 888f5d8c0e42..3aa746fe6786 100644
> --- a/test/core.c
> +++ b/test/core.c
> @@ -164,7 +164,7 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>  		 * Don't check for device-dax modules on kernels older
>  		 * than 4.7.
>  		 */
> -		if (strstr(name, "dax")
> +		if (strcmp(name, "dax") == 0
>  				&& !ndctl_test_attempt(test,
>  					KERNEL_VERSION(4, 7, 0)))
>  			continue;
> @@ -172,8 +172,8 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>  		/*
>  		 * Skip device-dax bus-model modules on pre-v5.1
>  		 */
> -		if ((strstr(name, "dax_pmem_core")
> -				|| strstr(name, "dax_pmem_compat"))
> +		if ((strcmp(name, "dax_pmem_core") == 0
> +				|| strcmp(name, "dax_pmem_compat") == 0)
>  				&& !ndctl_test_attempt(test,
>  					KERNEL_VERSION(5, 1, 0)))
>  			continue;
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [ndctl PATCH v2 2/2] ndctl/test: Relax dax_pmem_compat requirement
  2020-03-03 22:58   ` [ndctl PATCH v2 2/2] ndctl/test: Relax dax_pmem_compat requirement Dan Williams
@ 2020-03-04 12:44     ` Jan Kara
  0 siblings, 0 replies; 44+ messages in thread
From: Jan Kara @ 2020-03-04 12:44 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-nvdimm, Jan Kara

On Tue 03-03-20 14:58:35, Dan Williams wrote:
> While there are some tests that require the new "dax-bus" device model,
> none of the tests require compatibility mode. Drop the requirement so
> the tests work with DEV_DAX_PMEM_COMPAT=n kernels.
> 
> Link: http://lore.kernel.org/r/20200123154720.12097-1-jack@suse.cz
> Cc: Jan Kara <jack@suse.cz>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  test/core.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/test/core.c b/test/core.c
> index 3aa746fe6786..5118d86483d4 100644
> --- a/test/core.c
> +++ b/test/core.c
> @@ -180,6 +180,14 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>  
>  retry:
>  		rc = kmod_module_new_from_name(*ctx, name, mod);
> +
> +		/*
> +		 * dax_pmem_compat is not required, missing is ok,
> +		 * present-but-production is not ok.
> +		 */
> +		if (rc && strcmp(name, "dax_pmem_compat") == 0)
> +			continue;
> +
>  		if (rc) {
>  			log_err(&log_ctx, "%s.ko: missing\n", name);
>  			break;
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [ndctl PATCH 00/36] Multiple topics / backlog for v68
  2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
                   ` (35 preceding siblings ...)
  2020-02-29 20:23 ` [ndctl PATCH 36/36] ndctl/test: Regression test misaligned namespaces Dan Williams
@ 2020-03-19  4:13 ` Verma, Vishal L
  36 siblings, 0 replies; 44+ messages in thread
From: Verma, Vishal L @ 2020-03-19  4:13 UTC (permalink / raw)
  To: Williams, Dan J, linux-nvdimm

On Sat, 2020-02-29 at 12:20 -0800, Dan Williams wrote:
> Changes from review:
> - Add NDCTL_LIST_LINT to not regress list output (Jeff)
> - Add kernel-doc description for ndctl_region_set_align() (Jeff)
> 
> ---
> 
> About half of these have been posted previously, but have been reworked
> and revised as they have percolated in my tree relative to other
> arriving features. Yes, it is quite a lot to ingest at once, but given
> the interdependencies and need to catch up I decided to post it all
> together.
> 
> The recommendation on how to review is to start with the new tests,
> those introduce some new commands, and those new commands introduce some
> new library routines. The rest are miscellaneous updates, fixes, and
> cleanups.
> 
This all looks good to me - you can add:
Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>


_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

end of thread, other threads:[~2020-03-19  4:14 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-29 20:20 [ndctl PATCH 00/36] Multiple topics / backlog for v68 Dan Williams
2020-02-29 20:20 ` [ndctl PATCH 01/36] ndctl/list: Add 'target_node' to region and namespace verbose listings Dan Williams
2020-02-29 20:20 ` [ndctl PATCH 02/36] ndctl/docs: Fix mailing list sign-up link Dan Williams
2020-02-29 20:20 ` [ndctl PATCH 03/36] ndctl/list: Drop named list objects from verbose listing Dan Williams
2020-02-29 20:20 ` [ndctl PATCH 04/36] daxctl/list: Avoid memory operations without resource data Dan Williams
2020-02-29 20:20 ` [ndctl PATCH 05/36] ndctl/build: Fix distcheck Dan Williams
2020-02-29 20:20 ` [ndctl PATCH 06/36] ndctl/namespace: Fix destroy-namespace accounting relative to seed devices Dan Williams
2020-02-29 20:20 ` [ndctl PATCH 07/36] ndctl/region: Support ndctl_region_{get, set}_align() Dan Williams
2020-02-29 20:20 ` [ndctl PATCH 08/36] ndctl/namespace: Emit better errors on failure Dan Williams
2020-02-29 20:20 ` [ndctl PATCH 09/36] ndctl/namespace: Check for region alignment violations Dan Williams
2020-02-29 20:20 ` [ndctl PATCH 10/36] ndctl/util: Up-level is_power_of_2() and introduce IS_ALIGNED Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 11/36] ndctl/namespace: Validate resource alignment for dax-mode namespaces Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 12/36] ndctl/namespace: Add read-infoblock command Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 13/36] ndctl/test: Update dax-dev to handle multiple e820 ranges Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 14/36] ndctl/namespace: Always zero info-blocks Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 15/36] ndctl/namespace: Disable autorecovery of create-namespace failures Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 16/36] ndctl/build: Fix EXTRA_DIST already defined errors Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 17/36] ndctl/test: Checkout device-mapper + dax operation Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 18/36] ndctl/test: Exercise sub-section sized namespace creation/deletion Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 19/36] ndctl/namespace: Kill off the legacy mode names Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 20/36] ndctl/namespace: Introduce mode-to-name and name-to-mode helpers Dan Williams
2020-02-29 20:21 ` [ndctl PATCH 21/36] ndctl/namespace: Validate namespace size within validate_namespace_options() Dan Williams
2020-02-29 20:22 ` [ndctl PATCH 22/36] ndctl/namespace: Clarify 16M minimum size requirement Dan Williams
2020-02-29 20:22 ` [ndctl PATCH 23/36] ndctl/test: Regression test 'failed to track' Dan Williams
2020-02-29 20:22 ` [ndctl PATCH 24/36] ndctl/dimm: Rework dimm command status reporting Dan Williams
2020-02-29 20:22 ` [ndctl PATCH 25/36] ndctl/dimm: Rework iteration to drop unaligned pointers Dan Williams
2020-02-29 20:22 ` [ndctl PATCH 26/36] ndctl/test: Fix typos / loss of tpm.handle in security test Dan Williams
2020-02-29 20:22 ` [ndctl PATCH 27/36] ndctl/test: Relax dax_pmem_compat requirement Dan Williams
2020-03-03 13:28   ` Jan Kara
2020-03-03 21:05     ` Dan Williams
2020-03-03 22:58   ` [ndctl PATCH v2 1/2] ndctl/test: Cleanup test-vs-production nvdimm module detection Dan Williams
2020-03-04 12:44     ` Jan Kara
2020-03-03 22:58   ` [ndctl PATCH v2 2/2] ndctl/test: Relax dax_pmem_compat requirement Dan Williams
2020-03-04 12:44     ` Jan Kara
2020-02-29 20:22 ` [ndctl PATCH 28/36] ndctl/namespace: Fix namespace-action vs namespace-mode confusion Dan Williams
2020-02-29 20:22 ` [ndctl PATCH 29/36] ndctl/namespace: Update 'pfn' infoblock definition Dan Williams
2020-02-29 20:22 ` [ndctl PATCH 30/36] ndctl/util: Return 0 for NULL arguments to parse_size64() Dan Williams
2020-02-29 20:22 ` [ndctl PATCH 31/36] ndctl/namespace: Fix read-info-block vs read-infoblock Dan Williams
2020-02-29 20:22 ` [ndctl PATCH 32/36] ndctl/namespace: Parse infoblocks from stdin Dan Williams
2020-02-29 20:23 ` [ndctl PATCH 33/36] ndctl/namespace: Add write-infoblock command Dan Williams
2020-02-29 20:23 ` [ndctl PATCH 34/36] ndctl/list: Add option to list configured + disabled namespaces Dan Williams
2020-02-29 20:23 ` [ndctl PATCH 35/36] ndctl/lib/namespace: Fix resource retrieval after size change Dan Williams
2020-02-29 20:23 ` [ndctl PATCH 36/36] ndctl/test: Regression test misaligned namespaces Dan Williams
2020-03-19  4:13 ` [ndctl PATCH 00/36] Multiple topics / backlog for v68 Verma, Vishal L

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