Re: [PATCH v2 8/8] lightnvm: Inherit mdts from the parent nvme device

["Javier González" <javier@javigon.com>]

[-- Attachment #1: Type: text/plain, Size: 3253 bytes --]

> On 5 Mar 2019, at 15.34, Matias Bjørling <mb@lightnvm.io> wrote:
> 
> On 3/5/19 2:51 PM, Igor Konopko wrote:
>> Current lightnvm and pblk implementation does not care about NVMe max
>> data transfer size, which can be smaller than 64*K=256K. There are
>> existing NVMe controllers which NVMe max data transfer size is lower
>> that 256K (for example 128K, which happens for existing NVMe
>> controllers which are NVMe spec compliant). Such a controllers are not
>> able to handle command which contains 64 PPAs, since the the size of
>> DMAed buffer will be above the capabilities of such a controller.
>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com>
>> Reviewed-by: Javier González <javier@javigon.com>
>> ---
>>  drivers/lightnvm/core.c      | 9 +++++++--
>>  drivers/nvme/host/lightnvm.c | 1 +
>>  include/linux/lightnvm.h     | 1 +
>>  3 files changed, 9 insertions(+), 2 deletions(-)
>> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
>> index 5f82036..c01f83b 100644
>> --- a/drivers/lightnvm/core.c
>> +++ b/drivers/lightnvm/core.c
>> @@ -325,6 +325,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
>>  	struct nvm_target *t;
>>  	struct nvm_tgt_dev *tgt_dev;
>>  	void *targetdata;
>> +	unsigned int mdts;
>>  	int ret;
>>    	switch (create->conf.type) {
>> @@ -412,8 +413,12 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
>>  	tdisk->private_data = targetdata;
>>  	tqueue->queuedata = targetdata;
>>  -	blk_queue_max_hw_sectors(tqueue,
>> -			(dev->geo.csecs >> 9) * NVM_MAX_VLBA);
>> +	mdts = (dev->geo.csecs >> 9) * NVM_MAX_VLBA;
>> +	if (dev->geo.mdts) {
>> +		mdts = min_t(u32, dev->geo.mdts,
>> +				(dev->geo.csecs >> 9) * NVM_MAX_VLBA);
>> +	}
>> +	blk_queue_max_hw_sectors(tqueue, mdts);
>>    	set_capacity(tdisk, tt->capacity(targetdata));
>>  	add_disk(tdisk);
>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>> index 949e29e..4f20a10 100644
>> --- a/drivers/nvme/host/lightnvm.c
>> +++ b/drivers/nvme/host/lightnvm.c
>> @@ -977,6 +977,7 @@ int nvme_nvm_register(struct nvme_ns *ns, char *disk_name, int node)
>>  	geo->csecs = 1 << ns->lba_shift;
>>  	geo->sos = ns->ms;
>>  	geo->ext = ns->ext;
>> +	geo->mdts = ns->ctrl->max_hw_sectors;
>>    	dev->q = q;
>>  	memcpy(dev->name, disk_name, DISK_NAME_LEN);
>> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
>> index 5d865a5..d3b0270 100644
>> --- a/include/linux/lightnvm.h
>> +++ b/include/linux/lightnvm.h
>> @@ -358,6 +358,7 @@ struct nvm_geo {
>>  	u16	csecs;		/* sector size */
>>  	u16	sos;		/* out-of-band area size */
>>  	bool	ext;		/* metadata in extended data buffer */
>> +	u32	mdts;		/* Max data transfer size*/
>>    	/* device write constrains */
>>  	u32	ws_min;		/* minimum write size */
> 
> I think I can pick this up. I'll let Javier/Hans rereview.
> 
> Given Javier's feedback on broken existing mdts implementations. When
> they are brought to light, we may want to add in a quirk list to
> update MDTS accordingly. Javier, will this address the issue you have
> regarding using mdts?

Sounds good. Thanks for considering this.

Javier

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]