[PATCH 1/4] libndctl: Unify adding dimms for papr and nfit families

[PATCH 1/4] libndctl: Unify adding dimms for papr and nfit families

[Santosh Sivaraj]

In preparation for enabling tests on non-nfit devices, unify both, already very
similar, functions into one. This will help in adding all attributes needed for
the unit tests. Since the function doesn't fail if some of the dimm attributes
are missing, this will work fine on PAPR platforms though only part of the DIMM
attributes are provided (This doesn't mean that all of the DIMM attributes can
be missing).

Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
 ndctl/lib/libndctl.c | 103 ++++++++++++++++---------------------------
 1 file changed, 38 insertions(+), 65 deletions(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 36fb6fe..26b9317 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -1646,41 +1646,9 @@ static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
 static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
 static struct kmod_module *to_module(struct ndctl_ctx *ctx, const char *alias);
 
-static int add_papr_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
-{
-	int rc = -ENODEV;
-	char buf[SYSFS_ATTR_SIZE];
-	struct ndctl_ctx *ctx = dimm->bus->ctx;
-	char *path = calloc(1, strlen(dimm_base) + 100);
-	const char * const devname = ndctl_dimm_get_devname(dimm);
-
-	dbg(ctx, "%s: Probing of_pmem dimm at %s\n", devname, dimm_base);
-
-	if (!path)
-		return -ENOMEM;
-
-	/* construct path to the papr compatible dimm flags file */
-	sprintf(path, "%s/papr/flags", dimm_base);
-
-	if (ndctl_bus_is_papr_scm(dimm->bus) &&
-	    sysfs_read_attr(ctx, path, buf) == 0) {
-
-		dbg(ctx, "%s: Adding papr-scm dimm flags:\"%s\"\n", devname, buf);
-		dimm->cmd_family = NVDIMM_FAMILY_PAPR;
-
-		/* Parse dimm flags */
-		parse_papr_flags(dimm, buf);
-
-		/* Allocate monitor mode fd */
-		dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
-		rc = 0;
-	}
-
-	free(path);
-	return rc;
-}
-
-static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
+static int populate_dimm_attributes(struct ndctl_dimm *dimm,
+				    const char *dimm_base,
+				    const char *bus_prefix)
 {
 	int i, rc = -1;
 	char buf[SYSFS_ATTR_SIZE];
@@ -1694,7 +1662,7 @@ static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
 	 * 'unique_id' may not be available on older kernels, so don't
 	 * fail if the read fails.
 	 */
-	sprintf(path, "%s/nfit/id", dimm_base);
+	sprintf(path, "%s/%s/id", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0) {
 		unsigned int b[9];
 
@@ -1709,68 +1677,74 @@ static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
 		}
 	}
 
-	sprintf(path, "%s/nfit/handle", dimm_base);
+	sprintf(path, "%s/%s/handle", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		goto err_read;
 	dimm->handle = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/phys_id", dimm_base);
+	sprintf(path, "%s/%s/phys_id", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		goto err_read;
 	dimm->phys_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/serial", dimm_base);
+	sprintf(path, "%s/%s/serial", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->serial = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/vendor", dimm_base);
+	sprintf(path, "%s/%s/vendor", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->vendor_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/device", dimm_base);
+	sprintf(path, "%s/%s/device", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->device_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/rev_id", dimm_base);
+	sprintf(path, "%s/%s/rev_id", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->revision_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/dirty_shutdown", dimm_base);
+	sprintf(path, "%s/%s/dirty_shutdown", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->dirty_shutdown = strtoll(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/subsystem_vendor", dimm_base);
+	sprintf(path, "%s/%s/subsystem_vendor", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->subsystem_vendor_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/subsystem_device", dimm_base);
+	sprintf(path, "%s/%s/subsystem_device", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->subsystem_device_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/subsystem_rev_id", dimm_base);
+	sprintf(path, "%s/%s/subsystem_rev_id", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->subsystem_revision_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/family", dimm_base);
+	sprintf(path, "%s/%s/family", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->cmd_family = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/dsm_mask", dimm_base);
+	sprintf(path, "%s/%s/dsm_mask", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->nfit_dsm_mask = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/format", dimm_base);
+	sprintf(path, "%s/%s/format", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->format[0] = strtoul(buf, NULL, 0);
 	for (i = 1; i < dimm->formats; i++) {
-		sprintf(path, "%s/nfit/format%d", dimm_base, i);
+		sprintf(path, "%s/%s/format%d", dimm_base, bus_prefix, i);
 		if (sysfs_read_attr(ctx, path, buf) == 0)
 			dimm->format[i] = strtoul(buf, NULL, 0);
 	}
 
-	sprintf(path, "%s/nfit/flags", dimm_base);
-	if (sysfs_read_attr(ctx, path, buf) == 0)
-		parse_nfit_mem_flags(dimm, buf);
+	sprintf(path, "%s/%s/flags", dimm_base, bus_prefix);
+	if (sysfs_read_attr(ctx, path, buf) == 0) {
+		if (ndctl_bus_has_nfit(dimm->bus))
+			parse_nfit_mem_flags(dimm, buf);
+		else if (ndctl_bus_is_papr_scm(dimm->bus)) {
+			dimm->cmd_family = NVDIMM_FAMILY_PAPR;
+			parse_papr_flags(dimm, buf);
+		}
+	}
 
 	dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
 	rc = 0;
@@ -1792,7 +1766,8 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
 	if (!path)
 		return NULL;
 
-	sprintf(path, "%s/nfit/formats", dimm_base);
+	sprintf(path, "%s/%s/formats", dimm_base,
+		ndctl_bus_has_nfit(bus) ? "nfit" : "papr");
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		formats = 1;
 	else
@@ -1866,13 +1841,12 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
 	else
 		dimm->fwa_result = fwa_result_to_result(buf);
 
+	dimm->formats = formats;
 	/* Check if the given dimm supports nfit */
 	if (ndctl_bus_has_nfit(bus)) {
-		dimm->formats = formats;
-		rc = add_nfit_dimm(dimm, dimm_base);
-	} else if (ndctl_bus_has_of_node(bus)) {
-		rc = add_papr_dimm(dimm, dimm_base);
-	}
+		rc = populate_dimm_attributes(dimm, dimm_base, "nfit");
+	} else if (ndctl_bus_has_of_node(bus))
+		rc = populate_dimm_attributes(dimm, dimm_base, "papr");
 
 	if (rc == -ENODEV) {
 		/* Unprobed dimm with no family */
@@ -2531,13 +2505,12 @@ static void *add_region(void *parent, int id, const char *region_base)
 		goto err_read;
 	region->num_mappings = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/range_index", region_base);
-	if (ndctl_bus_has_nfit(bus)) {
-		if (sysfs_read_attr(ctx, path, buf) < 0)
-			goto err_read;
-		region->range_index = strtoul(buf, NULL, 0);
-	} else
+	sprintf(path, "%s/%s/range_index", region_base,
+		ndctl_bus_has_nfit(bus) ? "nfit": "papr");
+	if (sysfs_read_attr(ctx, path, buf) < 0)
 		region->range_index = -1;
+	else
+		region->range_index = strtoul(buf, NULL, 0);
 
 	sprintf(path, "%s/read_only", region_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
-- 
2.30.2
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 2/4] test: Don't skip tests if nfit modules are missing

[Santosh Sivaraj]

For NFIT to be available ACPI is a must, so don't fail when nfit modules
are missing on a platform that doesn't support ACPI.

Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
 test.h                        |  2 +-
 test/ack-shutdown-count-set.c |  2 +-
 test/blk_namespaces.c         |  2 +-
 test/core.c                   | 30 ++++++++++++++++++++++++++++--
 test/dpa-alloc.c              |  2 +-
 test/dsm-fail.c               |  2 +-
 test/libndctl.c               |  2 +-
 test/multi-pmem.c             |  2 +-
 test/parent-uuid.c            |  2 +-
 test/pmem_namespaces.c        |  2 +-
 10 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/test.h b/test.h
index cba8d41..7de13fe 100644
--- a/test.h
+++ b/test.h
@@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
 
 struct kmod_ctx;
 struct kmod_module;
-int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
+int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
 		struct ndctl_ctx *nd_ctx, int log_level,
 		struct ndctl_test *test);
 
diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
index fb1d82b..c561ff3 100644
--- a/test/ack-shutdown-count-set.c
+++ b/test/ack-shutdown-count-set.c
@@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct ndctl_test *test,
 	int result = EXIT_FAILURE, err;
 
 	ndctl_set_log_priority(ctx, loglevel);
-	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
+	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
 	if (err < 0) {
 		result = 77;
 		ndctl_test_skip(test);
diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
index d7f00cb..f076e85 100644
--- a/test/blk_namespaces.c
+++ b/test/blk_namespaces.c
@@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test *test,
 
 	if (!bus) {
 		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
-		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
+		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
 		ndctl_invalidate(ctx);
 		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
 		if (rc < 0 || !bus) {
diff --git a/test/core.c b/test/core.c
index cc7d8d9..44cb277 100644
--- a/test/core.c
+++ b/test/core.c
@@ -11,6 +11,7 @@
 #include <util/log.h>
 #include <util/sysfs.h>
 #include <ndctl/libndctl.h>
+#include <ndctl/ndctl.h>
 #include <ccan/array_size/array_size.h>
 
 #define KVER_STRLEN 20
@@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
 	return test->skip;
 }
 
-int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
+int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
 		struct ndctl_ctx *nd_ctx, int log_level,
 		struct ndctl_test *test)
 {
-	int rc;
+	int rc, family = -1;
 	unsigned int i;
 	const char *name;
 	struct ndctl_bus *bus;
@@ -127,10 +128,30 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
 		"nd_e820",
 		"nd_pmem",
 	};
+	char *test_env;
 
 	log_init(&log_ctx, "test/init", "NDCTL_TEST");
 	log_ctx.log_priority = log_level;
 
+	/*
+	 * The following two checks determine the platform family. For
+	 * Intel/platforms which support ACPI, check sysfs; for other platforms
+	 * determine from the environment variable NVDIMM_TEST_FAMILY
+	 */
+	if (access("/sys/bus/acpi", F_OK) == 0) {
+		if (errno == ENOENT)
+			family = NVDIMM_FAMILY_INTEL;
+	}
+
+	test_env = getenv("NDCTL_TEST_FAMILY");
+	if (test_env && strcmp(test_env, "PAPR") == 0)
+		family = NVDIMM_FAMILY_PAPR;
+
+	if (family == -1) {
+		log_err(&log_ctx, "Cannot determine NVDIMM family\n");
+		return -ENOTSUP;
+	}
+
 	*ctx = kmod_new(NULL, NULL);
 	if (!*ctx)
 		return -ENXIO;
@@ -185,6 +206,11 @@ retry:
 
 		path = kmod_module_get_path(*mod);
 		if (!path) {
+			if (family != NVDIMM_FAMILY_INTEL &&
+			    (strcmp(name, "nfit") == 0 ||
+			     strcmp(name, "nd_e820") == 0))
+				continue;
+
 			log_err(&log_ctx, "%s.ko: failed to get path\n", name);
 			break;
 		}
diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
index e922009..0b3bb7a 100644
--- a/test/dpa-alloc.c
+++ b/test/dpa-alloc.c
@@ -289,7 +289,7 @@ int test_dpa_alloc(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
 		return 77;
 
 	ndctl_set_log_priority(ctx, loglevel);
-	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
+	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
 	if (err < 0) {
 		ndctl_test_skip(test);
 		fprintf(stderr, "nfit_test unavailable skipping tests\n");
diff --git a/test/dsm-fail.c b/test/dsm-fail.c
index 9dfd8b0..0a6383d 100644
--- a/test/dsm-fail.c
+++ b/test/dsm-fail.c
@@ -346,7 +346,7 @@ int test_dsm_fail(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
 	int result = EXIT_FAILURE, err;
 
 	ndctl_set_log_priority(ctx, loglevel);
-	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
+	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
 	if (err < 0) {
 		result = 77;
 		ndctl_test_skip(test);
diff --git a/test/libndctl.c b/test/libndctl.c
index 24d72b3..0e88fce 100644
--- a/test/libndctl.c
+++ b/test/libndctl.c
@@ -2692,7 +2692,7 @@ int test_libndctl(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
 	daxctl_set_log_priority(daxctl_ctx, loglevel);
 	ndctl_set_private_data(ctx, test);
 
-	err = nfit_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
+	err = ndctl_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
 	if (err < 0) {
 		ndctl_test_skip(test);
 		fprintf(stderr, "nfit_test unavailable skipping tests\n");
diff --git a/test/multi-pmem.c b/test/multi-pmem.c
index 3d10952..3ea08cc 100644
--- a/test/multi-pmem.c
+++ b/test/multi-pmem.c
@@ -249,7 +249,7 @@ int test_multi_pmem(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx
 
 	ndctl_set_log_priority(ctx, loglevel);
 
-	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
+	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
 	if (err < 0) {
 		result = 77;
 		ndctl_test_skip(test);
diff --git a/test/parent-uuid.c b/test/parent-uuid.c
index 6424e9f..bded33a 100644
--- a/test/parent-uuid.c
+++ b/test/parent-uuid.c
@@ -218,7 +218,7 @@ int test_parent_uuid(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ct
 		return 77;
 
 	ndctl_set_log_priority(ctx, loglevel);
-	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
+	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
 	if (err < 0) {
 		ndctl_test_skip(test);
 		fprintf(stderr, "nfit_test unavailable skipping tests\n");
diff --git a/test/pmem_namespaces.c b/test/pmem_namespaces.c
index f0f2edd..a4db1ae 100644
--- a/test/pmem_namespaces.c
+++ b/test/pmem_namespaces.c
@@ -191,7 +191,7 @@ int test_pmem_namespaces(int log_level, struct ndctl_test *test,
 
 	if (!bus) {
 		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
-		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
+		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
 		ndctl_invalidate(ctx);
 		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
 		if (rc < 0 || !bus) {
-- 
2.30.2
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 2/4] test: Don't skip tests if nfit modules are missing

[Aneesh Kumar K.V]

Santosh Sivaraj <santosh@fossix.org> writes:

> For NFIT to be available ACPI is a must, so don't fail when nfit modules
> are missing on a platform that doesn't support ACPI.
>
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
>  test.h                        |  2 +-
>  test/ack-shutdown-count-set.c |  2 +-
>  test/blk_namespaces.c         |  2 +-
>  test/core.c                   | 30 ++++++++++++++++++++++++++++--
>  test/dpa-alloc.c              |  2 +-
>  test/dsm-fail.c               |  2 +-
>  test/libndctl.c               |  2 +-
>  test/multi-pmem.c             |  2 +-
>  test/parent-uuid.c            |  2 +-
>  test/pmem_namespaces.c        |  2 +-
>  10 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/test.h b/test.h
> index cba8d41..7de13fe 100644
> --- a/test.h
> +++ b/test.h
> @@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
>  
>  struct kmod_ctx;
>  struct kmod_module;
> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>  		struct ndctl_ctx *nd_ctx, int log_level,
>  		struct ndctl_test *test);
>  
> diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
> index fb1d82b..c561ff3 100644
> --- a/test/ack-shutdown-count-set.c
> +++ b/test/ack-shutdown-count-set.c
> @@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct ndctl_test *test,
>  	int result = EXIT_FAILURE, err;
>  
>  	ndctl_set_log_priority(ctx, loglevel);
> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>  	if (err < 0) {
>  		result = 77;
>  		ndctl_test_skip(test);
> diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
> index d7f00cb..f076e85 100644
> --- a/test/blk_namespaces.c
> +++ b/test/blk_namespaces.c
> @@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test *test,
>  
>  	if (!bus) {
>  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
> -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
> +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>  		ndctl_invalidate(ctx);
>  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>  		if (rc < 0 || !bus) {
> diff --git a/test/core.c b/test/core.c
> index cc7d8d9..44cb277 100644
> --- a/test/core.c
> +++ b/test/core.c
> @@ -11,6 +11,7 @@
>  #include <util/log.h>
>  #include <util/sysfs.h>
>  #include <ndctl/libndctl.h>
> +#include <ndctl/ndctl.h>
>  #include <ccan/array_size/array_size.h>
>  
>  #define KVER_STRLEN 20
> @@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
>  	return test->skip;
>  }
>  
> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>  		struct ndctl_ctx *nd_ctx, int log_level,
>  		struct ndctl_test *test)
>  {
> -	int rc;
> +	int rc, family = -1;
>  	unsigned int i;
>  	const char *name;
>  	struct ndctl_bus *bus;
> @@ -127,10 +128,30 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>  		"nd_e820",
>  		"nd_pmem",
>  	};
> +	char *test_env;
>  
>  	log_init(&log_ctx, "test/init", "NDCTL_TEST");
>  	log_ctx.log_priority = log_level;
>  
> +	/*
> +	 * The following two checks determine the platform family. For
> +	 * Intel/platforms which support ACPI, check sysfs; for other platforms
> +	 * determine from the environment variable NVDIMM_TEST_FAMILY
> +	 */
> +	if (access("/sys/bus/acpi", F_OK) == 0) {
> +		if (errno == ENOENT)
> +			family = NVDIMM_FAMILY_INTEL;
> +	}
> +
> +	test_env = getenv("NDCTL_TEST_FAMILY");
> +	if (test_env && strcmp(test_env, "PAPR") == 0)
> +		family = NVDIMM_FAMILY_PAPR;

I am wondering whether it is confusing to call this as
NVDIMM_FAMILY_PAPR. If you are looking at a platform agnoistic family we
should probably name it accordingly. Maybe NVDIMM_FAMILY_TEST ?


> +
> +	if (family == -1) {
> +		log_err(&log_ctx, "Cannot determine NVDIMM family\n");
> +		return -ENOTSUP;
> +	}
> +
>  	*ctx = kmod_new(NULL, NULL);
>  	if (!*ctx)
>  		return -ENXIO;
> @@ -185,6 +206,11 @@ retry:
>  
>  		path = kmod_module_get_path(*mod);
>  		if (!path) {
> +			if (family != NVDIMM_FAMILY_INTEL &&
> +			    (strcmp(name, "nfit") == 0 ||
> +			     strcmp(name, "nd_e820") == 0))
> +				continue;
> +
>  			log_err(&log_ctx, "%s.ko: failed to get path\n", name);
>  			break;
>  		}
> diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
> index e922009..0b3bb7a 100644
> --- a/test/dpa-alloc.c
> +++ b/test/dpa-alloc.c
> @@ -289,7 +289,7 @@ int test_dpa_alloc(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>  		return 77;
>  
>  	ndctl_set_log_priority(ctx, loglevel);
> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>  	if (err < 0) {
>  		ndctl_test_skip(test);
>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
> diff --git a/test/dsm-fail.c b/test/dsm-fail.c
> index 9dfd8b0..0a6383d 100644
> --- a/test/dsm-fail.c
> +++ b/test/dsm-fail.c
> @@ -346,7 +346,7 @@ int test_dsm_fail(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>  	int result = EXIT_FAILURE, err;
>  
>  	ndctl_set_log_priority(ctx, loglevel);
> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>  	if (err < 0) {
>  		result = 77;
>  		ndctl_test_skip(test);
> diff --git a/test/libndctl.c b/test/libndctl.c
> index 24d72b3..0e88fce 100644
> --- a/test/libndctl.c
> +++ b/test/libndctl.c
> @@ -2692,7 +2692,7 @@ int test_libndctl(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>  	daxctl_set_log_priority(daxctl_ctx, loglevel);
>  	ndctl_set_private_data(ctx, test);
>  
> -	err = nfit_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
>  	if (err < 0) {
>  		ndctl_test_skip(test);
>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
> diff --git a/test/multi-pmem.c b/test/multi-pmem.c
> index 3d10952..3ea08cc 100644
> --- a/test/multi-pmem.c
> +++ b/test/multi-pmem.c
> @@ -249,7 +249,7 @@ int test_multi_pmem(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx
>  
>  	ndctl_set_log_priority(ctx, loglevel);
>  
> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>  	if (err < 0) {
>  		result = 77;
>  		ndctl_test_skip(test);
> diff --git a/test/parent-uuid.c b/test/parent-uuid.c
> index 6424e9f..bded33a 100644
> --- a/test/parent-uuid.c
> +++ b/test/parent-uuid.c
> @@ -218,7 +218,7 @@ int test_parent_uuid(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ct
>  		return 77;
>  
>  	ndctl_set_log_priority(ctx, loglevel);
> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>  	if (err < 0) {
>  		ndctl_test_skip(test);
>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
> diff --git a/test/pmem_namespaces.c b/test/pmem_namespaces.c
> index f0f2edd..a4db1ae 100644
> --- a/test/pmem_namespaces.c
> +++ b/test/pmem_namespaces.c
> @@ -191,7 +191,7 @@ int test_pmem_namespaces(int log_level, struct ndctl_test *test,
>  
>  	if (!bus) {
>  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
> -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
> +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>  		ndctl_invalidate(ctx);
>  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>  		if (rc < 0 || !bus) {
> -- 
> 2.30.2
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 2/4] test: Don't skip tests if nfit modules are missing

[Santosh Sivaraj]

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:

> Santosh Sivaraj <santosh@fossix.org> writes:
>
>> For NFIT to be available ACPI is a must, so don't fail when nfit modules
>> are missing on a platform that doesn't support ACPI.
>>
>> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>> ---
>>  test.h                        |  2 +-
>>  test/ack-shutdown-count-set.c |  2 +-
>>  test/blk_namespaces.c         |  2 +-
>>  test/core.c                   | 30 ++++++++++++++++++++++++++++--
>>  test/dpa-alloc.c              |  2 +-
>>  test/dsm-fail.c               |  2 +-
>>  test/libndctl.c               |  2 +-
>>  test/multi-pmem.c             |  2 +-
>>  test/parent-uuid.c            |  2 +-
>>  test/pmem_namespaces.c        |  2 +-
>>  10 files changed, 37 insertions(+), 11 deletions(-)
>>
>> diff --git a/test.h b/test.h
>> index cba8d41..7de13fe 100644
>> --- a/test.h
>> +++ b/test.h
>> @@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
>>  
>>  struct kmod_ctx;
>>  struct kmod_module;
>> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>  		struct ndctl_ctx *nd_ctx, int log_level,
>>  		struct ndctl_test *test);
>>  
>> diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
>> index fb1d82b..c561ff3 100644
>> --- a/test/ack-shutdown-count-set.c
>> +++ b/test/ack-shutdown-count-set.c
>> @@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct ndctl_test *test,
>>  	int result = EXIT_FAILURE, err;
>>  
>>  	ndctl_set_log_priority(ctx, loglevel);
>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>  	if (err < 0) {
>>  		result = 77;
>>  		ndctl_test_skip(test);
>> diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
>> index d7f00cb..f076e85 100644
>> --- a/test/blk_namespaces.c
>> +++ b/test/blk_namespaces.c
>> @@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test *test,
>>  
>>  	if (!bus) {
>>  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
>> -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>> +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>>  		ndctl_invalidate(ctx);
>>  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>>  		if (rc < 0 || !bus) {
>> diff --git a/test/core.c b/test/core.c
>> index cc7d8d9..44cb277 100644
>> --- a/test/core.c
>> +++ b/test/core.c
>> @@ -11,6 +11,7 @@
>>  #include <util/log.h>
>>  #include <util/sysfs.h>
>>  #include <ndctl/libndctl.h>
>> +#include <ndctl/ndctl.h>
>>  #include <ccan/array_size/array_size.h>
>>  
>>  #define KVER_STRLEN 20
>> @@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
>>  	return test->skip;
>>  }
>>  
>> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>  		struct ndctl_ctx *nd_ctx, int log_level,
>>  		struct ndctl_test *test)
>>  {
>> -	int rc;
>> +	int rc, family = -1;
>>  	unsigned int i;
>>  	const char *name;
>>  	struct ndctl_bus *bus;
>> @@ -127,10 +128,30 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>  		"nd_e820",
>>  		"nd_pmem",
>>  	};
>> +	char *test_env;
>>  
>>  	log_init(&log_ctx, "test/init", "NDCTL_TEST");
>>  	log_ctx.log_priority = log_level;
>>  
>> +	/*
>> +	 * The following two checks determine the platform family. For
>> +	 * Intel/platforms which support ACPI, check sysfs; for other platforms
>> +	 * determine from the environment variable NVDIMM_TEST_FAMILY
>> +	 */
>> +	if (access("/sys/bus/acpi", F_OK) == 0) {
>> +		if (errno == ENOENT)
>> +			family = NVDIMM_FAMILY_INTEL;
>> +	}
>> +
>> +	test_env = getenv("NDCTL_TEST_FAMILY");
>> +	if (test_env && strcmp(test_env, "PAPR") == 0)
>> +		family = NVDIMM_FAMILY_PAPR;
>
> I am wondering whether it is confusing to call this as
> NVDIMM_FAMILY_PAPR. If you are looking at a platform agnoistic family we
> should probably name it accordingly. Maybe NVDIMM_FAMILY_TEST ?

This is intentional, so that we can reuse PAPR code for SMART, health and error
injection tests. Otherwise we will end up copying major parts of the code. Also
I am not sure that we should create a separate family for tests.

Thanks,
Santosh

>
>
>> +
>> +	if (family == -1) {
>> +		log_err(&log_ctx, "Cannot determine NVDIMM family\n");
>> +		return -ENOTSUP;
>> +	}
>> +
>>  	*ctx = kmod_new(NULL, NULL);
>>  	if (!*ctx)
>>  		return -ENXIO;
>> @@ -185,6 +206,11 @@ retry:
>>  
>>  		path = kmod_module_get_path(*mod);
>>  		if (!path) {
>> +			if (family != NVDIMM_FAMILY_INTEL &&
>> +			    (strcmp(name, "nfit") == 0 ||
>> +			     strcmp(name, "nd_e820") == 0))
>> +				continue;
>> +
>>  			log_err(&log_ctx, "%s.ko: failed to get path\n", name);
>>  			break;
>>  		}
>> diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
>> index e922009..0b3bb7a 100644
>> --- a/test/dpa-alloc.c
>> +++ b/test/dpa-alloc.c
>> @@ -289,7 +289,7 @@ int test_dpa_alloc(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>>  		return 77;
>>  
>>  	ndctl_set_log_priority(ctx, loglevel);
>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>  	if (err < 0) {
>>  		ndctl_test_skip(test);
>>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
>> diff --git a/test/dsm-fail.c b/test/dsm-fail.c
>> index 9dfd8b0..0a6383d 100644
>> --- a/test/dsm-fail.c
>> +++ b/test/dsm-fail.c
>> @@ -346,7 +346,7 @@ int test_dsm_fail(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>>  	int result = EXIT_FAILURE, err;
>>  
>>  	ndctl_set_log_priority(ctx, loglevel);
>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>  	if (err < 0) {
>>  		result = 77;
>>  		ndctl_test_skip(test);
>> diff --git a/test/libndctl.c b/test/libndctl.c
>> index 24d72b3..0e88fce 100644
>> --- a/test/libndctl.c
>> +++ b/test/libndctl.c
>> @@ -2692,7 +2692,7 @@ int test_libndctl(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>>  	daxctl_set_log_priority(daxctl_ctx, loglevel);
>>  	ndctl_set_private_data(ctx, test);
>>  
>> -	err = nfit_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
>>  	if (err < 0) {
>>  		ndctl_test_skip(test);
>>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
>> diff --git a/test/multi-pmem.c b/test/multi-pmem.c
>> index 3d10952..3ea08cc 100644
>> --- a/test/multi-pmem.c
>> +++ b/test/multi-pmem.c
>> @@ -249,7 +249,7 @@ int test_multi_pmem(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx
>>  
>>  	ndctl_set_log_priority(ctx, loglevel);
>>  
>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>  	if (err < 0) {
>>  		result = 77;
>>  		ndctl_test_skip(test);
>> diff --git a/test/parent-uuid.c b/test/parent-uuid.c
>> index 6424e9f..bded33a 100644
>> --- a/test/parent-uuid.c
>> +++ b/test/parent-uuid.c
>> @@ -218,7 +218,7 @@ int test_parent_uuid(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ct
>>  		return 77;
>>  
>>  	ndctl_set_log_priority(ctx, loglevel);
>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>  	if (err < 0) {
>>  		ndctl_test_skip(test);
>>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
>> diff --git a/test/pmem_namespaces.c b/test/pmem_namespaces.c
>> index f0f2edd..a4db1ae 100644
>> --- a/test/pmem_namespaces.c
>> +++ b/test/pmem_namespaces.c
>> @@ -191,7 +191,7 @@ int test_pmem_namespaces(int log_level, struct ndctl_test *test,
>>  
>>  	if (!bus) {
>>  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
>> -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>> +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>>  		ndctl_invalidate(ctx);
>>  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>>  		if (rc < 0 || !bus) {
>> -- 
>> 2.30.2
>> _______________________________________________
>> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
>> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 2/4] test: Don't skip tests if nfit modules are missing

[Verma, Vishal L]

On Sun, 2021-03-28 at 07:39 +0530, Santosh Sivaraj wrote:
> For NFIT to be available ACPI is a must, so don't fail when nfit modules
> are missing on a platform that doesn't support ACPI.
> 
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
>  test.h                        |  2 +-
>  test/ack-shutdown-count-set.c |  2 +-
>  test/blk_namespaces.c         |  2 +-
>  test/core.c                   | 30 ++++++++++++++++++++++++++++--
>  test/dpa-alloc.c              |  2 +-
>  test/dsm-fail.c               |  2 +-
>  test/libndctl.c               |  2 +-
>  test/multi-pmem.c             |  2 +-
>  test/parent-uuid.c            |  2 +-
>  test/pmem_namespaces.c        |  2 +-
>  10 files changed, 37 insertions(+), 11 deletions(-)
> 

I haven't looked deeper, but this seems to fail the blk-ns test with:

  ACPI.NFIT unavailable falling back to nfit_test
  test/init: ndctl_test_init: Cannot determine NVDIMM family
  __ndctl_test_skip: explicit skip test_blk_namespaces:235
  nfit_test unavailable skipping tests

> diff --git a/test.h b/test.h
> index cba8d41..7de13fe 100644
> --- a/test.h
> +++ b/test.h
> @@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
>  
> 
>  struct kmod_ctx;
>  struct kmod_module;
> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>  		struct ndctl_ctx *nd_ctx, int log_level,
>  		struct ndctl_test *test);
>  
> 
> diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
> index fb1d82b..c561ff3 100644
> --- a/test/ack-shutdown-count-set.c
> +++ b/test/ack-shutdown-count-set.c
> @@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct ndctl_test *test,
>  	int result = EXIT_FAILURE, err;
>  
> 
>  	ndctl_set_log_priority(ctx, loglevel);
> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>  	if (err < 0) {
>  		result = 77;
>  		ndctl_test_skip(test);
> diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
> index d7f00cb..f076e85 100644
> --- a/test/blk_namespaces.c
> +++ b/test/blk_namespaces.c
> @@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test *test,
>  
> 
>  	if (!bus) {
>  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
> -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
> +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>  		ndctl_invalidate(ctx);
>  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>  		if (rc < 0 || !bus) {
> diff --git a/test/core.c b/test/core.c
> index cc7d8d9..44cb277 100644
> --- a/test/core.c
> +++ b/test/core.c
> @@ -11,6 +11,7 @@
>  #include <util/log.h>
>  #include <util/sysfs.h>
>  #include <ndctl/libndctl.h>
> +#include <ndctl/ndctl.h>
>  #include <ccan/array_size/array_size.h>
>  
> 
>  #define KVER_STRLEN 20
> @@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
>  	return test->skip;
>  }
>  
> 
> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>  		struct ndctl_ctx *nd_ctx, int log_level,
>  		struct ndctl_test *test)
>  {
> -	int rc;
> +	int rc, family = -1;
>  	unsigned int i;
>  	const char *name;
>  	struct ndctl_bus *bus;
> @@ -127,10 +128,30 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>  		"nd_e820",
>  		"nd_pmem",
>  	};
> +	char *test_env;
>  
> 
>  	log_init(&log_ctx, "test/init", "NDCTL_TEST");
>  	log_ctx.log_priority = log_level;
>  
> 
> +	/*
> +	 * The following two checks determine the platform family. For
> +	 * Intel/platforms which support ACPI, check sysfs; for other platforms
> +	 * determine from the environment variable NVDIMM_TEST_FAMILY
> +	 */
> +	if (access("/sys/bus/acpi", F_OK) == 0) {
> +		if (errno == ENOENT)
> +			family = NVDIMM_FAMILY_INTEL;
> +	}
> +
> +	test_env = getenv("NDCTL_TEST_FAMILY");
> +	if (test_env && strcmp(test_env, "PAPR") == 0)
> +		family = NVDIMM_FAMILY_PAPR;
> +
> +	if (family == -1) {
> +		log_err(&log_ctx, "Cannot determine NVDIMM family\n");
> +		return -ENOTSUP;
> +	}
> +
>  	*ctx = kmod_new(NULL, NULL);
>  	if (!*ctx)
>  		return -ENXIO;
> @@ -185,6 +206,11 @@ retry:
>  
> 
>  		path = kmod_module_get_path(*mod);
>  		if (!path) {
> +			if (family != NVDIMM_FAMILY_INTEL &&
> +			    (strcmp(name, "nfit") == 0 ||
> +			     strcmp(name, "nd_e820") == 0))
> +				continue;
> +
>  			log_err(&log_ctx, "%s.ko: failed to get path\n", name);
>  			break;
>  		}
> diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
> index e922009..0b3bb7a 100644
> --- a/test/dpa-alloc.c
> +++ b/test/dpa-alloc.c
> @@ -289,7 +289,7 @@ int test_dpa_alloc(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>  		return 77;
>  
> 
>  	ndctl_set_log_priority(ctx, loglevel);
> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>  	if (err < 0) {
>  		ndctl_test_skip(test);
>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
> diff --git a/test/dsm-fail.c b/test/dsm-fail.c
> index 9dfd8b0..0a6383d 100644
> --- a/test/dsm-fail.c
> +++ b/test/dsm-fail.c
> @@ -346,7 +346,7 @@ int test_dsm_fail(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>  	int result = EXIT_FAILURE, err;
>  
> 
>  	ndctl_set_log_priority(ctx, loglevel);
> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>  	if (err < 0) {
>  		result = 77;
>  		ndctl_test_skip(test);
> diff --git a/test/libndctl.c b/test/libndctl.c
> index 24d72b3..0e88fce 100644
> --- a/test/libndctl.c
> +++ b/test/libndctl.c
> @@ -2692,7 +2692,7 @@ int test_libndctl(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>  	daxctl_set_log_priority(daxctl_ctx, loglevel);
>  	ndctl_set_private_data(ctx, test);
>  
> 
> -	err = nfit_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
>  	if (err < 0) {
>  		ndctl_test_skip(test);
>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
> diff --git a/test/multi-pmem.c b/test/multi-pmem.c
> index 3d10952..3ea08cc 100644
> --- a/test/multi-pmem.c
> +++ b/test/multi-pmem.c
> @@ -249,7 +249,7 @@ int test_multi_pmem(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx
>  
> 
>  	ndctl_set_log_priority(ctx, loglevel);
>  
> 
> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>  	if (err < 0) {
>  		result = 77;
>  		ndctl_test_skip(test);
> diff --git a/test/parent-uuid.c b/test/parent-uuid.c
> index 6424e9f..bded33a 100644
> --- a/test/parent-uuid.c
> +++ b/test/parent-uuid.c
> @@ -218,7 +218,7 @@ int test_parent_uuid(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ct
>  		return 77;
>  
> 
>  	ndctl_set_log_priority(ctx, loglevel);
> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>  	if (err < 0) {
>  		ndctl_test_skip(test);
>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
> diff --git a/test/pmem_namespaces.c b/test/pmem_namespaces.c
> index f0f2edd..a4db1ae 100644
> --- a/test/pmem_namespaces.c
> +++ b/test/pmem_namespaces.c
> @@ -191,7 +191,7 @@ int test_pmem_namespaces(int log_level, struct ndctl_test *test,
>  
> 
>  	if (!bus) {
>  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
> -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
> +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>  		ndctl_invalidate(ctx);
>  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>  		if (rc < 0 || !bus) {


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

Re: [PATCH 2/4] test: Don't skip tests if nfit modules are missing

[Santosh Sivaraj]

"Verma, Vishal L" <vishal.l.verma@intel.com> writes:

Hi Vishal,

> On Sun, 2021-03-28 at 07:39 +0530, Santosh Sivaraj wrote:
>> For NFIT to be available ACPI is a must, so don't fail when nfit modules
>> are missing on a platform that doesn't support ACPI.
>> 
>> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>> ---
>>  test.h                        |  2 +-
>>  test/ack-shutdown-count-set.c |  2 +-
>>  test/blk_namespaces.c         |  2 +-
>>  test/core.c                   | 30 ++++++++++++++++++++++++++++--
>>  test/dpa-alloc.c              |  2 +-
>>  test/dsm-fail.c               |  2 +-
>>  test/libndctl.c               |  2 +-
>>  test/multi-pmem.c             |  2 +-
>>  test/parent-uuid.c            |  2 +-
>>  test/pmem_namespaces.c        |  2 +-
>>  10 files changed, 37 insertions(+), 11 deletions(-)
>> 
>
> I haven't looked deeper, but this seems to fail the blk-ns test with:
>
>   ACPI.NFIT unavailable falling back to nfit_test
>   test/init: ndctl_test_init: Cannot determine NVDIMM family
>   __ndctl_test_skip: explicit skip test_blk_namespaces:235
>   nfit_test unavailable skipping tests

The first message will be emitted even without the changes if the bus is not
found. The second error will be emitted when check "/sys/bus/acpi" is not
found. We fail for all other buses by default except for NFIT as before and PAPR
tests are enabled only when NVDIMM_TEST_FAMILY is set to "PAPR".

All tests pass in my setup (x86_64 qemu guest) with the recent upstream kernel,
except for the the below warning from drivers/acpi/nfit/core.c:

[ 2426.727584] ------------[ cut here ]------------
[ 2426.728405] WARNING: CPU: 2 PID: 47504 at tools/testing/nvdimm/../../../drivers/acpi/nfit/core.c:3879 nfit_exit+0x]
[ 2426.730264] Modules linked in: dax_pmem(O) nd_pmem(O) nfit(O-) kmem dax_pmem_compat(O) nd_blk(O) dax_pmem_core(O) ]
[ 2426.733209] CPU: 2 PID: 47504 Comm: modprobe Tainted: G        W  O      5.12.0+ #3
[ 2426.734472] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.o4
[ 2426.736305] RIP: 0010:nfit_exit+0x2c/0x703 [nfit]
[ 2426.737099] Code: fd ff ff 48 c7 c7 00 f0 39 c0 e8 52 a1 38 da 48 8b 3d 6b 46 00 00 e8 e6 88 ee d9 48 8b 05 5f 3c 0
[ 2426.740046] RSP: 0018:ffffa8e800b77ed8 EFLAGS: 00010287
[ 2426.740990] RAX: ffff95b7e51935b0 RBX: 0000000000000800 RCX: ffffffff9b4a36a8
[ 2426.742236] RDX: 0000000000000000 RSI: 0000000000000083 RDI: ffff95b7c03e1554
[ 2426.743404] RBP: ffffffffc039f740 R08: 0000000000000400 R09: ffff95b7c03e0e50
[ 2426.744617] R10: ffff95b7fbd296f0 R11: 0000000000895440 R12: ffffa8e800b77f58
[ 2426.745792] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 2426.746946] FS:  00007f48297e3740(0000) GS:ffff95b7fbd00000(0000) knlGS:0000000000000000
[ 2426.748250] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2426.749198] CR2: 000056072aadc9f8 CR3: 0000000118b08000 CR4: 00000000000006e0
[ 2426.750349] Call Trace:
[ 2426.750754]  __do_sys_delete_module+0x19d/0x240
[ 2426.751472]  ? task_work_run+0x5c/0x90
[ 2426.751964]  ? exit_to_user_mode_prepare+0x2a/0x130
[ 2426.752637]  do_syscall_64+0x40/0x80
[ 2426.753121]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 2426.753810] RIP: 0033:0x7f482991361b
[ 2426.754274] Code: 73 01 c3 48 8b 0d 5d 18 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 8
[ 2426.756668] RSP: 002b:00007ffd46c89b98 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[ 2426.757676] RAX: ffffffffffffffda RBX: 000056072aad8f90 RCX: 00007f482991361b
[ 2426.758618] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000056072aad8ff8
[ 2426.759563] RBP: 000056072aad8f90 R08: 0000000000000000 R09: 0000000000000000
[ 2426.760513] R10: 00007f4829987ac0 R11: 0000000000000206 R12: 000056072aad8ff8
[ 2426.761463] R13: 0000000000000000 R14: 000056072aadb4e8 R15: 00007ffd46c89d18
[ 2426.762405] ---[ end trace 14a8748cda8b4777 ]---

This was not seen with the 5.11 kernel.

Thanks,
Santosh
>
>> diff --git a/test.h b/test.h
>> index cba8d41..7de13fe 100644
>> --- a/test.h
>> +++ b/test.h
>> @@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
>>  
>> 
>>  struct kmod_ctx;
>>  struct kmod_module;
>> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>  		struct ndctl_ctx *nd_ctx, int log_level,
>>  		struct ndctl_test *test);
>>  
>> 
>> diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
>> index fb1d82b..c561ff3 100644
>> --- a/test/ack-shutdown-count-set.c
>> +++ b/test/ack-shutdown-count-set.c
>> @@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct ndctl_test *test,
>>  	int result = EXIT_FAILURE, err;
>>  
>> 
>>  	ndctl_set_log_priority(ctx, loglevel);
>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>  	if (err < 0) {
>>  		result = 77;
>>  		ndctl_test_skip(test);
>> diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
>> index d7f00cb..f076e85 100644
>> --- a/test/blk_namespaces.c
>> +++ b/test/blk_namespaces.c
>> @@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test *test,
>>  
>> 
>>  	if (!bus) {
>>  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
>> -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>> +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>>  		ndctl_invalidate(ctx);
>>  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>>  		if (rc < 0 || !bus) {
>> diff --git a/test/core.c b/test/core.c
>> index cc7d8d9..44cb277 100644
>> --- a/test/core.c
>> +++ b/test/core.c
>> @@ -11,6 +11,7 @@
>>  #include <util/log.h>
>>  #include <util/sysfs.h>
>>  #include <ndctl/libndctl.h>
>> +#include <ndctl/ndctl.h>
>>  #include <ccan/array_size/array_size.h>
>>  
>> 
>>  #define KVER_STRLEN 20
>> @@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
>>  	return test->skip;
>>  }
>>  
>> 
>> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>  		struct ndctl_ctx *nd_ctx, int log_level,
>>  		struct ndctl_test *test)
>>  {
>> -	int rc;
>> +	int rc, family = -1;
>>  	unsigned int i;
>>  	const char *name;
>>  	struct ndctl_bus *bus;
>> @@ -127,10 +128,30 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>  		"nd_e820",
>>  		"nd_pmem",
>>  	};
>> +	char *test_env;
>>  
>> 
>>  	log_init(&log_ctx, "test/init", "NDCTL_TEST");
>>  	log_ctx.log_priority = log_level;
>>  
>> 
>> +	/*
>> +	 * The following two checks determine the platform family. For
>> +	 * Intel/platforms which support ACPI, check sysfs; for other platforms
>> +	 * determine from the environment variable NVDIMM_TEST_FAMILY
>> +	 */
>> +	if (access("/sys/bus/acpi", F_OK) == 0) {
>> +		if (errno == ENOENT)
>> +			family = NVDIMM_FAMILY_INTEL;
>> +	}
>> +
>> +	test_env = getenv("NDCTL_TEST_FAMILY");
>> +	if (test_env && strcmp(test_env, "PAPR") == 0)
>> +		family = NVDIMM_FAMILY_PAPR;
>> +
>> +	if (family == -1) {
>> +		log_err(&log_ctx, "Cannot determine NVDIMM family\n");
>> +		return -ENOTSUP;
>> +	}
>> +
>>  	*ctx = kmod_new(NULL, NULL);
>>  	if (!*ctx)
>>  		return -ENXIO;
>> @@ -185,6 +206,11 @@ retry:
>>  
>> 
>>  		path = kmod_module_get_path(*mod);
>>  		if (!path) {
>> +			if (family != NVDIMM_FAMILY_INTEL &&
>> +			    (strcmp(name, "nfit") == 0 ||
>> +			     strcmp(name, "nd_e820") == 0))
>> +				continue;
>> +
>>  			log_err(&log_ctx, "%s.ko: failed to get path\n", name);
>>  			break;
>>  		}
>> diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
>> index e922009..0b3bb7a 100644
>> --- a/test/dpa-alloc.c
>> +++ b/test/dpa-alloc.c
>> @@ -289,7 +289,7 @@ int test_dpa_alloc(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>>  		return 77;
>>  
>> 
>>  	ndctl_set_log_priority(ctx, loglevel);
>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>  	if (err < 0) {
>>  		ndctl_test_skip(test);
>>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
>> diff --git a/test/dsm-fail.c b/test/dsm-fail.c
>> index 9dfd8b0..0a6383d 100644
>> --- a/test/dsm-fail.c
>> +++ b/test/dsm-fail.c
>> @@ -346,7 +346,7 @@ int test_dsm_fail(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>>  	int result = EXIT_FAILURE, err;
>>  
>> 
>>  	ndctl_set_log_priority(ctx, loglevel);
>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>  	if (err < 0) {
>>  		result = 77;
>>  		ndctl_test_skip(test);
>> diff --git a/test/libndctl.c b/test/libndctl.c
>> index 24d72b3..0e88fce 100644
>> --- a/test/libndctl.c
>> +++ b/test/libndctl.c
>> @@ -2692,7 +2692,7 @@ int test_libndctl(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>>  	daxctl_set_log_priority(daxctl_ctx, loglevel);
>>  	ndctl_set_private_data(ctx, test);
>>  
>> 
>> -	err = nfit_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
>>  	if (err < 0) {
>>  		ndctl_test_skip(test);
>>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
>> diff --git a/test/multi-pmem.c b/test/multi-pmem.c
>> index 3d10952..3ea08cc 100644
>> --- a/test/multi-pmem.c
>> +++ b/test/multi-pmem.c
>> @@ -249,7 +249,7 @@ int test_multi_pmem(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx
>>  
>> 
>>  	ndctl_set_log_priority(ctx, loglevel);
>>  
>> 
>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>  	if (err < 0) {
>>  		result = 77;
>>  		ndctl_test_skip(test);
>> diff --git a/test/parent-uuid.c b/test/parent-uuid.c
>> index 6424e9f..bded33a 100644
>> --- a/test/parent-uuid.c
>> +++ b/test/parent-uuid.c
>> @@ -218,7 +218,7 @@ int test_parent_uuid(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ct
>>  		return 77;
>>  
>> 
>>  	ndctl_set_log_priority(ctx, loglevel);
>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>  	if (err < 0) {
>>  		ndctl_test_skip(test);
>>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
>> diff --git a/test/pmem_namespaces.c b/test/pmem_namespaces.c
>> index f0f2edd..a4db1ae 100644
>> --- a/test/pmem_namespaces.c
>> +++ b/test/pmem_namespaces.c
>> @@ -191,7 +191,7 @@ int test_pmem_namespaces(int log_level, struct ndctl_test *test,
>>  
>> 
>>  	if (!bus) {
>>  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
>> -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>> +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>>  		ndctl_invalidate(ctx);
>>  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>>  		if (rc < 0 || !bus) {
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 2/4] test: Don't skip tests if nfit modules are missing

[Verma, Vishal L]

On Sat, 2021-05-01 at 11:57 +0530, Santosh Sivaraj wrote:
> "Verma, Vishal L" <vishal.l.verma@intel.com> writes:
> 
> Hi Vishal,
> 
> > On Sun, 2021-03-28 at 07:39 +0530, Santosh Sivaraj wrote:
> > > For NFIT to be available ACPI is a must, so don't fail when nfit modules
> > > are missing on a platform that doesn't support ACPI.
> > > 
> > > Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> > > ---
> > >  test.h                        |  2 +-
> > >  test/ack-shutdown-count-set.c |  2 +-
> > >  test/blk_namespaces.c         |  2 +-
> > >  test/core.c                   | 30 ++++++++++++++++++++++++++++--
> > >  test/dpa-alloc.c              |  2 +-
> > >  test/dsm-fail.c               |  2 +-
> > >  test/libndctl.c               |  2 +-
> > >  test/multi-pmem.c             |  2 +-
> > >  test/parent-uuid.c            |  2 +-
> > >  test/pmem_namespaces.c        |  2 +-
> > >  10 files changed, 37 insertions(+), 11 deletions(-)
> > > 
> > 
> > I haven't looked deeper, but this seems to fail the blk-ns test with:
> > 
> >   ACPI.NFIT unavailable falling back to nfit_test
> >   test/init: ndctl_test_init: Cannot determine NVDIMM family
> >   __ndctl_test_skip: explicit skip test_blk_namespaces:235
> >   nfit_test unavailable skipping tests
> 
> The first message will be emitted even without the changes if the bus is not
> found. The second error will be emitted when check "/sys/bus/acpi" is not
> found. We fail for all other buses by default except for NFIT as before and PAPR
> tests are enabled only when NVDIMM_TEST_FAMILY is set to "PAPR".

See below on this.

> 
> All tests pass in my setup (x86_64 qemu guest) with the recent upstream kernel,
> except for the the below warning from drivers/acpi/nfit/core.c:

Hm I've not seen this with 5.11 or 5.12. What's the qemu command line
and is it just triggered from a unit test tun?

> 
> [ 2426.727584] ------------[ cut here ]------------
> [ 2426.728405] WARNING: CPU: 2 PID: 47504 at tools/testing/nvdimm/../../../drivers/acpi/nfit/core.c:3879 nfit_exit+0x]
> [ 2426.730264] Modules linked in: dax_pmem(O) nd_pmem(O) nfit(O-) kmem dax_pmem_compat(O) nd_blk(O) dax_pmem_core(O) ]
> [ 2426.733209] CPU: 2 PID: 47504 Comm: modprobe Tainted: G        W  O      5.12.0+ #3
> [ 2426.734472] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.o4
> [ 2426.736305] RIP: 0010:nfit_exit+0x2c/0x703 [nfit]
> [ 2426.737099] Code: fd ff ff 48 c7 c7 00 f0 39 c0 e8 52 a1 38 da 48 8b 3d 6b 46 00 00 e8 e6 88 ee d9 48 8b 05 5f 3c 0
> [ 2426.740046] RSP: 0018:ffffa8e800b77ed8 EFLAGS: 00010287
> [ 2426.740990] RAX: ffff95b7e51935b0 RBX: 0000000000000800 RCX: ffffffff9b4a36a8
> [ 2426.742236] RDX: 0000000000000000 RSI: 0000000000000083 RDI: ffff95b7c03e1554
> [ 2426.743404] RBP: ffffffffc039f740 R08: 0000000000000400 R09: ffff95b7c03e0e50
> [ 2426.744617] R10: ffff95b7fbd296f0 R11: 0000000000895440 R12: ffffa8e800b77f58
> [ 2426.745792] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 2426.746946] FS:  00007f48297e3740(0000) GS:ffff95b7fbd00000(0000) knlGS:0000000000000000
> [ 2426.748250] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 2426.749198] CR2: 000056072aadc9f8 CR3: 0000000118b08000 CR4: 00000000000006e0
> [ 2426.750349] Call Trace:
> [ 2426.750754]  __do_sys_delete_module+0x19d/0x240
> [ 2426.751472]  ? task_work_run+0x5c/0x90
> [ 2426.751964]  ? exit_to_user_mode_prepare+0x2a/0x130
> [ 2426.752637]  do_syscall_64+0x40/0x80
> [ 2426.753121]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 2426.753810] RIP: 0033:0x7f482991361b
> [ 2426.754274] Code: 73 01 c3 48 8b 0d 5d 18 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 8
> [ 2426.756668] RSP: 002b:00007ffd46c89b98 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> [ 2426.757676] RAX: ffffffffffffffda RBX: 000056072aad8f90 RCX: 00007f482991361b
> [ 2426.758618] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000056072aad8ff8
> [ 2426.759563] RBP: 000056072aad8f90 R08: 0000000000000000 R09: 0000000000000000
> [ 2426.760513] R10: 00007f4829987ac0 R11: 0000000000000206 R12: 000056072aad8ff8
> [ 2426.761463] R13: 0000000000000000 R14: 000056072aadb4e8 R15: 00007ffd46c89d18
> [ 2426.762405] ---[ end trace 14a8748cda8b4777 ]---
> 
> This was not seen with the 5.11 kernel.
> 
> Thanks,
> Santosh
> > 
> > > diff --git a/test.h b/test.h
> > > index cba8d41..7de13fe 100644
> > > --- a/test.h
> > > +++ b/test.h
> > > @@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
> > >  
> > > 
> > >  struct kmod_ctx;
> > >  struct kmod_module;
> > > -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> > > +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> > >  		struct ndctl_ctx *nd_ctx, int log_level,
> > >  		struct ndctl_test *test);
> > >  
> > > 
> > > diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
> > > index fb1d82b..c561ff3 100644
> > > --- a/test/ack-shutdown-count-set.c
> > > +++ b/test/ack-shutdown-count-set.c
> > > @@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct ndctl_test *test,
> > >  	int result = EXIT_FAILURE, err;
> > >  
> > > 
> > >  	ndctl_set_log_priority(ctx, loglevel);
> > > -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> > > +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
> > >  	if (err < 0) {
> > >  		result = 77;
> > >  		ndctl_test_skip(test);
> > > diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
> > > index d7f00cb..f076e85 100644
> > > --- a/test/blk_namespaces.c
> > > +++ b/test/blk_namespaces.c
> > > @@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test *test,
> > >  
> > > 
> > >  	if (!bus) {
> > >  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
> > > -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
> > > +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
> > >  		ndctl_invalidate(ctx);
> > >  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
> > >  		if (rc < 0 || !bus) {
> > > diff --git a/test/core.c b/test/core.c
> > > index cc7d8d9..44cb277 100644
> > > --- a/test/core.c
> > > +++ b/test/core.c
> > > @@ -11,6 +11,7 @@
> > >  #include <util/log.h>
> > >  #include <util/sysfs.h>
> > >  #include <ndctl/libndctl.h>
> > > +#include <ndctl/ndctl.h>
> > >  #include <ccan/array_size/array_size.h>
> > >  
> > > 
> > >  #define KVER_STRLEN 20
> > > @@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
> > >  	return test->skip;
> > >  }
> > >  
> > > 
> > > -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> > > +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> > >  		struct ndctl_ctx *nd_ctx, int log_level,
> > >  		struct ndctl_test *test)
> > >  {
> > > -	int rc;
> > > +	int rc, family = -1;
> > >  	unsigned int i;
> > >  	const char *name;
> > >  	struct ndctl_bus *bus;
> > > @@ -127,10 +128,30 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> > >  		"nd_e820",
> > >  		"nd_pmem",
> > >  	};
> > > +	char *test_env;
> > >  
> > > 
> > >  	log_init(&log_ctx, "test/init", "NDCTL_TEST");
> > >  	log_ctx.log_priority = log_level;
> > >  
> > > 
> > > +	/*
> > > +	 * The following two checks determine the platform family. For
> > > +	 * Intel/platforms which support ACPI, check sysfs; for other platforms
> > > +	 * determine from the environment variable NVDIMM_TEST_FAMILY
> > > +	 */
> > > +	if (access("/sys/bus/acpi", F_OK) == 0) {
> > > +		if (errno == ENOENT)
> > > +			family = NVDIMM_FAMILY_INTEL;
> > > +	}

Did you mean for the errno check to be if (errno != ENOENT) ?
This is what was causing the unit test failure for me. This patch on
top fixes it for me:

diff --git a/test/core.c b/test/core.c
index 44cb277..698bb66 100644
--- a/test/core.c
+++ b/test/core.c
@@ -139,7 +139,7 @@ int ndctl_test_init(struct kmod_ctx **ctx, struct
kmod_module **mod,
         * determine from the environment variable NVDIMM_TEST_FAMILY
         */
        if (access("/sys/bus/acpi", F_OK) == 0) {
-               if (errno == ENOENT)
+               if (errno != ENOENT)
                        family = NVDIMM_FAMILY_INTEL;
        }
 
If this looks okay do you want to send out a respin with this and I'll
pick it up.

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

Re: [PATCH 2/4] test: Don't skip tests if nfit modules are missing

[Verma, Vishal L]

On Wed, 2021-05-12 at 21:00 +0000, Verma, Vishal L wrote:
> 
> Did you mean for the errno check to be if (errno != ENOENT) ?
> This is what was causing the unit test failure for me. This patch on
> top fixes it for me:
> 
> diff --git a/test/core.c b/test/core.c
> index 44cb277..698bb66 100644
> --- a/test/core.c
> +++ b/test/core.c
> @@ -139,7 +139,7 @@ int ndctl_test_init(struct kmod_ctx **ctx, struct
> kmod_module **mod,
>          * determine from the environment variable NVDIMM_TEST_FAMILY
>          */
>         if (access("/sys/bus/acpi", F_OK) == 0) {
> -               if (errno == ENOENT)
> +               if (errno != ENOENT)
>                         family = NVDIMM_FAMILY_INTEL;
>         }

Also, looks like for access(<path>, F_OK), it should be sufficient to
just test for the return value instead of return value and errno.


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

Re: [PATCH 2/4] test: Don't skip tests if nfit modules are missing

[Santosh Sivaraj]

"Verma, Vishal L" <vishal.l.verma@intel.com> writes:

Hi Vishal,

> On Sat, 2021-05-01 at 11:57 +0530, Santosh Sivaraj wrote:
>> "Verma, Vishal L" <vishal.l.verma@intel.com> writes:
>> 
>> Hi Vishal,
>> 
>> > On Sun, 2021-03-28 at 07:39 +0530, Santosh Sivaraj wrote:
>> > > For NFIT to be available ACPI is a must, so don't fail when nfit modules
>> > > are missing on a platform that doesn't support ACPI.
>> > > 
>> > > Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>> > > ---
>> > >  test.h                        |  2 +-
>> > >  test/ack-shutdown-count-set.c |  2 +-
>> > >  test/blk_namespaces.c         |  2 +-
>> > >  test/core.c                   | 30 ++++++++++++++++++++++++++++--
>> > >  test/dpa-alloc.c              |  2 +-
>> > >  test/dsm-fail.c               |  2 +-
>> > >  test/libndctl.c               |  2 +-
>> > >  test/multi-pmem.c             |  2 +-
>> > >  test/parent-uuid.c            |  2 +-
>> > >  test/pmem_namespaces.c        |  2 +-
>> > >  10 files changed, 37 insertions(+), 11 deletions(-)
>> > > 
>> > 
>> > I haven't looked deeper, but this seems to fail the blk-ns test with:
>> > 
>> >   ACPI.NFIT unavailable falling back to nfit_test
>> >   test/init: ndctl_test_init: Cannot determine NVDIMM family
>> >   __ndctl_test_skip: explicit skip test_blk_namespaces:235
>> >   nfit_test unavailable skipping tests
>> 
>> The first message will be emitted even without the changes if the bus is not
>> found. The second error will be emitted when check "/sys/bus/acpi" is not
>> found. We fail for all other buses by default except for NFIT as before and PAPR
>> tests are enabled only when NVDIMM_TEST_FAMILY is set to "PAPR".
>
> See below on this.
>
>> 
>> All tests pass in my setup (x86_64 qemu guest) with the recent upstream kernel,
>> except for the the below warning from drivers/acpi/nfit/core.c:
>
> Hm I've not seen this with 5.11 or 5.12. What's the qemu command line
> and is it just triggered from a unit test tun?

This was on a 5.12 kernel, which I have mentioned earlier, but I am not able to
reproduce now, I had rebased the kernel now. But anyway it doesn't seem to be
reproducible now. Will come back if I see something again.

>
>> 
>> [ 2426.727584] ------------[ cut here ]------------
>> [ 2426.728405] WARNING: CPU: 2 PID: 47504 at tools/testing/nvdimm/../../../drivers/acpi/nfit/core.c:3879 nfit_exit+0x]
>> [ 2426.730264] Modules linked in: dax_pmem(O) nd_pmem(O) nfit(O-) kmem dax_pmem_compat(O) nd_blk(O) dax_pmem_core(O) ]
>> [ 2426.733209] CPU: 2 PID: 47504 Comm: modprobe Tainted: G        W  O      5.12.0+ #3
>> [ 2426.734472] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.o4
>> [ 2426.736305] RIP: 0010:nfit_exit+0x2c/0x703 [nfit]
>> [ 2426.737099] Code: fd ff ff 48 c7 c7 00 f0 39 c0 e8 52 a1 38 da 48 8b 3d 6b 46 00 00 e8 e6 88 ee d9 48 8b 05 5f 3c 0
>> [ 2426.740046] RSP: 0018:ffffa8e800b77ed8 EFLAGS: 00010287
>> [ 2426.740990] RAX: ffff95b7e51935b0 RBX: 0000000000000800 RCX: ffffffff9b4a36a8
>> [ 2426.742236] RDX: 0000000000000000 RSI: 0000000000000083 RDI: ffff95b7c03e1554
>> [ 2426.743404] RBP: ffffffffc039f740 R08: 0000000000000400 R09: ffff95b7c03e0e50
>> [ 2426.744617] R10: ffff95b7fbd296f0 R11: 0000000000895440 R12: ffffa8e800b77f58
>> [ 2426.745792] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
>> [ 2426.746946] FS:  00007f48297e3740(0000) GS:ffff95b7fbd00000(0000) knlGS:0000000000000000
>> [ 2426.748250] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 2426.749198] CR2: 000056072aadc9f8 CR3: 0000000118b08000 CR4: 00000000000006e0
>> [ 2426.750349] Call Trace:
>> [ 2426.750754]  __do_sys_delete_module+0x19d/0x240
>> [ 2426.751472]  ? task_work_run+0x5c/0x90
>> [ 2426.751964]  ? exit_to_user_mode_prepare+0x2a/0x130
>> [ 2426.752637]  do_syscall_64+0x40/0x80
>> [ 2426.753121]  entry_SYSCALL_64_after_hwframe+0x44/0xae
>> [ 2426.753810] RIP: 0033:0x7f482991361b
>> [ 2426.754274] Code: 73 01 c3 48 8b 0d 5d 18 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 8
>> [ 2426.756668] RSP: 002b:00007ffd46c89b98 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
>> [ 2426.757676] RAX: ffffffffffffffda RBX: 000056072aad8f90 RCX: 00007f482991361b
>> [ 2426.758618] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000056072aad8ff8
>> [ 2426.759563] RBP: 000056072aad8f90 R08: 0000000000000000 R09: 0000000000000000
>> [ 2426.760513] R10: 00007f4829987ac0 R11: 0000000000000206 R12: 000056072aad8ff8
>> [ 2426.761463] R13: 0000000000000000 R14: 000056072aadb4e8 R15: 00007ffd46c89d18
>> [ 2426.762405] ---[ end trace 14a8748cda8b4777 ]---
>> 
>> This was not seen with the 5.11 kernel.
>> 
>> Thanks,
>> Santosh
>> > 
>> > > diff --git a/test.h b/test.h
>> > > index cba8d41..7de13fe 100644
>> > > --- a/test.h
>> > > +++ b/test.h
>> > > @@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
>> > >  
>> > > 
>> > >  struct kmod_ctx;
>> > >  struct kmod_module;
>> > > -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> > > +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> > >  		struct ndctl_ctx *nd_ctx, int log_level,
>> > >  		struct ndctl_test *test);
>> > >  
>> > > 
>> > > diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
>> > > index fb1d82b..c561ff3 100644
>> > > --- a/test/ack-shutdown-count-set.c
>> > > +++ b/test/ack-shutdown-count-set.c
>> > > @@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct ndctl_test *test,
>> > >  	int result = EXIT_FAILURE, err;
>> > >  
>> > > 
>> > >  	ndctl_set_log_priority(ctx, loglevel);
>> > > -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> > > +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>> > >  	if (err < 0) {
>> > >  		result = 77;
>> > >  		ndctl_test_skip(test);
>> > > diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
>> > > index d7f00cb..f076e85 100644
>> > > --- a/test/blk_namespaces.c
>> > > +++ b/test/blk_namespaces.c
>> > > @@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test *test,
>> > >  
>> > > 
>> > >  	if (!bus) {
>> > >  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
>> > > -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>> > > +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>> > >  		ndctl_invalidate(ctx);
>> > >  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>> > >  		if (rc < 0 || !bus) {
>> > > diff --git a/test/core.c b/test/core.c
>> > > index cc7d8d9..44cb277 100644
>> > > --- a/test/core.c
>> > > +++ b/test/core.c
>> > > @@ -11,6 +11,7 @@
>> > >  #include <util/log.h>
>> > >  #include <util/sysfs.h>
>> > >  #include <ndctl/libndctl.h>
>> > > +#include <ndctl/ndctl.h>
>> > >  #include <ccan/array_size/array_size.h>
>> > >  
>> > > 
>> > >  #define KVER_STRLEN 20
>> > > @@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
>> > >  	return test->skip;
>> > >  }
>> > >  
>> > > 
>> > > -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> > > +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> > >  		struct ndctl_ctx *nd_ctx, int log_level,
>> > >  		struct ndctl_test *test)
>> > >  {
>> > > -	int rc;
>> > > +	int rc, family = -1;
>> > >  	unsigned int i;
>> > >  	const char *name;
>> > >  	struct ndctl_bus *bus;
>> > > @@ -127,10 +128,30 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> > >  		"nd_e820",
>> > >  		"nd_pmem",
>> > >  	};
>> > > +	char *test_env;
>> > >  
>> > > 
>> > >  	log_init(&log_ctx, "test/init", "NDCTL_TEST");
>> > >  	log_ctx.log_priority = log_level;
>> > >  
>> > > 
>> > > +	/*
>> > > +	 * The following two checks determine the platform family. For
>> > > +	 * Intel/platforms which support ACPI, check sysfs; for other platforms
>> > > +	 * determine from the environment variable NVDIMM_TEST_FAMILY
>> > > +	 */
>> > > +	if (access("/sys/bus/acpi", F_OK) == 0) {
>> > > +		if (errno == ENOENT)
>> > > +			family = NVDIMM_FAMILY_INTEL;
>> > > +	}
>
> Did you mean for the errno check to be if (errno != ENOENT) ?
> This is what was causing the unit test failure for me. This patch on
> top fixes it for me:
>
> diff --git a/test/core.c b/test/core.c
> index 44cb277..698bb66 100644
> --- a/test/core.c
> +++ b/test/core.c
> @@ -139,7 +139,7 @@ int ndctl_test_init(struct kmod_ctx **ctx, struct
> kmod_module **mod,
>          * determine from the environment variable NVDIMM_TEST_FAMILY
>          */
>         if (access("/sys/bus/acpi", F_OK) == 0) {
> -               if (errno == ENOENT)
> +               if (errno != ENOENT)
>                         family = NVDIMM_FAMILY_INTEL;
>         }
>  
> If this looks okay do you want to send out a respin with this and I'll
> pick it up.

That's this looks okay. I will send out an updated version today.

Thanks,
Santosh
>
> Thanks,
> -Vishal
>> > 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 2/4] test: Don't skip tests if nfit modules are missing

[Santosh Sivaraj]

Santosh Sivaraj <santosh@fossix.org> writes:

> "Verma, Vishal L" <vishal.l.verma@intel.com> writes:
>
> Hi Vishal,
>
>> On Sat, 2021-05-01 at 11:57 +0530, Santosh Sivaraj wrote:
>>> "Verma, Vishal L" <vishal.l.verma@intel.com> writes:
>>> 
>>> Hi Vishal,
>>> 
>>> > On Sun, 2021-03-28 at 07:39 +0530, Santosh Sivaraj wrote:
>>> > > For NFIT to be available ACPI is a must, so don't fail when nfit modules
>>> > > are missing on a platform that doesn't support ACPI.
>>> > > 
>>> > > Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>>> > > ---
>>> > >  test.h                        |  2 +-
>>> > >  test/ack-shutdown-count-set.c |  2 +-
>>> > >  test/blk_namespaces.c         |  2 +-
>>> > >  test/core.c                   | 30 ++++++++++++++++++++++++++++--
>>> > >  test/dpa-alloc.c              |  2 +-
>>> > >  test/dsm-fail.c               |  2 +-
>>> > >  test/libndctl.c               |  2 +-
>>> > >  test/multi-pmem.c             |  2 +-
>>> > >  test/parent-uuid.c            |  2 +-
>>> > >  test/pmem_namespaces.c        |  2 +-
>>> > >  10 files changed, 37 insertions(+), 11 deletions(-)
>>> > > 
>>> > 
>>> > I haven't looked deeper, but this seems to fail the blk-ns test with:
>>> > 
>>> >   ACPI.NFIT unavailable falling back to nfit_test
>>> >   test/init: ndctl_test_init: Cannot determine NVDIMM family
>>> >   __ndctl_test_skip: explicit skip test_blk_namespaces:235
>>> >   nfit_test unavailable skipping tests
>>> 
>>> The first message will be emitted even without the changes if the bus is not
>>> found. The second error will be emitted when check "/sys/bus/acpi" is not
>>> found. We fail for all other buses by default except for NFIT as before and PAPR
>>> tests are enabled only when NVDIMM_TEST_FAMILY is set to "PAPR".
>>
>> See below on this.
>>
>>> 
>>> All tests pass in my setup (x86_64 qemu guest) with the recent upstream kernel,
>>> except for the the below warning from drivers/acpi/nfit/core.c:
>>
>> Hm I've not seen this with 5.11 or 5.12. What's the qemu command line
>> and is it just triggered from a unit test tun?
>
> This was on a 5.12 kernel, which I have mentioned earlier, but I am not able to
> reproduce now, I had rebased the kernel now. But anyway it doesn't seem to be
> reproducible now. Will come back if I see something again.

I could reproduce this with

make check TESTS=create.sh

and it reproduces with all the tests when the module is being removed.

kernel: 5.13-rc1 (6efb943b8616ec53a5e444193dccf1af9ad627b5)
ndctl: ea014c0c9ec8d0ef945d072dcc52b306c7a686f9

qemu --version: QEMU emulator version 5.0.1 (v5.0.1)

$QEMU_X86 -smp 4 -enable-kvm -hda $DISK -boot c \
          -m 4G,slots=4,maxmem=8G \
          -M pc,accel=kvm,nvdimm \
          -name guest=santosh-x86,debug-threads=on \
          -netdev user,id=ethernet.0,hostfwd=tcp::12121-:22 -device e1000,netdev=ethernet.0 \
          -serial mon:stdio -display none -vga none -nographic \
          -kernel $1 -append "console=ttyS0 root=$ROOT" \
          -hdb $DISK2

Thanks,
Santosh

>
>>
>>> 
>>> [ 2426.727584] ------------[ cut here ]------------
>>> [ 2426.728405] WARNING: CPU: 2 PID: 47504 at tools/testing/nvdimm/../../../drivers/acpi/nfit/core.c:3879 nfit_exit+0x]
>>> [ 2426.730264] Modules linked in: dax_pmem(O) nd_pmem(O) nfit(O-) kmem dax_pmem_compat(O) nd_blk(O) dax_pmem_core(O) ]
>>> [ 2426.733209] CPU: 2 PID: 47504 Comm: modprobe Tainted: G        W  O      5.12.0+ #3
>>> [ 2426.734472] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.o4
>>> [ 2426.736305] RIP: 0010:nfit_exit+0x2c/0x703 [nfit]
>>> [ 2426.737099] Code: fd ff ff 48 c7 c7 00 f0 39 c0 e8 52 a1 38 da 48 8b 3d 6b 46 00 00 e8 e6 88 ee d9 48 8b 05 5f 3c 0
>>> [ 2426.740046] RSP: 0018:ffffa8e800b77ed8 EFLAGS: 00010287
>>> [ 2426.740990] RAX: ffff95b7e51935b0 RBX: 0000000000000800 RCX: ffffffff9b4a36a8
>>> [ 2426.742236] RDX: 0000000000000000 RSI: 0000000000000083 RDI: ffff95b7c03e1554
>>> [ 2426.743404] RBP: ffffffffc039f740 R08: 0000000000000400 R09: ffff95b7c03e0e50
>>> [ 2426.744617] R10: ffff95b7fbd296f0 R11: 0000000000895440 R12: ffffa8e800b77f58
>>> [ 2426.745792] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
>>> [ 2426.746946] FS:  00007f48297e3740(0000) GS:ffff95b7fbd00000(0000) knlGS:0000000000000000
>>> [ 2426.748250] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [ 2426.749198] CR2: 000056072aadc9f8 CR3: 0000000118b08000 CR4: 00000000000006e0
>>> [ 2426.750349] Call Trace:
>>> [ 2426.750754]  __do_sys_delete_module+0x19d/0x240
>>> [ 2426.751472]  ? task_work_run+0x5c/0x90
>>> [ 2426.751964]  ? exit_to_user_mode_prepare+0x2a/0x130
>>> [ 2426.752637]  do_syscall_64+0x40/0x80
>>> [ 2426.753121]  entry_SYSCALL_64_after_hwframe+0x44/0xae
>>> [ 2426.753810] RIP: 0033:0x7f482991361b
>>> [ 2426.754274] Code: 73 01 c3 48 8b 0d 5d 18 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 8
>>> [ 2426.756668] RSP: 002b:00007ffd46c89b98 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
>>> [ 2426.757676] RAX: ffffffffffffffda RBX: 000056072aad8f90 RCX: 00007f482991361b
>>> [ 2426.758618] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000056072aad8ff8
>>> [ 2426.759563] RBP: 000056072aad8f90 R08: 0000000000000000 R09: 0000000000000000
>>> [ 2426.760513] R10: 00007f4829987ac0 R11: 0000000000000206 R12: 000056072aad8ff8
>>> [ 2426.761463] R13: 0000000000000000 R14: 000056072aadb4e8 R15: 00007ffd46c89d18
>>> [ 2426.762405] ---[ end trace 14a8748cda8b4777 ]---
>>> 
>>> This was not seen with the 5.11 kernel.
>>> 
>>> Thanks,
>>> Santosh
>>> > 
>>> > > diff --git a/test.h b/test.h
>>> > > index cba8d41..7de13fe 100644
>>> > > --- a/test.h
>>> > > +++ b/test.h
>>> > > @@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
>>> > >  
>>> > > 
>>> > >  struct kmod_ctx;
>>> > >  struct kmod_module;
>>> > > -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>> > > +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>> > >  		struct ndctl_ctx *nd_ctx, int log_level,
>>> > >  		struct ndctl_test *test);
>>> > >  
>>> > > 
>>> > > diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
>>> > > index fb1d82b..c561ff3 100644
>>> > > --- a/test/ack-shutdown-count-set.c
>>> > > +++ b/test/ack-shutdown-count-set.c
>>> > > @@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct ndctl_test *test,
>>> > >  	int result = EXIT_FAILURE, err;
>>> > >  
>>> > > 
>>> > >  	ndctl_set_log_priority(ctx, loglevel);
>>> > > -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>> > > +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>> > >  	if (err < 0) {
>>> > >  		result = 77;
>>> > >  		ndctl_test_skip(test);
>>> > > diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
>>> > > index d7f00cb..f076e85 100644
>>> > > --- a/test/blk_namespaces.c
>>> > > +++ b/test/blk_namespaces.c
>>> > > @@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test *test,
>>> > >  
>>> > > 
>>> > >  	if (!bus) {
>>> > >  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
>>> > > -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>>> > > +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>>> > >  		ndctl_invalidate(ctx);
>>> > >  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>>> > >  		if (rc < 0 || !bus) {
>>> > > diff --git a/test/core.c b/test/core.c
>>> > > index cc7d8d9..44cb277 100644
>>> > > --- a/test/core.c
>>> > > +++ b/test/core.c
>>> > > @@ -11,6 +11,7 @@
>>> > >  #include <util/log.h>
>>> > >  #include <util/sysfs.h>
>>> > >  #include <ndctl/libndctl.h>
>>> > > +#include <ndctl/ndctl.h>
>>> > >  #include <ccan/array_size/array_size.h>
>>> > >  
>>> > > 
>>> > >  #define KVER_STRLEN 20
>>> > > @@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
>>> > >  	return test->skip;
>>> > >  }
>>> > >  
>>> > > 
>>> > > -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>> > > +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>> > >  		struct ndctl_ctx *nd_ctx, int log_level,
>>> > >  		struct ndctl_test *test)
>>> > >  {
>>> > > -	int rc;
>>> > > +	int rc, family = -1;
>>> > >  	unsigned int i;
>>> > >  	const char *name;
>>> > >  	struct ndctl_bus *bus;
>>> > > @@ -127,10 +128,30 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>> > >  		"nd_e820",
>>> > >  		"nd_pmem",
>>> > >  	};
>>> > > +	char *test_env;
>>> > >  
>>> > > 
>>> > >  	log_init(&log_ctx, "test/init", "NDCTL_TEST");
>>> > >  	log_ctx.log_priority = log_level;
>>> > >  
>>> > > 
>>> > > +	/*
>>> > > +	 * The following two checks determine the platform family. For
>>> > > +	 * Intel/platforms which support ACPI, check sysfs; for other platforms
>>> > > +	 * determine from the environment variable NVDIMM_TEST_FAMILY
>>> > > +	 */
>>> > > +	if (access("/sys/bus/acpi", F_OK) == 0) {
>>> > > +		if (errno == ENOENT)
>>> > > +			family = NVDIMM_FAMILY_INTEL;
>>> > > +	}
>>
>> Did you mean for the errno check to be if (errno != ENOENT) ?
>> This is what was causing the unit test failure for me. This patch on
>> top fixes it for me:
>>
>> diff --git a/test/core.c b/test/core.c
>> index 44cb277..698bb66 100644
>> --- a/test/core.c
>> +++ b/test/core.c
>> @@ -139,7 +139,7 @@ int ndctl_test_init(struct kmod_ctx **ctx, struct
>> kmod_module **mod,
>>          * determine from the environment variable NVDIMM_TEST_FAMILY
>>          */
>>         if (access("/sys/bus/acpi", F_OK) == 0) {
>> -               if (errno == ENOENT)
>> +               if (errno != ENOENT)
>>                         family = NVDIMM_FAMILY_INTEL;
>>         }
>>  
>> If this looks okay do you want to send out a respin with this and I'll
>> pick it up.
>
> That's this looks okay. I will send out an updated version today.
>
> Thanks,
> Santosh
>>
>> Thanks,
>> -Vishal
>>> > 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 3/4] papr: Add support to parse save_fail flag for dimm

[Santosh Sivaraj]

This will help in getting the dimm fail tests to run on papr family too.
Also add nvdimm_test compatibility string for recognizing the test module.

Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
 ndctl/lib/libndctl.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 26b9317..dd1a5fc 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -805,6 +805,8 @@ static void parse_papr_flags(struct ndctl_dimm *dimm, char *flags)
 			dimm->flags.f_restore = 1;
 		else if (strcmp(start, "smart_notify") == 0)
 			dimm->flags.f_smart = 1;
+		else if (strcmp(start, "save_fail") == 0)
+			dimm->flags.f_save = 1;
 		start = end + 1;
 	}
 	if (end != start)
@@ -1035,7 +1037,8 @@ NDCTL_EXPORT int ndctl_bus_is_papr_scm(struct ndctl_bus *bus)
 	if (sysfs_read_attr(bus->ctx, bus->bus_buf, buf) < 0)
 		return 0;
 
-	return (strcmp(buf, "ibm,pmemory") == 0);
+	return (strcmp(buf, "ibm,pmemory") == 0 ||
+		strcmp(buf, "nvdimm_test") == 0);
 }
 
 /**
-- 
2.30.2
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 4/4] Use page size as alignment value

[Santosh Sivaraj]

The alignment sizes passed to ndctl in the tests are all hardcoded to 4k,
the default page size on x86. Change those to the default page size on that
architecture (sysconf/getconf). No functional changes otherwise.

Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
 test/dpa-alloc.c    | 15 ++++++++-------
 test/multi-dax.sh   |  6 ++++--
 test/sector-mode.sh |  4 +++-
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
index 0b3bb7a..59185cf 100644
--- a/test/dpa-alloc.c
+++ b/test/dpa-alloc.c
@@ -38,12 +38,13 @@ static int do_test(struct ndctl_ctx *ctx, struct ndctl_test *test)
 	struct ndctl_region *region, *blk_region = NULL;
 	struct ndctl_namespace *ndns;
 	struct ndctl_dimm *dimm;
-	unsigned long size;
+	unsigned long size, page_size;
 	struct ndctl_bus *bus;
 	char uuid_str[40];
 	int round;
 	int rc;
 
+	page_size = sysconf(_SC_PAGESIZE);
 	/* disable nfit_test.1, not used in this test */
 	bus = ndctl_bus_get_by_provider(ctx, NFIT_PROVIDER1);
 	if (!bus)
@@ -124,11 +125,11 @@ static int do_test(struct ndctl_ctx *ctx, struct ndctl_test *test)
 			return rc;
 		}
 		ndctl_namespace_disable_invalidate(ndns);
-		rc = ndctl_namespace_set_size(ndns, SZ_4K);
+		rc = ndctl_namespace_set_size(ndns, page_size);
 		if (rc) {
-			fprintf(stderr, "failed to init %s to size: %d\n",
+			fprintf(stderr, "failed to init %s to size: %lu\n",
 					ndctl_namespace_get_devname(ndns),
-					SZ_4K);
+					page_size);
 			return rc;
 		}
 		namespaces[i].ndns = ndns;
@@ -150,7 +151,7 @@ static int do_test(struct ndctl_ctx *ctx, struct ndctl_test *test)
 		ndns = namespaces[i % ARRAY_SIZE(namespaces)].ndns;
 		if (i % ARRAY_SIZE(namespaces) == 0)
 			round++;
-		size = SZ_4K * round;
+		size = page_size * round;
 		rc = ndctl_namespace_set_size(ndns, size);
 		if (rc) {
 			fprintf(stderr, "%s: set_size: %lx failed: %d\n",
@@ -166,7 +167,7 @@ static int do_test(struct ndctl_ctx *ctx, struct ndctl_test *test)
 	i--;
 	round++;
 	ndns = namespaces[i % ARRAY_SIZE(namespaces)].ndns;
-	size = SZ_4K * round;
+	size = page_size * round;
 	rc = ndctl_namespace_set_size(ndns, size);
 	if (rc) {
 		fprintf(stderr, "%s failed to update while labels full\n",
@@ -175,7 +176,7 @@ static int do_test(struct ndctl_ctx *ctx, struct ndctl_test *test)
 	}
 
 	round--;
-	size = SZ_4K * round;
+	size = page_size * round;
 	rc = ndctl_namespace_set_size(ndns, size);
 	if (rc) {
 		fprintf(stderr, "%s failed to reduce size while labels full\n",
diff --git a/test/multi-dax.sh b/test/multi-dax.sh
index e932569..9451ed0 100755
--- a/test/multi-dax.sh
+++ b/test/multi-dax.sh
@@ -12,6 +12,8 @@ check_min_kver "4.13" || do_skip "may lack multi-dax support"
 
 trap 'err $LINENO' ERR
 
+ALIGN_SIZE=`getconf PAGESIZE`
+
 # setup (reset nfit_test dimms)
 modprobe nfit_test
 $NDCTL disable-region -b $NFIT_TEST_BUS0 all
@@ -22,9 +24,9 @@ rc=1
 query=". | sort_by(.available_size) | reverse | .[0].dev"
 region=$($NDCTL list -b $NFIT_TEST_BUS0 -t pmem -Ri | jq -r "$query")
 
-json=$($NDCTL create-namespace -b $NFIT_TEST_BUS0 -r $region -t pmem -m devdax -a 4096 -s 16M)
+json=$($NDCTL create-namespace -b $NFIT_TEST_BUS0 -r $region -t pmem -m devdax -a $ALIGN_SIZE -s 16M)
 chardev1=$(echo $json | jq ". | select(.mode == \"devdax\") | .daxregion.devices[0].chardev")
-json=$($NDCTL create-namespace -b $NFIT_TEST_BUS0 -r $region -t pmem -m devdax -a 4096 -s 16M)
+json=$($NDCTL create-namespace -b $NFIT_TEST_BUS0 -r $region -t pmem -m devdax -a $ALIGN_SIZE -s 16M)
 chardev2=$(echo $json | jq ". | select(.mode == \"devdax\") | .daxregion.devices[0].chardev")
 
 _cleanup
diff --git a/test/sector-mode.sh b/test/sector-mode.sh
index dd7013e..d03c0ca 100755
--- a/test/sector-mode.sh
+++ b/test/sector-mode.sh
@@ -9,6 +9,8 @@ rc=77
 set -e
 trap 'err $LINENO' ERR
 
+ALIGN_SIZE=`getconf PAGESIZE`
+
 # setup (reset nfit_test dimms)
 modprobe nfit_test
 $NDCTL disable-region -b $NFIT_TEST_BUS0 all
@@ -25,7 +27,7 @@ NAMESPACE=$($NDCTL list -b $NFIT_TEST_BUS1 -N | jq -r "$query")
 REGION=$($NDCTL list -R --namespace=$NAMESPACE | jq -r "(.[]) | .dev")
 echo 0 > /sys/bus/nd/devices/$REGION/read_only
 $NDCTL create-namespace --no-autolabel -e $NAMESPACE -m sector -f -l 4K
-$NDCTL create-namespace --no-autolabel -e $NAMESPACE -m dax -f -a 4K
+$NDCTL create-namespace --no-autolabel -e $NAMESPACE -m dax -f -a $ALIGN_SIZE
 $NDCTL create-namespace --no-autolabel -e $NAMESPACE -m sector -f -l 4K
 
 _cleanup
-- 
2.30.2
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 1/4] libndctl: Unify adding dimms for papr and nfit families

[Santosh Sivaraj]

Sorry, I missed to provide the version in the subject, this is v4 of the series.

Santosh Sivaraj <santosh@fossix.org> writes:

> In preparation for enabling tests on non-nfit devices, unify both, already very
> similar, functions into one. This will help in adding all attributes needed for
> the unit tests. Since the function doesn't fail if some of the dimm attributes
> are missing, this will work fine on PAPR platforms though only part of the DIMM
> attributes are provided (This doesn't mean that all of the DIMM attributes can
> be missing).
>
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
>  ndctl/lib/libndctl.c | 103 ++++++++++++++++---------------------------
>  1 file changed, 38 insertions(+), 65 deletions(-)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 36fb6fe..26b9317 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -1646,41 +1646,9 @@ static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
>  static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
>  static struct kmod_module *to_module(struct ndctl_ctx *ctx, const char *alias);
>  
> -static int add_papr_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
> -{
> -	int rc = -ENODEV;
> -	char buf[SYSFS_ATTR_SIZE];
> -	struct ndctl_ctx *ctx = dimm->bus->ctx;
> -	char *path = calloc(1, strlen(dimm_base) + 100);
> -	const char * const devname = ndctl_dimm_get_devname(dimm);
> -
> -	dbg(ctx, "%s: Probing of_pmem dimm at %s\n", devname, dimm_base);
> -
> -	if (!path)
> -		return -ENOMEM;
> -
> -	/* construct path to the papr compatible dimm flags file */
> -	sprintf(path, "%s/papr/flags", dimm_base);
> -
> -	if (ndctl_bus_is_papr_scm(dimm->bus) &&
> -	    sysfs_read_attr(ctx, path, buf) == 0) {
> -
> -		dbg(ctx, "%s: Adding papr-scm dimm flags:\"%s\"\n", devname, buf);
> -		dimm->cmd_family = NVDIMM_FAMILY_PAPR;
> -
> -		/* Parse dimm flags */
> -		parse_papr_flags(dimm, buf);
> -
> -		/* Allocate monitor mode fd */
> -		dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
> -		rc = 0;
> -	}
> -
> -	free(path);
> -	return rc;
> -}
> -
> -static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
> +static int populate_dimm_attributes(struct ndctl_dimm *dimm,
> +				    const char *dimm_base,
> +				    const char *bus_prefix)
>  {
>  	int i, rc = -1;
>  	char buf[SYSFS_ATTR_SIZE];
> @@ -1694,7 +1662,7 @@ static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
>  	 * 'unique_id' may not be available on older kernels, so don't
>  	 * fail if the read fails.
>  	 */
> -	sprintf(path, "%s/nfit/id", dimm_base);
> +	sprintf(path, "%s/%s/id", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0) {
>  		unsigned int b[9];
>  
> @@ -1709,68 +1677,74 @@ static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
>  		}
>  	}
>  
> -	sprintf(path, "%s/nfit/handle", dimm_base);
> +	sprintf(path, "%s/%s/handle", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
>  		goto err_read;
>  	dimm->handle = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/phys_id", dimm_base);
> +	sprintf(path, "%s/%s/phys_id", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
>  		goto err_read;
>  	dimm->phys_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/serial", dimm_base);
> +	sprintf(path, "%s/%s/serial", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->serial = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/vendor", dimm_base);
> +	sprintf(path, "%s/%s/vendor", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->vendor_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/device", dimm_base);
> +	sprintf(path, "%s/%s/device", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->device_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/rev_id", dimm_base);
> +	sprintf(path, "%s/%s/rev_id", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->revision_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/dirty_shutdown", dimm_base);
> +	sprintf(path, "%s/%s/dirty_shutdown", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->dirty_shutdown = strtoll(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/subsystem_vendor", dimm_base);
> +	sprintf(path, "%s/%s/subsystem_vendor", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->subsystem_vendor_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/subsystem_device", dimm_base);
> +	sprintf(path, "%s/%s/subsystem_device", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->subsystem_device_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/subsystem_rev_id", dimm_base);
> +	sprintf(path, "%s/%s/subsystem_rev_id", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->subsystem_revision_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/family", dimm_base);
> +	sprintf(path, "%s/%s/family", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->cmd_family = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/dsm_mask", dimm_base);
> +	sprintf(path, "%s/%s/dsm_mask", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->nfit_dsm_mask = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/format", dimm_base);
> +	sprintf(path, "%s/%s/format", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->format[0] = strtoul(buf, NULL, 0);
>  	for (i = 1; i < dimm->formats; i++) {
> -		sprintf(path, "%s/nfit/format%d", dimm_base, i);
> +		sprintf(path, "%s/%s/format%d", dimm_base, bus_prefix, i);
>  		if (sysfs_read_attr(ctx, path, buf) == 0)
>  			dimm->format[i] = strtoul(buf, NULL, 0);
>  	}
>  
> -	sprintf(path, "%s/nfit/flags", dimm_base);
> -	if (sysfs_read_attr(ctx, path, buf) == 0)
> -		parse_nfit_mem_flags(dimm, buf);
> +	sprintf(path, "%s/%s/flags", dimm_base, bus_prefix);
> +	if (sysfs_read_attr(ctx, path, buf) == 0) {
> +		if (ndctl_bus_has_nfit(dimm->bus))
> +			parse_nfit_mem_flags(dimm, buf);
> +		else if (ndctl_bus_is_papr_scm(dimm->bus)) {
> +			dimm->cmd_family = NVDIMM_FAMILY_PAPR;
> +			parse_papr_flags(dimm, buf);
> +		}
> +	}
>  
>  	dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
>  	rc = 0;
> @@ -1792,7 +1766,8 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
>  	if (!path)
>  		return NULL;
>  
> -	sprintf(path, "%s/nfit/formats", dimm_base);
> +	sprintf(path, "%s/%s/formats", dimm_base,
> +		ndctl_bus_has_nfit(bus) ? "nfit" : "papr");
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
>  		formats = 1;
>  	else
> @@ -1866,13 +1841,12 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
>  	else
>  		dimm->fwa_result = fwa_result_to_result(buf);
>  
> +	dimm->formats = formats;
>  	/* Check if the given dimm supports nfit */
>  	if (ndctl_bus_has_nfit(bus)) {
> -		dimm->formats = formats;
> -		rc = add_nfit_dimm(dimm, dimm_base);
> -	} else if (ndctl_bus_has_of_node(bus)) {
> -		rc = add_papr_dimm(dimm, dimm_base);
> -	}
> +		rc = populate_dimm_attributes(dimm, dimm_base, "nfit");
> +	} else if (ndctl_bus_has_of_node(bus))
> +		rc = populate_dimm_attributes(dimm, dimm_base, "papr");
>  
>  	if (rc == -ENODEV) {
>  		/* Unprobed dimm with no family */
> @@ -2531,13 +2505,12 @@ static void *add_region(void *parent, int id, const char *region_base)
>  		goto err_read;
>  	region->num_mappings = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/range_index", region_base);
> -	if (ndctl_bus_has_nfit(bus)) {
> -		if (sysfs_read_attr(ctx, path, buf) < 0)
> -			goto err_read;
> -		region->range_index = strtoul(buf, NULL, 0);
> -	} else
> +	sprintf(path, "%s/%s/range_index", region_base,
> +		ndctl_bus_has_nfit(bus) ? "nfit": "papr");
> +	if (sysfs_read_attr(ctx, path, buf) < 0)
>  		region->range_index = -1;
> +	else
> +		region->range_index = strtoul(buf, NULL, 0);
>  
>  	sprintf(path, "%s/read_only", region_base);
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
> -- 
> 2.30.2
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 2/4] test: Don't skip tests if nfit modules are missing

[Santosh Sivaraj]

Santosh Sivaraj <santosh@fossix.org> writes:

> "Verma, Vishal L" <vishal.l.verma@intel.com> writes:
>
> Hi Vishal,
>
>> On Sun, 2021-03-28 at 07:39 +0530, Santosh Sivaraj wrote:
>>> For NFIT to be available ACPI is a must, so don't fail when nfit modules
>>> are missing on a platform that doesn't support ACPI.
>>> 
>>> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>>> ---
>>>  test.h                        |  2 +-
>>>  test/ack-shutdown-count-set.c |  2 +-
>>>  test/blk_namespaces.c         |  2 +-
>>>  test/core.c                   | 30 ++++++++++++++++++++++++++++--
>>>  test/dpa-alloc.c              |  2 +-
>>>  test/dsm-fail.c               |  2 +-
>>>  test/libndctl.c               |  2 +-
>>>  test/multi-pmem.c             |  2 +-
>>>  test/parent-uuid.c            |  2 +-
>>>  test/pmem_namespaces.c        |  2 +-
>>>  10 files changed, 37 insertions(+), 11 deletions(-)
>>> 
>>
>> I haven't looked deeper, but this seems to fail the blk-ns test with:
>>
>>   ACPI.NFIT unavailable falling back to nfit_test
>>   test/init: ndctl_test_init: Cannot determine NVDIMM family
>>   __ndctl_test_skip: explicit skip test_blk_namespaces:235
>>   nfit_test unavailable skipping tests
>
> The first message will be emitted even without the changes if the bus is not
> found. The second error will be emitted when /sys/bus/acpi is not found. We fail
> for all other buses by default as before. PAPR tests are enabled only when
> NVDIMM_TEST_FAMILY is set to "PAPR".
>
> All tests pass in my setup with the recent upstream kernel, except for the the
> below warning from drivers/acpi/nfit/core.c:
>
> [ 2426.727584] ------------[ cut here ]------------
> [ 2426.728405] WARNING: CPU: 2 PID: 47504 at tools/testing/nvdimm/../../../drivers/acpi/nfit/core.c:3879 nfit_exit+0x]
> [ 2426.730264] Modules linked in: dax_pmem(O) nd_pmem(O) nfit(O-) kmem dax_pmem_compat(O) nd_blk(O) dax_pmem_core(O) ]
> [ 2426.733209] CPU: 2 PID: 47504 Comm: modprobe Tainted: G        W  O      5.12.0+ #3
> [ 2426.734472] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.o4
> [ 2426.736305] RIP: 0010:nfit_exit+0x2c/0x703 [nfit]
> [ 2426.737099] Code: fd ff ff 48 c7 c7 00 f0 39 c0 e8 52 a1 38 da 48 8b 3d 6b 46 00 00 e8 e6 88 ee d9 48 8b 05 5f 3c 0
> [ 2426.740046] RSP: 0018:ffffa8e800b77ed8 EFLAGS: 00010287
> [ 2426.740990] RAX: ffff95b7e51935b0 RBX: 0000000000000800 RCX: ffffffff9b4a36a8
> [ 2426.742236] RDX: 0000000000000000 RSI: 0000000000000083 RDI: ffff95b7c03e1554
> [ 2426.743404] RBP: ffffffffc039f740 R08: 0000000000000400 R09: ffff95b7c03e0e50
> [ 2426.744617] R10: ffff95b7fbd296f0 R11: 0000000000895440 R12: ffffa8e800b77f58
> [ 2426.745792] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 2426.746946] FS:  00007f48297e3740(0000) GS:ffff95b7fbd00000(0000) knlGS:0000000000000000
> [ 2426.748250] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 2426.749198] CR2: 000056072aadc9f8 CR3: 0000000118b08000 CR4: 00000000000006e0
> [ 2426.750349] Call Trace:
> [ 2426.750754]  __do_sys_delete_module+0x19d/0x240
> [ 2426.751472]  ? task_work_run+0x5c/0x90
> [ 2426.751964]  ? exit_to_user_mode_prepare+0x2a/0x130
> [ 2426.752637]  do_syscall_64+0x40/0x80
> [ 2426.753121]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 2426.753810] RIP: 0033:0x7f482991361b
> [ 2426.754274] Code: 73 01 c3 48 8b 0d 5d 18 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 8
> [ 2426.756668] RSP: 002b:00007ffd46c89b98 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> [ 2426.757676] RAX: ffffffffffffffda RBX: 000056072aad8f90 RCX: 00007f482991361b
> [ 2426.758618] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000056072aad8ff8
> [ 2426.759563] RBP: 000056072aad8f90 R08: 0000000000000000 R09: 0000000000000000
> [ 2426.760513] R10: 00007f4829987ac0 R11: 0000000000000206 R12: 000056072aad8ff8
> [ 2426.761463] R13: 0000000000000000 R14: 000056072aadb4e8 R15: 00007ffd46c89d18
> [ 2426.762405] ---[ end trace 14a8748cda8b4777 ]---
>
> This was not seen with the 5.11 kernel.
>

Reverting cf16b05c607b ("ACPICA: ACPI 6.4: NFIT: add Location Cookie field")
doesn't produce the warning.

Thanks,
Santosh

>>
>>> diff --git a/test.h b/test.h
>>> index cba8d41..7de13fe 100644
>>> --- a/test.h
>>> +++ b/test.h
>>> @@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
>>>  
>>> 
>>>  struct kmod_ctx;
>>>  struct kmod_module;
>>> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>>  		struct ndctl_ctx *nd_ctx, int log_level,
>>>  		struct ndctl_test *test);
>>>  
>>> 
>>> diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
>>> index fb1d82b..c561ff3 100644
>>> --- a/test/ack-shutdown-count-set.c
>>> +++ b/test/ack-shutdown-count-set.c
>>> @@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct ndctl_test *test,
>>>  	int result = EXIT_FAILURE, err;
>>>  
>>> 
>>>  	ndctl_set_log_priority(ctx, loglevel);
>>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>>  	if (err < 0) {
>>>  		result = 77;
>>>  		ndctl_test_skip(test);
>>> diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
>>> index d7f00cb..f076e85 100644
>>> --- a/test/blk_namespaces.c
>>> +++ b/test/blk_namespaces.c
>>> @@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test *test,
>>>  
>>> 
>>>  	if (!bus) {
>>>  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
>>> -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>>> +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>>>  		ndctl_invalidate(ctx);
>>>  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>>>  		if (rc < 0 || !bus) {
>>> diff --git a/test/core.c b/test/core.c
>>> index cc7d8d9..44cb277 100644
>>> --- a/test/core.c
>>> +++ b/test/core.c
>>> @@ -11,6 +11,7 @@
>>>  #include <util/log.h>
>>>  #include <util/sysfs.h>
>>>  #include <ndctl/libndctl.h>
>>> +#include <ndctl/ndctl.h>
>>>  #include <ccan/array_size/array_size.h>
>>>  
>>> 
>>>  #define KVER_STRLEN 20
>>> @@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
>>>  	return test->skip;
>>>  }
>>>  
>>> 
>>> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>>  		struct ndctl_ctx *nd_ctx, int log_level,
>>>  		struct ndctl_test *test)
>>>  {
>>> -	int rc;
>>> +	int rc, family = -1;
>>>  	unsigned int i;
>>>  	const char *name;
>>>  	struct ndctl_bus *bus;
>>> @@ -127,10 +128,30 @@ int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>>>  		"nd_e820",
>>>  		"nd_pmem",
>>>  	};
>>> +	char *test_env;
>>>  
>>> 
>>>  	log_init(&log_ctx, "test/init", "NDCTL_TEST");
>>>  	log_ctx.log_priority = log_level;
>>>  
>>> 
>>> +	/*
>>> +	 * The following two checks determine the platform family. For
>>> +	 * Intel/platforms which support ACPI, check sysfs; for other platforms
>>> +	 * determine from the environment variable NVDIMM_TEST_FAMILY
>>> +	 */
>>> +	if (access("/sys/bus/acpi", F_OK) == 0) {
>>> +		if (errno == ENOENT)
>>> +			family = NVDIMM_FAMILY_INTEL;
>>> +	}
>>> +
>>> +	test_env = getenv("NDCTL_TEST_FAMILY");
>>> +	if (test_env && strcmp(test_env, "PAPR") == 0)
>>> +		family = NVDIMM_FAMILY_PAPR;
>>> +
>>> +	if (family == -1) {
>>> +		log_err(&log_ctx, "Cannot determine NVDIMM family\n");
>>> +		return -ENOTSUP;
>>> +	}
>>> +
>>>  	*ctx = kmod_new(NULL, NULL);
>>>  	if (!*ctx)
>>>  		return -ENXIO;
>>> @@ -185,6 +206,11 @@ retry:
>>>  
>>> 
>>>  		path = kmod_module_get_path(*mod);
>>>  		if (!path) {
>>> +			if (family != NVDIMM_FAMILY_INTEL &&
>>> +			    (strcmp(name, "nfit") == 0 ||
>>> +			     strcmp(name, "nd_e820") == 0))
>>> +				continue;
>>> +
>>>  			log_err(&log_ctx, "%s.ko: failed to get path\n", name);
>>>  			break;
>>>  		}
>>> diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
>>> index e922009..0b3bb7a 100644
>>> --- a/test/dpa-alloc.c
>>> +++ b/test/dpa-alloc.c
>>> @@ -289,7 +289,7 @@ int test_dpa_alloc(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>>>  		return 77;
>>>  
>>> 
>>>  	ndctl_set_log_priority(ctx, loglevel);
>>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>>  	if (err < 0) {
>>>  		ndctl_test_skip(test);
>>>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
>>> diff --git a/test/dsm-fail.c b/test/dsm-fail.c
>>> index 9dfd8b0..0a6383d 100644
>>> --- a/test/dsm-fail.c
>>> +++ b/test/dsm-fail.c
>>> @@ -346,7 +346,7 @@ int test_dsm_fail(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>>>  	int result = EXIT_FAILURE, err;
>>>  
>>> 
>>>  	ndctl_set_log_priority(ctx, loglevel);
>>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>>  	if (err < 0) {
>>>  		result = 77;
>>>  		ndctl_test_skip(test);
>>> diff --git a/test/libndctl.c b/test/libndctl.c
>>> index 24d72b3..0e88fce 100644
>>> --- a/test/libndctl.c
>>> +++ b/test/libndctl.c
>>> @@ -2692,7 +2692,7 @@ int test_libndctl(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx)
>>>  	daxctl_set_log_priority(daxctl_ctx, loglevel);
>>>  	ndctl_set_private_data(ctx, test);
>>>  
>>> 
>>> -	err = nfit_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
>>> +	err = ndctl_test_init(&kmod_ctx, &mod, ctx, loglevel, test);
>>>  	if (err < 0) {
>>>  		ndctl_test_skip(test);
>>>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
>>> diff --git a/test/multi-pmem.c b/test/multi-pmem.c
>>> index 3d10952..3ea08cc 100644
>>> --- a/test/multi-pmem.c
>>> +++ b/test/multi-pmem.c
>>> @@ -249,7 +249,7 @@ int test_multi_pmem(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx
>>>  
>>> 
>>>  	ndctl_set_log_priority(ctx, loglevel);
>>>  
>>> 
>>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>>  	if (err < 0) {
>>>  		result = 77;
>>>  		ndctl_test_skip(test);
>>> diff --git a/test/parent-uuid.c b/test/parent-uuid.c
>>> index 6424e9f..bded33a 100644
>>> --- a/test/parent-uuid.c
>>> +++ b/test/parent-uuid.c
>>> @@ -218,7 +218,7 @@ int test_parent_uuid(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ct
>>>  		return 77;
>>>  
>>> 
>>>  	ndctl_set_log_priority(ctx, loglevel);
>>> -	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>> +	err = ndctl_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
>>>  	if (err < 0) {
>>>  		ndctl_test_skip(test);
>>>  		fprintf(stderr, "nfit_test unavailable skipping tests\n");
>>> diff --git a/test/pmem_namespaces.c b/test/pmem_namespaces.c
>>> index f0f2edd..a4db1ae 100644
>>> --- a/test/pmem_namespaces.c
>>> +++ b/test/pmem_namespaces.c
>>> @@ -191,7 +191,7 @@ int test_pmem_namespaces(int log_level, struct ndctl_test *test,
>>>  
>>> 
>>>  	if (!bus) {
>>>  		fprintf(stderr, "ACPI.NFIT unavailable falling back to nfit_test\n");
>>> -		rc = nfit_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>>> +		rc = ndctl_test_init(&kmod_ctx, &mod, NULL, log_level, test);
>>>  		ndctl_invalidate(ctx);
>>>  		bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>>>  		if (rc < 0 || !bus) {
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org