From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751437AbdH3Maa (ORCPT ); Wed, 30 Aug 2017 08:30:30 -0400 Received: from mail-wm0-f45.google.com ([74.125.82.45]:36860 "EHLO mail-wm0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751323AbdH3Ma2 (ORCPT ); Wed, 30 Aug 2017 08:30:28 -0400 MIME-Version: 1.0 In-Reply-To: References: <1503322344-5900-1-git-send-email-suganath-prabu.subramani@broadcom.com> From: Suganath Prabu Subramani Date: Wed, 30 Aug 2017 18:00:26 +0530 Message-ID: Subject: Re: [PATCH v4 00/14] mpt3sas driver NVMe support: To: "Martin K. Petersen" Cc: linux-scsi@vger.kernel.org, Sathya Prakash , Kashyap Desai , linux-kernel@vger.kernel.org, Chaitra Basappa , Sreekanth Reddy , linux-nvme@lists.infradead.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 ). 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 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 > ). 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: suganath-prabu.subramani@broadcom.com (Suganath Prabu Subramani) Date: Wed, 30 Aug 2017 18:00:26 +0530 Subject: [PATCH v4 00/14] mpt3sas driver NVMe support: In-Reply-To: References: <1503322344-5900-1-git-send-email-suganath-prabu.subramani@broadcom.com> Message-ID: 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 ). And you don't need that long CC: list. Just send the patch series to linux-scsi at 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 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 > ). And you don't need that > long CC: list. Just send the patch series to linux-scsi at vger.kernel.org. > > Thanks! > > -- > Martin K. Petersen Oracle Linux Engineering