[PATCH 2 1/2] ndctl: introduce 4k allocation support for creating namespace

[PATCH 2 1/2] ndctl: introduce 4k allocation support for creating namespace

[Dave Jiang]

Existing implementation defaults all pages allocated as 2M superpages.
For nfit_test dax device we need 4k pages allocated to work properly.
Adding an --align,-a option to provide the alignment. Will accept
4k and 2M at the moment as proper parameter. No -a option still defaults
to 2M.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 ndctl/builtin-xaction-namespace.c |   34 +++++++++++++++++++++++++++++++---
 util/size.h                       |    1 +
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/ndctl/builtin-xaction-namespace.c b/ndctl/builtin-xaction-namespace.c
index 8257eb9..038e59f 100644
--- a/ndctl/builtin-xaction-namespace.c
+++ b/ndctl/builtin-xaction-namespace.c
@@ -49,6 +49,7 @@ static struct parameters {
 	const char *region;
 	const char *reconfig;
 	const char *sector_size;
+	const char *align;
 } param;
 
 void builtin_xaction_namespace_reset(void)
@@ -71,6 +72,7 @@ struct parsed_parameters {
 	enum ndctl_namespace_mode mode;
 	unsigned long long size;
 	unsigned long sector_size;
+	unsigned long align;
 };
 
 #define debug(fmt, ...) \
@@ -104,6 +106,8 @@ OPT_STRING('l', "sector-size", &param.sector_size, "lba-size", \
 	"specify the logical sector size in bytes"), \
 OPT_STRING('t', "type", &param.type, "type", \
 	"specify the type of namespace to create 'pmem' or 'blk'"), \
+OPT_STRING('a', "align", &param.align, "align", \
+	"specify the namespace alignment in bytes (default: 0x200000 (2M))"), \
 OPT_BOOLEAN('f', "force", &force, "reconfigure namespace even if currently active")
 
 static const struct option base_options[] = {
@@ -319,14 +323,13 @@ static int setup_namespace(struct ndctl_region *region,
 
 		try(ndctl_pfn, set_uuid, pfn, uuid);
 		try(ndctl_pfn, set_location, pfn, p->loc);
-
 		/*
 		 * TODO: when we allow setting a non-default alignment
 		 * we'll need to check for "has_align" earlier and fail
 		 * non-default attempts on older kernels.
 		 */
 		if (ndctl_pfn_has_align(pfn))
-			try(ndctl_pfn, set_align, pfn, SZ_2M);
+			try(ndctl_pfn, set_align, pfn, p->align);
 		try(ndctl_pfn, set_namespace, pfn, ndns);
 		rc = ndctl_pfn_enable(pfn);
 	} else if (p->mode == NDCTL_NS_MODE_DAX) {
@@ -335,7 +338,7 @@ static int setup_namespace(struct ndctl_region *region,
 		try(ndctl_dax, set_uuid, dax, uuid);
 		try(ndctl_dax, set_location, dax, p->loc);
 		/* device-dax assumes 'align' attribute present */
-		try(ndctl_dax, set_align, dax, SZ_2M);
+		try(ndctl_dax, set_align, dax, p->align);
 		try(ndctl_dax, set_namespace, dax, ndns);
 		rc = ndctl_dax_enable(dax);
 	} else if (p->mode == NDCTL_NS_MODE_SAFE) {
@@ -439,6 +442,31 @@ static int validate_namespace_options(struct ndctl_region *region,
 	} else if (ndns)
 		p->mode = ndctl_namespace_get_mode(ndns);
 
+	if ((p->mode == NDCTL_NS_MODE_MEMORY) ||
+			(p->mode == NDCTL_NS_MODE_DAX)) {
+		struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
+
+		if (!ndctl_pfn_has_align(pfn))
+			return -EINVAL;
+
+		if (param.align) {
+			p->align = parse_size64(param.align);
+			switch (p->align) {
+			case SZ_4K:
+			case SZ_2M:
+				break;
+			case SZ_1G: /* unsupported yet... */
+			default:
+				debug("%s: invalid align\n", __func__);
+				return -EINVAL;
+			}
+		} else
+			p->align = SZ_2M;
+	} else if (param.align) {
+		debug("%s: does not support align\n", __func__);
+		return -EINVAL;
+	}
+
 	if (param.sector_size) {
 		struct ndctl_btt *btt;
 		int num, i;
diff --git a/util/size.h b/util/size.h
index 50917a5..634c926 100644
--- a/util/size.h
+++ b/util/size.h
@@ -2,6 +2,7 @@
 #define _NDCTL_SIZE_H_
 
 #define SZ_1K     0x00000400
+#define SZ_4K     0x00001000
 #define SZ_1M     0x00100000
 #define SZ_2M     0x00200000
 #define SZ_4M     0x00400000

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

[PATCH 2 2/2] ndctl: Add test for device-dax error testing path

[Dave Jiang]

The test creates a dax device via the nfit_test with injected error.
The test uses fallocate(PUNCH_HOLE) to clear the error and then check
again to see if the error has been cleared. This requires code that
will be going into 4.10 kernel to discover the badblocks on the device
and fallocate support to clear the errors.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 test/Makefile.am          |    8 ++-
 test/device-dax-errors.c  |  130 +++++++++++++++++++++++++++++++++++++++++++++
 test/device-dax-errors.sh |   52 ++++++++++++++++++
 3 files changed, 188 insertions(+), 2 deletions(-)
 create mode 100644 test/device-dax-errors.c
 create mode 100755 test/device-dax-errors.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index 46a1acf..6dce75e 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -8,7 +8,8 @@ TESTS =\
 	multi-pmem \
 	create.sh \
 	clear.sh \
-	dax-errors.sh
+	dax-errors.sh \
+	device-dax-errors.sh
 
 check_PROGRAMS =\
 	libndctl \
@@ -16,7 +17,8 @@ check_PROGRAMS =\
 	dpa-alloc \
 	parent-uuid \
 	multi-pmem \
-	dax-errors
+	dax-errors \
+	device-dax-errors
 
 if ENABLE_DESTRUCTIVE
 TESTS +=\
@@ -69,6 +71,8 @@ dax_dev_LDADD = $(LIBNDCTL_LIB)
 dax_pmd_SOURCES = dax-pmd.c
 mmap_SOURCES = mmap.c
 dax_errors_SOURCES = dax-errors.c
+device_dax_errors_SOURCES = device-dax-errors.c core.c
+device_dax_errors_LDADD = $(LIBNDCTL_LIB)
 device_dax_SOURCES = \
 		device-dax.c \
 		dax-dev.c \
diff --git a/test/device-dax-errors.c b/test/device-dax-errors.c
new file mode 100644
index 0000000..9705ac6
--- /dev/null
+++ b/test/device-dax-errors.c
@@ -0,0 +1,130 @@
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <linux/falloc.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <libgen.h>
+#include <linux/version.h>
+#include <ndctl/builtin.h>
+#include <test.h>
+
+static int clear_badblocks(int dev_fd, char *dev_path);
+
+static int clear_badblocks(int dev_fd, char *dev_path)
+{
+	char *base, *bb_file;
+	char bb_path[128] = "/sys/class/dax/";
+	int rc, i;
+	unsigned long bbs[32][2];
+	unsigned long bb[2];
+	FILE *fp;
+
+	memset(bb, 0, sizeof(unsigned long) * 32 * 2);
+	base = basename(dev_path);
+	bb_file = strcat(strcat(bb_path, base), "/badblocks");
+
+	printf("opening %s\n", bb_file);
+	fp = fopen(bb_file, "r");
+	if (!fp) {
+		perror("open failed");
+		return -ENXIO;
+	}
+
+	i = 0;
+
+	/* read out all the bad blocks */
+	while ((fscanf(fp, "%lu %lu", &bbs[i][0], &bbs[i][1]) != EOF) &&
+	       (i++ < 32))
+		printf("badblock %d @ %lu for len %lu\n",
+			i-1, bbs[i-1][0], bbs[i-1][1]);
+
+	i = 0;
+	while (bbs[i][1]) {
+		printf("fallocate @ sector %lu for %lu bytes\n",
+			bbs[i][0] * 512, bbs[i][1] * 512);
+		rc = fallocate(dev_fd,
+			FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+			bbs[i][0] * 512, bbs[i][1] * 512);
+		if (rc < 0) {
+			perror("fallocate failed");
+			return -ENXIO;
+		}
+		i++;
+	}
+
+	if (!i) {
+		printf("No badblocks found\n");
+		goto exit;
+	}
+
+	fseek(fp, 0, SEEK_SET);
+
+	if (fscanf(fp, "%lu %lu", &bb[0], &bb[1]) != EOF) {
+		perror("reading badblocks");
+		rc = -ENXIO;
+		goto exit;
+	}
+
+	if (bb[0] || bb[1])
+		rc = -ENXIO;
+	else
+		rc = 0;
+
+exit:
+	fclose(fp);
+	return rc;
+}
+
+int main(int argc, char *argv[])
+{
+	int fd, rc;
+	char *path;
+	struct stat st;
+	struct ndctl_test *test = ndctl_test_new(0);
+
+	if (!test) {
+		fprintf(stderr, "failed to initialize test\n");
+		return EXIT_FAILURE;
+	}
+
+	if (!ndctl_test_attempt(test, KERNEL_VERSION(4, 10, 0)))
+		return EXIT_FAILURE;
+
+	if (argc < 2) {
+		perror("argc invalid");
+		return -EINVAL;
+	}
+
+	if (stat(argv[1], &st) < 0) {
+		perror("invalid file");
+		return EXIT_FAILURE;
+	}
+
+	if (!S_ISCHR(st.st_mode)) {
+		fprintf(stderr, "%s not char device\n", argv[1]);
+		return EXIT_FAILURE;
+	}
+
+	fd = open(argv[1], O_RDWR);
+	if (fd < 0) {
+		perror("fd");
+		return EXIT_FAILURE;
+	}
+
+	path = strdup(argv[1]);
+	rc = clear_badblocks(fd, path);
+	if (rc < 0) {
+		fprintf(stderr, "Failed to clear badblocks on %s\n", argv[1]);
+		return EXIT_FAILURE;
+	}
+
+	free(path);
+	close(fd);
+
+	return EXIT_SUCCESS;
+}
diff --git a/test/device-dax-errors.sh b/test/device-dax-errors.sh
new file mode 100755
index 0000000..54c43de
--- /dev/null
+++ b/test/device-dax-errors.sh
@@ -0,0 +1,52 @@
+#!/bin/bash -x
+
+DEV=""
+NDCTL="../ndctl/ndctl"
+BUS="-b nfit_test.0"
+json2var="s/[{}\",]//g; s/:/=/g"
+rc=77
+
+err() {
+	rc=1
+	echo "test/device-dax-errors: failed at line $1"
+	exit $rc
+}
+
+eval $(uname -r | awk -F. '{print "maj="$1 ";" "min="$2}')
+if [ $maj -lt 4 ]; then
+	echo "kernel $maj.$min lacks dax error handling"
+	exit $rc
+elif [ $maj -eq 4 -a $min -lt 7 ]; then
+	echo "kernel $maj.$min lacks dax error handling"
+	exit $rc
+fi
+
+set -e
+trap 'err $LINENO' ERR
+
+# setup (reset nfit_test dimms)
+modprobe nfit_test
+$NDCTL disable-region $BUS all
+$NDCTL zero-labels $BUS all
+$NDCTL enable-region $BUS all
+
+rc=1
+
+# create device dax
+dev="x"
+json=$($NDCTL create-namespace $BUS -t pmem -m dax -a 0x1000)
+eval $(echo $json | sed -e 's/\]//g' -e "$json2var")
+[ $dev = "x" ] && echo "fail: $LINENO" && exit 1
+[ $mode != "dax" ] && echo "fail: $LINENO" && exit 1
+
+# run the dax-errors test
+test -x ./device-dax-errors
+echo "device-dax-errors /dev/$chardev"
+./device-dax-errors /dev/$chardev
+
+# cleanup
+$NDCTL disable-region $BUS all
+$NDCTL disable-region $BUS1 all
+modprobe -r nfit_test
+
+exit 0

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

Re: [PATCH 2 1/2] ndctl: introduce 4k allocation support for creating namespace

[Vishal Verma]

On 12/08, Dave Jiang wrote:
> Existing implementation defaults all pages allocated as 2M superpages.
> For nfit_test dax device we need 4k pages allocated to work properly.
> Adding an --align,-a option to provide the alignment. Will accept
> 4k and 2M at the moment as proper parameter. No -a option still defaults
> to 2M.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  ndctl/builtin-xaction-namespace.c |   34 +++++++++++++++++++++++++++++++---
>  util/size.h                       |    1 +
>  2 files changed, 32 insertions(+), 3 deletions(-)

A patch for bash completiion for this is at the bottom.

A minor nit below, but other than that:

Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>

> 
> diff --git a/ndctl/builtin-xaction-namespace.c b/ndctl/builtin-xaction-namespace.c
> index 8257eb9..038e59f 100644
> --- a/ndctl/builtin-xaction-namespace.c
> +++ b/ndctl/builtin-xaction-namespace.c
> @@ -49,6 +49,7 @@ static struct parameters {
>  	const char *region;
>  	const char *reconfig;
>  	const char *sector_size;
> +	const char *align;
>  } param;
>  
>  void builtin_xaction_namespace_reset(void)
> @@ -71,6 +72,7 @@ struct parsed_parameters {
>  	enum ndctl_namespace_mode mode;
>  	unsigned long long size;
>  	unsigned long sector_size;
> +	unsigned long align;
>  };
>  
>  #define debug(fmt, ...) \
> @@ -104,6 +106,8 @@ OPT_STRING('l', "sector-size", &param.sector_size, "lba-size", \
>  	"specify the logical sector size in bytes"), \
>  OPT_STRING('t', "type", &param.type, "type", \
>  	"specify the type of namespace to create 'pmem' or 'blk'"), \
> +OPT_STRING('a', "align", &param.align, "align", \
> +	"specify the namespace alignment in bytes (default: 0x200000 (2M))"), \
>  OPT_BOOLEAN('f', "force", &force, "reconfigure namespace even if currently active")
>  
>  static const struct option base_options[] = {
> @@ -319,14 +323,13 @@ static int setup_namespace(struct ndctl_region *region,
>  
>  		try(ndctl_pfn, set_uuid, pfn, uuid);
>  		try(ndctl_pfn, set_location, pfn, p->loc);
> -
>  		/*
>  		 * TODO: when we allow setting a non-default alignment
>  		 * we'll need to check for "has_align" earlier and fail
>  		 * non-default attempts on older kernels.
>  		 */
>  		if (ndctl_pfn_has_align(pfn))
> -			try(ndctl_pfn, set_align, pfn, SZ_2M);
> +			try(ndctl_pfn, set_align, pfn, p->align);
>  		try(ndctl_pfn, set_namespace, pfn, ndns);
>  		rc = ndctl_pfn_enable(pfn);
>  	} else if (p->mode == NDCTL_NS_MODE_DAX) {
> @@ -335,7 +338,7 @@ static int setup_namespace(struct ndctl_region *region,
>  		try(ndctl_dax, set_uuid, dax, uuid);
>  		try(ndctl_dax, set_location, dax, p->loc);
>  		/* device-dax assumes 'align' attribute present */
> -		try(ndctl_dax, set_align, dax, SZ_2M);
> +		try(ndctl_dax, set_align, dax, p->align);
>  		try(ndctl_dax, set_namespace, dax, ndns);
>  		rc = ndctl_dax_enable(dax);
>  	} else if (p->mode == NDCTL_NS_MODE_SAFE) {
> @@ -439,6 +442,31 @@ static int validate_namespace_options(struct ndctl_region *region,
>  	} else if (ndns)
>  		p->mode = ndctl_namespace_get_mode(ndns);
>  
> +	if ((p->mode == NDCTL_NS_MODE_MEMORY) ||
> +			(p->mode == NDCTL_NS_MODE_DAX)) {
> +		struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
> +
> +		if (!ndctl_pfn_has_align(pfn))
> +			return -EINVAL;
> +
> +		if (param.align) {
> +			p->align = parse_size64(param.align);
> +			switch (p->align) {
> +			case SZ_4K:
> +			case SZ_2M:
> +				break;
> +			case SZ_1G: /* unsupported yet... */
> +			default:
> +				debug("%s: invalid align\n", __func__);

s/align/alignment ?

<snip>

8<----

>From e705e371fd3c5ab00572db30955f2ee18ba74134 Mon Sep 17 00:00:00 2001
From: Vishal Verma <vishal.l.verma@intel.com>
Date: Thu, 8 Dec 2016 17:16:45 -0700
Subject: [ndctl PATCH] ndctl: add the new --align option to bash completion

Add completion for --align, and provide only the expected options of 4K,
2M, and 1G.

Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 contrib/ndctl | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contrib/ndctl b/contrib/ndctl
index ea7303c..162d47f 100755
--- a/contrib/ndctl
+++ b/contrib/ndctl
@@ -91,7 +91,7 @@ __ndctlcomp()
 
 	COMPREPLY=( $( compgen -W "$1" -- "$2" ) )
 	for cword in "${COMPREPLY[@]}"; do
-		if [[ "$cword" == @(--bus|--region|--type|--mode|--size|--dimm|--reconfig|--uuid|--name|--sector-size|--map|--namespace) ]]; then
+		if [[ "$cword" == @(--bus|--region|--type|--mode|--size|--dimm|--reconfig|--uuid|--name|--sector-size|--map|--namespace|--align) ]]; then
 			COMPREPLY[$i]="${cword}="
 		else
 			COMPREPLY[$i]="${cword} "
@@ -171,6 +171,9 @@ __ndctl_comp_options()
 		--map)
 			opts="mem dev"
 			;;
+		--align)
+			opts="4K 2M 1G"
+			;;
 		*)
 			return
 			;;
-- 
2.7.4

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