Re: [RFC] nvme/pci: allocate separate interrupt for reserved non-polled IO queue

[Christoph Hellwig <hch@lst.de>]

On Tue, Sep 22, 2020 at 12:28:16PM +0800, Jeffle Xu wrote:
> One queue will be reserved for non-polled IO when nvme.poll_queues is
> greater or equal than the number of IO queues that the nvme controller
> can provide. Currently the reserved queue for non-polled IO will reuse
> the interrupt used by admin queue in this case, e.g, vector 0.
> 
> This can work and the performance may not be an issue since the admin
> queue is used unfrequently. However this behaviour may be inconsistent
> with that when nvme.poll_queues is smaller than the number of IO
> queues available.
> 
> Thus allocate separate interrupt for this reserved queue, and thus make
> the behaviour consistent.
> 
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>

This code looks good, but the function already is a mess without the
addition.  What do you think about this variant?


diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 899d2f4d7ab612..43055138d59a47 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2038,31 +2038,29 @@ static int nvme_setup_irqs(struct nvme_dev *dev, unsigned int nr_io_queues)
 		.calc_sets	= nvme_calc_irq_sets,
 		.priv		= dev,
 	};
-	unsigned int irq_queues, this_p_queues;
+	unsigned int irq_queues, poll_queues;
 
 	/*
-	 * Poll queues don't need interrupts, but we need at least one IO
-	 * queue left over for non-polled IO.
+	 * Poll queues don't need interrupts, but we need at least one I/O queue
+	 * left over for non-polled I/O.
 	 */
-	this_p_queues = dev->nr_poll_queues;
-	if (this_p_queues >= nr_io_queues) {
-		this_p_queues = nr_io_queues - 1;
-		irq_queues = 1;
-	} else {
-		irq_queues = nr_io_queues - this_p_queues + 1;
-	}
-	dev->io_queues[HCTX_TYPE_POLL] = this_p_queues;
+	poll_queues = min(dev->nr_poll_queues, nr_io_queues - 1);
+	dev->io_queues[HCTX_TYPE_POLL] = poll_queues;
 
-	/* Initialize for the single interrupt case */
+	/*
+	 * Initialize for the single interrupt case, will be updated in
+	 * nvme_calc_irq_sets().
+	 */
 	dev->io_queues[HCTX_TYPE_DEFAULT] = 1;
 	dev->io_queues[HCTX_TYPE_READ] = 0;
 
 	/*
-	 * Some Apple controllers require all queues to use the
-	 * first vector.
+	 * Some Apple controllers require all queues to use the first vector.
 	 */
 	if (dev->ctrl.quirks & NVME_QUIRK_SINGLE_VECTOR)
 		irq_queues = 1;
+	else
+		irq_queues = 1 + (nr_io_queues - poll_queues);
 
 	return pci_alloc_irq_vectors_affinity(pdev, 1, irq_queues,
 			      PCI_IRQ_ALL_TYPES | PCI_IRQ_AFFINITY, &affd);

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme