* [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines
2014-02-21 15:49 ` [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines Alexander Gordeev
@ 2014-02-22 19:55 ` Alexander Gordeev
2014-03-04 0:12 ` Jon Mason
2014-03-11 16:00 ` Alexander Gordeev
2 siblings, 0 replies; 18+ messages in thread
From: Alexander Gordeev @ 2014-02-22 19:55 UTC (permalink / raw)
To: linux-kernel; +Cc: Jon Mason, linux-pci
This is an cleanup effort to make ntb_setup_msix() more
readable - use ntb_setup_bwd_msix() to init MSI-Xs on
BWD hardware and ntb_setup_snb_msix() - on SNB hardware.
Function ntb_setup_snb_msix() also initializes MSI-Xs the
way it should has been done - looping pci_enable_msix()
until success or failure.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Cc: Jon Mason <jon.mason@intel.com>
Cc: linux-pci@vger.kernel.org
---
drivers/ntb/ntb_hw.c | 171 +++++++++++++++++++++++++++++++-------------------
1 files changed, 106 insertions(+), 65 deletions(-)
diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 53468f4..679d4d0 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1079,10 +1079,110 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
return IRQ_HANDLED;
}
-static int ntb_setup_msix(struct ntb_device *ndev)
+static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
+{
+ struct pci_dev *pdev = ndev->pdev;
+ struct msix_entry *msix;
+ int rc, i;
+
+ if (msix_entries < SNB_MSIX_CNT)
+ return -ENOSPC;
+
+ rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+ if (rc < 0)
+ return rc;
+ else if (rc > 0)
+ return -ENOSPC;
+
+ for (i = 0; i < msix_entries; i++) {
+ msix = &ndev->msix_entries[i];
+ WARN_ON(!msix->vector);
+
+ if (i == msix_entries - 1) {
+ rc = request_irq(msix->vector,
+ xeon_event_msix_irq, 0,
+ "ntb-event-msix", ndev);
+ if (rc)
+ goto err;
+ } else {
+ rc = request_irq(msix->vector,
+ xeon_callback_msix_irq, 0,
+ "ntb-callback-msix",
+ &ndev->db_cb[i]);
+ if (rc)
+ goto err;
+ }
+ }
+
+ ndev->num_msix = msix_entries;
+ ndev->max_cbs = msix_entries - 1;
+
+ return 0;
+
+err:
+ while (--i >= 0) {
+ msix = &ndev->msix_entries[i];
+ WARN_ON(i == ndev->num_msix - 1);
+
+ if (i == ndev->num_msix - 1)
+ free_irq(msix->vector, ndev);
+ else
+ free_irq(msix->vector, &ndev->db_cb[i]);
+ }
+
+ pci_disable_msix(pdev);
+ ndev->num_msix = 0;
+
+ return rc;
+}
+
+static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
{
struct pci_dev *pdev = ndev->pdev;
struct msix_entry *msix;
+ int rc, i;
+
+retry:
+ rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+ if (rc < 0)
+ return rc;
+ else if (rc > 0) {
+ dev_warn(&pdev->dev,
+ "Only %d MSI-X vectors. "
+ "Limiting the number of queues to that number.\n",
+ rc);
+ msix_entries = rc;
+ goto retry;
+ }
+
+ for (i = 0; i < msix_entries; i++) {
+ msix = &ndev->msix_entries[i];
+ WARN_ON(!msix->vector);
+
+ rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
+ "ntb-callback-msix", &ndev->db_cb[i]);
+ if (rc)
+ goto err;
+ }
+
+ ndev->num_msix = msix_entries;
+ ndev->max_cbs = msix_entries;
+
+ return 0;
+
+err:
+ while (--i >= 0)
+ free_irq(msix->vector, &ndev->db_cb[i]);
+
+ pci_disable_msix(pdev);
+ ndev->num_msix = 0;
+
+ return rc;
+}
+
+static int ntb_setup_msix(struct ntb_device *ndev)
+{
+ struct pci_dev *pdev = ndev->pdev;
int msix_entries;
int rc, i;
@@ -1105,78 +1205,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
for (i = 0; i < msix_entries; i++)
ndev->msix_entries[i].entry = i;
- rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
- if (rc < 0)
- goto err1;
- if (rc > 0) {
- /* On SNB, the link interrupt is always tied to 4th vector. If
- * we can't get all 4, then we can't use MSI-X.
- */
- if (ndev->hw_type != BWD_HW) {
- rc = -EIO;
- goto err1;
- }
-
- dev_warn(&pdev->dev,
- "Only %d MSI-X vectors. Limiting the number of queues to that number.\n",
- rc);
- msix_entries = rc;
-
- rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
- if (rc)
- goto err1;
- }
-
- for (i = 0; i < msix_entries; i++) {
- msix = &ndev->msix_entries[i];
- WARN_ON(!msix->vector);
-
- /* Use the last MSI-X vector for Link status */
- if (ndev->hw_type == BWD_HW) {
- rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
- "ntb-callback-msix", &ndev->db_cb[i]);
- if (rc)
- goto err2;
- } else {
- if (i == msix_entries - 1) {
- rc = request_irq(msix->vector,
- xeon_event_msix_irq, 0,
- "ntb-event-msix", ndev);
- if (rc)
- goto err2;
- } else {
- rc = request_irq(msix->vector,
- xeon_callback_msix_irq, 0,
- "ntb-callback-msix",
- &ndev->db_cb[i]);
- if (rc)
- goto err2;
- }
- }
- }
-
- ndev->num_msix = msix_entries;
if (ndev->hw_type == BWD_HW)
- ndev->max_cbs = msix_entries;
+ rc = ntb_setup_bwd_msix(ndev, msix_entries);
else
- ndev->max_cbs = msix_entries - 1;
+ rc = ntb_setup_snb_msix(ndev, msix_entries);
+ if (rc)
+ goto err1;
return 0;
-err2:
- while (--i >= 0) {
- msix = &ndev->msix_entries[i];
- if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
- free_irq(msix->vector, ndev);
- else
- free_irq(msix->vector, &ndev->db_cb[i]);
- }
- pci_disable_msix(pdev);
err1:
kfree(ndev->msix_entries);
- dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
err:
- ndev->num_msix = 0;
+ dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
return rc;
}
--
1.7.7.6
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines
2014-02-21 15:49 ` [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines Alexander Gordeev
2014-02-22 19:55 ` Alexander Gordeev
@ 2014-03-04 0:12 ` Jon Mason
2014-03-04 16:41 ` Alexander Gordeev
2014-03-11 16:00 ` Alexander Gordeev
2 siblings, 1 reply; 18+ messages in thread
From: Jon Mason @ 2014-03-04 0:12 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: linux-kernel, linux-pci
On Fri, Feb 21, 2014 at 04:49:31PM +0100, Alexander Gordeev wrote:
> This is an cleanup effort to make ntb_setup_msix() more
> readable - use ntb_setup_bwd_msix() to init MSI-Xs on
> BWD hardware and ntb_setup_snb_msix() - on SNB hardware.
>
> Function ntb_setup_snb_msix() also initializes MSI-Xs the
> way it should has been done - looping pci_enable_msix()
> until success or failure.
>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> Cc: Jon Mason <jon.mason@intel.com>
> Cc: linux-pci@vger.kernel.org
> ---
> drivers/ntb/ntb_hw.c | 170 +++++++++++++++++++++++++++++++-------------------
> 1 files changed, 106 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 53468f4..0472045 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1079,6 +1079,107 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
> return IRQ_HANDLED;
> }
>
> +static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
> +{
> + struct pci_dev *pdev = ndev->pdev;
> + struct msix_entry *msix;
> + int rc, i;
> +
> + if (msix_entries < SNB_MSIX_CNT)
I would prefer ndev->limits.msix_cnt here to keep it generic. I could
foresee this number changing on upcoming hardware with a similar
design.
> + return -ENOSPC;
> +
> + rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> + if (rc < 0)
> + return rc;
> + else if (rc > 0)
> + return -ENOSPC;
> +
> + for (i = 0; i < msix_entries; i++) {
> + msix = &ndev->msix_entries[i];
> + WARN_ON(!msix->vector);
> +
> + if (i == msix_entries - 1) {
> + rc = request_irq(msix->vector,
> + xeon_event_msix_irq, 0,
> + "ntb-event-msix", ndev);
> + if (rc)
> + goto err;
> + } else {
> + rc = request_irq(msix->vector,
> + xeon_callback_msix_irq, 0,
> + "ntb-callback-msix",
> + &ndev->db_cb[i]);
> + if (rc)
> + goto err;
> + }
> + }
> +
> + ndev->num_msix = msix_entries;
> + ndev->max_cbs = msix_entries - 1;
> +
> + return 0;
> +
> +err:
> + while (--i >= 0) {
> + msix = &ndev->msix_entries[i];
> + WARN_ON(i == ndev->num_msix - 1);
> +
> + if (i == ndev->num_msix - 1)
Why have a WARN followed by a if statement on the same condition? As
it stands right now, there is no way it could be hit. So I'd say
remove both of them.
Aside from that it looks good. Thanks for taking the time to cleanup
the code!
Thanks,
Jon
> + free_irq(msix->vector, ndev);
> + else
> + free_irq(msix->vector, &ndev->db_cb[i]);
> + }
> +
> + pci_disable_msix(pdev);
> + ndev->num_msix = 0;
> +
> + return rc;
> +}
> +
> +static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
> +{
> + struct pci_dev *pdev = ndev->pdev;
> + struct msix_entry *msix;
> + int rc, i;
> +
> +retry:
> + rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> + if (rc < 0)
> + return rc;
> + else if (rc > 0) {
> + dev_warn(&pdev->dev,
> + "Only %d MSI-X vectors. "
> + "Limiting the number of queues to that number.\n",
> + rc);
> + msix_entries = rc;
> + goto retry;
> + }
> +
> + for (i = 0; i < msix_entries; i++) {
> + msix = &ndev->msix_entries[i];
> + WARN_ON(!msix->vector);
> +
> + rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> + "ntb-callback-msix", &ndev->db_cb[i]);
> + if (rc)
> + goto err;
> + }
> +
> + ndev->num_msix = msix_entries;
> + ndev->max_cbs = msix_entries;
> +
> + return 0;
> +
> +err:
> + while (--i >= 0)
> + free_irq(msix->vector, &ndev->db_cb[i]);
> +
> + pci_disable_msix(pdev);
> + ndev->num_msix = 0;
> +
> + return rc;
> +}
> +
> static int ntb_setup_msix(struct ntb_device *ndev)
> {
> struct pci_dev *pdev = ndev->pdev;
> @@ -1105,78 +1206,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
> for (i = 0; i < msix_entries; i++)
> ndev->msix_entries[i].entry = i;
>
> - rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> - if (rc < 0)
> - goto err1;
> - if (rc > 0) {
> - /* On SNB, the link interrupt is always tied to 4th vector. If
> - * we can't get all 4, then we can't use MSI-X.
> - */
> - if (ndev->hw_type != BWD_HW) {
> - rc = -EIO;
> - goto err1;
> - }
> -
> - dev_warn(&pdev->dev,
> - "Only %d MSI-X vectors. Limiting the number of queues to that number.\n",
> - rc);
> - msix_entries = rc;
> -
> - rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> - if (rc)
> - goto err1;
> - }
> -
> - for (i = 0; i < msix_entries; i++) {
> - msix = &ndev->msix_entries[i];
> - WARN_ON(!msix->vector);
> -
> - /* Use the last MSI-X vector for Link status */
> - if (ndev->hw_type == BWD_HW) {
> - rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> - "ntb-callback-msix", &ndev->db_cb[i]);
> - if (rc)
> - goto err2;
> - } else {
> - if (i == msix_entries - 1) {
> - rc = request_irq(msix->vector,
> - xeon_event_msix_irq, 0,
> - "ntb-event-msix", ndev);
> - if (rc)
> - goto err2;
> - } else {
> - rc = request_irq(msix->vector,
> - xeon_callback_msix_irq, 0,
> - "ntb-callback-msix",
> - &ndev->db_cb[i]);
> - if (rc)
> - goto err2;
> - }
> - }
> - }
> -
> - ndev->num_msix = msix_entries;
> if (ndev->hw_type == BWD_HW)
> - ndev->max_cbs = msix_entries;
> + rc = ntb_setup_bwd_msix(ndev, msix_entries);
> else
> - ndev->max_cbs = msix_entries - 1;
> + rc = ntb_setup_snb_msix(ndev, msix_entries);
> + if (rc)
> + goto err1;
>
> return 0;
>
> -err2:
> - while (--i >= 0) {
> - msix = &ndev->msix_entries[i];
> - if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
> - free_irq(msix->vector, ndev);
> - else
> - free_irq(msix->vector, &ndev->db_cb[i]);
> - }
> - pci_disable_msix(pdev);
> err1:
> kfree(ndev->msix_entries);
> - dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> err:
> - ndev->num_msix = 0;
> + dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> return rc;
> }
>
> --
> 1.7.7.6
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines
2014-03-04 0:12 ` Jon Mason
@ 2014-03-04 16:41 ` Alexander Gordeev
2014-03-10 21:18 ` Jon Mason
0 siblings, 1 reply; 18+ messages in thread
From: Alexander Gordeev @ 2014-03-04 16:41 UTC (permalink / raw)
To: Jon Mason; +Cc: linux-kernel, linux-pci
On Mon, Mar 03, 2014 at 05:12:25PM -0700, Jon Mason wrote:
> On Fri, Feb 21, 2014 at 04:49:31PM +0100, Alexander Gordeev wrote:
> > This is an cleanup effort to make ntb_setup_msix() more
> > readable - use ntb_setup_bwd_msix() to init MSI-Xs on
> > BWD hardware and ntb_setup_snb_msix() - on SNB hardware.
> >
> > Function ntb_setup_snb_msix() also initializes MSI-Xs the
> > way it should has been done - looping pci_enable_msix()
> > until success or failure.
> >
> > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > Cc: Jon Mason <jon.mason@intel.com>
> > Cc: linux-pci@vger.kernel.org
> > ---
> > drivers/ntb/ntb_hw.c | 170 +++++++++++++++++++++++++++++++-------------------
> > 1 files changed, 106 insertions(+), 64 deletions(-)
> >
> > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > index 53468f4..0472045 100644
> > --- a/drivers/ntb/ntb_hw.c
> > +++ b/drivers/ntb/ntb_hw.c
> > @@ -1079,6 +1079,107 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
> > return IRQ_HANDLED;
> > }
> >
> > +static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
> > +{
> > + struct pci_dev *pdev = ndev->pdev;
> > + struct msix_entry *msix;
> > + int rc, i;
> > +
> > + if (msix_entries < SNB_MSIX_CNT)
>
> I would prefer ndev->limits.msix_cnt here to keep it generic. I could
> foresee this number changing on upcoming hardware with a similar
> design.
>
> > + return -ENOSPC;
> > +
> > + rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > + if (rc < 0)
> > + return rc;
> > + else if (rc > 0)
> > + return -ENOSPC;
> > +
> > + for (i = 0; i < msix_entries; i++) {
> > + msix = &ndev->msix_entries[i];
> > + WARN_ON(!msix->vector);
> > +
> > + if (i == msix_entries - 1) {
> > + rc = request_irq(msix->vector,
> > + xeon_event_msix_irq, 0,
> > + "ntb-event-msix", ndev);
> > + if (rc)
> > + goto err;
> > + } else {
> > + rc = request_irq(msix->vector,
> > + xeon_callback_msix_irq, 0,
> > + "ntb-callback-msix",
> > + &ndev->db_cb[i]);
> > + if (rc)
> > + goto err;
> > + }
> > + }
> > +
> > + ndev->num_msix = msix_entries;
> > + ndev->max_cbs = msix_entries - 1;
> > +
> > + return 0;
> > +
> > +err:
> > + while (--i >= 0) {
> > + msix = &ndev->msix_entries[i];
> > + WARN_ON(i == ndev->num_msix - 1);
> > +
> > + if (i == ndev->num_msix - 1)
>
> Why have a WARN followed by a if statement on the same condition? As
> it stands right now, there is no way it could be hit. So I'd say
> remove both of them.
Right, WARN_ON() is stupid here. But we can not remove the condition,
because of 'dev_id' parameter for free_irq() is 'ndev' for the last
MSI-X, unlike &ndev->db_cb[i] for all other MSI-Xs, unless I am
missing something.
>
> Aside from that it looks good. Thanks for taking the time to cleanup
> the code!
>
> Thanks,
> Jon
>
> > + free_irq(msix->vector, ndev);
> > + else
> > + free_irq(msix->vector, &ndev->db_cb[i]);
> > + }
> > +
> > + pci_disable_msix(pdev);
> > + ndev->num_msix = 0;
> > +
> > + return rc;
> > +}
> > +
> > +static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
> > +{
> > + struct pci_dev *pdev = ndev->pdev;
> > + struct msix_entry *msix;
> > + int rc, i;
> > +
> > +retry:
> > + rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > + if (rc < 0)
> > + return rc;
> > + else if (rc > 0) {
> > + dev_warn(&pdev->dev,
> > + "Only %d MSI-X vectors. "
> > + "Limiting the number of queues to that number.\n",
> > + rc);
> > + msix_entries = rc;
> > + goto retry;
> > + }
> > +
> > + for (i = 0; i < msix_entries; i++) {
> > + msix = &ndev->msix_entries[i];
> > + WARN_ON(!msix->vector);
> > +
> > + rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> > + "ntb-callback-msix", &ndev->db_cb[i]);
> > + if (rc)
> > + goto err;
> > + }
> > +
> > + ndev->num_msix = msix_entries;
> > + ndev->max_cbs = msix_entries;
> > +
> > + return 0;
> > +
> > +err:
> > + while (--i >= 0)
> > + free_irq(msix->vector, &ndev->db_cb[i]);
> > +
> > + pci_disable_msix(pdev);
> > + ndev->num_msix = 0;
> > +
> > + return rc;
> > +}
> > +
> > static int ntb_setup_msix(struct ntb_device *ndev)
> > {
> > struct pci_dev *pdev = ndev->pdev;
> > @@ -1105,78 +1206,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
> > for (i = 0; i < msix_entries; i++)
> > ndev->msix_entries[i].entry = i;
> >
> > - rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > - if (rc < 0)
> > - goto err1;
> > - if (rc > 0) {
> > - /* On SNB, the link interrupt is always tied to 4th vector. If
> > - * we can't get all 4, then we can't use MSI-X.
> > - */
> > - if (ndev->hw_type != BWD_HW) {
> > - rc = -EIO;
> > - goto err1;
> > - }
> > -
> > - dev_warn(&pdev->dev,
> > - "Only %d MSI-X vectors. Limiting the number of queues to that number.\n",
> > - rc);
> > - msix_entries = rc;
> > -
> > - rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > - if (rc)
> > - goto err1;
> > - }
> > -
> > - for (i = 0; i < msix_entries; i++) {
> > - msix = &ndev->msix_entries[i];
> > - WARN_ON(!msix->vector);
> > -
> > - /* Use the last MSI-X vector for Link status */
> > - if (ndev->hw_type == BWD_HW) {
> > - rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> > - "ntb-callback-msix", &ndev->db_cb[i]);
> > - if (rc)
> > - goto err2;
> > - } else {
> > - if (i == msix_entries - 1) {
> > - rc = request_irq(msix->vector,
> > - xeon_event_msix_irq, 0,
> > - "ntb-event-msix", ndev);
> > - if (rc)
> > - goto err2;
> > - } else {
> > - rc = request_irq(msix->vector,
> > - xeon_callback_msix_irq, 0,
> > - "ntb-callback-msix",
> > - &ndev->db_cb[i]);
> > - if (rc)
> > - goto err2;
> > - }
> > - }
> > - }
> > -
> > - ndev->num_msix = msix_entries;
> > if (ndev->hw_type == BWD_HW)
> > - ndev->max_cbs = msix_entries;
> > + rc = ntb_setup_bwd_msix(ndev, msix_entries);
> > else
> > - ndev->max_cbs = msix_entries - 1;
> > + rc = ntb_setup_snb_msix(ndev, msix_entries);
> > + if (rc)
> > + goto err1;
> >
> > return 0;
> >
> > -err2:
> > - while (--i >= 0) {
> > - msix = &ndev->msix_entries[i];
> > - if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
> > - free_irq(msix->vector, ndev);
> > - else
> > - free_irq(msix->vector, &ndev->db_cb[i]);
> > - }
> > - pci_disable_msix(pdev);
> > err1:
> > kfree(ndev->msix_entries);
> > - dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> > err:
> > - ndev->num_msix = 0;
> > + dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> > return rc;
> > }
> >
> > --
> > 1.7.7.6
> >
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines
2014-03-04 16:41 ` Alexander Gordeev
@ 2014-03-10 21:18 ` Jon Mason
0 siblings, 0 replies; 18+ messages in thread
From: Jon Mason @ 2014-03-10 21:18 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: linux-kernel, linux-pci
On Tue, Mar 04, 2014 at 05:41:28PM +0100, Alexander Gordeev wrote:
> On Mon, Mar 03, 2014 at 05:12:25PM -0700, Jon Mason wrote:
> > On Fri, Feb 21, 2014 at 04:49:31PM +0100, Alexander Gordeev wrote:
> > > This is an cleanup effort to make ntb_setup_msix() more
> > > readable - use ntb_setup_bwd_msix() to init MSI-Xs on
> > > BWD hardware and ntb_setup_snb_msix() - on SNB hardware.
> > >
> > > Function ntb_setup_snb_msix() also initializes MSI-Xs the
> > > way it should has been done - looping pci_enable_msix()
> > > until success or failure.
> > >
> > > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > > Cc: Jon Mason <jon.mason@intel.com>
> > > Cc: linux-pci@vger.kernel.org
> > > ---
> > > drivers/ntb/ntb_hw.c | 170 +++++++++++++++++++++++++++++++-------------------
> > > 1 files changed, 106 insertions(+), 64 deletions(-)
> > >
> > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > index 53468f4..0472045 100644
> > > --- a/drivers/ntb/ntb_hw.c
> > > +++ b/drivers/ntb/ntb_hw.c
> > > @@ -1079,6 +1079,107 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
> > > return IRQ_HANDLED;
> > > }
> > >
> > > +static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
> > > +{
> > > + struct pci_dev *pdev = ndev->pdev;
> > > + struct msix_entry *msix;
> > > + int rc, i;
> > > +
> > > + if (msix_entries < SNB_MSIX_CNT)
> >
> > I would prefer ndev->limits.msix_cnt here to keep it generic. I could
> > foresee this number changing on upcoming hardware with a similar
> > design.
> >
> > > + return -ENOSPC;
> > > +
> > > + rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > > + if (rc < 0)
> > > + return rc;
> > > + else if (rc > 0)
> > > + return -ENOSPC;
> > > +
> > > + for (i = 0; i < msix_entries; i++) {
> > > + msix = &ndev->msix_entries[i];
> > > + WARN_ON(!msix->vector);
> > > +
> > > + if (i == msix_entries - 1) {
> > > + rc = request_irq(msix->vector,
> > > + xeon_event_msix_irq, 0,
> > > + "ntb-event-msix", ndev);
> > > + if (rc)
> > > + goto err;
> > > + } else {
> > > + rc = request_irq(msix->vector,
> > > + xeon_callback_msix_irq, 0,
> > > + "ntb-callback-msix",
> > > + &ndev->db_cb[i]);
> > > + if (rc)
> > > + goto err;
> > > + }
> > > + }
> > > +
> > > + ndev->num_msix = msix_entries;
> > > + ndev->max_cbs = msix_entries - 1;
> > > +
> > > + return 0;
> > > +
> > > +err:
> > > + while (--i >= 0) {
> > > + msix = &ndev->msix_entries[i];
> > > + WARN_ON(i == ndev->num_msix - 1);
> > > +
> > > + if (i == ndev->num_msix - 1)
> >
> > Why have a WARN followed by a if statement on the same condition? As
> > it stands right now, there is no way it could be hit. So I'd say
> > remove both of them.
>
> Right, WARN_ON() is stupid here. But we can not remove the condition,
> because of 'dev_id' parameter for free_irq() is 'ndev' for the last
> MSI-X, unlike &ndev->db_cb[i] for all other MSI-Xs, unless I am
> missing something.
The "i == ndev->num_msix - 1" cannot be hit. If a failure hits in
request_irq with i < msix_entries - 1, then there is no need to free
the irq for xeon_event_msix_irq allocation because it was never
allocated. If i == msix_entries - 1 when it fails, there is no need
to free the irq because the allocation failed. These 2 cases being
true, it's never possible to hit the case and the clean-up code is
unnecessary.
Thanks,
Jon
>
> >
> > Aside from that it looks good. Thanks for taking the time to cleanup
> > the code!
> >
> > Thanks,
> > Jon
> >
> > > + free_irq(msix->vector, ndev);
> > > + else
> > > + free_irq(msix->vector, &ndev->db_cb[i]);
> > > + }
> > > +
> > > + pci_disable_msix(pdev);
> > > + ndev->num_msix = 0;
> > > +
> > > + return rc;
> > > +}
> > > +
> > > +static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
> > > +{
> > > + struct pci_dev *pdev = ndev->pdev;
> > > + struct msix_entry *msix;
> > > + int rc, i;
> > > +
> > > +retry:
> > > + rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > > + if (rc < 0)
> > > + return rc;
> > > + else if (rc > 0) {
> > > + dev_warn(&pdev->dev,
> > > + "Only %d MSI-X vectors. "
> > > + "Limiting the number of queues to that number.\n",
> > > + rc);
> > > + msix_entries = rc;
> > > + goto retry;
> > > + }
> > > +
> > > + for (i = 0; i < msix_entries; i++) {
> > > + msix = &ndev->msix_entries[i];
> > > + WARN_ON(!msix->vector);
> > > +
> > > + rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> > > + "ntb-callback-msix", &ndev->db_cb[i]);
> > > + if (rc)
> > > + goto err;
> > > + }
> > > +
> > > + ndev->num_msix = msix_entries;
> > > + ndev->max_cbs = msix_entries;
> > > +
> > > + return 0;
> > > +
> > > +err:
> > > + while (--i >= 0)
> > > + free_irq(msix->vector, &ndev->db_cb[i]);
> > > +
> > > + pci_disable_msix(pdev);
> > > + ndev->num_msix = 0;
> > > +
> > > + return rc;
> > > +}
> > > +
> > > static int ntb_setup_msix(struct ntb_device *ndev)
> > > {
> > > struct pci_dev *pdev = ndev->pdev;
> > > @@ -1105,78 +1206,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
> > > for (i = 0; i < msix_entries; i++)
> > > ndev->msix_entries[i].entry = i;
> > >
> > > - rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > > - if (rc < 0)
> > > - goto err1;
> > > - if (rc > 0) {
> > > - /* On SNB, the link interrupt is always tied to 4th vector. If
> > > - * we can't get all 4, then we can't use MSI-X.
> > > - */
> > > - if (ndev->hw_type != BWD_HW) {
> > > - rc = -EIO;
> > > - goto err1;
> > > - }
> > > -
> > > - dev_warn(&pdev->dev,
> > > - "Only %d MSI-X vectors. Limiting the number of queues to that number.\n",
> > > - rc);
> > > - msix_entries = rc;
> > > -
> > > - rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > > - if (rc)
> > > - goto err1;
> > > - }
> > > -
> > > - for (i = 0; i < msix_entries; i++) {
> > > - msix = &ndev->msix_entries[i];
> > > - WARN_ON(!msix->vector);
> > > -
> > > - /* Use the last MSI-X vector for Link status */
> > > - if (ndev->hw_type == BWD_HW) {
> > > - rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> > > - "ntb-callback-msix", &ndev->db_cb[i]);
> > > - if (rc)
> > > - goto err2;
> > > - } else {
> > > - if (i == msix_entries - 1) {
> > > - rc = request_irq(msix->vector,
> > > - xeon_event_msix_irq, 0,
> > > - "ntb-event-msix", ndev);
> > > - if (rc)
> > > - goto err2;
> > > - } else {
> > > - rc = request_irq(msix->vector,
> > > - xeon_callback_msix_irq, 0,
> > > - "ntb-callback-msix",
> > > - &ndev->db_cb[i]);
> > > - if (rc)
> > > - goto err2;
> > > - }
> > > - }
> > > - }
> > > -
> > > - ndev->num_msix = msix_entries;
> > > if (ndev->hw_type == BWD_HW)
> > > - ndev->max_cbs = msix_entries;
> > > + rc = ntb_setup_bwd_msix(ndev, msix_entries);
> > > else
> > > - ndev->max_cbs = msix_entries - 1;
> > > + rc = ntb_setup_snb_msix(ndev, msix_entries);
> > > + if (rc)
> > > + goto err1;
> > >
> > > return 0;
> > >
> > > -err2:
> > > - while (--i >= 0) {
> > > - msix = &ndev->msix_entries[i];
> > > - if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
> > > - free_irq(msix->vector, ndev);
> > > - else
> > > - free_irq(msix->vector, &ndev->db_cb[i]);
> > > - }
> > > - pci_disable_msix(pdev);
> > > err1:
> > > kfree(ndev->msix_entries);
> > > - dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> > > err:
> > > - ndev->num_msix = 0;
> > > + dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> > > return rc;
> > > }
> > >
> > > --
> > > 1.7.7.6
> > >
>
> --
> Regards,
> Alexander Gordeev
> agordeev@redhat.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines
2014-02-21 15:49 ` [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines Alexander Gordeev
2014-02-22 19:55 ` Alexander Gordeev
2014-03-04 0:12 ` Jon Mason
@ 2014-03-11 16:00 ` Alexander Gordeev
2014-03-11 17:14 ` Jon Mason
2 siblings, 1 reply; 18+ messages in thread
From: Alexander Gordeev @ 2014-03-11 16:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Jon Mason, linux-pci
This is an cleanup effort to make ntb_setup_msix() more
readable - use ntb_setup_bwd_msix() to init MSI-Xs on
BWD hardware and ntb_setup_snb_msix() - on SNB hardware.
Function ntb_setup_snb_msix() also initializes MSI-Xs the
way it should has been done - looping pci_enable_msix()
until success or failure.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Cc: Jon Mason <jon.mason@intel.com>
Cc: linux-pci@vger.kernel.org
---
drivers/ntb/ntb_hw.c | 167 ++++++++++++++++++++++++++++++-------------------
1 files changed, 102 insertions(+), 65 deletions(-)
diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 53468f4..1f8decd 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1079,10 +1079,106 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
return IRQ_HANDLED;
}
-static int ntb_setup_msix(struct ntb_device *ndev)
+static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
{
struct pci_dev *pdev = ndev->pdev;
struct msix_entry *msix;
+ int rc, i;
+
+ if (msix_entries < ndev->limits.msix_cnt)
+ return -ENOSPC;
+
+ rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+ if (rc < 0)
+ return rc;
+ else if (rc > 0)
+ return -ENOSPC;
+
+ for (i = 0; i < msix_entries; i++) {
+ msix = &ndev->msix_entries[i];
+ WARN_ON(!msix->vector);
+
+ if (i == msix_entries - 1) {
+ rc = request_irq(msix->vector,
+ xeon_event_msix_irq, 0,
+ "ntb-event-msix", ndev);
+ if (rc)
+ goto err;
+ } else {
+ rc = request_irq(msix->vector,
+ xeon_callback_msix_irq, 0,
+ "ntb-callback-msix",
+ &ndev->db_cb[i]);
+ if (rc)
+ goto err;
+ }
+ }
+
+ ndev->num_msix = msix_entries;
+ ndev->max_cbs = msix_entries - 1;
+
+ return 0;
+
+err:
+ while (--i >= 0) {
+ /* Code never reaches here for entry nr 'ndev->num_msix - 1' */
+ msix = &ndev->msix_entries[i];
+ free_irq(msix->vector, &ndev->db_cb[i]);
+ }
+
+ pci_disable_msix(pdev);
+ ndev->num_msix = 0;
+
+ return rc;
+}
+
+static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
+{
+ struct pci_dev *pdev = ndev->pdev;
+ struct msix_entry *msix;
+ int rc, i;
+
+retry:
+ rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+ if (rc < 0)
+ return rc;
+ else if (rc > 0) {
+ dev_warn(&pdev->dev,
+ "Only %d MSI-X vectors. "
+ "Limiting the number of queues to that number.\n",
+ rc);
+ msix_entries = rc;
+ goto retry;
+ }
+
+ for (i = 0; i < msix_entries; i++) {
+ msix = &ndev->msix_entries[i];
+ WARN_ON(!msix->vector);
+
+ rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
+ "ntb-callback-msix", &ndev->db_cb[i]);
+ if (rc)
+ goto err;
+ }
+
+ ndev->num_msix = msix_entries;
+ ndev->max_cbs = msix_entries;
+
+ return 0;
+
+err:
+ while (--i >= 0)
+ free_irq(msix->vector, &ndev->db_cb[i]);
+
+ pci_disable_msix(pdev);
+ ndev->num_msix = 0;
+
+ return rc;
+}
+
+static int ntb_setup_msix(struct ntb_device *ndev)
+{
+ struct pci_dev *pdev = ndev->pdev;
int msix_entries;
int rc, i;
@@ -1105,78 +1201,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
for (i = 0; i < msix_entries; i++)
ndev->msix_entries[i].entry = i;
- rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
- if (rc < 0)
- goto err1;
- if (rc > 0) {
- /* On SNB, the link interrupt is always tied to 4th vector. If
- * we can't get all 4, then we can't use MSI-X.
- */
- if (ndev->hw_type != BWD_HW) {
- rc = -EIO;
- goto err1;
- }
-
- dev_warn(&pdev->dev,
- "Only %d MSI-X vectors. Limiting the number of queues to that number.\n",
- rc);
- msix_entries = rc;
-
- rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
- if (rc)
- goto err1;
- }
-
- for (i = 0; i < msix_entries; i++) {
- msix = &ndev->msix_entries[i];
- WARN_ON(!msix->vector);
-
- /* Use the last MSI-X vector for Link status */
- if (ndev->hw_type == BWD_HW) {
- rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
- "ntb-callback-msix", &ndev->db_cb[i]);
- if (rc)
- goto err2;
- } else {
- if (i == msix_entries - 1) {
- rc = request_irq(msix->vector,
- xeon_event_msix_irq, 0,
- "ntb-event-msix", ndev);
- if (rc)
- goto err2;
- } else {
- rc = request_irq(msix->vector,
- xeon_callback_msix_irq, 0,
- "ntb-callback-msix",
- &ndev->db_cb[i]);
- if (rc)
- goto err2;
- }
- }
- }
-
- ndev->num_msix = msix_entries;
if (ndev->hw_type == BWD_HW)
- ndev->max_cbs = msix_entries;
+ rc = ntb_setup_bwd_msix(ndev, msix_entries);
else
- ndev->max_cbs = msix_entries - 1;
+ rc = ntb_setup_snb_msix(ndev, msix_entries);
+ if (rc)
+ goto err1;
return 0;
-err2:
- while (--i >= 0) {
- msix = &ndev->msix_entries[i];
- if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
- free_irq(msix->vector, ndev);
- else
- free_irq(msix->vector, &ndev->db_cb[i]);
- }
- pci_disable_msix(pdev);
err1:
kfree(ndev->msix_entries);
- dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
err:
- ndev->num_msix = 0;
+ dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
return rc;
}
--
1.7.7.6
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines
2014-03-11 16:00 ` Alexander Gordeev
@ 2014-03-11 17:14 ` Jon Mason
0 siblings, 0 replies; 18+ messages in thread
From: Jon Mason @ 2014-03-11 17:14 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: linux-kernel, linux-pci
On Tue, Mar 11, 2014 at 05:00:22PM +0100, Alexander Gordeev wrote:
> This is an cleanup effort to make ntb_setup_msix() more
> readable - use ntb_setup_bwd_msix() to init MSI-Xs on
> BWD hardware and ntb_setup_snb_msix() - on SNB hardware.
>
> Function ntb_setup_snb_msix() also initializes MSI-Xs the
> way it should has been done - looping pci_enable_msix()
> until success or failure.
>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> Cc: Jon Mason <jon.mason@intel.com>
Looks good. I will include it in my next release of the NTB driver
(e.g. 3.15).
Thanks,
Jon
> Cc: linux-pci@vger.kernel.org
> ---
> drivers/ntb/ntb_hw.c | 167 ++++++++++++++++++++++++++++++-------------------
> 1 files changed, 102 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 53468f4..1f8decd 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1079,10 +1079,106 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
> return IRQ_HANDLED;
> }
>
> -static int ntb_setup_msix(struct ntb_device *ndev)
> +static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
> {
> struct pci_dev *pdev = ndev->pdev;
> struct msix_entry *msix;
> + int rc, i;
> +
> + if (msix_entries < ndev->limits.msix_cnt)
> + return -ENOSPC;
> +
> + rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> + if (rc < 0)
> + return rc;
> + else if (rc > 0)
> + return -ENOSPC;
> +
> + for (i = 0; i < msix_entries; i++) {
> + msix = &ndev->msix_entries[i];
> + WARN_ON(!msix->vector);
> +
> + if (i == msix_entries - 1) {
> + rc = request_irq(msix->vector,
> + xeon_event_msix_irq, 0,
> + "ntb-event-msix", ndev);
> + if (rc)
> + goto err;
> + } else {
> + rc = request_irq(msix->vector,
> + xeon_callback_msix_irq, 0,
> + "ntb-callback-msix",
> + &ndev->db_cb[i]);
> + if (rc)
> + goto err;
> + }
> + }
> +
> + ndev->num_msix = msix_entries;
> + ndev->max_cbs = msix_entries - 1;
> +
> + return 0;
> +
> +err:
> + while (--i >= 0) {
> + /* Code never reaches here for entry nr 'ndev->num_msix - 1' */
> + msix = &ndev->msix_entries[i];
> + free_irq(msix->vector, &ndev->db_cb[i]);
> + }
> +
> + pci_disable_msix(pdev);
> + ndev->num_msix = 0;
> +
> + return rc;
> +}
> +
> +static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
> +{
> + struct pci_dev *pdev = ndev->pdev;
> + struct msix_entry *msix;
> + int rc, i;
> +
> +retry:
> + rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> + if (rc < 0)
> + return rc;
> + else if (rc > 0) {
> + dev_warn(&pdev->dev,
> + "Only %d MSI-X vectors. "
> + "Limiting the number of queues to that number.\n",
> + rc);
> + msix_entries = rc;
> + goto retry;
> + }
> +
> + for (i = 0; i < msix_entries; i++) {
> + msix = &ndev->msix_entries[i];
> + WARN_ON(!msix->vector);
> +
> + rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> + "ntb-callback-msix", &ndev->db_cb[i]);
> + if (rc)
> + goto err;
> + }
> +
> + ndev->num_msix = msix_entries;
> + ndev->max_cbs = msix_entries;
> +
> + return 0;
> +
> +err:
> + while (--i >= 0)
> + free_irq(msix->vector, &ndev->db_cb[i]);
> +
> + pci_disable_msix(pdev);
> + ndev->num_msix = 0;
> +
> + return rc;
> +}
> +
> +static int ntb_setup_msix(struct ntb_device *ndev)
> +{
> + struct pci_dev *pdev = ndev->pdev;
> int msix_entries;
> int rc, i;
>
> @@ -1105,78 +1201,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
> for (i = 0; i < msix_entries; i++)
> ndev->msix_entries[i].entry = i;
>
> - rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> - if (rc < 0)
> - goto err1;
> - if (rc > 0) {
> - /* On SNB, the link interrupt is always tied to 4th vector. If
> - * we can't get all 4, then we can't use MSI-X.
> - */
> - if (ndev->hw_type != BWD_HW) {
> - rc = -EIO;
> - goto err1;
> - }
> -
> - dev_warn(&pdev->dev,
> - "Only %d MSI-X vectors. Limiting the number of queues to that number.\n",
> - rc);
> - msix_entries = rc;
> -
> - rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> - if (rc)
> - goto err1;
> - }
> -
> - for (i = 0; i < msix_entries; i++) {
> - msix = &ndev->msix_entries[i];
> - WARN_ON(!msix->vector);
> -
> - /* Use the last MSI-X vector for Link status */
> - if (ndev->hw_type == BWD_HW) {
> - rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> - "ntb-callback-msix", &ndev->db_cb[i]);
> - if (rc)
> - goto err2;
> - } else {
> - if (i == msix_entries - 1) {
> - rc = request_irq(msix->vector,
> - xeon_event_msix_irq, 0,
> - "ntb-event-msix", ndev);
> - if (rc)
> - goto err2;
> - } else {
> - rc = request_irq(msix->vector,
> - xeon_callback_msix_irq, 0,
> - "ntb-callback-msix",
> - &ndev->db_cb[i]);
> - if (rc)
> - goto err2;
> - }
> - }
> - }
> -
> - ndev->num_msix = msix_entries;
> if (ndev->hw_type == BWD_HW)
> - ndev->max_cbs = msix_entries;
> + rc = ntb_setup_bwd_msix(ndev, msix_entries);
> else
> - ndev->max_cbs = msix_entries - 1;
> + rc = ntb_setup_snb_msix(ndev, msix_entries);
> + if (rc)
> + goto err1;
>
> return 0;
>
> -err2:
> - while (--i >= 0) {
> - msix = &ndev->msix_entries[i];
> - if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
> - free_irq(msix->vector, ndev);
> - else
> - free_irq(msix->vector, &ndev->db_cb[i]);
> - }
> - pci_disable_msix(pdev);
> err1:
> kfree(ndev->msix_entries);
> - dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> err:
> - ndev->num_msix = 0;
> + dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> return rc;
> }
>
> --
> 1.7.7.6
>
> --
> Regards,
> Alexander Gordeev
> agordeev@redhat.com
^ permalink raw reply [flat|nested] 18+ messages in thread