* [PATCH] ndctl: Use the same align value as original namespace on reconfigure
@ 2019-08-07 4:44 Aneesh Kumar K.V
2019-08-27 8:39 ` Aneesh Kumar K.V
2019-08-27 18:19 ` Verma, Vishal L
0 siblings, 2 replies; 6+ messages in thread
From: Aneesh Kumar K.V @ 2019-08-07 4:44 UTC (permalink / raw)
To: dan.j.williams; +Cc: Aneesh Kumar K.V, linux-nvdimm
When using reconfigure command to add a `name` to the namespace we end
up updating the align attribute. Avoid this by using the value from
the original namespace. Do this only if we are keeping the namespace mode
same.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
ndctl/namespace.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 1f212a2b3a9b..24e51bb35ae1 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -596,6 +596,22 @@ static int validate_namespace_options(struct ndctl_region *region,
return -ENXIO;
}
} else {
+
+ /*
+ * If we are tryint to reconfigure with the same namespace mode
+ * Use the align details from the origin namespace. Otherwise
+ * pick the align details from seed namespace
+ */
+ if (ndns && p->mode == ndctl_namespace_get_mode(ndns)) {
+ struct ndctl_pfn *ns_pfn = ndctl_namespace_get_pfn(ndns);
+ struct ndctl_dax *ns_dax = ndctl_namespace_get_dax(ndns);
+ if (ns_pfn)
+ p->align = ndctl_pfn_get_align(ns_pfn);
+ else if (ns_dax)
+ p->align = ndctl_dax_get_align(ns_dax);
+ else
+ p->align = sysconf(_SC_PAGE_SIZE);
+ } else
/*
* Use the seed namespace alignment as the default if we need
* one. If we don't then use PAGE_SIZE so the size_align
--
2.21.0
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ndctl: Use the same align value as original namespace on reconfigure
2019-08-07 4:44 [PATCH] ndctl: Use the same align value as original namespace on reconfigure Aneesh Kumar K.V
@ 2019-08-27 8:39 ` Aneesh Kumar K.V
2019-08-27 18:19 ` Verma, Vishal L
1 sibling, 0 replies; 6+ messages in thread
From: Aneesh Kumar K.V @ 2019-08-27 8:39 UTC (permalink / raw)
To: dan.j.williams, vishal.l.verma; +Cc: linux-nvdimm
Hi Dan/Vishal,
Any update on this?
-aneesh
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> When using reconfigure command to add a `name` to the namespace we end
> up updating the align attribute. Avoid this by using the value from
> the original namespace. Do this only if we are keeping the namespace mode
> same.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> ndctl/namespace.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> index 1f212a2b3a9b..24e51bb35ae1 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c
> @@ -596,6 +596,22 @@ static int validate_namespace_options(struct ndctl_region *region,
> return -ENXIO;
> }
> } else {
> +
> + /*
> + * If we are tryint to reconfigure with the same namespace mode
> + * Use the align details from the origin namespace. Otherwise
> + * pick the align details from seed namespace
> + */
> + if (ndns && p->mode == ndctl_namespace_get_mode(ndns)) {
> + struct ndctl_pfn *ns_pfn = ndctl_namespace_get_pfn(ndns);
> + struct ndctl_dax *ns_dax = ndctl_namespace_get_dax(ndns);
> + if (ns_pfn)
> + p->align = ndctl_pfn_get_align(ns_pfn);
> + else if (ns_dax)
> + p->align = ndctl_dax_get_align(ns_dax);
> + else
> + p->align = sysconf(_SC_PAGE_SIZE);
> + } else
> /*
> * Use the seed namespace alignment as the default if we need
> * one. If we don't then use PAGE_SIZE so the size_align
> --
> 2.21.0
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ndctl: Use the same align value as original namespace on reconfigure
2019-08-07 4:44 [PATCH] ndctl: Use the same align value as original namespace on reconfigure Aneesh Kumar K.V
2019-08-27 8:39 ` Aneesh Kumar K.V
@ 2019-08-27 18:19 ` Verma, Vishal L
2019-08-28 9:43 ` Aneesh Kumar K.V
1 sibling, 1 reply; 6+ messages in thread
From: Verma, Vishal L @ 2019-08-27 18:19 UTC (permalink / raw)
To: aneesh.kumar, Williams, Dan; +Cc: linux-nvdimm
On Wed, 2019-08-07 at 10:14 +0530, Aneesh Kumar K.V wrote:
> When using reconfigure command to add a `name` to the namespace we end
> up updating the align attribute. Avoid this by using the value from
> the original namespace. Do this only if we are keeping the namespace mode
> same.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> ndctl/namespace.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
Hi Aneesh,
A few comments below:
>
> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> index 1f212a2b3a9b..24e51bb35ae1 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c
> @@ -596,6 +596,22 @@ static int validate_namespace_options(struct ndctl_region *region,
> return -ENXIO;
> }
> } else {
> +
> + /*
> + * If we are tryint to reconfigure with the same namespace mode
^trying
> + * Use the align details from the origin namespace. Otherwise
s/origin/original/
> + * pick the align details from seed namespace
> + */
> + if (ndns && p->mode == ndctl_namespace_get_mode(ndns)) {
Do we need to depend on the mode here?
I'm thinking it should be sufficient to do:
1. Check We're in 'reconfigure'
2. Check param.align was not supplied
3. Get alignment from the pfn/dax personality, and just use that.
Does this make sense (Maybe I'm missing something).
> + struct ndctl_pfn *ns_pfn = ndctl_namespace_get_pfn(ndns);
> + struct ndctl_dax *ns_dax = ndctl_namespace_get_dax(ndns);
> + if (ns_pfn)
> + p->align = ndctl_pfn_get_align(ns_pfn);
> + else if (ns_dax)
> + p->align = ndctl_dax_get_align(ns_dax);
> + else
> + p->align = sysconf(_SC_PAGE_SIZE);
Do we need the page size fallback here - there are other checks after
this point that also do a similar fallback, do they not catch the
default case?
> + } else
> /*
> * Use the seed namespace alignment as the default if we need
> * one. If we don't then use PAGE_SIZE so the size_align
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ndctl: Use the same align value as original namespace on reconfigure
2019-08-27 18:19 ` Verma, Vishal L
@ 2019-08-28 9:43 ` Aneesh Kumar K.V
2019-10-18 10:25 ` Aneesh Kumar K.V
0 siblings, 1 reply; 6+ messages in thread
From: Aneesh Kumar K.V @ 2019-08-28 9:43 UTC (permalink / raw)
To: Verma, Vishal L, Williams; +Cc: linux-nvdimm
"Verma, Vishal L" <vishal.l.verma@intel.com> writes:
> On Wed, 2019-08-07 at 10:14 +0530, Aneesh Kumar K.V wrote:
>> When using reconfigure command to add a `name` to the namespace we end
>> up updating the align attribute. Avoid this by using the value from
>> the original namespace. Do this only if we are keeping the namespace mode
>> same.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> ndctl/namespace.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>
> Hi Aneesh,
>
> A few comments below:
>
>>
>> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
>> index 1f212a2b3a9b..24e51bb35ae1 100644
>> --- a/ndctl/namespace.c
>> +++ b/ndctl/namespace.c
>> @@ -596,6 +596,22 @@ static int validate_namespace_options(struct ndctl_region *region,
>> return -ENXIO;
>> }
>> } else {
>> +
>> + /*
>> + * If we are tryint to reconfigure with the same namespace mode
>
> ^trying
>
>> + * Use the align details from the origin namespace. Otherwise
>
> s/origin/original/
>
>> + * pick the align details from seed namespace
>> + */
>> + if (ndns && p->mode == ndctl_namespace_get_mode(ndns)) {
>
> Do we need to depend on the mode here?
>
> I'm thinking it should be sufficient to do:
> 1. Check We're in 'reconfigure'
> 2. Check param.align was not supplied
> 3. Get alignment from the pfn/dax personality, and just use that.
>
> Does this make sense (Maybe I'm missing something).
We want to use the align value from the seed when we are trying
to reconfigure a namespace with a different mode. ie, if we are moving a
fsdax namespace with align value 64K to a devdax, IMHO we should pick 16M
as alignment for devdax.
>
>> + struct ndctl_pfn *ns_pfn = ndctl_namespace_get_pfn(ndns);
>> + struct ndctl_dax *ns_dax = ndctl_namespace_get_dax(ndns);
>> + if (ns_pfn)
>> + p->align = ndctl_pfn_get_align(ns_pfn);
>> + else if (ns_dax)
>> + p->align = ndctl_dax_get_align(ns_dax);
>> + else
>> + p->align = sysconf(_SC_PAGE_SIZE);
>
> Do we need the page size fallback here - there are other checks after
> this point that also do a similar fallback, do they not catch the
> default case?
I did that to simplify the code with that `else if`
>
>> + } else
>> /*
>> * Use the seed namespace alignment as the default if we need
>> * one. If we don't then use PAGE_SIZE so the size_align
-aneesh
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ndctl: Use the same align value as original namespace on reconfigure
2019-08-28 9:43 ` Aneesh Kumar K.V
@ 2019-10-18 10:25 ` Aneesh Kumar K.V
2019-10-18 17:18 ` Verma, Vishal L
0 siblings, 1 reply; 6+ messages in thread
From: Aneesh Kumar K.V @ 2019-10-18 10:25 UTC (permalink / raw)
To: Verma, Vishal L, Williams, Dan J; +Cc: linux-nvdimm
Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> writes:
> "Verma, Vishal L" <vishal.l.verma@intel.com> writes:
>
>> On Wed, 2019-08-07 at 10:14 +0530, Aneesh Kumar K.V wrote:
>>> When using reconfigure command to add a `name` to the namespace we end
>>> up updating the align attribute. Avoid this by using the value from
>>> the original namespace. Do this only if we are keeping the namespace mode
>>> same.
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>> ---
>>> ndctl/namespace.c | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>
>> Hi Aneesh,
>>
>> A few comments below:
>>
>>>
>>> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
>>> index 1f212a2b3a9b..24e51bb35ae1 100644
>>> --- a/ndctl/namespace.c
>>> +++ b/ndctl/namespace.c
>>> @@ -596,6 +596,22 @@ static int validate_namespace_options(struct ndctl_region *region,
>>> return -ENXIO;
>>> }
>>> } else {
>>> +
>>> + /*
>>> + * If we are tryint to reconfigure with the same namespace mode
>>
>> ^trying
>>
>>> + * Use the align details from the origin namespace. Otherwise
>>
>> s/origin/original/
>>
>>> + * pick the align details from seed namespace
>>> + */
>>> + if (ndns && p->mode == ndctl_namespace_get_mode(ndns)) {
>>
>> Do we need to depend on the mode here?
>>
>> I'm thinking it should be sufficient to do:
>> 1. Check We're in 'reconfigure'
>> 2. Check param.align was not supplied
>> 3. Get alignment from the pfn/dax personality, and just use that.
>>
>> Does this make sense (Maybe I'm missing something).
>
> We want to use the align value from the seed when we are trying
> to reconfigure a namespace with a different mode. ie, if we are moving a
> fsdax namespace with align value 64K to a devdax, IMHO we should pick 16M
> as alignment for devdax.
>
>>
>>> + struct ndctl_pfn *ns_pfn = ndctl_namespace_get_pfn(ndns);
>>> + struct ndctl_dax *ns_dax = ndctl_namespace_get_dax(ndns);
>>> + if (ns_pfn)
>>> + p->align = ndctl_pfn_get_align(ns_pfn);
>>> + else if (ns_dax)
>>> + p->align = ndctl_dax_get_align(ns_dax);
>>> + else
>>> + p->align = sysconf(_SC_PAGE_SIZE);
>>
>> Do we need the page size fallback here - there are other checks after
>> this point that also do a similar fallback, do they not catch the
>> default case?
>
>
> I did that to simplify the code with that `else if`
>
>>
>>> + } else
>>> /*
>>> * Use the seed namespace alignment as the default if we need
>>> * one. If we don't then use PAGE_SIZE so the size_align
Any update on this.?
-aneesh
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ndctl: Use the same align value as original namespace on reconfigure
2019-10-18 10:25 ` Aneesh Kumar K.V
@ 2019-10-18 17:18 ` Verma, Vishal L
0 siblings, 0 replies; 6+ messages in thread
From: Verma, Vishal L @ 2019-10-18 17:18 UTC (permalink / raw)
To: aneesh.kumar, Williams, Dan J; +Cc: linux-nvdimm
On Fri, 2019-10-18 at 15:55 +0530, Aneesh Kumar K.V wrote:
> Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> writes:
>
> > "Verma, Vishal L" <vishal.l.verma@intel.com> writes:
> >
> > > On Wed, 2019-08-07 at 10:14 +0530, Aneesh Kumar K.V wrote:
> > > > When using reconfigure command to add a `name` to the namespace we end
> > > > up updating the align attribute. Avoid this by using the value from
> > > > the original namespace. Do this only if we are keeping the namespace mode
> > > > same.
> > > >
> > > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > > > ---
> > > > ndctl/namespace.c | 16 ++++++++++++++++
> > > > 1 file changed, 16 insertions(+)
> > >
> > > Hi Aneesh,
> > >
> > > A few comments below:
> > >
> > > > diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> > > > index 1f212a2b3a9b..24e51bb35ae1 100644
> > > > --- a/ndctl/namespace.c
> > > > +++ b/ndctl/namespace.c
> > > > @@ -596,6 +596,22 @@ static int validate_namespace_options(struct ndctl_region *region,
> > > > return -ENXIO;
> > > > }
> > > > } else {
> > > > +
> > > > + /*
> > > > + * If we are tryint to reconfigure with the same namespace mode
> > >
> > > ^trying
> > >
> > > > + * Use the align details from the origin namespace. Otherwise
> > >
> > > s/origin/original/
> > >
> > > > + * pick the align details from seed namespace
> > > > + */
> > > > + if (ndns && p->mode == ndctl_namespace_get_mode(ndns)) {
> > >
> > > Do we need to depend on the mode here?
> > >
> > > I'm thinking it should be sufficient to do:
> > > 1. Check We're in 'reconfigure'
> > > 2. Check param.align was not supplied
> > > 3. Get alignment from the pfn/dax personality, and just use that.
> > >
> > > Does this make sense (Maybe I'm missing something).
> >
> > We want to use the align value from the seed when we are trying
> > to reconfigure a namespace with a different mode. ie, if we are moving a
> > fsdax namespace with align value 64K to a devdax, IMHO we should pick 16M
> > as alignment for devdax.
> >
> > > > + struct ndctl_pfn *ns_pfn = ndctl_namespace_get_pfn(ndns);
> > > > + struct ndctl_dax *ns_dax = ndctl_namespace_get_dax(ndns);
> > > > + if (ns_pfn)
> > > > + p->align = ndctl_pfn_get_align(ns_pfn);
> > > > + else if (ns_dax)
> > > > + p->align = ndctl_dax_get_align(ns_dax);
> > > > + else
> > > > + p->align = sysconf(_SC_PAGE_SIZE);
> > >
> > > Do we need the page size fallback here - there are other checks after
> > > this point that also do a similar fallback, do they not catch the
> > > default case?
> >
> > I did that to simplify the code with that `else if`
> >
> > > > + } else
> > > > /*
> > > > * Use the seed namespace alignment as the default if we need
> > > > * one. If we don't then use PAGE_SIZE so the size_align
>
> Any update on this.?
Yes - I've applied it to the pending branch, and it will be in v67.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-18 17:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07 4:44 [PATCH] ndctl: Use the same align value as original namespace on reconfigure Aneesh Kumar K.V
2019-08-27 8:39 ` Aneesh Kumar K.V
2019-08-27 18:19 ` Verma, Vishal L
2019-08-28 9:43 ` Aneesh Kumar K.V
2019-10-18 10:25 ` Aneesh Kumar K.V
2019-10-18 17:18 ` Verma, Vishal L
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).