[0/2] qla2xxx - add nvme map_queues support

Message ID	20210823125649.16061-1-njavali@marvell.com (mailing list archive)
Headers	show Return-Path: <linux-scsi-owner@kernel.org> From: Nilesh Javali <njavali@marvell.com> To: <martin.petersen@oracle.com>, <linux-nvme@lists.infradead.org> CC: <linux-scsi@vger.kernel.org>, <GR-QLogic-Storage-Upstream@marvell.com> Subject: [PATCH 0/2] qla2xxx - add nvme map_queues support Date: Mon, 23 Aug 2021 05:56:47 -0700 Message-ID: <20210823125649.16061-1-njavali@marvell.com> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk
Series	qla2xxx - add nvme map_queues support \| expand [0/2] qla2xxx - add nvme map_queues support [1/2] nvme-fc: Add support for map_queues. [2/2] qla2xxx: Add map_queues support to nvme.

Nilesh Javali Aug. 23, 2021, 12:56 p.m. UTC

Currently nvme fc doesn't support map queue functionality. This patch
set adds map_queue functionality to nvme_fc_mq_ops and
nvme_fc_port_template, providing an option to LLDs to map queues
similar to SCSI. For qla2xxx, minimum 10% improvement is noticed
with this change as it helps in reducing cpu thrashing.

Saurav Kashyap (2):
  nvme-fc: Add support for map_queues.
  qla2xxx: Add map_queues support to nvme.

 drivers/nvme/host/fc.c          | 25 +++++++++++++++++++++++++
 drivers/scsi/qla2xxx/qla_nvme.c | 14 ++++++++++++++
 include/linux/nvme-fc-driver.h  |  7 +++++++
 3 files changed, 46 insertions(+)


base-commit: 92cc94adfce4683d0b421cbf59013703368aaeb9

Sagi Grimberg Aug. 23, 2021, 5:21 p.m. UTC | #1

On 8/23/21 5:56 AM, Nilesh Javali wrote:
> Currently nvme fc doesn't support map queue functionality. This patch
> set adds map_queue functionality to nvme_fc_mq_ops and
> nvme_fc_port_template, providing an option to LLDs to map queues
> similar to SCSI. For qla2xxx, minimum 10% improvement is noticed
> with this change as it helps in reducing cpu thrashing.

Does this make nvme-fc use managed irq?

CCing Ming to see if this affects his patchset.

Saurav Kashyap Aug. 24, 2021, 3:38 a.m. UTC | #2

Hi Sagi,
Comments inline

> -----Original Message-----
> From: Sagi Grimberg <sagi@grimberg.me>
> Sent: Monday, August 23, 2021 10:51 PM
> To: Nilesh Javali <njavali@marvell.com>; martin.petersen@oracle.com; linux-
> nvme@lists.infradead.org; Ming Lei <ming.lei@redhat.com>
> Cc: linux-scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-
> Storage-Upstream@marvell.com>
> Subject: Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> 
> 
> On 8/23/21 5:56 AM, Nilesh Javali wrote:
> > Currently nvme fc doesn't support map queue functionality. This patch
> > set adds map_queue functionality to nvme_fc_mq_ops and
> > nvme_fc_port_template, providing an option to LLDs to map queues
> > similar to SCSI. For qla2xxx, minimum 10% improvement is noticed
> > with this change as it helps in reducing cpu thrashing.
> 
> Does this make nvme-fc use managed irq?

qla2xxx driver uses pci_alloc_irq_vectors_affinity to have affinity with each MSI-X vector. Currently nvme queue are not mapped based on affinity and irq offset. The change is to use blk_mq_pci_map_queues for mapping, this function consider irq affinity as well as irq offset.

Thanks,
~Saurav
> 
> CCing Ming to see if this affects his patchset.

Ming Lei Aug. 24, 2021, 3:46 a.m. UTC | #3

On Tue, Aug 24, 2021 at 03:38:24AM +0000, Saurav Kashyap wrote:
> Hi Sagi,
> Comments inline
> 
> > -----Original Message-----
> > From: Sagi Grimberg <sagi@grimberg.me>
> > Sent: Monday, August 23, 2021 10:51 PM
> > To: Nilesh Javali <njavali@marvell.com>; martin.petersen@oracle.com; linux-
> > nvme@lists.infradead.org; Ming Lei <ming.lei@redhat.com>
> > Cc: linux-scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-
> > Storage-Upstream@marvell.com>
> > Subject: Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> > 
> > 
> > On 8/23/21 5:56 AM, Nilesh Javali wrote:
> > > Currently nvme fc doesn't support map queue functionality. This patch
> > > set adds map_queue functionality to nvme_fc_mq_ops and
> > > nvme_fc_port_template, providing an option to LLDs to map queues
> > > similar to SCSI. For qla2xxx, minimum 10% improvement is noticed
> > > with this change as it helps in reducing cpu thrashing.
> > 
> > Does this make nvme-fc use managed irq?
> 
> qla2xxx driver uses pci_alloc_irq_vectors_affinity to have affinity with each MSI-X vector. Currently nvme queue are not mapped based on affinity and irq offset. The change is to use blk_mq_pci_map_queues for mapping, this function consider irq affinity as well as irq offset.
> 

OK, got it. Even though without this patchset, nvme-fc actually relies
on managed irq since qla2xxx driver uses pci_alloc_irq_vectors_affinity.

Now the patchset[1] isn't good for addressing the issue in
blk_mq_alloc_request_hctx().

[1] https://lore.kernel.org/linux-block/YR7demOSG6MKFVAF@T590/T/#t


Thanks,
Ming

Saurav Kashyap Aug. 25, 2021, 4:58 a.m. UTC | #4

Hi Ming,
Comments inline

> -----Original Message-----
> From: Ming Lei <ming.lei@redhat.com>
> Sent: Tuesday, August 24, 2021 9:17 AM
> To: Saurav Kashyap <skashyap@marvell.com>
> Cc: Sagi Grimberg <sagi@grimberg.me>; Nilesh Javali <njavali@marvell.com>;
> martin.petersen@oracle.com; linux-nvme@lists.infradead.org; linux-
> scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
> Upstream@marvell.com>
> Subject: [EXT] Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Tue, Aug 24, 2021 at 03:38:24AM +0000, Saurav Kashyap wrote:
> > Hi Sagi,
> > Comments inline
> >
> > > -----Original Message-----
> > > From: Sagi Grimberg <sagi@grimberg.me>
> > > Sent: Monday, August 23, 2021 10:51 PM
> > > To: Nilesh Javali <njavali@marvell.com>; martin.petersen@oracle.com;
> linux-
> > > nvme@lists.infradead.org; Ming Lei <ming.lei@redhat.com>
> > > Cc: linux-scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-
> > > Storage-Upstream@marvell.com>
> > > Subject: Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> > >
> > >
> > > On 8/23/21 5:56 AM, Nilesh Javali wrote:
> > > > Currently nvme fc doesn't support map queue functionality. This patch
> > > > set adds map_queue functionality to nvme_fc_mq_ops and
> > > > nvme_fc_port_template, providing an option to LLDs to map queues
> > > > similar to SCSI. For qla2xxx, minimum 10% improvement is noticed
> > > > with this change as it helps in reducing cpu thrashing.
> > >
> > > Does this make nvme-fc use managed irq?
> >
> > qla2xxx driver uses pci_alloc_irq_vectors_affinity to have affinity with each
> MSI-X vector. Currently nvme queue are not mapped based on affinity and irq
> offset. The change is to use blk_mq_pci_map_queues for mapping, this function
> consider irq affinity as well as irq offset.
> >
> 
> OK, got it. Even though without this patchset, nvme-fc actually relies
> on managed irq since qla2xxx driver uses pci_alloc_irq_vectors_affinity.
> 
> Now the patchset[1] isn't good for addressing the issue in
> blk_mq_alloc_request_hctx().

Can you please elaborate on this? Is there something needs to be done from my side?

Thanks,
~Saurav
> 
> [1] https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__lore.kernel.org_linux-2Dblock_YR7demOSG6MKFVAF-40T590_T_-
> 23t&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=ZHZbmY_LbM3DUZK_BDO1
> OITP3ot_Vkb_5w-
> gas5TBMQ&m=CqFDnfAsZphubKXkUx5gsRF6RZ2Qe6sxWkYq4pBfFD0&s=2Nba
> EUI5eB6_R6PxW8ld1Xn2OU3_UdD6D30uvFAWhow&e=
> 
> 
> Thanks,
> Ming

Saurav Kashyap Sept. 7, 2021, 9:18 a.m. UTC | #5

Hi,

Can I get a review for this patch set?

Thanks,
~Saurav

> -----Original Message-----
> From: Saurav Kashyap <skashyap@marvell.com>
> Sent: Wednesday, August 25, 2021 10:29 AM
> To: Ming Lei <ming.lei@redhat.com>
> Cc: Sagi Grimberg <sagi@grimberg.me>; Nilesh Javali <njavali@marvell.com>;
> martin.petersen@oracle.com; linux-nvme@lists.infradead.org; linux-
> scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
> Upstream@marvell.com>
> Subject: RE: [EXT] Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> 
> Hi Ming,
> Comments inline
> 
> > -----Original Message-----
> > From: Ming Lei <ming.lei@redhat.com>
> > Sent: Tuesday, August 24, 2021 9:17 AM
> > To: Saurav Kashyap <skashyap@marvell.com>
> > Cc: Sagi Grimberg <sagi@grimberg.me>; Nilesh Javali <njavali@marvell.com>;
> > martin.petersen@oracle.com; linux-nvme@lists.infradead.org; linux-
> > scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
> > Upstream@marvell.com>
> > Subject: [EXT] Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > On Tue, Aug 24, 2021 at 03:38:24AM +0000, Saurav Kashyap wrote:
> > > Hi Sagi,
> > > Comments inline
> > >
> > > > -----Original Message-----
> > > > From: Sagi Grimberg <sagi@grimberg.me>
> > > > Sent: Monday, August 23, 2021 10:51 PM
> > > > To: Nilesh Javali <njavali@marvell.com>; martin.petersen@oracle.com;
> > linux-
> > > > nvme@lists.infradead.org; Ming Lei <ming.lei@redhat.com>
> > > > Cc: linux-scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-
> > > > Storage-Upstream@marvell.com>
> > > > Subject: Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> > > >
> > > >
> > > > On 8/23/21 5:56 AM, Nilesh Javali wrote:
> > > > > Currently nvme fc doesn't support map queue functionality. This patch
> > > > > set adds map_queue functionality to nvme_fc_mq_ops and
> > > > > nvme_fc_port_template, providing an option to LLDs to map queues
> > > > > similar to SCSI. For qla2xxx, minimum 10% improvement is noticed
> > > > > with this change as it helps in reducing cpu thrashing.
> > > >
> > > > Does this make nvme-fc use managed irq?
> > >
> > > qla2xxx driver uses pci_alloc_irq_vectors_affinity to have affinity with each
> > MSI-X vector. Currently nvme queue are not mapped based on affinity and irq
> > offset. The change is to use blk_mq_pci_map_queues for mapping, this
> function
> > consider irq affinity as well as irq offset.
> > >
> >
> > OK, got it. Even though without this patchset, nvme-fc actually relies
> > on managed irq since qla2xxx driver uses pci_alloc_irq_vectors_affinity.
> >
> > Now the patchset[1] isn't good for addressing the issue in
> > blk_mq_alloc_request_hctx().
> 
> Can you please elaborate on this? Is there something needs to be done from my
> side?
> 
> Thanks,
> ~Saurav
> >
> > [1] https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__lore.kernel.org_linux-2Dblock_YR7demOSG6MKFVAF-40T590_T_-
> >
> 23t&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=ZHZbmY_LbM3DUZK_BDO1
> > OITP3ot_Vkb_5w-
> >
> gas5TBMQ&m=CqFDnfAsZphubKXkUx5gsRF6RZ2Qe6sxWkYq4pBfFD0&s=2Nba
> > EUI5eB6_R6PxW8ld1Xn2OU3_UdD6D30uvFAWhow&e=
> >
> >
> > Thanks,
> > Ming

Saurav Kashyap Sept. 14, 2021, 8:07 a.m. UTC | #6

Hi Sagi/Christoph,

I haven't heard anything on this and there are no review comments on this patch set, kindly include this in nvme tree.

Thanks,
~Saurav


> -----Original Message-----
> From: Saurav Kashyap
> Sent: Tuesday, September 7, 2021 2:48 PM
> To: Ming Lei <ming.lei@redhat.com>; Sagi Grimberg <sagi@grimberg.me>;
> linux-nvme@lists.infradead.org
> Cc: Nilesh Javali <njavali@marvell.com>; martin.petersen@oracle.com; linux-
> scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
> Upstream@marvell.com>
> Subject: RE: [EXT] Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> 
> Hi,
> 
> Can I get a review for this patch set?
> 
> Thanks,
> ~Saurav
> 
> > -----Original Message-----
> > From: Saurav Kashyap <skashyap@marvell.com>
> > Sent: Wednesday, August 25, 2021 10:29 AM
> > To: Ming Lei <ming.lei@redhat.com>
> > Cc: Sagi Grimberg <sagi@grimberg.me>; Nilesh Javali <njavali@marvell.com>;
> > martin.petersen@oracle.com; linux-nvme@lists.infradead.org; linux-
> > scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
> > Upstream@marvell.com>
> > Subject: RE: [EXT] Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> >
> > Hi Ming,
> > Comments inline
> >
> > > -----Original Message-----
> > > From: Ming Lei <ming.lei@redhat.com>
> > > Sent: Tuesday, August 24, 2021 9:17 AM
> > > To: Saurav Kashyap <skashyap@marvell.com>
> > > Cc: Sagi Grimberg <sagi@grimberg.me>; Nilesh Javali
> <njavali@marvell.com>;
> > > martin.petersen@oracle.com; linux-nvme@lists.infradead.org; linux-
> > > scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
> > > Upstream@marvell.com>
> > > Subject: [EXT] Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> > >
> > > External Email
> > >
> > > ----------------------------------------------------------------------
> > > On Tue, Aug 24, 2021 at 03:38:24AM +0000, Saurav Kashyap wrote:
> > > > Hi Sagi,
> > > > Comments inline
> > > >
> > > > > -----Original Message-----
> > > > > From: Sagi Grimberg <sagi@grimberg.me>
> > > > > Sent: Monday, August 23, 2021 10:51 PM
> > > > > To: Nilesh Javali <njavali@marvell.com>; martin.petersen@oracle.com;
> > > linux-
> > > > > nvme@lists.infradead.org; Ming Lei <ming.lei@redhat.com>
> > > > > Cc: linux-scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-
> QLogic-
> > > > > Storage-Upstream@marvell.com>
> > > > > Subject: Re: [PATCH 0/2] qla2xxx - add nvme map_queues support
> > > > >
> > > > >
> > > > > On 8/23/21 5:56 AM, Nilesh Javali wrote:
> > > > > > Currently nvme fc doesn't support map queue functionality. This patch
> > > > > > set adds map_queue functionality to nvme_fc_mq_ops and
> > > > > > nvme_fc_port_template, providing an option to LLDs to map queues
> > > > > > similar to SCSI. For qla2xxx, minimum 10% improvement is noticed
> > > > > > with this change as it helps in reducing cpu thrashing.
> > > > >
> > > > > Does this make nvme-fc use managed irq?
> > > >
> > > > qla2xxx driver uses pci_alloc_irq_vectors_affinity to have affinity with
> each
> > > MSI-X vector. Currently nvme queue are not mapped based on affinity and
> irq
> > > offset. The change is to use blk_mq_pci_map_queues for mapping, this
> > function
> > > consider irq affinity as well as irq offset.
> > > >
> > >
> > > OK, got it. Even though without this patchset, nvme-fc actually relies
> > > on managed irq since qla2xxx driver uses pci_alloc_irq_vectors_affinity.
> > >
> > > Now the patchset[1] isn't good for addressing the issue in
> > > blk_mq_alloc_request_hctx().
> >
> > Can you please elaborate on this? Is there something needs to be done from
> my
> > side?
> >
> > Thanks,
> > ~Saurav
> > >
> > > [1] https://urldefense.proofpoint.com/v2/url?u=https-
> > > 3A__lore.kernel.org_linux-2Dblock_YR7demOSG6MKFVAF-40T590_T_-
> > >
> >
> 23t&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=ZHZbmY_LbM3DUZK_BDO1
> > > OITP3ot_Vkb_5w-
> > >
> >
> gas5TBMQ&m=CqFDnfAsZphubKXkUx5gsRF6RZ2Qe6sxWkYq4pBfFD0&s=2Nba
> > > EUI5eB6_R6PxW8ld1Xn2OU3_UdD6D30uvFAWhow&e=
> > >
> > >
> > > Thanks,
> > > Ming

Christoph Hellwig Sept. 14, 2021, 8:32 a.m. UTC | #7

On Tue, Sep 14, 2021 at 08:07:02AM +0000, Saurav Kashyap wrote:
> Hi Sagi/Christoph,
> 
> I haven't heard anything on this and there are no review comments on this patch set, kindly include this in nvme tree.
> 

I'll queue this up once the 5.16 tree opens unless James voices any
objections.

Daniel Wagner Sept. 14, 2021, 9:31 a.m. UTC | #8

On Tue, Aug 24, 2021 at 11:46:32AM +0800, Ming Lei wrote:
> OK, got it. Even though without this patchset, nvme-fc actually relies
> on managed irq since qla2xxx driver uses pci_alloc_irq_vectors_affinity.
> 
> Now the patchset[1] isn't good for addressing the issue in
> blk_mq_alloc_request_hctx().
> 
> [1] https://lore.kernel.org/linux-block/YR7demOSG6MKFVAF@T590/T/#t

Ming, do you have any ideas/suggestions how to address it? I can work
on it.

Saurav Kashyap Sept. 22, 2021, 4:28 a.m. UTC | #9

> On Tue, Sep 14, 2021 at 08:07:02AM +0000, Saurav Kashyap wrote:
> > Hi Sagi/Christoph,
> >
> > I haven't heard anything on this and there are no review comments on this
> patch set, kindly include this in nvme tree.
> >
> 
> I'll queue this up once the 5.16 tree opens unless James voices any
> objections.

Sounds good.

Thanks,
~Saurav

Saurav Kashyap Oct. 13, 2021, 5:09 a.m. UTC | #10

Hi Christoph,

> > Hi Sagi/Christoph,
> >
> > I haven't heard anything on this and there are no review comments on this
> patch set, kindly include this in nvme tree.
> >
> 
> I'll queue this up once the 5.16 tree opens unless James voices any
> objections.

Any updates on this one?

Thanks,
~Saurav

Christoph Hellwig Oct. 13, 2021, 5:11 a.m. UTC | #11

On Wed, Oct 13, 2021 at 05:09:04AM +0000, Saurav Kashyap wrote:
> Hi Christoph,
> 
> > > Hi Sagi/Christoph,
> > >
> > > I haven't heard anything on this and there are no review comments on this
> > patch set, kindly include this in nvme tree.
> > >
> > 
> > I'll queue this up once the 5.16 tree opens unless James voices any
> > objections.
> 
> Any updates on this one?

http://git.infradead.org/nvme.git/commitdiff/062b5291ef352d6198c6c8414c71ae2debd06cf7