diff mbox

[ndctl] ndctl, create-namespace: fix size alignment validation

Message ID 150109896285.13943.15686058156974485950.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive)
State Accepted
Commit 5c763a54625e
Headers show

Commit Message

Dan Williams July 26, 2017, 7:56 p.m. UTC
Linda reports:

    I was creating a namespace on a 4-way interleave set and got an error
    I didn't expect:

    $ sudo ndctl create-namespace -m sector -s 10G -n number2
      Error: '--size=' must align to interleave-width: 4 and alignment: 2097152
      did you intend --size=12G?

    What's happening is that since I specified my size in units of G,
    the function wants the namespace to be 1G * 4 aligned rather than
    2M * 4 aligned.

Fix this by using the base "align * interleave_ways" for the alignment
check, and later calculate a 'friendly' recommended size value that
takes into account the units specified for --size.

Reported-by: Linda Knippers <linda.knippers@hpe.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Linda Knippers July 26, 2017, 8:12 p.m. UTC | #1
On 7/26/2017 3:56 PM, Dan Williams wrote:
> Linda reports:
>
>     I was creating a namespace on a 4-way interleave set and got an error
>     I didn't expect:
>
>     $ sudo ndctl create-namespace -m sector -s 10G -n number2
>       Error: '--size=' must align to interleave-width: 4 and alignment: 2097152
>       did you intend --size=12G?
>
>     What's happening is that since I specified my size in units of G,
>     the function wants the namespace to be 1G * 4 aligned rather than
>     2M * 4 aligned.
>
> Fix this by using the base "align * interleave_ways" for the alignment
> check, and later calculate a 'friendly' recommended size value that
> takes into account the units specified for --size.
>
> Reported-by: Linda Knippers <linda.knippers@hpe.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  ndctl/namespace.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> index 73a422633a0f..c4d70c39c6c4 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c
> @@ -557,8 +557,7 @@ static int validate_namespace_options(struct ndctl_region *region,
>
>  	/* (re-)validate that the size satisfies the alignment */
>  	ways = ndctl_region_get_interleave_ways(region);
> -	size_align = max(units, size_align) * ways;
> -	if (p->size % size_align) {
> +	if (p->size % (size_align * ways)) {
>  		char *suffix = "";
>
>  		if (units == SZ_1K)
> @@ -570,6 +569,12 @@ static int validate_namespace_options(struct ndctl_region *region,
>  		else if (units == SZ_1T)
>  			suffix = "T";
>
> +		/*
> +		 * Make the recommendation in the units of the '--size'
> +		 * option
> +		 */
> +		size_align = max(units, size_align) * ways;

I can't test this because I don't have the system now but doesn't this
introduce the same problem, but only in the failure case?  I think I'll
get a different recommendation if I use a size of 1M vs. 1024K vs. just bytes.
Or is that what you want?  You want the closest value in that unit rather than
the closest value that would meet the requirements?

> +
>  		p->size /= size_align;
>  		p->size++;
>  		p->size *= size_align;
>
Dan Williams July 26, 2017, 8:19 p.m. UTC | #2
On Wed, Jul 26, 2017 at 1:12 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
> On 7/26/2017 3:56 PM, Dan Williams wrote:
>>
>> Linda reports:
>>
>>     I was creating a namespace on a 4-way interleave set and got an error
>>     I didn't expect:
>>
>>     $ sudo ndctl create-namespace -m sector -s 10G -n number2
>>       Error: '--size=' must align to interleave-width: 4 and alignment:
>> 2097152
>>       did you intend --size=12G?
>>
>>     What's happening is that since I specified my size in units of G,
>>     the function wants the namespace to be 1G * 4 aligned rather than
>>     2M * 4 aligned.
>>
>> Fix this by using the base "align * interleave_ways" for the alignment
>> check, and later calculate a 'friendly' recommended size value that
>> takes into account the units specified for --size.
>>
>> Reported-by: Linda Knippers <linda.knippers@hpe.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>>  ndctl/namespace.c |    9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
>> index 73a422633a0f..c4d70c39c6c4 100644
>> --- a/ndctl/namespace.c
>> +++ b/ndctl/namespace.c
>> @@ -557,8 +557,7 @@ static int validate_namespace_options(struct
>> ndctl_region *region,
>>
>>         /* (re-)validate that the size satisfies the alignment */
>>         ways = ndctl_region_get_interleave_ways(region);
>> -       size_align = max(units, size_align) * ways;
>> -       if (p->size % size_align) {
>> +       if (p->size % (size_align * ways)) {
>>                 char *suffix = "";
>>
>>                 if (units == SZ_1K)
>> @@ -570,6 +569,12 @@ static int validate_namespace_options(struct
>> ndctl_region *region,
>>                 else if (units == SZ_1T)
>>                         suffix = "T";
>>
>> +               /*
>> +                * Make the recommendation in the units of the '--size'
>> +                * option
>> +                */
>> +               size_align = max(units, size_align) * ways;
>
>
> I can't test this because I don't have the system now but doesn't this
> introduce the same problem, but only in the failure case?  I think I'll
> get a different recommendation if I use a size of 1M vs. 1024K vs. just
> bytes.
> Or is that what you want?  You want the closest value in that unit rather
> than
> the closest value that would meet the requirements?

Right, the former, i.e. the closest without changing units.
Linda Knippers July 26, 2017, 8:21 p.m. UTC | #3
On 7/26/2017 4:19 PM, Dan Williams wrote:
>>> @@ -570,6 +569,12 @@ static int validate_namespace_options(struct
>>> ndctl_region *region,
>>>                 else if (units == SZ_1T)
>>>                         suffix = "T";
>>>
>>> +               /*
>>> +                * Make the recommendation in the units of the '--size'
>>> +                * option
>>> +                */
>>> +               size_align = max(units, size_align) * ways;
>>
>>
>> I can't test this because I don't have the system now but doesn't this
>> introduce the same problem, but only in the failure case?  I think I'll
>> get a different recommendation if I use a size of 1M vs. 1024K vs. just
>> bytes.
>> Or is that what you want?  You want the closest value in that unit rather
>> than
>> the closest value that would meet the requirements?
>
> Right, the former, i.e. the closest without changing units.

Since it's just a recommendation, sounds harmless and probably what
half the users will want.

Thanks,

-- ljk
diff mbox

Patch

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 73a422633a0f..c4d70c39c6c4 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -557,8 +557,7 @@  static int validate_namespace_options(struct ndctl_region *region,
 
 	/* (re-)validate that the size satisfies the alignment */
 	ways = ndctl_region_get_interleave_ways(region);
-	size_align = max(units, size_align) * ways;
-	if (p->size % size_align) {
+	if (p->size % (size_align * ways)) {
 		char *suffix = "";
 
 		if (units == SZ_1K)
@@ -570,6 +569,12 @@  static int validate_namespace_options(struct ndctl_region *region,
 		else if (units == SZ_1T)
 			suffix = "T";
 
+		/*
+		 * Make the recommendation in the units of the '--size'
+		 * option
+		 */
+		size_align = max(units, size_align) * ways;
+
 		p->size /= size_align;
 		p->size++;
 		p->size *= size_align;