Re: [PATCH V2 2/4] vDPA/ifcvf: implement device MSIX vector allocator

On 1/26/2022 3:36 AM, Michael S. Tsirkin wrote:

On Tue, Jan 25, 2022 at 05:17:42PM +0800, Zhu Lingshan wrote:

quoted

This commit implements a MSIX vector allocation helper
for vqs and config interrupts.

Signed-off-by: Zhu Lingshan <redacted>
---
  drivers/vdpa/ifcvf/ifcvf_main.c | 30 ++++++++++++++++++++++++++++--
  1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index d1a6b5ab543c..7e2af2d2aaf5 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c

@@ -58,14 +58,40 @@ static void ifcvf_free_irq(struct ifcvf_adapter *adapter, int queues)
  	ifcvf_free_irq_vectors(pdev);
  }
  
+static int ifcvf_alloc_vectors(struct ifcvf_adapter *adapter)

So the helper returns an int...

can fix in V3, thanks for your help!

quoted

+{
+	struct pci_dev *pdev = adapter->pdev;
+	struct ifcvf_hw *vf = &adapter->vf;
+	u16 max_intr, ret;
+
+	/* all queues and config interrupt  */
+	max_intr = vf->nr_vring + 1;
+	ret = pci_alloc_irq_vectors(pdev, 1, max_intr, PCI_IRQ_MSIX | PCI_IRQ_AFFINITY);
+
+	if (ret < 0) {
+		IFCVF_ERR(pdev, "Failed to alloc IRQ vectors\n");
+		return ret;

which is negative on error...

quoted

+	}
+
+	if (ret < max_intr)
+		IFCVF_INFO(pdev,
+			   "Requested %u vectors, however only %u allocated, lower performance\n",
+			   max_intr, ret);
+
+	return ret;
+}
+
  static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
  {
  	struct pci_dev *pdev = adapter->pdev;
  	struct ifcvf_hw *vf = &adapter->vf;
  	int vector, i, ret, irq;
-	u16 max_intr;
+	u16 nvectors, max_intr;
+
+	nvectors = ifcvf_alloc_vectors(adapter);

which you proceed to stash into an unsigned int ...

quoted

+	if (!(nvectors > 0))

and then compare to zero ...

quoted

+		return nvectors;

correct error handling is unlikely as a result.

   

quoted

-	/* all queues and config interrupt  */
  	max_intr = vf->nr_vring + 1;
  
  	ret = pci_alloc_irq_vectors(pdev, max_intr,

As long as you are introducing a helper, document it's return
type and behaviour and then use correctly.

quoted

-- 
2.27.0