diff mbox series

[2/3] lightnvm: do no update csecs and sos on 1.2

Message ID 1535537370-10729-3-git-send-email-javier@cnexlabs.com (mailing list archive)
State New, archived
Headers show
Series lightnvm: pblk: support variable OOB size | expand

Commit Message

Javier González Aug. 29, 2018, 10:09 a.m. UTC
In the OCSSD 2.0 spec., the sector and metadata sizes are reported though
the standard nvme identify command. Thus, the lightnvm subsystem needs
to update this information on the geometry structure on bootup.

Since 1.2 devices report these values on the OCSSD geometry identify,
avoid this update is it is unnecessary and can also corrupt the geometry
if the devices does not report the nvme sizes correctly (which is not
required by the OCSSD 1.2 spec either)

Signed-off-by: Javier González <javier@cnexlabs.com>
---
 drivers/nvme/host/lightnvm.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Matias Bjorling Sept. 7, 2018, 10:28 a.m. UTC | #1
On 08/29/2018 12:09 PM, Javier González wrote:
> In the OCSSD 2.0 spec., the sector and metadata sizes are reported though
> the standard nvme identify command. Thus, the lightnvm subsystem needs
> to update this information on the geometry structure on bootup.
> 
> Since 1.2 devices report these values on the OCSSD geometry identify,
> avoid this update is it is unnecessary and can also corrupt the geometry
> if the devices does not report the nvme sizes correctly (which is not
> required by the OCSSD 1.2 spec either) >
> Signed-off-by: Javier González <javier@cnexlabs.com>
> ---
>   drivers/nvme/host/lightnvm.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
> index 5bfa354c5dd5..33ed09f8410e 100644
> --- a/drivers/nvme/host/lightnvm.c
> +++ b/drivers/nvme/host/lightnvm.c
> @@ -980,6 +980,9 @@ void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
>   	struct nvm_dev *ndev = ns->ndev;
>   	struct nvm_geo *geo = &ndev->geo;
>   
> +	if (geo->version == NVM_OCSSD_SPEC_12)
> +		return;

The lba format sizes are not explicit in 2.0 either. For a conforming 
drive, it should implement the NVM Command Set and expose the LBA format 
correctly. Although, I do get your incentive, and if it is okay with 
you, I'll reword the commit message to this and apply it?:

"1.2 devices exposes their data and metadata size through the separate
identify command. Make sure that the NVMe LBA format does not override
these values."


> +
>   	geo->csecs = 1 << ns->lba_shift;
>   	geo->sos = ns->ms;
>   }
>
Javier Gonzalez Sept. 11, 2018, 8:50 a.m. UTC | #2
> On 7 Sep 2018, at 12.28, Matias Bjørling <mb@lightnvm.io> wrote:
> 
> On 08/29/2018 12:09 PM, Javier González wrote:
>> In the OCSSD 2.0 spec., the sector and metadata sizes are reported though
>> the standard nvme identify command. Thus, the lightnvm subsystem needs
>> to update this information on the geometry structure on bootup.
>> Since 1.2 devices report these values on the OCSSD geometry identify,
>> avoid this update is it is unnecessary and can also corrupt the geometry
>> if the devices does not report the nvme sizes correctly (which is not
>> required by the OCSSD 1.2 spec either) >
>> Signed-off-by: Javier González <javier@cnexlabs.com>
>> ---
>>  drivers/nvme/host/lightnvm.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>> index 5bfa354c5dd5..33ed09f8410e 100644
>> --- a/drivers/nvme/host/lightnvm.c
>> +++ b/drivers/nvme/host/lightnvm.c
>> @@ -980,6 +980,9 @@ void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
>>  	struct nvm_dev *ndev = ns->ndev;
>>  	struct nvm_geo *geo = &ndev->geo;
>>  +	if (geo->version == NVM_OCSSD_SPEC_12)
>> +		return;
> 
> The lba format sizes are not explicit in 2.0 either. For a conforming drive, it should implement the NVM Command Set and expose the LBA format correctly. Although, I do get your incentive, and if it is okay with you, I'll reword the commit message to this and apply it?:
> 
> "1.2 devices exposes their data and metadata size through the separate
> identify command. Make sure that the NVMe LBA format does not override
> these values."
> 

Sounds good. I'll resend either way as I rebased on top of for-4.20/core
- you can change the commit as you think it suits.

Javier
Javier Gonzalez Sept. 11, 2018, 11:30 a.m. UTC | #3
> On 11 Sep 2018, at 10.50, Javier Gonzalez <javier@cnexlabs.com> wrote:
> 
>> On 7 Sep 2018, at 12.28, Matias Bjørling <mb@lightnvm.io> wrote:
>> 
>> On 08/29/2018 12:09 PM, Javier González wrote:
>>> In the OCSSD 2.0 spec., the sector and metadata sizes are reported though
>>> the standard nvme identify command. Thus, the lightnvm subsystem needs
>>> to update this information on the geometry structure on bootup.
>>> Since 1.2 devices report these values on the OCSSD geometry identify,
>>> avoid this update is it is unnecessary and can also corrupt the geometry
>>> if the devices does not report the nvme sizes correctly (which is not
>>> required by the OCSSD 1.2 spec either) >
>>> Signed-off-by: Javier González <javier@cnexlabs.com>
>>> ---
>>> drivers/nvme/host/lightnvm.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>>> index 5bfa354c5dd5..33ed09f8410e 100644
>>> --- a/drivers/nvme/host/lightnvm.c
>>> +++ b/drivers/nvme/host/lightnvm.c
>>> @@ -980,6 +980,9 @@ void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
>>> 	struct nvm_dev *ndev = ns->ndev;
>>> 	struct nvm_geo *geo = &ndev->geo;
>>> +	if (geo->version == NVM_OCSSD_SPEC_12)
>>> +		return;
>> 
>> The lba format sizes are not explicit in 2.0 either. For a conforming drive, it should implement the NVM Command Set and expose the LBA format correctly. Although, I do get your incentive, and if it is okay with you, I'll reword the commit message to this and apply it?:
>> 
>> "1.2 devices exposes their data and metadata size through the separate
>> identify command. Make sure that the NVMe LBA format does not override
>> these values."
> 
> Sounds good. I'll resend either way as I rebased on top of for-4.20/core
> - you can change the commit as you think it suits.

Since Igor will send a patch for the OOB, can you pick this up
separately? No changes needed after rebase.

Thanks,
Javier
Matias Bjorling Sept. 18, 2018, 8:37 a.m. UTC | #4
On 09/11/2018 01:30 PM, Javier Gonzalez wrote:
>> On 11 Sep 2018, at 10.50, Javier Gonzalez <javier@cnexlabs.com> wrote:
>>
>>> On 7 Sep 2018, at 12.28, Matias Bjørling <mb@lightnvm.io> wrote:
>>>
>>> On 08/29/2018 12:09 PM, Javier González wrote:
>>>> In the OCSSD 2.0 spec., the sector and metadata sizes are reported though
>>>> the standard nvme identify command. Thus, the lightnvm subsystem needs
>>>> to update this information on the geometry structure on bootup.
>>>> Since 1.2 devices report these values on the OCSSD geometry identify,
>>>> avoid this update is it is unnecessary and can also corrupt the geometry
>>>> if the devices does not report the nvme sizes correctly (which is not
>>>> required by the OCSSD 1.2 spec either) >
>>>> Signed-off-by: Javier González <javier@cnexlabs.com>
>>>> ---
>>>> drivers/nvme/host/lightnvm.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>>>> index 5bfa354c5dd5..33ed09f8410e 100644
>>>> --- a/drivers/nvme/host/lightnvm.c
>>>> +++ b/drivers/nvme/host/lightnvm.c
>>>> @@ -980,6 +980,9 @@ void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
>>>> 	struct nvm_dev *ndev = ns->ndev;
>>>> 	struct nvm_geo *geo = &ndev->geo;
>>>> +	if (geo->version == NVM_OCSSD_SPEC_12)
>>>> +		return;
>>>
>>> The lba format sizes are not explicit in 2.0 either. For a conforming drive, it should implement the NVM Command Set and expose the LBA format correctly. Although, I do get your incentive, and if it is okay with you, I'll reword the commit message to this and apply it?:
>>>
>>> "1.2 devices exposes their data and metadata size through the separate
>>> identify command. Make sure that the NVMe LBA format does not override
>>> these values."
>>
>> Sounds good. I'll resend either way as I rebased on top of for-4.20/core
>> - you can change the commit as you think it suits.
> 
> Since Igor will send a patch for the OOB, can you pick this up
> separately? No changes needed after rebase.
> 
> Thanks,
> Javier
> 

Applied for 4.20.
diff mbox series

Patch

diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index 5bfa354c5dd5..33ed09f8410e 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -980,6 +980,9 @@  void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
 	struct nvm_dev *ndev = ns->ndev;
 	struct nvm_geo *geo = &ndev->geo;
 
+	if (geo->version == NVM_OCSSD_SPEC_12)
+		return;
+
 	geo->csecs = 1 << ns->lba_shift;
 	geo->sos = ns->ms;
 }