blk-mq: Allow PCI vector offset for mapping queues
Subsystems (5)
PATCH 1/3 blk-mq: Allow PCI vector offset for mapping queues
The PCI interrupt vectors intended to be associated with a queue may not start at 0; a driver may allocate pre_vectors for special use. This patch adds an offset parameter so blk-mq may find the intended affinity mask and updates all drivers using this API accordingly.
---
v1 -> v2:
Update blk-mq API directly instead of chaining a default parameter to a new API, and update all drivers accordingly.
block/blk-mq-pci.c | 6 ++++-- drivers/nvme/host/pci.c | 2 +- drivers/scsi/qla2xxx/qla_os.c | 2 +- drivers/scsi/smartpqi/smartpqi_init.c | 2 +- include/linux/blk-mq-pci.h | 3 ++- 5 files changed, 9 insertions(+), 6 deletions(-)
block/blk-mq-pci.c +4 -2
@@ -21,6 +21,7 @@* blk_mq_pci_map_queues - provide a default queue mapping for PCI device * @set: tagset to provide the mapping for * @pdev: PCI device associated with @set. +* @offset: Offset to use for the pci irq vector * * This function assumes the PCI device @pdev has at least as many available * interrupt vectors as @set has queues. It will then query the vector
@@ -28,13 +29,14 @@* that maps a queue to the CPUs that have irq affinity for the corresponding * vector. */ -int blk_mq_pci_map_queues (struct blk_mq_tag_set *set,struct pci_dev *pdev) +int blk_mq_pci_map_queues (struct blk_mq_tag_set *set,struct pci_dev *pdev, +int offset) {const struct cpumask *mask;unsigned int queue, cpu;for (queue =0 ; queue < set->nr_hw_queues; queue++) { - mask =pci_irq_get_affinity (pdev, queue); + mask =pci_irq_get_affinity (pdev, queue + offset);if (!mask) goto fallback;
drivers/nvme/host/pci.c +1 -1
@@ -414,7 +414,7 @@ static int nvme_pci_map_queues(struct blk_mq_tag_set *set) {struct nvme_dev *dev = set->driver_data; -return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev)); +return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev),0 ); }/**
drivers/scsi/qla2xxx/qla_os.c +1 -1
@@ -6805,7 +6805,7 @@ static int qla2xxx_map_queues(struct Scsi_Host *shost)if (USER_CTRL_IRQ (vha->hw)) rc =blk_mq_map_queues (&shost->tag_set);else - rc =blk_mq_pci_map_queues (&shost->tag_set, vha->hw->pdev); + rc =blk_mq_pci_map_queues (&shost->tag_set, vha->hw->pdev,0 );return rc; }
drivers/scsi/smartpqi/smartpqi_init.c +1 -1
@@ -5348,7 +5348,7 @@ static int pqi_map_queues(struct Scsi_Host *shost) {struct pqi_ctrl_info *ctrl_info =shost_to_hba (shost); -return blk_mq_pci_map_queues (&shost->tag_set, ctrl_info->pci_dev); +return blk_mq_pci_map_queues (&shost->tag_set, ctrl_info->pci_dev,0 ); } static int pqi_getpciinfo_ioctl(struct pqi_ctrl_info *ctrl_info,
include/linux/blk-mq-pci.h +2 -1
@@ -5,6 +5,7 @@struct blk_mq_tag_set ;struct pci_dev ; -int blk_mq_pci_map_queues (struct blk_mq_tag_set *set,struct pci_dev *pdev); +int blk_mq_pci_map_queues (struct blk_mq_tag_set *set,struct pci_dev *pdev, +int offset);#endif /* _LINUX_BLK_MQ_PCI_H */
Signature · 1 line
2.14.3
PATCH 2/3 nvme-pci: Remove unused queue parameter
All the queue memory is allocated up front. We don't take the node into consideration when creating queues anymore, so removing the unused parameter.
---
v1 -> v2:
Added review.
drivers/nvme/host/pci.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
drivers/nvme/host/pci.c +3 -7
@@ -1379,8 +1379,7 @@ static int nvme_alloc_sq_cmds(struct nvme_dev *dev, struct nvme_queue *nvmeq,return 0 ; } -static int nvme_alloc_queue (struct nvme_dev *dev,int qid, -int depth,int node) +static int nvme_alloc_queue (struct nvme_dev *dev,int qid,int depth) {struct nvme_queue *nvmeq = &dev->queues[qid];
@@ -1595,8 +1594,7 @@ static int nvme_pci_configure_admin_queue(struct nvme_dev *dev)if (result <0 )return result; - result =nvme_alloc_queue (dev,0 ,NVME_AQ_DEPTH , -dev_to_node (dev->dev)); + result =nvme_alloc_queue (dev,0 ,NVME_AQ_DEPTH );if (result)return result;
@@ -1629,9 +1627,7 @@ static int nvme_create_io_queues(struct nvme_dev *dev)int ret =0 ;for (i = dev->ctrl.queue_count; i <= dev->max_qid; i++) { -/* vector == qid - 1, match nvme_create_queue */ -if (nvme_alloc_queue (dev, i, dev->q_depth, -pci_irq_get_node (to_pci_dev (dev->dev), i -1 ))) { +if (nvme_alloc_queue (dev, i, dev->q_depth)) { ret = -ENOMEM ;break ; }
Signature · 1 line
2.14.3
Re: [PATCH 2/3] nvme-pci: Remove unused queue parameter
Quoted 52 lines · Keith Busch
All the queue memory is allocated up front. We don't take the node into consideration when creating queues anymore, so removing the unused parameter.
Signed-off-by Keith Busch <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>---
v1 -> v2:Added review.
drivers/nvme/host/pci.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)drivers/nvme/host/pci.c +3
-7@@ -1379,8 +1379,7 @@ static int nvme_alloc_sq_cmds(struct nvme_dev *dev, struct nvme_queue *nvmeq,return 0 ; } -static int nvme_alloc_queue (struct nvme_dev *dev,int qid, -int depth,int node) +static int nvme_alloc_queue (struct nvme_dev *dev,int qid,int depth) {struct nvme_queue *nvmeq = &dev->queues[qid];@@ -1595,8 +1594,7 @@ static int nvme_pci_configure_admin_queue(struct nvme_dev *dev)if (result <0 )return result; - result =nvme_alloc_queue (dev,0 ,NVME_AQ_DEPTH , -dev_to_node (dev->dev)); + result =nvme_alloc_queue (dev,0 ,NVME_AQ_DEPTH );if (result)return result;@@ -1629,9 +1627,7 @@ static int nvme_create_io_queues(struct nvme_dev *dev)int ret =0 ;for (i = dev->ctrl.queue_count; i <= dev->max_qid; i++) { -/* vector == qid - 1, match nvme_create_queue */ -if (nvme_alloc_queue (dev, i, dev->q_depth, -pci_irq_get_node (to_pci_dev (dev->dev), i -1 ))) { +if (nvme_alloc_queue (dev, i, dev->q_depth)) { ret = -ENOMEM ;break ; }Signature · 1 line
2.14.3
Signature · 1 line
Ming
PATCH 3/3 nvme-pci: Separate IO and admin queue IRQ vectors
In reply to Keith BuschThe admin and first IO queues shared the first irq vector, which has an affinity mask including cpu0. If a system allows cpu0 to be offlined, the admin queue may not be usable if no other CPUs in the affinity mask are online. This is a problem since unlike IO queues, there is only one admin queue that always needs to be usable.
To fix, this patch allocates one pre_vector for the admin queue that is assigned all CPUs, so will always be accessible. The IO queues are assigned the remaining managed vectors.
In case a controller has only one interrupt vector available, the admin and IO queues will share the pre_vector with all CPUs assigned.
---
v1 -> v2:
Update to use new blk-mq API.
Removed unnecessary braces, inline functions, and temp variables.
Amended author (this has evolved significantly from the original).
drivers/nvme/host/pci.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
drivers/nvme/host/pci.c +17 -6
@@ -84,6 +84,7 @@ struct nvme_dev {struct dma_pool *prp_small_pool;unsigned online_queues;unsigned max_qid; +unsigned int num_vecs;int q_depth;u32 db_stride;void __iomem *bar;
@@ -414,7 +415,8 @@ static int nvme_pci_map_queues(struct blk_mq_tag_set *set) {struct nvme_dev *dev = set->driver_data; -return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev),0 ); +return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev), + dev->num_vecs >1 ); }/**
@@ -1455,7 +1457,11 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid) nvmeq->sq_cmds_io = dev->cmb + offset; } - nvmeq->cq_vector = qid -1 ; +/* +* A queue's vector matches the queue identifier unless the controller +* has only one vector available. +*/ + nvmeq->cq_vector = dev->num_vecs ==1 ?0 : qid; result =adapter_alloc_cq (dev, qid, nvmeq);if (result <0 ) goto release_vector;
@@ -1909,6 +1915,10 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)int result, nr_io_queues;unsigned long size; +struct irq_affinity affd = { + .pre_vectors =1 + }; + nr_io_queues =num_present_cpus (); result =nvme_set_queue_count (&dev->ctrl, &nr_io_queues);if (result <0 )
@@ -1944,11 +1954,12 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)* setting up the full range we need. */ pci_free_irq_vectors (pdev); - nr_io_queues =pci_alloc_irq_vectors (pdev,1 , nr_io_queues, -PCI_IRQ_ALL_TYPES |PCI_IRQ_AFFINITY ); -if (nr_io_queues <=0 ) + result =pci_alloc_irq_vectors_affinity (pdev,1 , nr_io_queues +1 , +PCI_IRQ_ALL_TYPES |PCI_IRQ_AFFINITY , &affd); +if (result <=0 )return -EIO ; - dev->max_qid = nr_io_queues; + dev->num_vecs = result; + dev->max_qid =max (result -1 ,1 );/* * Should investigate if there's a performance win from allocating
Signature · 1 line
2.14.3
Re: [PATCH 3/3] nvme-pci: Separate IO and admin queue IRQ vectors
Quoted 94 lines · Keith Busch
The admin and first IO queues shared the first irq vector, which has an affinity mask including cpu0. If a system allows cpu0 to be offlined, the admin queue may not be usable if no other CPUs in the affinity mask are online. This is a problem since unlike IO queues, there is only one admin queue that always needs to be usable.
To fix, this patch allocates one
pre_vectorfor the admin queue that is assigned all CPUs, so will always be accessible. The IO queues are assigned the remaining managed vectors.In case a controller has only one interrupt vector available, the admin and IO queues will share the
pre_vectorwith all CPUs assigned.Cc Jianchao Wang <[email protected]>Cc Ming Lei <[email protected]>Signed-off-by Keith Busch <[email protected]>---
v1 -> v2:Update to use new blk-mq API.
Removed unnecessary braces, inline functions, and temp variables.
Amended author (this has evolved significantly from the original).
drivers/nvme/host/pci.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)drivers/nvme/host/pci.c +17
-6@@ -84,6 +84,7 @@ struct nvme_dev {struct dma_pool *prp_small_pool;unsigned online_queues;unsigned max_qid; +unsigned int num_vecs;int q_depth;u32 db_stride;void __iomem *bar;@@ -414,7 +415,8 @@ static int nvme_pci_map_queues(struct blk_mq_tag_set *set) {struct nvme_dev *dev = set->driver_data; -return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev),0 ); +return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev), + dev->num_vecs >1 ); }/** @@ -1455,7 +1457,11 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid) nvmeq->sq_cmds_io = dev->cmb + offset; } - nvmeq->cq_vector = qid -1 ; +/* +* A queue's vector matches the queue identifier unless the controller +* has only one vector available. +*/ + nvmeq->cq_vector = dev->num_vecs ==1 ?0 : qid; result =adapter_alloc_cq (dev, qid, nvmeq);if (result <0 ) goto release_vector;@@ -1909,6 +1915,10 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)int result, nr_io_queues;unsigned long size; +struct irq_affinity affd = { + .pre_vectors =1 + }; + nr_io_queues =num_present_cpus (); result =nvme_set_queue_count (&dev->ctrl, &nr_io_queues);if (result <0 )@@ -1944,11 +1954,12 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)* setting up the full range we need. */ pci_free_irq_vectors (pdev); - nr_io_queues =pci_alloc_irq_vectors (pdev,1 , nr_io_queues, -PCI_IRQ_ALL_TYPES |PCI_IRQ_AFFINITY ); -if (nr_io_queues <=0 ) + result =pci_alloc_irq_vectors_affinity (pdev,1 , nr_io_queues +1 , +PCI_IRQ_ALL_TYPES |PCI_IRQ_AFFINITY , &affd); +if (result <=0 )return -EIO ; - dev->max_qid = nr_io_queues; + dev->num_vecs = result; + dev->max_qid =max (result -1 ,1 );/* * Should investigate if there's a performance win from allocating Signature · 1 line
2.14.3
Signature · 1 line
Ming
Re: [PATCH 3/3] nvme-pci: Separate IO and admin queue IRQ vectors
In reply to Keith Buschdrivers/nvme/host/pci.c static int nvme_pci_map_queues(struct blk_mq_tag_set *set) · line 419
Quoted 3 lines · Keith Busch
drivers/nvme/host/pci.c +2
-1-return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev),0 ); +return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev), + dev->num_vecs >1 );
Can you turn this into:
- return blk_mq_pci_map_queues(set, to_pci_dev(dev->dev), 0); dev->num_vecs > 1 ? 1 /* admin queue */ : 0);
no functional change, but much easier to understand.
Except for that the whole series looks good:
Re: [PATCH 3/3] nvme-pci: Separate IO and admin queue IRQ vectors
Quoted 10 lines · Christoph Hellwig
drivers/nvme/host/pci.c static int nvme_pci_map_queues(struct blk_mq_tag_set *set) · line 419
Quoted 3 lines · Keith Busch
drivers/nvme/host/pci.c +2
-1-return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev),0 ); +return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev), + dev->num_vecs >1 );Can you turn this into:
drivers/nvme/host/pci.c +0
-1-return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev),0 );dev->num_vecs > 1 ? 1 /* admin queue */ : 0);
Quoted 3 lines
no functional change, but much easier to understand.
Except for that the whole series looks good:
Sounds good, thanks for the reviews, Christoph and Ming.
Updated with your suggestion and applied (Jens picked up the blk-mq part; nvme-4.17 is rebased to that).
Quoted 106 lines · Keith Busch
The PCI interrupt vectors intended to be associated with a queue may not start at 0; a driver may allocate
pre_vectorsfor special use. This patch adds an offset parameter so blk-mq may find the intended affinity mask and updates all drivers using this API accordingly.Cc Don Brace <[email protected]>Cc <[email protected]>Cc <[email protected]>Signed-off-by Keith Busch <[email protected]>---
v1 -> v2:Update blk-mq API directly instead of chaining a default parameter to a new API, and update all drivers accordingly.
block/blk-mq-pci.c | 6 ++++-- drivers/nvme/host/pci.c | 2 +- drivers/scsi/qla2xxx/qla_os.c | 2 +- drivers/scsi/smartpqi/smartpqi_init.c | 2 +- include/linux/blk-mq-pci.h | 3 ++- 5 files changed, 9 insertions(+), 6 deletions(-)block/blk-mq-pci.c +4
-2@@ -21,6 +21,7 @@* blk_mq_pci_map_queues - provide a default queue mapping for PCI device * @set: tagset to provide the mapping for * @pdev: PCI device associated with @set. +* @offset: Offset to use for the pci irq vector * * This function assumes the PCI device @pdev has at least as many available * interrupt vectors as @set has queues. It will then query the vector @@ -28,13 +29,14 @@* that maps a queue to the CPUs that have irq affinity for the corresponding * vector. */ -int blk_mq_pci_map_queues (struct blk_mq_tag_set *set,struct pci_dev *pdev) +int blk_mq_pci_map_queues (struct blk_mq_tag_set *set,struct pci_dev *pdev, +int offset) {const struct cpumask *mask;unsigned int queue, cpu;for (queue =0 ; queue < set->nr_hw_queues; queue++) { - mask =pci_irq_get_affinity (pdev, queue); + mask =pci_irq_get_affinity (pdev, queue + offset);if (!mask) goto fallback;drivers/nvme/host/pci.c +1
-1@@ -414,7 +414,7 @@ static int nvme_pci_map_queues(struct blk_mq_tag_set *set) {struct nvme_dev *dev = set->driver_data; -return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev)); +return blk_mq_pci_map_queues (set,to_pci_dev (dev->dev),0 ); }/** drivers/scsi/qla2xxx/qla_os.c +1
-1@@ -6805,7 +6805,7 @@ static int qla2xxx_map_queues(struct Scsi_Host *shost)if (USER_CTRL_IRQ (vha->hw)) rc =blk_mq_map_queues (&shost->tag_set);else - rc =blk_mq_pci_map_queues (&shost->tag_set, vha->hw->pdev); + rc =blk_mq_pci_map_queues (&shost->tag_set, vha->hw->pdev,0 );return rc; }drivers/scsi/smartpqi/smartpqi_init.c +1
-1@@ -5348,7 +5348,7 @@ static int pqi_map_queues(struct Scsi_Host *shost) {struct pqi_ctrl_info *ctrl_info =shost_to_hba (shost); -return blk_mq_pci_map_queues (&shost->tag_set, ctrl_info->pci_dev); +return blk_mq_pci_map_queues (&shost->tag_set, ctrl_info->pci_dev,0 ); } static int pqi_getpciinfo_ioctl(struct pqi_ctrl_info *ctrl_info,include/linux/blk-mq-pci.h +2
-1@@ -5,6 +5,7 @@struct blk_mq_tag_set ;struct pci_dev ; -int blk_mq_pci_map_queues (struct blk_mq_tag_set *set,struct pci_dev *pdev); +int blk_mq_pci_map_queues (struct blk_mq_tag_set *set,struct pci_dev *pdev, +int offset);#endif /* _LINUX_BLK_MQ_PCI_H */ Signature · 1 line
2.14.3
Signature · 1 line
Ming
Jens Axboe
maintainerQuoted 4 lines · Keith Busch
The PCI interrupt vectors intended to be associated with a queue may not start at 0; a driver may allocate
pre_vectorsfor special use. This patch adds an offset parameter so blk-mq may find the intended affinity mask and updates all drivers using this API accordingly.
Looks good to me, I'll queue it up for 4.17.
Signature · 1 line
Jens Axboe
Quoted 46 lines
-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Keith Busch
Sent: Tuesday, March 27, 2018 10:39 AM
To: Linux NVMe <[email protected]>; Linux Block <linux- [email protected]>
Cc: Christoph Hellwig <[email protected]>; Sagi Grimberg <[email protected]>; Jianchao Wang <[email protected]>; Ming Lei <[email protected]>; Jens Axboe <[email protected]>; Keith Busch <[email protected]>; Don Brace <[email protected]>; qla2xxx- [email protected]; [email protected]
Subject: [PATCH 1/3] blk-mq: Allow PCI vector offset for mapping queues =20
EXTERNAL EMAIL
=20
=20
The PCI interrupt vectors intended to be associated with a queue may not start at 0; a driver may allocatepre_vectorsfor special use. This patch adds an offset parameter so blk-mq may find the intended affinity mask and updates all drivers using this API accordingly.
=20Cc Don Brace <[email protected]>Cc <[email protected]>Cc <[email protected]>Signed-off-by Keith Busch <[email protected]>---
v1 -> v2:
=20
Update blk-mq API directly instead of chaining a default parameter to a new API, and update all drivers accordingly.
=20block/blk-mq-pci.c | 6 ++++-- drivers/nvme/host/pci.c | 2 +- drivers/scsi/qla2xxx/qla_os.c | 2 +- drivers/scsi/smartpqi/smartpqi_init.c | 2 +- include/linux/blk-mq-pci.h | 3 ++- 5 files changed, 9 insertions(+), 6 deletions(-)=20
drivers/scsi/smartpqi/smartpqi_init.c +0
-0@@ -5348,7 +5348,7 @@ static int pqi_map_queues(struct Scsi_Host *shost) { struct pqi_ctrl_info *ctrl_info =3D shost_to_hba(shost);
Quoted 2 lines
=20
- returnblk_mq_pci_map_queues(&shost->tag_set, ctrl_info->pci_dev)=
;
Quoted 1 line
+ return blk_mq_pci_map_queues(&shost->tag_set, ctrl_info->pci_dev,=
0);
Quoted 2 lines
}
=20