Re: [PATCH 0/4] nvme: Threaded interrupt handling improvements

[Christoph Hellwig <hch@lst.de>]

FYI, this is how I'd imagine my comments to look like on top of your
tree, modulo the posted interrupt disabling part that will need
changes outside nvme.  If we want to be fancy we can split the irq
disable/enable into separate helpers.

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index e62fede7d4e4..1d6a222ddcc3 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1042,50 +1042,45 @@ static irqreturn_t nvme_irq(int irq, void *data)
 	return ret;
 }
 
-static void nvme_irq_spin(int irq, void *data)
-{
-	while (nvme_irq(irq, data) != IRQ_NONE)
-		cond_resched();
-}
-
 static irqreturn_t nvme_irq_thread(int irq, void *data)
-{
-	nvme_irq_spin(irq, data);
-	__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 0);
-	return IRQ_HANDLED;
-}
-
-static irqreturn_t nvme_irq_check(int irq, void *data)
 {
 	struct nvme_queue *nvmeq = data;
+	u16 start, end;
 
-	if (nvme_cqe_pending(nvmeq)) {
-		__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 1);
-		return IRQ_WAKE_THREAD;
+	/*
+	 * The rmb/wmb pair ensures we see all updates from a previous run of
+	 * the irq thread, even if that was on another CPU.
+	 */
+	rmb();
+	for (;;) {
+		nvme_process_cq(nvmeq, &start, &end, -1);
+		nvmeq->last_cq_head = nvmeq->cq_head;
+		if (start == end)
+			break;
+		nvme_complete_cqes(nvmeq, start, end);
+		cond_resched();
 	}
-	return IRQ_NONE;
-}
-
-static irqreturn_t nvme_irq_thread_msi(int irq, void *data)
-{
-	struct nvme_queue *nvmeq = data;
-	struct nvme_dev *dev = nvmeq->dev;
+	wmb();
 
-	nvme_irq_spin(irq, data);
-	writel(1 << nvmeq->cq_vector, dev->bar + NVME_REG_INTMC);
+	if (to_pci_dev(nvmeq->dev->dev)->msix_enabled)
+		__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 0);
+	else
+		writel(1 << nvmeq->cq_vector, nvmeq->dev->bar + NVME_REG_INTMC);
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t nvme_irq_check_msi(int irq, void *data)
+static irqreturn_t nvme_irq_check(int irq, void *data)
 {
 	struct nvme_queue *nvmeq = data;
-	struct nvme_dev *dev = nvmeq->dev;
 
-	if (nvme_cqe_pending(nvmeq)) {
-		writel(1 << nvmeq->cq_vector, dev->bar + NVME_REG_INTMS);
-		return IRQ_WAKE_THREAD;
-	}
-	return IRQ_NONE;
+	if (!nvme_cqe_pending(nvmeq))
+		return IRQ_NONE;
+
+	if (to_pci_dev(nvmeq->dev->dev)->msix_enabled)
+		__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 1);
+	else
+		writel(1 << nvmeq->cq_vector, nvmeq->dev->bar + NVME_REG_INTMS);
+	return IRQ_WAKE_THREAD;
 }
 
 /*
@@ -1542,11 +1537,6 @@ static int queue_request_irq(struct nvme_queue *nvmeq)
 	int nr = nvmeq->dev->ctrl.instance;
 
 	if (use_threaded_interrupts) {
-		/* MSI and Legacy use the same NVMe IRQ masking */
-		if (!pdev->msix_enabled)
-			return pci_request_irq(pdev, nvmeq->cq_vector,
-				nvme_irq_check_msi, nvme_irq_thread_msi,
-				nvmeq, "nvme%dq%d", nr, nvmeq->qid);
 		return pci_request_irq(pdev, nvmeq->cq_vector, nvme_irq_check,
 				nvme_irq_thread, nvmeq, "nvme%dq%d", nr,
 				nvmeq->qid);

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