Discussion:
[PATCH RFC 11/77] benet: Return -ENOSPC when not enough MSI-Xs available
Alexander Gordeev
2013-10-02 10:48:27 UTC
Permalink
Signed-off-by: Alexander Gordeev <***@redhat.com>
---
drivers/net/ethernet/emulex/benet/be_main.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 100b528..3e2c834 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -2378,6 +2378,8 @@ static int be_msix_enable(struct be_adapter *adapter)
num_vec);
if (!status)
goto done;
+ } else (status > 0) {
+ status = -ENOSPC;
}

dev_warn(dev, "MSIx enable failed\n");
--
1.7.7.6
Alexander Gordeev
2013-10-02 10:49:14 UTC
Permalink
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.

Signed-off-by: Alexander Gordeev <***@redhat.com>
---
drivers/infiniband/hw/qib/qib_pcie.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c
index 3f14009..9580903 100644
--- a/drivers/infiniband/hw/qib/qib_pcie.c
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -218,10 +218,6 @@ static void qib_msix_setup(struct qib_devdata *dd, int pos, u32 *msixcnt,
if (tabsize > *msixcnt)
tabsize = *msixcnt;
ret = pci_enable_msix(dd->pcidev, msix_entry, tabsize);
- if (ret > 0) {
- tabsize = ret;
- ret = pci_enable_msix(dd->pcidev, msix_entry, tabsize);
- }
do_intx:
if (ret) {
qib_dev_err(dd,
--
1.7.7.6
Alexander Gordeev
2013-10-02 10:48:50 UTC
Permalink
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.

Signed-off-by: Alexander Gordeev <***@redhat.com>
---
drivers/dma/ioat/dma.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c
index acee3b2..b352ee6 100644
--- a/drivers/dma/ioat/dma.c
+++ b/drivers/dma/ioat/dma.c
@@ -915,15 +915,19 @@ int ioat_dma_setup_interrupts(struct ioatdma_device *device)

msix:
/* The number of MSI-X vectors should equal the number of channels */
+ err = pci_msix_table_size(pdev);
+ if (err < 0)
+ goto msi;
+ if (err < device->common.chancnt)
+ goto msix_single_vector;
+
msixcnt = device->common.chancnt;
for (i = 0; i < msixcnt; i++)
device->msix_entries[i].entry = i;

err = pci_enable_msix(pdev, device->msix_entries, msixcnt);
- if (err < 0)
+ if (err)
goto msi;
- if (err > 0)
- goto msix_single_vector;

for (i = 0; i < msixcnt; i++) {
msix = &device->msix_entries[i];
--
1.7.7.6
Williams, Dan J
2013-10-02 17:31:09 UTC
Permalink
[ dropping all cc's except lkml ]
Post by Alexander Gordeev
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.
What happens if this patch is not applied. This changelog does not
have enough information for me to judge the patch. Unless I missed it
I do not see a patch 00/77 explaining the background of the change,
and a quick search did not turn up a discussion of why drivers now
need to call pci_msix_table_size() prior to pci_enable_msix.
Post by Alexander Gordeev
---
Not sure why everybody needed to be cc'd on this one patch. Please
put targeted cc's in the patches themselves. If anything only patch
00/77 should go to the union of all addresses.

--
Dan
Williams, Dan J
2013-10-02 18:00:51 UTC
Permalink
On Wed, Oct 2, 2013 at 10:31 AM, Williams, Dan J
Post by Williams, Dan J
[ dropping all cc's except lkml ]
Post by Alexander Gordeev
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.
What happens if this patch is not applied. This changelog does not
have enough information for me to judge the patch. Unless I missed it
I do not see a patch 00/77 explaining the background of the change,
and a quick search did not turn up a discussion of why drivers now
need to call pci_msix_table_size() prior to pci_enable_msix.
I see patch 00/77 now and I still don't see the need for ioatdma to be
updated. If we fail the subsequent request_irq, the driver will still
fall back to trying less interrupts.
Alexander Gordeev
2013-10-03 20:12:05 UTC
Permalink
Post by Williams, Dan J
I see patch 00/77 now and I still don't see the need for ioatdma to be
updated. If we fail the subsequent request_irq, the driver will still
fall back to trying less interrupts.
In the proposed design pci_enable_msix() will never return a positive value.
Therefore if the driver is not updated it will fallback to single MSI if
device->common.chancnt MSI-X vectors were not allocated while it used to
fallback to a single MSI-X in the same situation. Not to mention 'if (err > 0)'
dead code.
--
Regards,
Alexander Gordeev
***@redhat.com
Dan Williams
2013-10-03 21:21:12 UTC
Permalink
Post by Alexander Gordeev
Post by Williams, Dan J
I see patch 00/77 now and I still don't see the need for ioatdma to be
updated. If we fail the subsequent request_irq, the driver will still
fall back to trying less interrupts.
In the proposed design pci_enable_msix() will never return a positive value.
Therefore if the driver is not updated it will fallback to single MSI if
device->common.chancnt MSI-X vectors were not allocated while it used to
fallback to a single MSI-X in the same situation.
That's fine. I just don't like how this patch makes the driver more
complex. If we're making things simpler lets not expose msix internal
details and just fallback to msi directly without worrying about the
number of vectors. I think the msix_single_vector intermediate step
should go.

...and now that I look I see a bug in ioat3_irq_reinit.
Post by Alexander Gordeev
Not to mention 'if (err > 0)'
dead code.
Ok, then just this piece for now, but more preferable to fold it into
a msix_single_vector removal patch.

- if (err < 0)
+ if (err)
goto msi;
- if (err > 0)
- goto msix_single_vector;

--
Dan
Alexander Gordeev
2013-10-02 10:49:08 UTC
Permalink
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.

Signed-off-by: Alexander Gordeev <***@redhat.com>
---
drivers/net/ethernet/sun/niu.c | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
index f28460c..54c41b9 100644
--- a/drivers/net/ethernet/sun/niu.c
+++ b/drivers/net/ethernet/sun/niu.c
@@ -9051,26 +9051,28 @@ static void niu_try_msix(struct niu *np, u8 *ldg_num_map)
(np->port == 0 ? 3 : 1));
BUG_ON(num_irqs > (NIU_NUM_LDG / parent->num_ports));

-retry:
+ err = pci_msix_table_size(pdev);
+ if (err < 0)
+ goto fail;
+
+ num_irqs = min(num_irqs, err);
for (i = 0; i < num_irqs; i++) {
msi_vec[i].vector = 0;
msi_vec[i].entry = i;
}

err = pci_enable_msix(pdev, msi_vec, num_irqs);
- if (err < 0) {
- np->flags &= ~NIU_FLAGS_MSIX;
- return;
- }
- if (err > 0) {
- num_irqs = err;
- goto retry;
- }
+ if (err)
+ goto fail;

np->flags |= NIU_FLAGS_MSIX;
for (i = 0; i < num_irqs; i++)
np->ldg[i].irq = msi_vec[i].vector;
np->num_ldg = num_irqs;
+ return;
+
+fail:
+ np->flags &= ~NIU_FLAGS_MSIX;
}

static int niu_n2_irq_init(struct niu *np, u8 *ldg_num_map)
--
1.7.7.6
Alexander Gordeev
2013-10-02 10:49:07 UTC
Permalink
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.

Signed-off-by: Alexander Gordeev <***@redhat.com>
---
drivers/infiniband/hw/mthca/mthca_main.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_main.c b/drivers/infiniband/hw/mthca/mthca_main.c
index 87897b9..3d44ca4 100644
--- a/drivers/infiniband/hw/mthca/mthca_main.c
+++ b/drivers/infiniband/hw/mthca/mthca_main.c
@@ -854,17 +854,23 @@ static int mthca_enable_msi_x(struct mthca_dev *mdev)
struct msix_entry entries[3];
int err;

+ err = pci_msix_table_size(mdev->pdev);
+ if (err < 0)
+ return err;
+ if (err < ARRAY_SIZE(entries)) {
+ mthca_info(mdev, "Only %d MSI-X vectors available, "
+ "not using MSI-X\n", err);
+
+ return -ENOSPC;
+ }
+
entries[0].entry = 0;
entries[1].entry = 1;
entries[2].entry = 2;

err = pci_enable_msix(mdev->pdev, entries, ARRAY_SIZE(entries));
- if (err) {
- if (err > 0)
- mthca_info(mdev, "Only %d MSI-X vectors available, "
- "not using MSI-X\n", err);
+ if (err)
return err;
- }

mdev->eq_table.eq[MTHCA_EQ_COMP ].msi_x_vector = entries[0].vector;
mdev->eq_table.eq[MTHCA_EQ_ASYNC].msi_x_vector = entries[1].vector;
--
1.7.7.6
Jack Morgenstein
2013-10-03 16:11:00 UTC
Permalink
On Wed, 2 Oct 2013 12:49:07 +0200
Subject: [PATCH RFC 51/77] mthca: Update MSI/MSI-X interrupts
enablement code Date: Wed, 2 Oct 2013 12:49:07 +0200
X-Mailer: git-send-email 1.7.7.6
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.
---
ACK.

-Jack
Alexander Gordeev
2013-10-02 10:49:26 UTC
Permalink
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.

Signed-off-by: Alexander Gordeev <***@redhat.com>
---
drivers/misc/vmw_vmci/vmci_guest.c | 22 +++++++++++++++-------
1 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_guest.c b/drivers/misc/vmw_vmci/vmci_guest.c
index b3a2b76..af5caf8 100644
--- a/drivers/misc/vmw_vmci/vmci_guest.c
+++ b/drivers/misc/vmw_vmci/vmci_guest.c
@@ -377,19 +377,27 @@ static int vmci_enable_msix(struct pci_dev *pdev,
{
int i;
int result;
+ int nvec;

- for (i = 0; i < VMCI_MAX_INTRS; ++i) {
+ result = pci_msix_table_size(pdev);
+ if (result < 0)
+ return result;
+
+ nvec = min(result, VMCI_MAX_INTRS);
+ if (nvec < VMCI_MAX_INTRS)
+ nvec = 1;
+
+ for (i = 0; i < nvec; ++i) {
vmci_dev->msix_entries[i].entry = i;
vmci_dev->msix_entries[i].vector = i;
}

- result = pci_enable_msix(pdev, vmci_dev->msix_entries, VMCI_MAX_INTRS);
- if (result == 0)
- vmci_dev->exclusive_vectors = true;
- else if (result > 0)
- result = pci_enable_msix(pdev, vmci_dev->msix_entries, 1);
+ result = pci_enable_msix(pdev, vmci_dev->msix_entries, nvec);
+ if (result)
+ return result;

- return result;
+ vmci_dev->exclusive_vectors = true;
+ return 0;
}

/*
--
1.7.7.6
Alexander Gordeev
2013-10-02 10:49:15 UTC
Permalink
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.

Signed-off-by: Alexander Gordeev <***@redhat.com>
---
drivers/scsi/qla2xxx/qla_isr.c | 18 +++++++++++-------
1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index df1b30b..6c11ab9 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -2836,16 +2836,20 @@ qla24xx_enable_msix(struct qla_hw_data *ha, struct rsp_que *rsp)
for (i = 0; i < ha->msix_count; i++)
entries[i].entry = i;

- ret = pci_enable_msix(ha->pdev, entries, ha->msix_count);
- if (ret) {
+ ret = pci_msix_table_size(ha->pdev);
+ if (ret < 0) {
+ goto msix_failed;
+ } else {
if (ret < MIN_MSIX_COUNT)
goto msix_failed;

- ql_log(ql_log_warn, vha, 0x00c6,
- "MSI-X: Failed to enable support "
- "-- %d/%d\n Retry with %d vectors.\n",
- ha->msix_count, ret, ret);
- ha->msix_count = ret;
+ if (ret < ha->msix_count) {
+ ql_log(ql_log_warn, vha, 0x00c6,
+ "MSI-X: Failed to enable support "
+ "-- %d/%d\n Retry with %d vectors.\n",
+ ha->msix_count, ret, ret);
+ ha->msix_count = ret;
+ }
ret = pci_enable_msix(ha->pdev, entries, ha->msix_count);
if (ret) {
msix_failed:
--
1.7.7.6
Saurav Kashyap
2013-10-03 17:42:33 UTC
Permalink
Post by Alexander Gordeev
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.
---
drivers/scsi/qla2xxx/qla_isr.c | 18 +++++++++++-------
1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_isr.c
b/drivers/scsi/qla2xxx/qla_isr.c
index df1b30b..6c11ab9 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -2836,16 +2836,20 @@ qla24xx_enable_msix(struct qla_hw_data *ha, struct rsp_que *rsp)
for (i = 0; i < ha->msix_count; i++)
entries[i].entry = i;
- ret = pci_enable_msix(ha->pdev, entries, ha->msix_count);
- if (ret) {
+ ret = pci_msix_table_size(ha->pdev);
+ if (ret < 0) {
+ goto msix_failed;
+ } else {
if (ret < MIN_MSIX_COUNT)
goto msix_failed;
- ql_log(ql_log_warn, vha, 0x00c6,
- "MSI-X: Failed to enable support "
- "-- %d/%d\n Retry with %d vectors.\n",
- ha->msix_count, ret, ret);
- ha->msix_count = ret;
+ if (ret < ha->msix_count) {
+ ql_log(ql_log_warn, vha, 0x00c6,
+ "MSI-X: Failed to enable support "
+ "-- %d/%d\n Retry with %d vectors.\n",
+ ha->msix_count, ret, ret);
+ ha->msix_count = ret;
+ }
ret = pci_enable_msix(ha->pdev, entries, ha->msix_count);
if (ret) {
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Alexander Gordeev
2013-10-02 10:49:01 UTC
Permalink
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.

Signed-off-by: Alexander Gordeev <***@redhat.com>
---
drivers/scsi/megaraid/megaraid_sas_base.c | 20 +++++++++-----------
1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 3020921..b5973e4 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3727,18 +3727,16 @@ static int megasas_init_fw(struct megasas_instance *instance)
(unsigned int)num_online_cpus());
for (i = 0; i < instance->msix_vectors; i++)
instance->msixentry[i].entry = i;
- i = pci_enable_msix(instance->pdev, instance->msixentry,
- instance->msix_vectors);
- if (i >= 0) {
- if (i) {
- if (!pci_enable_msix(instance->pdev,
- instance->msixentry, i))
- instance->msix_vectors = i;
- else
- instance->msix_vectors = 0;
- }
- } else
+ i = pci_msix_table_size(instance->pdev);
+ if (i < 0) {
instance->msix_vectors = 0;
+ } else {
+ if (!pci_enable_msix(instance->pdev,
+ instance->msixentry, i))
+ instance->msix_vectors = i;
+ else
+ instance->msix_vectors = 0;
+ }

dev_info(&instance->pdev->dev, "[scsi%d]: FW supports"
"<%d> MSIX vector,Online CPUs: <%d>,"
--
1.7.7.6
Alexander Gordeev
2013-10-02 10:48:31 UTC
Permalink
As result of recent re-design of the MSI/MSI-X interrupts enabling
pattern this driver has to be updated to use the new technique to
obtain a optimal number of MSI/MSI-X interrupts required.

Signed-off-by: Alexander Gordeev <***@redhat.com>
---
drivers/net/ethernet/broadcom/bnx2.c | 27 ++++++++++++++-------------
1 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
index e838a3f..c902627 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -6202,25 +6202,26 @@ bnx2_enable_msix(struct bnx2 *bp, int msix_vecs)
* is setup properly */
BNX2_RD(bp, BNX2_PCI_MSIX_CONTROL);

- for (i = 0; i < BNX2_MAX_MSIX_VEC; i++) {
- msix_ent[i].entry = i;
- msix_ent[i].vector = 0;
- }
-
total_vecs = msix_vecs;
#ifdef BCM_CNIC
total_vecs++;
#endif
- rc = -ENOSPC;
- while (total_vecs >= BNX2_MIN_MSIX_VEC) {
- rc = pci_enable_msix(bp->pdev, msix_ent, total_vecs);
- if (rc <= 0)
- break;
- if (rc > 0)
- total_vecs = rc;
+ rc = pci_msix_table_size(bp->pdev);
+ if (rc < 0)
+ return;
+
+ total_vecs = min(total_vecs, rc);
+ if (total_vecs < BNX2_MIN_MSIX_VEC)
+ return;
+
+ BUG_ON(total_vecs > ARRAY_SIZE(msix_ent));
+ for (i = 0; i < total_vecs; i++) {
+ msix_ent[i].entry = i;
+ msix_ent[i].vector = 0;
}

- if (rc != 0)
+ rc = pci_enable_msix(bp->pdev, msix_ent, total_vecs);
+ if (rc)
return;

msix_vecs = total_vecs;
--
1.7.7.6