Hi Jason Wang Thank you for your review.
Yes, you're correct. I will update the patch accordingly to address the issues you pointed out. V2：https://lore.kernel.org/all/[email protected]/ Here is the content of the V2 version: From: Yuxue Liu <[email protected]> When there is a ctlq and it doesn't require interrupt callbacks,the original method of calculating vectors wastes hardware MSI or MSI-X resources as well as system IRQ resources. Referencing the per_vq_vectors mode in the vp_find_vqs_msix function, calculate the required number of vectors based on whether the callback is set. Signed-off-by: Yuxue Liu <[email protected]> --- V1 -> V2: fix when allocating IRQs, scan all queues. V1: https://lore.kernel.org/all/[email protected]/ --- drivers/vdpa/virtio_pci/vp_vdpa.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c index 281287fae89f..87329d4358ce 100644 --- a/drivers/vdpa/virtio_pci/vp_vdpa.c +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c @@ -160,8 +160,15 @@ static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa) struct pci_dev *pdev = mdev->pci_dev; int i, ret, irq; int queues = vp_vdpa->queues; - int vectors = queues + 1; + int allocated_vectors, vectors = 0; + u16 msix_vec; + for (i = 0; i < queues; i++) { + if (vp_vdpa->vring[i].cb.callback != NULL) + vectors++; + } + /*By default, config interrupts request a single vector*/ + vectors = vectors + 1; ret = pci_alloc_irq_vectors(pdev, vectors, vectors, PCI_IRQ_MSIX); if (ret != vectors) { dev_err(&pdev->dev, @@ -169,13 +176,17 @@ static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa) vectors, ret); return ret; } - vp_vdpa->vectors = vectors; for (i = 0; i < queues; i++) { + if (vp_vdpa->vring[i].cb.callback == NULL) + continue; + else + msix_vec = allocated_vectors++; + snprintf(vp_vdpa->vring[i].msix_name, VP_VDPA_NAME_SIZE, "vp-vdpa[%s]-%d\n", pci_name(pdev), i); - irq = pci_irq_vector(pdev, i); + irq = pci_irq_vector(pdev, msix_vec); ret = devm_request_irq(&pdev->dev, irq, vp_vdpa_vq_handler, 0, vp_vdpa->vring[i].msix_name, @@ -185,13 +196,14 @@ static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa) "vp_vdpa: fail to request irq for vq %d\n", i); goto err; } - vp_modern_queue_vector(mdev, i, i); + vp_modern_queue_vector(mdev, i, msix_vec); vp_vdpa->vring[i].irq = irq; } + msix_vec = allocated_vectors; snprintf(vp_vdpa->msix_name, VP_VDPA_NAME_SIZE, "vp-vdpa[%s]-config\n", pci_name(pdev)); - irq = pci_irq_vector(pdev, queues); + irq = pci_irq_vector(pdev, msix_vec); ret = devm_request_irq(&pdev->dev, irq, vp_vdpa_config_handler, 0, vp_vdpa->msix_name, vp_vdpa); if (ret) { @@ -199,7 +211,7 @@ static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa) "vp_vdpa: fail to request irq for vq %d\n", i); goto err; } - vp_modern_config_vector(mdev, queues); + vp_modern_config_vector(mdev, msix_vec); vp_vdpa->config_irq = irq; return 0; ------- —————————————————————————————————————— Best regards, Gavin ----- Original Message ----- From: Jason Wang [email protected] Sent: March 18, 2024, 12:06 To: Gavin Liu [email protected] Cc: [email protected]; [email protected]; [email protected]; Angus Chen [email protected]; Gavin Liu [email protected] Subject: Re: [PATCH] vp_vdpa: fix the method of calculating vectors External Mail: This email originated from OUTSIDE of the organization! Do not click links, open attachments or provide ANY information unless you recognize the sender and know the content is safe. On Mon, Mar 18, 2024 at 11:01 AM gavin.liu <[email protected]> wrote: > > From: Yuxue Liu <[email protected]> > > When there is a ctlq and it doesn't require interrupt callbacks,the > original method of calculating vectors wastes hardware MSI or MSI-X > resources as well as system IRQ resources. Referencing the > per_vq_vectors mode in the vp_find_vqs_msix function, calculate the > required number of vectors based on whether the callback is set. > > Signed-off-by: Yuxue Liu <[email protected]> > --- > drivers/vdpa/virtio_pci/vp_vdpa.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c > b/drivers/vdpa/virtio_pci/vp_vdpa.c > index 281287fae89f..5066970b2570 100644 > --- a/drivers/vdpa/virtio_pci/vp_vdpa.c > +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c > @@ -160,7 +160,13 @@ static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa) > struct pci_dev *pdev = mdev->pci_dev; > int i, ret, irq; > int queues = vp_vdpa->queues; > - int vectors = queues + 1; > + int vectors = 0; > + > + for (i = 0; i < queues; i++) { > + if (vp_vdpa->vring[i].cb.callback != NULL) > + vectors++; > + } > + vectors = vectors + 1; > > ret = pci_alloc_irq_vectors(pdev, vectors, vectors, PCI_IRQ_MSIX); > if (ret != vectors) { > @@ -172,7 +178,7 @@ static int vp_vdpa_request_irq(struct vp_vdpa > *vp_vdpa) > > vp_vdpa->vectors = vectors; > > - for (i = 0; i < queues; i++) { > + for (i = 0; i < vectors - 1; i++) { This seems to be buggy. You didn't scan all queues so you will miss the IRQ for the last queue. Thanks > snprintf(vp_vdpa->vring[i].msix_name, VP_VDPA_NAME_SIZE, > "vp-vdpa[%s]-%d\n", pci_name(pdev), i); > irq = pci_irq_vector(pdev, i); @@ -191,7 +197,7 @@ > static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa) > > snprintf(vp_vdpa->msix_name, VP_VDPA_NAME_SIZE, > "vp-vdpa[%s]-config\n", > pci_name(pdev)); > - irq = pci_irq_vector(pdev, queues); > + irq = pci_irq_vector(pdev, vectors - 1); > ret = devm_request_irq(&pdev->dev, irq, vp_vdpa_config_handler, 0, > vp_vdpa->msix_name, vp_vdpa); > if (ret) { > @@ -199,7 +205,7 @@ static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa) > "vp_vdpa: fail to request irq for vq %d\n", i); > goto err; > } > - vp_modern_config_vector(mdev, queues); > + vp_modern_config_vector(mdev, vectors - 1); > vp_vdpa->config_irq = irq; > > return 0; > -- > 2.43.0 >
