diff mbox

[08/20] lightnvm: complete geo structure with maxoc*

Message ID 1519205218-26994-9-git-send-email-javier@cnexlabs.com (mailing list archive)
State New, archived
Headers show

Commit Message

=?UTF-8?q?Javier=20Gonz=C3=A1lez?= Feb. 21, 2018, 9:26 a.m. UTC
Complete the generic geometry structure with the maxoc and maxocpu
felds, present in the 2.0 spec.

Signed-off-by: Javier González <javier@cnexlabs.com>
---
 drivers/nvme/host/lightnvm.c | 4 ++++
 include/linux/lightnvm.h     | 2 ++
 2 files changed, 6 insertions(+)

Comments

Matias Bjorling Feb. 22, 2018, 7:45 a.m. UTC | #1
On 02/21/2018 10:26 AM, Javier González wrote:
> Complete the generic geometry structure with the maxoc and maxocpu
> felds, present in the 2.0 spec.
> 
> Signed-off-by: Javier González <javier@cnexlabs.com>
> ---
>   drivers/nvme/host/lightnvm.c | 4 ++++
>   include/linux/lightnvm.h     | 2 ++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
> index cca32da05316..9c1f8225c4e1 100644
> --- a/drivers/nvme/host/lightnvm.c
> +++ b/drivers/nvme/host/lightnvm.c
> @@ -318,6 +318,8 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id,
>   	dev_geo->c.ws_min = sec_per_pg;
>   	dev_geo->c.ws_opt = sec_per_pg;
>   	dev_geo->c.mw_cunits = 8;		/* default to MLC safe values */
> +	dev_geo->c.maxoc = dev_geo->all_luns;	/* default to 1 chunk per LUN */
> +	dev_geo->c.maxocpu = 1;			/* default to 1 chunk per LUN */

One can't assume that it is 1 open chunk per lun. If you need this for 
specific hardware, make a quirk for it.

>   
>   	dev_geo->c.mccap = le32_to_cpu(src->mccap);
>   
> @@ -405,6 +407,8 @@ static int nvme_nvm_setup_20(struct nvme_nvm_id20 *id,
>   	dev_geo->c.ws_min = le32_to_cpu(id->ws_min);
>   	dev_geo->c.ws_opt = le32_to_cpu(id->ws_opt);
>   	dev_geo->c.mw_cunits = le32_to_cpu(id->mw_cunits);
> +	dev_geo->c.maxoc = le32_to_cpu(id->maxoc);
> +	dev_geo->c.maxocpu = le32_to_cpu(id->maxocpu);
>   
>   	dev_geo->c.mccap = le32_to_cpu(id->mccap);
>   
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index ccc5faa63cb7..e1c4292ea33d 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -215,6 +215,8 @@ struct nvm_common_geo {
>   	u32	ws_min;		/* minimum write size */
>   	u32	ws_opt;		/* optimal write size */
>   	u32	mw_cunits;	/* distance required for successful read */
> +	u32	maxoc;		/* maximum open chunks */
> +	u32	maxocpu;	/* maximum open chunks per parallel unit */
>   
>   	/* device capabilities */
>   	u32	mccap;
>
Javier Gonzalez Feb. 22, 2018, 7:55 a.m. UTC | #2
> On 22 Feb 2018, at 08.45, Matias Bjørling <mb@lightnvm.io> wrote:
> 
> On 02/21/2018 10:26 AM, Javier González wrote:
>> Complete the generic geometry structure with the maxoc and maxocpu
>> felds, present in the 2.0 spec.
>> Signed-off-by: Javier González <javier@cnexlabs.com>
>> ---
>>  drivers/nvme/host/lightnvm.c | 4 ++++
>>  include/linux/lightnvm.h     | 2 ++
>>  2 files changed, 6 insertions(+)
>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>> index cca32da05316..9c1f8225c4e1 100644
>> --- a/drivers/nvme/host/lightnvm.c
>> +++ b/drivers/nvme/host/lightnvm.c
>> @@ -318,6 +318,8 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id,
>>  	dev_geo->c.ws_min = sec_per_pg;
>>  	dev_geo->c.ws_opt = sec_per_pg;
>>  	dev_geo->c.mw_cunits = 8;		/* default to MLC safe values */
>> +	dev_geo->c.maxoc = dev_geo->all_luns;	/* default to 1 chunk per LUN */
>> +	dev_geo->c.maxocpu = 1;			/* default to 1 chunk per LUN */
> 
> One can't assume that it is 1 open chunk per lun. If you need this for specific hardware, make a quirk for it.
> 

Which default you want for 1.2 if not specified then? I use 1 because it
has been the implicit default until now.

Javier
Matias Bjorling Feb. 22, 2018, 9:45 a.m. UTC | #3
On 02/22/2018 08:55 AM, Javier Gonzalez wrote:
>> On 22 Feb 2018, at 08.45, Matias Bjørling <mb@lightnvm.io> wrote:
>>
>> On 02/21/2018 10:26 AM, Javier González wrote:
>>> Complete the generic geometry structure with the maxoc and maxocpu
>>> felds, present in the 2.0 spec.
>>> Signed-off-by: Javier González <javier@cnexlabs.com>
>>> ---
>>>   drivers/nvme/host/lightnvm.c | 4 ++++
>>>   include/linux/lightnvm.h     | 2 ++
>>>   2 files changed, 6 insertions(+)
>>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>>> index cca32da05316..9c1f8225c4e1 100644
>>> --- a/drivers/nvme/host/lightnvm.c
>>> +++ b/drivers/nvme/host/lightnvm.c
>>> @@ -318,6 +318,8 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id,
>>>   	dev_geo->c.ws_min = sec_per_pg;
>>>   	dev_geo->c.ws_opt = sec_per_pg;
>>>   	dev_geo->c.mw_cunits = 8;		/* default to MLC safe values */
>>> +	dev_geo->c.maxoc = dev_geo->all_luns;	/* default to 1 chunk per LUN */
>>> +	dev_geo->c.maxocpu = 1;			/* default to 1 chunk per LUN */
>>
>> One can't assume that it is 1 open chunk per lun. If you need this for specific hardware, make a quirk for it.
>>
> 
> Which default you want for 1.2 if not specified then? I use 1 because it
> has been the implicit default until now.

INT_MAX, since it then allows the maximum of open chunks. It cannot be 
assumed that other 1.2 devices is limited to a single open chunk.

> 
> Javier
>
Javier Gonzalez Feb. 22, 2018, 9:52 a.m. UTC | #4
> On 22 Feb 2018, at 10.45, Matias Bjørling <mb@lightnvm.io> wrote:
> 
> On 02/22/2018 08:55 AM, Javier Gonzalez wrote:
>>> On 22 Feb 2018, at 08.45, Matias Bjørling <mb@lightnvm.io> wrote:
>>> 
>>> On 02/21/2018 10:26 AM, Javier González wrote:
>>>> Complete the generic geometry structure with the maxoc and maxocpu
>>>> felds, present in the 2.0 spec.
>>>> Signed-off-by: Javier González <javier@cnexlabs.com>
>>>> ---
>>>>  drivers/nvme/host/lightnvm.c | 4 ++++
>>>>  include/linux/lightnvm.h     | 2 ++
>>>>  2 files changed, 6 insertions(+)
>>>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>>>> index cca32da05316..9c1f8225c4e1 100644
>>>> --- a/drivers/nvme/host/lightnvm.c
>>>> +++ b/drivers/nvme/host/lightnvm.c
>>>> @@ -318,6 +318,8 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id,
>>>>  	dev_geo->c.ws_min = sec_per_pg;
>>>>  	dev_geo->c.ws_opt = sec_per_pg;
>>>>  	dev_geo->c.mw_cunits = 8;		/* default to MLC safe values */
>>>> +	dev_geo->c.maxoc = dev_geo->all_luns;	/* default to 1 chunk per LUN */
>>>> +	dev_geo->c.maxocpu = 1;			/* default to 1 chunk per LUN */
>>> 
>>> One can't assume that it is 1 open chunk per lun. If you need this for specific hardware, make a quirk for it.
>> Which default you want for 1.2 if not specified then? I use 1 because it
>> has been the implicit default until now.
> 
> INT_MAX, since it then allows the maximum of open chunks. It cannot be assumed that other 1.2 devices is limited to a single open chunk.

So you want the default to be that all blocks on the device can be
opened at the same time. Interesting... I guess that such a SSD will
have a AA battery attached to it, but fine by me if that's how you want
it.

Assuming this, can we instead set it to the reported number of chunks,
since this is the hard limit anyway.

Javier
Matias Bjorling Feb. 22, 2018, 10 a.m. UTC | #5
On 02/22/2018 10:52 AM, Javier Gonzalez wrote:
>> On 22 Feb 2018, at 10.45, Matias Bjørling <mb@lightnvm.io> wrote:
>>
>> On 02/22/2018 08:55 AM, Javier Gonzalez wrote:
>>>> On 22 Feb 2018, at 08.45, Matias Bjørling <mb@lightnvm.io> wrote:
>>>>
>>>> On 02/21/2018 10:26 AM, Javier González wrote:
>>>>> Complete the generic geometry structure with the maxoc and maxocpu
>>>>> felds, present in the 2.0 spec.
>>>>> Signed-off-by: Javier González <javier@cnexlabs.com>
>>>>> ---
>>>>>   drivers/nvme/host/lightnvm.c | 4 ++++
>>>>>   include/linux/lightnvm.h     | 2 ++
>>>>>   2 files changed, 6 insertions(+)
>>>>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>>>>> index cca32da05316..9c1f8225c4e1 100644
>>>>> --- a/drivers/nvme/host/lightnvm.c
>>>>> +++ b/drivers/nvme/host/lightnvm.c
>>>>> @@ -318,6 +318,8 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id,
>>>>>   	dev_geo->c.ws_min = sec_per_pg;
>>>>>   	dev_geo->c.ws_opt = sec_per_pg;
>>>>>   	dev_geo->c.mw_cunits = 8;		/* default to MLC safe values */
>>>>> +	dev_geo->c.maxoc = dev_geo->all_luns;	/* default to 1 chunk per LUN */
>>>>> +	dev_geo->c.maxocpu = 1;			/* default to 1 chunk per LUN */
>>>>
>>>> One can't assume that it is 1 open chunk per lun. If you need this for specific hardware, make a quirk for it.
>>> Which default you want for 1.2 if not specified then? I use 1 because it
>>> has been the implicit default until now.
>>
>> INT_MAX, since it then allows the maximum of open chunks. It cannot be assumed that other 1.2 devices is limited to a single open chunk.
> 
> So you want the default to be that all blocks on the device can be
> opened at the same time. Interesting... I guess that such a SSD will
> have a AA battery attached to it, but fine by me if that's how you want
> it.

I feel you're a bit sarcastic here. One may think of SLC and other 
memories that does one-shot programming. In that case no caching is 
needed, and therefore power-caps can be limited on the hardware.

> 
> Assuming this, can we instead set it to the reported number of chunks,
> since this is the hard limit anyway.
> 

Works for me.
Javier Gonzalez Feb. 22, 2018, 10:03 a.m. UTC | #6
Javier

> On 22 Feb 2018, at 11.00, Matias Bjørling <mb@lightnvm.io> wrote:
> 
> On 02/22/2018 10:52 AM, Javier Gonzalez wrote:
>>> On 22 Feb 2018, at 10.45, Matias Bjørling <mb@lightnvm.io> wrote:
>>> 
>>> On 02/22/2018 08:55 AM, Javier Gonzalez wrote:
>>>>> On 22 Feb 2018, at 08.45, Matias Bjørling <mb@lightnvm.io> wrote:
>>>>> 
>>>>> On 02/21/2018 10:26 AM, Javier González wrote:
>>>>>> Complete the generic geometry structure with the maxoc and maxocpu
>>>>>> felds, present in the 2.0 spec.
>>>>>> Signed-off-by: Javier González <javier@cnexlabs.com>
>>>>>> ---
>>>>>>  drivers/nvme/host/lightnvm.c | 4 ++++
>>>>>>  include/linux/lightnvm.h     | 2 ++
>>>>>>  2 files changed, 6 insertions(+)
>>>>>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>>>>>> index cca32da05316..9c1f8225c4e1 100644
>>>>>> --- a/drivers/nvme/host/lightnvm.c
>>>>>> +++ b/drivers/nvme/host/lightnvm.c
>>>>>> @@ -318,6 +318,8 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id,
>>>>>>  	dev_geo->c.ws_min = sec_per_pg;
>>>>>>  	dev_geo->c.ws_opt = sec_per_pg;
>>>>>>  	dev_geo->c.mw_cunits = 8;		/* default to MLC safe values */
>>>>>> +	dev_geo->c.maxoc = dev_geo->all_luns;	/* default to 1 chunk per LUN */
>>>>>> +	dev_geo->c.maxocpu = 1;			/* default to 1 chunk per LUN */
>>>>> 
>>>>> One can't assume that it is 1 open chunk per lun. If you need this for specific hardware, make a quirk for it.
>>>> Which default you want for 1.2 if not specified then? I use 1 because it
>>>> has been the implicit default until now.
>>> 
>>> INT_MAX, since it then allows the maximum of open chunks. It cannot be assumed that other 1.2 devices is limited to a single open chunk.
>> So you want the default to be that all blocks on the device can be
>> opened at the same time. Interesting... I guess that such a SSD will
>> have a AA battery attached to it, but fine by me if that's how you want
>> it.
> 
> I feel you're a bit sarcastic here. One may think of SLC and other memories that does one-shot programming. In that case no caching is needed, and therefore power-caps can be limited on the hardware.

Sure. Hope people move to 2.0 then for all >SLC memories out there,
otherwise we'll see a lot of quirks coming in. Thought we wanted to be
generic.

> 
>> Assuming this, can we instead set it to the reported number of chunks,
>> since this is the hard limit anyway.
> 
> Works for me.

Cool. I'll add this to the next version.

Javier
diff mbox

Patch

diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index cca32da05316..9c1f8225c4e1 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -318,6 +318,8 @@  static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id,
 	dev_geo->c.ws_min = sec_per_pg;
 	dev_geo->c.ws_opt = sec_per_pg;
 	dev_geo->c.mw_cunits = 8;		/* default to MLC safe values */
+	dev_geo->c.maxoc = dev_geo->all_luns;	/* default to 1 chunk per LUN */
+	dev_geo->c.maxocpu = 1;			/* default to 1 chunk per LUN */
 
 	dev_geo->c.mccap = le32_to_cpu(src->mccap);
 
@@ -405,6 +407,8 @@  static int nvme_nvm_setup_20(struct nvme_nvm_id20 *id,
 	dev_geo->c.ws_min = le32_to_cpu(id->ws_min);
 	dev_geo->c.ws_opt = le32_to_cpu(id->ws_opt);
 	dev_geo->c.mw_cunits = le32_to_cpu(id->mw_cunits);
+	dev_geo->c.maxoc = le32_to_cpu(id->maxoc);
+	dev_geo->c.maxocpu = le32_to_cpu(id->maxocpu);
 
 	dev_geo->c.mccap = le32_to_cpu(id->mccap);
 
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index ccc5faa63cb7..e1c4292ea33d 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -215,6 +215,8 @@  struct nvm_common_geo {
 	u32	ws_min;		/* minimum write size */
 	u32	ws_opt;		/* optimal write size */
 	u32	mw_cunits;	/* distance required for successful read */
+	u32	maxoc;		/* maximum open chunks */
+	u32	maxocpu;	/* maximum open chunks per parallel unit */
 
 	/* device capabilities */
 	u32	mccap;