| Message ID | 20220125091744.115996-3-lingshan.zhu@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | vDPA/ifcvf: implement shared IRQ feature | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Tue, Jan 25, 2022 at 05:17:42PM +0800, Zhu Lingshan wrote: > This commit implements a MSIX vector allocation helper > for vqs and config interrupts. > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > --- > 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... > +{ > + 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... > + } > + > + 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 ... > + if (!(nvectors > 0)) and then compare to zero ... > + return nvectors; > correct error handling is unlikely as a result. > - /* 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. > -- > 2.27.0
On 1/26/2022 3:36 AM, Michael S. Tsirkin wrote: > On Tue, Jan 25, 2022 at 05:17:42PM +0800, Zhu Lingshan wrote: >> This commit implements a MSIX vector allocation helper >> for vqs and config interrupts. >> >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >> --- >> 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! > >> +{ >> + 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... > >> + } >> + >> + 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 ... > >> + if (!(nvectors > 0)) > and then compare to zero ... > >> + return nvectors; >> > correct error handling is unlikely as a result. > > >> - /* 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. > >> -- >> 2.27.0
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) +{ + 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; + } + + 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); + if (!(nvectors > 0)) + return nvectors; - /* all queues and config interrupt */ max_intr = vf->nr_vring + 1; ret = pci_alloc_irq_vectors(pdev, max_intr,
This commit implements a MSIX vector allocation helper for vqs and config interrupts. Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> --- drivers/vdpa/ifcvf/ifcvf_main.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)