diff mbox series

[ndctl] infoblock: Set the default alignment to the platform alignment

Message ID 20200707005641.3936295-1-santosh@fossix.org (mailing list archive)
State Superseded
Headers show
Series [ndctl] infoblock: Set the default alignment to the platform alignment | expand

Commit Message

Santosh Sivaraj July 7, 2020, 12:56 a.m. UTC 
The default alignment for write-infoblock command is set to 2M. Change
that to use the platform's supported alignment or PAGE_SIZE. The first
supported alignment is taken as the default.

Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
 ndctl/namespace.c | 56 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 46 insertions(+), 10 deletions(-)

Comments

Ira Weiny July 7, 2020, 9:12 p.m. UTC | #1 
On Tue, Jul 07, 2020 at 06:26:41AM +0530, Santosh Sivaraj wrote:
> The default alignment for write-infoblock command is set to 2M. Change
> that to use the platform's supported alignment or PAGE_SIZE. The first
> supported alignment is taken as the default.
> 
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---

[snip]

> @@ -1992,12 +2001,36 @@ static int namespace_rw_infoblock(struct ndctl_namespace *ndns,
>  	const char *save;
>  	const char *cmd = write ? "write-infoblock" : "read-infoblock";
>  	const char *devname = ndctl_namespace_get_devname(ndns);
> +	unsigned long long align;
>  
>  	if (ndctl_namespace_is_active(ndns)) {
>  		pr_verbose("%s: %s enabled, must be disabled\n", cmd, devname);
>  		return -EBUSY;
>  	}
>  
> +	if (write) {
> +		if (!param.align) {
> +			align = ndctl_get_default_alignment(ndns);
> +
> +			if (asprintf((char **)&param.align, "%llu", align) < 0) {

If we are looping through namespaces doesn't param.align need to be localized
to this function as well?

Ira

> +				rc = -EINVAL;
> +				goto out;
> +			}
> +		}
> +
> +		if (param.size) {
> +			unsigned long long size = parse_size64(param.size);
> +			align = parse_size64(param.align);
> +
> +			if (align < ULLONG_MAX && !IS_ALIGNED(size, align)) {
> +				error("--size=%s not aligned to %s\n", param.size,
> +				      param.align);
> +				rc = -EINVAL;
> +				goto out;
> +			}
> +		}
> +	}
> +
>  	ndctl_namespace_set_raw_mode(ndns, 1);
>  	rc = ndctl_namespace_enable(ndns);
>  	if (rc < 0) {
> @@ -2060,6 +2093,9 @@ static int do_xaction_namespace(const char *namespace,
>  	}
>  
>  	if (action == ACTION_WRITE_INFOBLOCK && !namespace) {
> +		if (!param.align)
> +			param.align = "2M";
> +
>  		rc = file_write_infoblock(param.outfile);
>  		if (rc >= 0)
>  			(*processed)++;
> -- 
> 2.26.2
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
Santosh Sivaraj July 8, 2020, 6:33 a.m. UTC | #2 
Ira Weiny <ira.weiny@intel.com> writes:

> On Tue, Jul 07, 2020 at 06:26:41AM +0530, Santosh Sivaraj wrote:
>> The default alignment for write-infoblock command is set to 2M. Change
>> that to use the platform's supported alignment or PAGE_SIZE. The first
>> supported alignment is taken as the default.
>> 
>> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>> ---
>
> [snip]
>
>> @@ -1992,12 +2001,36 @@ static int namespace_rw_infoblock(struct ndctl_namespace *ndns,
>>  	const char *save;
>>  	const char *cmd = write ? "write-infoblock" : "read-infoblock";
>>  	const char *devname = ndctl_namespace_get_devname(ndns);
>> +	unsigned long long align;
>>  
>>  	if (ndctl_namespace_is_active(ndns)) {
>>  		pr_verbose("%s: %s enabled, must be disabled\n", cmd, devname);
>>  		return -EBUSY;
>>  	}
>>  
>> +	if (write) {
>> +		if (!param.align) {
>> +			align = ndctl_get_default_alignment(ndns);
>> +
>> +			if (asprintf((char **)&param.align, "%llu", align) < 0) {
>
> If we are looping through namespaces doesn't param.align need to be localized
> to this function as well?

Thanks for reviewing!

Right, I missed the "all" case. I will get that fixed this in v2.

Thanks,
Santosh

>
> Ira
>
>> +				rc = -EINVAL;
>> +				goto out;
>> +			}
>> +		}
>> +
>> +		if (param.size) {
>> +			unsigned long long size = parse_size64(param.size);
>> +			align = parse_size64(param.align);
>> +
>> +			if (align < ULLONG_MAX && !IS_ALIGNED(size, align)) {
>> +				error("--size=%s not aligned to %s\n", param.size,
>> +				      param.align);
>> +				rc = -EINVAL;
>> +				goto out;
>> +			}
>> +		}
>> +	}
>> +
>>  	ndctl_namespace_set_raw_mode(ndns, 1);
>>  	rc = ndctl_namespace_enable(ndns);
>>  	if (rc < 0) {
>> @@ -2060,6 +2093,9 @@ static int do_xaction_namespace(const char *namespace,
>>  	}
>>  
>>  	if (action == ACTION_WRITE_INFOBLOCK && !namespace) {
>> +		if (!param.align)
>> +			param.align = "2M";
>> +
>>  		rc = file_write_infoblock(param.outfile);
>>  		if (rc >= 0)
>>  			(*processed)++;
>> -- 
>> 2.26.2
>> _______________________________________________
>> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
>> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
diff mbox series

Patch

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 0550580..4f056b7 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -175,7 +175,7 @@  OPT_STRING('m', "mode", &param.mode, "operation-mode", \
 OPT_STRING('s', "size", &param.size, "size", \
 	"override the image size to instantiate the infoblock"), \
 OPT_STRING('a', "align", &param.align, "align", \
-	"specify the expected physical alignment (default: 2M)"), \
+	"specify the expected physical alignment"), \
 OPT_STRING('u', "uuid", &param.uuid, "uuid", \
 	"specify the uuid for the infoblock (default: autogenerate)"), \
 OPT_STRING('M', "map", &param.map, "memmap-location", \
@@ -325,23 +325,15 @@  static int set_defaults(enum device_action action)
 					sysconf(_SC_PAGE_SIZE));
 			rc = -EINVAL;
 		}
-	} else if (action == ACTION_WRITE_INFOBLOCK)
-		param.align = "2M";
+	}
 
 	if (param.size) {
 		unsigned long long size = parse_size64(param.size);
-		unsigned long long align = parse_size64(param.align);
 
 		if (size == ULLONG_MAX) {
 			error("failed to parse namespace size '%s'\n",
 					param.size);
 			rc = -EINVAL;
-		} else if (action == ACTION_WRITE_INFOBLOCK
-				&& align < ULLONG_MAX
-				&& !IS_ALIGNED(size, align)) {
-			error("--size=%s not aligned to %s\n", param.size,
-					param.align);
-			rc = -EINVAL;
 		}
 	}
 
@@ -1982,6 +1974,23 @@  out:
 	return rc;
 }
 
+static unsigned long ndctl_get_default_alignment(struct ndctl_namespace *ndns)
+{
+	unsigned long long align = 0;
+	struct ndctl_dax *dax = ndctl_namespace_get_dax(ndns);
+	struct ndctl_pfn *pfn = ndctl_namespace_get_pfn(ndns);
+
+	if (ndctl_namespace_get_mode(ndns) == NDCTL_NS_MODE_FSDAX && pfn)
+		align = ndctl_pfn_get_supported_alignment(pfn, 1);
+	else if (ndctl_namespace_get_mode(ndns) == NDCTL_NS_MODE_DEVDAX && dax)
+		align = ndctl_dax_get_supported_alignment(dax, 1);
+
+	if (!align)
+		align =  sysconf(_SC_PAGE_SIZE);
+
+	return align;
+}
+
 static int namespace_rw_infoblock(struct ndctl_namespace *ndns,
 		struct read_infoblock_ctx *ri_ctx, int write)
 {
@@ -1992,12 +2001,36 @@  static int namespace_rw_infoblock(struct ndctl_namespace *ndns,
 	const char *save;
 	const char *cmd = write ? "write-infoblock" : "read-infoblock";
 	const char *devname = ndctl_namespace_get_devname(ndns);
+	unsigned long long align;
 
 	if (ndctl_namespace_is_active(ndns)) {
 		pr_verbose("%s: %s enabled, must be disabled\n", cmd, devname);
 		return -EBUSY;
 	}
 
+	if (write) {
+		if (!param.align) {
+			align = ndctl_get_default_alignment(ndns);
+
+			if (asprintf((char **)&param.align, "%llu", align) < 0) {
+				rc = -EINVAL;
+				goto out;
+			}
+		}
+
+		if (param.size) {
+			unsigned long long size = parse_size64(param.size);
+			align = parse_size64(param.align);
+
+			if (align < ULLONG_MAX && !IS_ALIGNED(size, align)) {
+				error("--size=%s not aligned to %s\n", param.size,
+				      param.align);
+				rc = -EINVAL;
+				goto out;
+			}
+		}
+	}
+
 	ndctl_namespace_set_raw_mode(ndns, 1);
 	rc = ndctl_namespace_enable(ndns);
 	if (rc < 0) {
@@ -2060,6 +2093,9 @@  static int do_xaction_namespace(const char *namespace,
 	}
 
 	if (action == ACTION_WRITE_INFOBLOCK && !namespace) {
+		if (!param.align)
+			param.align = "2M";
+
 		rc = file_write_infoblock(param.outfile);
 		if (rc >= 0)
 			(*processed)++;