Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1723268 > unrolled thread

Re: [PATCH v4 00/14] mpt3sas driver NVMe support:

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 00/14] mpt3sas driver NVMe support: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com> - 2017-08-30 14:40 +0200
    Re: [PATCH v4 00/14] mpt3sas driver NVMe support: "Martin K. Petersen" <martin.petersen@oracle.com> - 2017-08-31 05:10 +0200
      Re: [PATCH v4 00/14] mpt3sas driver NVMe support: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com> - 2017-08-31 07:00 +0200
        Re: [PATCH v4 00/14] mpt3sas driver NVMe support: "Martin K. Petersen" <martin.petersen@oracle.com> - 2017-09-01 05:30 +0200
          Re: [PATCH v4 00/14] mpt3sas driver NVMe support: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com> - 2017-09-01 10:50 +0200

#1723268 — Re: [PATCH v4 00/14] mpt3sas driver NVMe support:

Hi Martin,

Replied in line.

- I don't understand why you go through all these hoops to decide
  whether to use PRPs or IEEE scatterlists. If the firmware translation
  is slow, why even bother with the SG format in the first place? Set
  the max I/O size to match MDTS and you're done.

=>  We will  set MDTS value as max hw sectors using
blk_queue_max_hw_sectors(). As of now, we see correct MDTS value is
being set to block layer via VPD page 0xb0 (Block limits VPD page )
response from FW for NVME device.
=> I will remove MDTS checks in IO path.


- What's the benefit of using SG for regular I/O commands?

=>  Broadcom's IT Tri-mode HBA hardware has a capability of
translating IEEE SGLs to PRP's only up to ~4 page block size.
If the IO block size is greater than that (along with other condition
described code base_is_prp_possible), driver has to frame the PRP's to
avoid FW intervention. Both the case is a fast path, but for smaller
IO (up to 20K) size will frame IEEE SGL and large IO size will frame
PRP format SGL. Theoretically we want to use h/w capability (to
translate IEEE to PRP) for smaller IO size to leverage h/w capability.
We are investigating if at all we can send all PRP and avoid checks in
driver, but that exercise may take time as we have many different
opinions. We prefer to use existing code as it is stable and in-line
with h/w requirement.


- If the unmap translation in firmware is slow, why don't you translate
  WRITE SAME/w UNMAP set to DSM DEALLOCATE without requiring
  applications to do encapsulated passthrough?

=> As of now, current FW supports UNMAP command but not WRITE_SAME for
NVME drive. We did some experiment to convert UMAP command in driver,
but that is not really giving any performance improvement. We would
like to continue with UNMAP (and all other non-read/write commands) to
be handled in FW.


- Also make sure you attribute your patches correctly (From: root
<root@dhcp-135-24-192-103.dhcp.avagotech.net>). And you don't need
that long CC: list. Just send the patch series to
linux-scsi@vger.kernel.org.

=>  I will fix this type of issue going forward

Thanks,
Suganath Prabu S

On Wed, Aug 23, 2017 at 7:48 AM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
> Suganath,
>
>>   mpt3sas: SGL to PRP Translation for I/Os to NVMe  devices
>
> I'm still confused about this patch.
>
>  - I don't understand why you go through all these hoops to decide
>    whether to use PRPs or IEEE scatterlists. If the firmware translation
>    is slow, why even bother with the SG format in the first place? Set
>    the max I/O size to match MDTS and you're done.
>
>  - What's the benefit of using SG for regular I/O commands?
>
>  - If the unmap translation in firmware is slow, why don't you translate
>    WRITE SAME/w UNMAP set to DSM DEALLOCATE without requiring
>    applications to do encapsulated passthrough?
>
> Also make sure you attribute your patches correctly (From: root
> <root@dhcp-135-24-192-103.dhcp.avagotech.net>). And you don't need that
> long CC: list. Just send the patch series to linux-scsi@vger.kernel.org.
>
> Thanks!
>
> --
> Martin K. Petersen      Oracle Linux Engineering

[toc] | [next] | [standalone]

#1723782

Hi Suganath,

> Theoretically we want to use h/w capability (to translate IEEE to PRP)
> for smaller IO size to leverage h/w capability.

Nobody says we have to use the capability just because the hardware has
it.

Unlike some other operating systems, Linux will only submit I/Os to the
driver that conform to the reported underlying constraints of the
hardware. I fail to understand how letting the HBA firmware translate an
SGL to a PRP for a subset of I/Os could do anything but add latency.
Plus complexity in the hot path of the driver.

> - If the unmap translation in firmware is slow, why don't you translate
>   WRITE SAME/w UNMAP set to DSM DEALLOCATE without requiring
>   applications to do encapsulated passthrough?

> => As of now, current FW supports UNMAP command but not WRITE_SAME for
> NVME drive. We did some experiment to convert UMAP command in driver,
> but that is not really giving any performance improvement.

It is imperative that the common use case, Linux' discard
infrastructure, is working correctly and is as performant as any
application-driven passthrough workaround.

Unlike SCSI-to-SATA translation you have the benefit of a 1:1 mapping
between UNMAP and DEALLOCATE. I'm not even sure why there would be a
significant performance penalty in the firmware?

> We would like to continue with UNMAP (and all other non-read/write
> commands) to be handled in FW.

And yet patch 4 circumvents that statement by adding support for
encapsulated commands to bypass the FW translation...

-- 
Martin K. Petersen	Oracle Linux Engineering

[toc] | [prev] | [next] | [standalone]

#1723829

Hi Martin,
Replied inline.

Thanks,
Suganath Prabu S

On Thu, Aug 31, 2017 at 8:35 AM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
> Hi Suganath,
>
>> Theoretically we want to use h/w capability (to translate IEEE to PRP)
>> for smaller IO size to leverage h/w capability.
>
> Nobody says we have to use the capability just because the hardware has
> it.
>
> Unlike some other operating systems, Linux will only submit I/Os to the
> driver that conform to the reported underlying constraints of the
> hardware.

<Suganath> In general, h/w constraints are handled. What we missed is
Fast Path h/w
which is not exposed to OS.

> I fail to understand how letting the HBA firmware translate an
> SGL to a PRP for a subset of I/Os could do anything but add latency.

<Suganath> Let me explain - NVME device fast path is possible in two ways.
IEEE SGL and PRP SGL. Due to h/w constraint we choose IEEE SGL only for
smaller IO size.
Both above is true h/w Fast Path and no firmware involvement.

> Plus complexity in the hot path of the driver.

<Suganath> Agree with you. We are planning to see if we can keep only
simple Fast
Path using only PRP. It will take some time to finalize as we have to
engage h/w and f/w team.  BTW - This area is h/w dependent and we do not
see further changes in this area.

>
>> - If the unmap translation in firmware is slow, why don't you translate
>>   WRITE SAME/w UNMAP set to DSM DEALLOCATE without requiring
>>   applications to do encapsulated passthrough?
>
>> => As of now, current FW supports UNMAP command but not WRITE_SAME for
>> NVME drive. We did some experiment to convert UMAP command in driver,
>> but that is not really giving any performance improvement.
>
> It is imperative that the common use case, Linux' discard
> infrastructure, is working correctly and is as performant as any
> application-driven passthrough workaround.
>
> Unlike SCSI-to-SATA translation you have the benefit of a 1:1 mapping
> between UNMAP and DEALLOCATE. I'm not even sure why there would be a
> significant performance penalty in the firmware?

<Suganath> I agree. Currently there is no performance issue for UNMAP
translation in
FW.


>> We would like to continue with UNMAP (and all other non-read/write
>> commands) to be handled in FW.
>
> And yet patch 4 circumvents that statement by adding support for
> encapsulated commands to bypass the FW translation...

<Suganath> This path is not due to performance reason. User wants to
interact with
NVME drive in native NVME command for management.

>
> --
> Martin K. Petersen      Oracle Linux Engineering

[toc] | [prev] | [next] | [standalone]

#1724765

Hi Suganath,

> Let me explain - NVME device fast path is possible in two ways.  IEEE
> SGL and PRP SGL. Due to h/w constraint we choose IEEE SGL only for
> smaller IO size.  Both above is true h/w Fast Path and no firmware
> involvement.

> Agree with you. We are planning to see if we can keep only simple Fast
> Path using only PRP.

That would be great, thank you!

> Currently there is no performance issue for UNMAP translation in FW.

Good!

>> And yet patch 4 circumvents that statement by adding support for
>> encapsulated commands to bypass the FW translation...
>
> This path is not due to performance reason. User wants to interact
> with NVME drive in native NVME command for management.

Patch 4 states:

"This encapsulated NVMe command is used by applications to send direct
NVMe commands to NVMe drives or for handling unmap where the translation
at controller/firmware level is having performance issues."

-- 
Martin K. Petersen	Oracle Linux Engineering

[toc] | [prev] | [next] | [standalone]

#1724879

Hi Martin,

On Fri, Sep 1, 2017 at 8:52 AM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
> Hi Suganath,
>
>> Let me explain - NVME device fast path is possible in two ways.  IEEE
>> SGL and PRP SGL. Due to h/w constraint we choose IEEE SGL only for
>> smaller IO size.  Both above is true h/w Fast Path and no firmware
>> involvement.
>
>> Agree with you. We are planning to see if we can keep only simple Fast
>> Path using only PRP.
>
> That would be great, thank you!
>
>> Currently there is no performance issue for UNMAP translation in FW.
>
> Good!
>
>>> And yet patch 4 circumvents that statement by adding support for
>>> encapsulated commands to bypass the FW translation...
>>
>> This path is not due to performance reason. User wants to interact
>> with NVME drive in native NVME command for management.
>
> Patch 4 states:
>
> "This encapsulated NVMe command is used by applications to send direct
> NVMe commands to NVMe drives or for handling unmap where the translation
> at controller/firmware level is having performance issues."
>
> --

The statement in description of patch 4 is added by mistake, We ll
correct the description and re sending that.

> Martin K. Petersen      Oracle Linux Engineering

Thanks,
Suganath Prabu S

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web