NVMe: IRQ vector sharing depends on resources
Subsystems (2)
PATCH 1/1 NVMe: IRQ vector sharing depends on resources
If the number of io queues + admin queue is less than or equal to the number of vectors, no IRQs will be shared. If there are more io queues + admin queue than there are vectors, qid 0 (admin queue) and qid 1 will share a vector
---
May improve IRQ coalescing capabilities
drivers/block/nvme-core.c | 17 ++++++++++++----- include/linux/nvme.h | 1 + 2 files changed, 13 insertions(+), 5 deletions(-)
drivers/block/nvme-core.c +12 -5
@@ -1457,7 +1457,8 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid)struct nvme_dev *dev = nvmeq->dev;int result; - nvmeq->cq_vector = qid -1 ; + nvmeq->cq_vector = qid - dev->shared_vec; + result =adapter_alloc_cq (dev, qid, nvmeq);if (result <0 )return result;
@@ -2265,11 +2266,12 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)if (!pdev->irq)pci_disable_msix (pdev); -for (i =0 ; i < nr_io_queues; i++) +for (i =0 ; i <= nr_io_queues; i++) dev->entry[i].entry = i; - vecs =pci_enable_msix_range (pdev, dev->entry,1 , nr_io_queues); + vecs =pci_enable_msix_range (pdev, dev->entry,1 , nr_io_queues +1 ); +if (vecs <0 ) { - vecs =pci_enable_msi_range (pdev,1 ,min (nr_io_queues,32 )); + vecs =pci_enable_msi_range (pdev,1 ,min (nr_io_queues +1 ,32 ));if (vecs <0 ) { vecs =1 ; }else {
@@ -2278,6 +2280,9 @@ static int nvme_setup_io_queues(struct nvme_dev *dev) } } +if (vecs < nr_io_queues +1 ) + dev->shared_vec =1 ; +/* * Should investigate if there's a performance win from allocating * more queues than interrupt vectors; it might allow the submission
@@ -2285,6 +2290,8 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)* number of interrupts. */ nr_io_queues = vecs; +if (!dev->shared_vec) + --nr_io_queues; dev->max_qid = nr_io_queues; result =queue_request_irq (dev, adminq, adminq->irqname);
@@ -2971,7 +2978,7 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id) dev =kzalloc_node (sizeof (*dev),GFP_KERNEL , node);if (!dev)return -ENOMEM ; - dev->entry =kzalloc_node (num_possible_cpus () *sizeof (*dev->entry), + dev->entry =kzalloc_node ((num_possible_cpus () +1 ) *sizeof (*dev->entry),GFP_KERNEL , node);if (!dev->entry) goto free;
include/linux/nvme.h +1 -0
@@ -85,6 +85,7 @@ struct nvme_dev {u32 db_stride;u32 ctrl_config;struct msix_entry *entry; +int shared_vec;struct nvme_bar __iomem *bar;struct list_head namespaces;struct kref kref;
Signature · 1 line
2.1.4
PATCH 1/1 NVMe: IRQ vector sharing depends on resources
Quoted 4 lines · Jon Derrick
If the number of io queues + admin queue is less than or equal to the number of vectors, no IRQs will be shared. If there are more io queues + admin queue than there are vectors, qid 0 (admin queue) and qid 1 will share a vector
I like the idea. The implementation, while it looks correct, reads a bit funny.
drivers/block/nvme-core.c static int nvme_create_queue(struct nvme_queue *nvmeq, int qid) · line 1464
Quoted 10 lines
drivers/block/nvme-core.c +2
-1@@ -1457,7 +1457,8 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid)struct nvme_dev *dev = nvmeq->dev;int result; - nvmeq->cq_vector = qid -1 ; + nvmeq->cq_vector = qid - dev->shared_vec; + result =adapter_alloc_cq (dev, qid, nvmeq);if (result <0 )return result;
This usage says to me "shared_vec is a count of the number of shared vectors".
drivers/block/nvme-core.c static int nvme_setup_io_queues(struct nvme_dev *dev) · line 2288
Quoted 10 lines
drivers/block/nvme-core.c +3
-0@@ -2278,6 +2280,9 @@ static int nvme_setup_io_queues(struct nvme_dev *dev) } } +if (vecs < nr_io_queues +1 ) + dev->shared_vec =1 ; +/* * Should investigate if there's a performance win from allocating * more queues than interrupt vectors; it might allow the submission
Here it looks like it's being used as a truth value. Maybe you're one of these newfangled C programmers who use 'bool' and 'true', but a lot of Linux programmers are dinosaurs who invented their own values of truth.
drivers/block/nvme-core.c static int nvme_setup_io_queues(struct nvme_dev *dev) · line 2297
Quoted 9 lines
drivers/block/nvme-core.c +2
-0@@ -2285,6 +2290,8 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)* number of interrupts. */ nr_io_queues = vecs; +if (!dev->shared_vec) + --nr_io_queues; dev->max_qid = nr_io_queues; result =queue_request_irq (dev, adminq, adminq->irqname);
Here it looks like a truth value again.
include/linux/nvme.h struct nvme_dev { · line 91
Quoted 12 lines
include/linux/nvme.h +1
-0@@ -85,6 +85,7 @@ struct nvme_dev {u32 db_stride;u32 ctrl_config;struct msix_entry *entry; +int shared_vec;struct nvme_bar __iomem *bar;struct list_head namespaces;struct kref kref;
This is a bad place to put an 'int' -- between two pointers. It'd fit better after ctrl_config, but ...
What I think you probably ought to do is have a 'vec_count' variable, right after 'queue_count'. That gets us a little closer to answering this question:
/*
* Should investigate if there's a performance win from allocating
* more queues than interrupt vectors; it might allow the submission
* path to scale better, even if the receive path is limited by the
* number of interrupts.
*/
I don't know exactly how 'vec_count' will work out; feel free to come back and tell me that we really do need to store the difference between the number of vectors and the number of queues.