* [PATCH] csiostor: switch to pci_alloc_irq_vectors
@ 2017-01-13 16:30 Christoph Hellwig
2017-01-21 0:27 ` Martin K. Petersen
0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2017-01-13 16:30 UTC (permalink / raw)
To: linux-scsi; +Cc: hariprasad, praveenm
And get automatic MSI-X affinity for free.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/csiostor/csio_hw.h | 4 +-
drivers/scsi/csiostor/csio_init.c | 9 +--
drivers/scsi/csiostor/csio_isr.c | 127 +++++++++++++-------------------------
3 files changed, 51 insertions(+), 89 deletions(-)
diff --git a/drivers/scsi/csiostor/csio_hw.h b/drivers/scsi/csiostor/csio_hw.h
index 029bef8..a084d83 100644
--- a/drivers/scsi/csiostor/csio_hw.h
+++ b/drivers/scsi/csiostor/csio_hw.h
@@ -95,7 +95,6 @@ enum {
};
struct csio_msix_entries {
- unsigned short vector; /* Assigned MSI-X vector */
void *dev_id; /* Priv object associated w/ this msix*/
char desc[24]; /* Description of this vector */
};
@@ -591,8 +590,9 @@ int csio_enqueue_evt(struct csio_hw *, enum csio_evt, void *, uint16_t);
void csio_evtq_flush(struct csio_hw *hw);
int csio_request_irqs(struct csio_hw *);
+void csio_free_irqs(struct csio_hw *);
void csio_intr_enable(struct csio_hw *);
-void csio_intr_disable(struct csio_hw *, bool);
+void csio_intr_disable(struct csio_hw *);
void csio_hw_fatal_err(struct csio_hw *);
struct csio_lnode *csio_lnode_alloc(struct csio_hw *);
diff --git a/drivers/scsi/csiostor/csio_init.c b/drivers/scsi/csiostor/csio_init.c
index dbe416f..7e60699 100644
--- a/drivers/scsi/csiostor/csio_init.c
+++ b/drivers/scsi/csiostor/csio_init.c
@@ -461,8 +461,7 @@ csio_config_queues(struct csio_hw *hw)
return 0;
intr_disable:
- csio_intr_disable(hw, false);
-
+ csio_intr_disable(hw);
return -EINVAL;
}
@@ -575,7 +574,8 @@ static struct csio_hw *csio_hw_alloc(struct pci_dev *pdev)
static void
csio_hw_free(struct csio_hw *hw)
{
- csio_intr_disable(hw, true);
+ csio_free_irqs(hw);
+ csio_intr_disable(hw);
csio_hw_exit_workers(hw);
csio_hw_exit(hw);
iounmap(hw->regstart);
@@ -1068,7 +1068,8 @@ csio_pci_error_detected(struct pci_dev *pdev, pci_channel_state_t state)
spin_unlock_irq(&hw->lock);
csio_lnodes_unblock_request(hw);
csio_lnodes_exit(hw, 0);
- csio_intr_disable(hw, true);
+ csio_free_irqs(hw);
+ csio_intr_disable(hw);
pci_disable_device(pdev);
return state == pci_channel_io_perm_failure ?
PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET;
diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
index 2fb71c6..a5bf51b7 100644
--- a/drivers/scsi/csiostor/csio_isr.c
+++ b/drivers/scsi/csiostor/csio_isr.c
@@ -383,17 +383,15 @@ csio_request_irqs(struct csio_hw *hw)
int rv, i, j, k = 0;
struct csio_msix_entries *entryp = &hw->msix_entries[0];
struct csio_scsi_cpu_info *info;
+ struct pci_dev *pdev = hw->pdev;
if (hw->intr_mode != CSIO_IM_MSIX) {
- rv = request_irq(hw->pdev->irq, csio_fcoe_isr,
- (hw->intr_mode == CSIO_IM_MSI) ?
- 0 : IRQF_SHARED,
- KBUILD_MODNAME, hw);
+ rv = request_irq(pci_irq_vector(pdev, 0), csio_fcoe_isr,
+ hw->intr_mode == CSIO_IM_MSI ? 0 : IRQF_SHARED,
+ KBUILD_MODNAME, hw);
if (rv) {
- if (hw->intr_mode == CSIO_IM_MSI)
- pci_disable_msi(hw->pdev);
csio_err(hw, "Failed to allocate interrupt line.\n");
- return -EINVAL;
+ goto out_free_irqs;
}
goto out;
@@ -402,22 +400,22 @@ csio_request_irqs(struct csio_hw *hw)
/* Add the MSIX vector descriptions */
csio_add_msix_desc(hw);
- rv = request_irq(entryp[k].vector, csio_nondata_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_nondata_isr, 0,
entryp[k].desc, hw);
if (rv) {
csio_err(hw, "IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
- entryp[k++].dev_id = (void *)hw;
+ entryp[k++].dev_id = hw;
- rv = request_irq(entryp[k].vector, csio_fwevt_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_fwevt_isr, 0,
entryp[k].desc, hw);
if (rv) {
csio_err(hw, "IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
entryp[k++].dev_id = (void *)hw;
@@ -429,51 +427,31 @@ csio_request_irqs(struct csio_hw *hw)
struct csio_scsi_qset *sqset = &hw->sqset[i][j];
struct csio_q *q = hw->wrm.q_arr[sqset->iq_idx];
- rv = request_irq(entryp[k].vector, csio_scsi_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_scsi_isr, 0,
entryp[k].desc, q);
if (rv) {
csio_err(hw,
"IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
- entryp[k].dev_id = (void *)q;
+ entryp[k].dev_id = q;
} /* for all scsi cpus */
} /* for all ports */
out:
hw->flags |= CSIO_HWF_HOST_INTR_ENABLED;
-
return 0;
-err:
- for (i = 0; i < k; i++) {
- entryp = &hw->msix_entries[i];
- free_irq(entryp->vector, entryp->dev_id);
- }
- pci_disable_msix(hw->pdev);
-
+out_free_irqs:
+ for (i = 0; i < k; i++)
+ free_irq(pci_irq_vector(pdev, i), hw->msix_entries[i].dev_id);
+ pci_free_irq_vectors(hw->pdev);
return -EINVAL;
}
-static void
-csio_disable_msix(struct csio_hw *hw, bool free)
-{
- int i;
- struct csio_msix_entries *entryp;
- int cnt = hw->num_sqsets + CSIO_EXTRA_VECS;
-
- if (free) {
- for (i = 0; i < cnt; i++) {
- entryp = &hw->msix_entries[i];
- free_irq(entryp->vector, entryp->dev_id);
- }
- }
- pci_disable_msix(hw->pdev);
-}
-
/* Reduce per-port max possible CPUs */
static void
csio_reduce_sqsets(struct csio_hw *hw, int cnt)
@@ -500,10 +478,9 @@ static int
csio_enable_msix(struct csio_hw *hw)
{
int i, j, k, n, min, cnt;
- struct csio_msix_entries *entryp;
- struct msix_entry *entries;
int extra = CSIO_EXTRA_VECS;
struct csio_scsi_cpu_info *info;
+ struct irq_affinity desc = { .pre_vectors = 2 };
min = hw->num_pports + extra;
cnt = hw->num_sqsets + extra;
@@ -512,50 +489,35 @@ csio_enable_msix(struct csio_hw *hw)
if (hw->flags & CSIO_HWF_USING_SOFT_PARAMS || !csio_is_hw_master(hw))
cnt = min_t(uint8_t, hw->cfg_niq, cnt);
- entries = kzalloc(sizeof(struct msix_entry) * cnt, GFP_KERNEL);
- if (!entries)
- return -ENOMEM;
-
- for (i = 0; i < cnt; i++)
- entries[i].entry = (uint16_t)i;
-
csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
- cnt = pci_enable_msix_range(hw->pdev, entries, min, cnt);
- if (cnt < 0) {
- kfree(entries);
+ cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt, PCI_IRQ_MSIX,
+ &desc);
+ if (cnt < 0)
return cnt;
- }
if (cnt < (hw->num_sqsets + extra)) {
csio_dbg(hw, "Reducing sqsets to %d\n", cnt - extra);
csio_reduce_sqsets(hw, cnt - extra);
}
- /* Save off vectors */
- for (i = 0; i < cnt; i++) {
- entryp = &hw->msix_entries[i];
- entryp->vector = entries[i].vector;
- }
-
/* Distribute vectors */
k = 0;
- csio_set_nondata_intr_idx(hw, entries[k].entry);
- csio_set_mb_intr_idx(csio_hw_to_mbm(hw), entries[k++].entry);
- csio_set_fwevt_intr_idx(hw, entries[k++].entry);
+ csio_set_nondata_intr_idx(hw, k);
+ csio_set_mb_intr_idx(csio_hw_to_mbm(hw), k++);
+ csio_set_fwevt_intr_idx(hw, k++);
for (i = 0; i < hw->num_pports; i++) {
info = &hw->scsi_cpu_info[i];
for (j = 0; j < hw->num_scsi_msix_cpus; j++) {
n = (j % info->max_cpus) + k;
- hw->sqset[i][j].intr_idx = entries[n].entry;
+ hw->sqset[i][j].intr_idx = n;
}
k += info->max_cpus;
}
- kfree(entries);
return 0;
}
@@ -593,26 +555,25 @@ csio_intr_enable(struct csio_hw *hw)
}
void
-csio_intr_disable(struct csio_hw *hw, bool free)
+csio_free_irqs(struct csio_hw *hw)
{
- csio_hw_intr_disable(hw);
+ if (hw->intr_mode == CSIO_IM_MSIX) {
+ int i;
- switch (hw->intr_mode) {
- case CSIO_IM_MSIX:
- csio_disable_msix(hw, free);
- break;
- case CSIO_IM_MSI:
- if (free)
- free_irq(hw->pdev->irq, hw);
- pci_disable_msi(hw->pdev);
- break;
- case CSIO_IM_INTX:
- if (free)
- free_irq(hw->pdev->irq, hw);
- break;
- default:
- break;
+ for (i = 0; i < hw->num_sqsets + CSIO_EXTRA_VECS; i++) {
+ free_irq(pci_irq_vector(hw->pdev, i),
+ hw->msix_entries[i].dev_id);
+ }
+ } else {
+ free_irq(pci_irq_vector(hw->pdev, 0), hw);
}
+}
+
+void
+csio_intr_disable(struct csio_hw *hw)
+{
+ csio_hw_intr_disable(hw);
+ pci_free_irq_vectors(hw->pdev);
hw->intr_mode = CSIO_IM_NONE;
hw->flags &= ~CSIO_HWF_HOST_INTR_ENABLED;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] csiostor: switch to pci_alloc_irq_vectors
2017-01-13 16:30 [PATCH] csiostor: switch to pci_alloc_irq_vectors Christoph Hellwig
@ 2017-01-21 0:27 ` Martin K. Petersen
2017-03-31 6:55 ` Christoph Hellwig
0 siblings, 1 reply; 11+ messages in thread
From: Martin K. Petersen @ 2017-01-21 0:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-scsi, hariprasad, praveenm
>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:
Christoph> And get automatic MSI-X affinity for free.
Chelsio folks: Please review and test!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] csiostor: switch to pci_alloc_irq_vectors
2017-01-21 0:27 ` Martin K. Petersen
@ 2017-03-31 6:55 ` Christoph Hellwig
2017-04-03 14:51 ` Varun Prakash
0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2017-03-31 6:55 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: Christoph Hellwig, linux-scsi, hariprasad, praveenm
On Fri, Jan 20, 2017 at 07:27:02PM -0500, Martin K. Petersen wrote:
> >>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:
>
> Christoph> And get automatic MSI-X affinity for free.
>
> Chelsio folks: Please review and test!
ping!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] csiostor: switch to pci_alloc_irq_vectors
2017-03-31 6:55 ` Christoph Hellwig
@ 2017-04-03 14:51 ` Varun Prakash
2017-04-04 6:46 ` Christoph Hellwig
0 siblings, 1 reply; 11+ messages in thread
From: Varun Prakash @ 2017-04-03 14:51 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Martin K. Petersen, linux-scsi
On Fri, Mar 31, 2017 at 12:25:27PM +0530, Christoph Hellwig wrote:
> On Fri, Jan 20, 2017 at 07:27:02PM -0500, Martin K. Petersen wrote:
> > >>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:
> >
> > Christoph> And get automatic MSI-X affinity for free.
> >
> > Chelsio folks: Please review and test!
>
> ping!
csiostor driver is triggering WARN_ON with this patch
drivers/pci/msi.c:1193
1172 int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
1173 unsigned int max_vecs, unsigned int flags,
1174 const struct irq_affinity *affd)
1175 {
1176 static const struct irq_affinity msi_default_affd;
1177 int vecs = -ENOSPC;
1178
1179 if (flags & PCI_IRQ_AFFINITY) {
...
1192 } else {
1193 if (WARN_ON(affd))
1194 affd = NULL;
1195 }
PCI_IRQ_AFFINITY flag is missing
+ cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt, PCI_IRQ_MSIX,
+ &desc);
+ if (cnt < 0)
return cnt;
- }
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] csiostor: switch to pci_alloc_irq_vectors
2017-04-03 14:51 ` Varun Prakash
@ 2017-04-04 6:46 ` Christoph Hellwig
2017-04-04 8:19 ` Varun Prakash
0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2017-04-04 6:46 UTC (permalink / raw)
To: Varun Prakash; +Cc: Christoph Hellwig, Martin K. Petersen, linux-scsi
Does this one work better?
---
>From e5a4178cb810be581b6d9b8f48f13b12e88eae74 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Thu, 12 Jan 2017 11:17:29 +0100
Subject: csiostor: switch to pci_alloc_irq_vectors
And get automatic MSI-X affinity for free.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/csiostor/csio_hw.h | 4 +-
drivers/scsi/csiostor/csio_init.c | 9 +--
drivers/scsi/csiostor/csio_isr.c | 127 +++++++++++++-------------------------
3 files changed, 51 insertions(+), 89 deletions(-)
diff --git a/drivers/scsi/csiostor/csio_hw.h b/drivers/scsi/csiostor/csio_hw.h
index 029bef82c057..a084d83ce70f 100644
--- a/drivers/scsi/csiostor/csio_hw.h
+++ b/drivers/scsi/csiostor/csio_hw.h
@@ -95,7 +95,6 @@ enum {
};
struct csio_msix_entries {
- unsigned short vector; /* Assigned MSI-X vector */
void *dev_id; /* Priv object associated w/ this msix*/
char desc[24]; /* Description of this vector */
};
@@ -591,8 +590,9 @@ int csio_enqueue_evt(struct csio_hw *, enum csio_evt, void *, uint16_t);
void csio_evtq_flush(struct csio_hw *hw);
int csio_request_irqs(struct csio_hw *);
+void csio_free_irqs(struct csio_hw *);
void csio_intr_enable(struct csio_hw *);
-void csio_intr_disable(struct csio_hw *, bool);
+void csio_intr_disable(struct csio_hw *);
void csio_hw_fatal_err(struct csio_hw *);
struct csio_lnode *csio_lnode_alloc(struct csio_hw *);
diff --git a/drivers/scsi/csiostor/csio_init.c b/drivers/scsi/csiostor/csio_init.c
index dbe416ff46c2..7e60699c8b55 100644
--- a/drivers/scsi/csiostor/csio_init.c
+++ b/drivers/scsi/csiostor/csio_init.c
@@ -461,8 +461,7 @@ csio_config_queues(struct csio_hw *hw)
return 0;
intr_disable:
- csio_intr_disable(hw, false);
-
+ csio_intr_disable(hw);
return -EINVAL;
}
@@ -575,7 +574,8 @@ static struct csio_hw *csio_hw_alloc(struct pci_dev *pdev)
static void
csio_hw_free(struct csio_hw *hw)
{
- csio_intr_disable(hw, true);
+ csio_free_irqs(hw);
+ csio_intr_disable(hw);
csio_hw_exit_workers(hw);
csio_hw_exit(hw);
iounmap(hw->regstart);
@@ -1068,7 +1068,8 @@ csio_pci_error_detected(struct pci_dev *pdev, pci_channel_state_t state)
spin_unlock_irq(&hw->lock);
csio_lnodes_unblock_request(hw);
csio_lnodes_exit(hw, 0);
- csio_intr_disable(hw, true);
+ csio_free_irqs(hw);
+ csio_intr_disable(hw);
pci_disable_device(pdev);
return state == pci_channel_io_perm_failure ?
PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET;
diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
index 2fb71c6c3b37..a4ad847e9c53 100644
--- a/drivers/scsi/csiostor/csio_isr.c
+++ b/drivers/scsi/csiostor/csio_isr.c
@@ -383,17 +383,15 @@ csio_request_irqs(struct csio_hw *hw)
int rv, i, j, k = 0;
struct csio_msix_entries *entryp = &hw->msix_entries[0];
struct csio_scsi_cpu_info *info;
+ struct pci_dev *pdev = hw->pdev;
if (hw->intr_mode != CSIO_IM_MSIX) {
- rv = request_irq(hw->pdev->irq, csio_fcoe_isr,
- (hw->intr_mode == CSIO_IM_MSI) ?
- 0 : IRQF_SHARED,
- KBUILD_MODNAME, hw);
+ rv = request_irq(pci_irq_vector(pdev, 0), csio_fcoe_isr,
+ hw->intr_mode == CSIO_IM_MSI ? 0 : IRQF_SHARED,
+ KBUILD_MODNAME, hw);
if (rv) {
- if (hw->intr_mode == CSIO_IM_MSI)
- pci_disable_msi(hw->pdev);
csio_err(hw, "Failed to allocate interrupt line.\n");
- return -EINVAL;
+ goto out_free_irqs;
}
goto out;
@@ -402,22 +400,22 @@ csio_request_irqs(struct csio_hw *hw)
/* Add the MSIX vector descriptions */
csio_add_msix_desc(hw);
- rv = request_irq(entryp[k].vector, csio_nondata_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_nondata_isr, 0,
entryp[k].desc, hw);
if (rv) {
csio_err(hw, "IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
- entryp[k++].dev_id = (void *)hw;
+ entryp[k++].dev_id = hw;
- rv = request_irq(entryp[k].vector, csio_fwevt_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_fwevt_isr, 0,
entryp[k].desc, hw);
if (rv) {
csio_err(hw, "IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
entryp[k++].dev_id = (void *)hw;
@@ -429,51 +427,31 @@ csio_request_irqs(struct csio_hw *hw)
struct csio_scsi_qset *sqset = &hw->sqset[i][j];
struct csio_q *q = hw->wrm.q_arr[sqset->iq_idx];
- rv = request_irq(entryp[k].vector, csio_scsi_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_scsi_isr, 0,
entryp[k].desc, q);
if (rv) {
csio_err(hw,
"IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
- entryp[k].dev_id = (void *)q;
+ entryp[k].dev_id = q;
} /* for all scsi cpus */
} /* for all ports */
out:
hw->flags |= CSIO_HWF_HOST_INTR_ENABLED;
-
return 0;
-err:
- for (i = 0; i < k; i++) {
- entryp = &hw->msix_entries[i];
- free_irq(entryp->vector, entryp->dev_id);
- }
- pci_disable_msix(hw->pdev);
-
+out_free_irqs:
+ for (i = 0; i < k; i++)
+ free_irq(pci_irq_vector(pdev, i), hw->msix_entries[i].dev_id);
+ pci_free_irq_vectors(hw->pdev);
return -EINVAL;
}
-static void
-csio_disable_msix(struct csio_hw *hw, bool free)
-{
- int i;
- struct csio_msix_entries *entryp;
- int cnt = hw->num_sqsets + CSIO_EXTRA_VECS;
-
- if (free) {
- for (i = 0; i < cnt; i++) {
- entryp = &hw->msix_entries[i];
- free_irq(entryp->vector, entryp->dev_id);
- }
- }
- pci_disable_msix(hw->pdev);
-}
-
/* Reduce per-port max possible CPUs */
static void
csio_reduce_sqsets(struct csio_hw *hw, int cnt)
@@ -500,10 +478,9 @@ static int
csio_enable_msix(struct csio_hw *hw)
{
int i, j, k, n, min, cnt;
- struct csio_msix_entries *entryp;
- struct msix_entry *entries;
int extra = CSIO_EXTRA_VECS;
struct csio_scsi_cpu_info *info;
+ struct irq_affinity desc = { .pre_vectors = 2 };
min = hw->num_pports + extra;
cnt = hw->num_sqsets + extra;
@@ -512,50 +489,35 @@ csio_enable_msix(struct csio_hw *hw)
if (hw->flags & CSIO_HWF_USING_SOFT_PARAMS || !csio_is_hw_master(hw))
cnt = min_t(uint8_t, hw->cfg_niq, cnt);
- entries = kzalloc(sizeof(struct msix_entry) * cnt, GFP_KERNEL);
- if (!entries)
- return -ENOMEM;
-
- for (i = 0; i < cnt; i++)
- entries[i].entry = (uint16_t)i;
-
csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
- cnt = pci_enable_msix_range(hw->pdev, entries, min, cnt);
- if (cnt < 0) {
- kfree(entries);
+ cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
+ PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, &desc);
+ if (cnt < 0)
return cnt;
- }
if (cnt < (hw->num_sqsets + extra)) {
csio_dbg(hw, "Reducing sqsets to %d\n", cnt - extra);
csio_reduce_sqsets(hw, cnt - extra);
}
- /* Save off vectors */
- for (i = 0; i < cnt; i++) {
- entryp = &hw->msix_entries[i];
- entryp->vector = entries[i].vector;
- }
-
/* Distribute vectors */
k = 0;
- csio_set_nondata_intr_idx(hw, entries[k].entry);
- csio_set_mb_intr_idx(csio_hw_to_mbm(hw), entries[k++].entry);
- csio_set_fwevt_intr_idx(hw, entries[k++].entry);
+ csio_set_nondata_intr_idx(hw, k);
+ csio_set_mb_intr_idx(csio_hw_to_mbm(hw), k++);
+ csio_set_fwevt_intr_idx(hw, k++);
for (i = 0; i < hw->num_pports; i++) {
info = &hw->scsi_cpu_info[i];
for (j = 0; j < hw->num_scsi_msix_cpus; j++) {
n = (j % info->max_cpus) + k;
- hw->sqset[i][j].intr_idx = entries[n].entry;
+ hw->sqset[i][j].intr_idx = n;
}
k += info->max_cpus;
}
- kfree(entries);
return 0;
}
@@ -593,26 +555,25 @@ csio_intr_enable(struct csio_hw *hw)
}
void
-csio_intr_disable(struct csio_hw *hw, bool free)
+csio_free_irqs(struct csio_hw *hw)
{
- csio_hw_intr_disable(hw);
+ if (hw->intr_mode == CSIO_IM_MSIX) {
+ int i;
- switch (hw->intr_mode) {
- case CSIO_IM_MSIX:
- csio_disable_msix(hw, free);
- break;
- case CSIO_IM_MSI:
- if (free)
- free_irq(hw->pdev->irq, hw);
- pci_disable_msi(hw->pdev);
- break;
- case CSIO_IM_INTX:
- if (free)
- free_irq(hw->pdev->irq, hw);
- break;
- default:
- break;
+ for (i = 0; i < hw->num_sqsets + CSIO_EXTRA_VECS; i++) {
+ free_irq(pci_irq_vector(hw->pdev, i),
+ hw->msix_entries[i].dev_id);
+ }
+ } else {
+ free_irq(pci_irq_vector(hw->pdev, 0), hw);
}
+}
+
+void
+csio_intr_disable(struct csio_hw *hw)
+{
+ csio_hw_intr_disable(hw);
+ pci_free_irq_vectors(hw->pdev);
hw->intr_mode = CSIO_IM_NONE;
hw->flags &= ~CSIO_HWF_HOST_INTR_ENABLED;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] csiostor: switch to pci_alloc_irq_vectors
2017-04-04 6:46 ` Christoph Hellwig
@ 2017-04-04 8:19 ` Varun Prakash
2017-04-05 6:26 ` Christoph Hellwig
0 siblings, 1 reply; 11+ messages in thread
From: Varun Prakash @ 2017-04-04 8:19 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Martin K. Petersen, linux-scsi
On Tue, Apr 04, 2017 at 08:46:14AM +0200, Christoph Hellwig wrote:
> Does this one work better?
>
csiostor driver is triggering following warning during module unload.
WARNING: CPU: 8 PID: 20636 at kernel/irq/manage.c:1480 __free_irq+0xa6/0x2b0
Trying to free already-free IRQ 53
CPU: 8 PID: 20636 Comm: rmmod Tainted: G B W OE 4.11.0-rc5+ #2
Call Trace:
dump_stack+0x63/0x84
__warn+0xd1/0xf0
warn_slowpath_fmt+0x5f/0x80
__free_irq+0xa6/0x2b0
free_irq+0x39/0x90
csio_free_irqs+0x34/0x90 [csiostor]
csio_hw_free+0x12/0xb0 [csiostor]
csio_remove_one+0x6e/0x90 [csiostor]
pci_device_remove+0x39/0xc0
device_release_driver_internal+0x141/0x1f0
driver_detach+0x3f/0x80
bus_remove_driver+0x55/0xd0
driver_unregister+0x2c/0x50
pci_unregister_driver+0x2a/0xa0
csio_exit+0x10/0xf70 [csiostor]
SyS_delete_module+0x1ba/0x220
do_syscall_64+0x67/0x180
entry_SYSCALL64_slow_path+0x25/0x25
kernel/irq/manage.c:1480
1457 static struct irqaction *__free_irq(unsigned int irq, void *dev_id)
1458 {
1459 struct irq_desc *desc = irq_to_desc(irq);
1460 struct irqaction *action, **action_ptr;
1461 unsigned long flags;
1462
...
1475 action_ptr = &desc->action;
1476 for (;;) {
1477 action = *action_ptr;
1478
1479 if (!action) {
1480 WARN(1, "Trying to free already-free IRQ %d\n", irq);
1481 raw_spin_unlock_irqrestore(&desc->lock, flags);
1482 chip_bus_sync_unlock(desc);
1483 return NULL;
1484 }
1485
1486 if (action->dev_id == dev_id)
1487 break;
1488 action_ptr = &action->next;
1489 }
...
1546 }
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] csiostor: switch to pci_alloc_irq_vectors
2017-04-04 8:19 ` Varun Prakash
@ 2017-04-05 6:26 ` Christoph Hellwig
2017-04-05 15:39 ` Varun Prakash
0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2017-04-05 6:26 UTC (permalink / raw)
To: Varun Prakash; +Cc: Christoph Hellwig, Martin K. Petersen, linux-scsi
On Tue, Apr 04, 2017 at 01:49:44PM +0530, Varun Prakash wrote:
> On Tue, Apr 04, 2017 at 08:46:14AM +0200, Christoph Hellwig wrote:
> > Does this one work better?
> >
>
> csiostor driver is triggering following warning during module unload.
Looks like we need to explicitly ignore the CSIO_IM_NONE case in
csio_free_irqs. New patch below:
---
>From e5a4178cb810be581b6d9b8f48f13b12e88eae74 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Thu, 12 Jan 2017 11:17:29 +0100
Subject: csiostor: switch to pci_alloc_irq_vectors
And get automatic MSI-X affinity for free.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/csiostor/csio_hw.h | 4 +-
drivers/scsi/csiostor/csio_init.c | 9 +--
drivers/scsi/csiostor/csio_isr.c | 127 +++++++++++++-------------------------
3 files changed, 51 insertions(+), 89 deletions(-)
diff --git a/drivers/scsi/csiostor/csio_hw.h b/drivers/scsi/csiostor/csio_hw.h
index 029bef82c057..a084d83ce70f 100644
--- a/drivers/scsi/csiostor/csio_hw.h
+++ b/drivers/scsi/csiostor/csio_hw.h
@@ -95,7 +95,6 @@ enum {
};
struct csio_msix_entries {
- unsigned short vector; /* Assigned MSI-X vector */
void *dev_id; /* Priv object associated w/ this msix*/
char desc[24]; /* Description of this vector */
};
@@ -591,8 +590,9 @@ int csio_enqueue_evt(struct csio_hw *, enum csio_evt, void *, uint16_t);
void csio_evtq_flush(struct csio_hw *hw);
int csio_request_irqs(struct csio_hw *);
+void csio_free_irqs(struct csio_hw *);
void csio_intr_enable(struct csio_hw *);
-void csio_intr_disable(struct csio_hw *, bool);
+void csio_intr_disable(struct csio_hw *);
void csio_hw_fatal_err(struct csio_hw *);
struct csio_lnode *csio_lnode_alloc(struct csio_hw *);
diff --git a/drivers/scsi/csiostor/csio_init.c b/drivers/scsi/csiostor/csio_init.c
index dbe416ff46c2..7e60699c8b55 100644
--- a/drivers/scsi/csiostor/csio_init.c
+++ b/drivers/scsi/csiostor/csio_init.c
@@ -461,8 +461,7 @@ csio_config_queues(struct csio_hw *hw)
return 0;
intr_disable:
- csio_intr_disable(hw, false);
-
+ csio_intr_disable(hw);
return -EINVAL;
}
@@ -575,7 +574,8 @@ static struct csio_hw *csio_hw_alloc(struct pci_dev *pdev)
static void
csio_hw_free(struct csio_hw *hw)
{
- csio_intr_disable(hw, true);
+ csio_free_irqs(hw);
+ csio_intr_disable(hw);
csio_hw_exit_workers(hw);
csio_hw_exit(hw);
iounmap(hw->regstart);
@@ -1068,7 +1068,8 @@ csio_pci_error_detected(struct pci_dev *pdev, pci_channel_state_t state)
spin_unlock_irq(&hw->lock);
csio_lnodes_unblock_request(hw);
csio_lnodes_exit(hw, 0);
- csio_intr_disable(hw, true);
+ csio_free_irqs(hw);
+ csio_intr_disable(hw);
pci_disable_device(pdev);
return state == pci_channel_io_perm_failure ?
PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET;
diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
index 2fb71c6c3b37..a4ad847e9c53 100644
--- a/drivers/scsi/csiostor/csio_isr.c
+++ b/drivers/scsi/csiostor/csio_isr.c
@@ -383,17 +383,15 @@ csio_request_irqs(struct csio_hw *hw)
int rv, i, j, k = 0;
struct csio_msix_entries *entryp = &hw->msix_entries[0];
struct csio_scsi_cpu_info *info;
+ struct pci_dev *pdev = hw->pdev;
if (hw->intr_mode != CSIO_IM_MSIX) {
- rv = request_irq(hw->pdev->irq, csio_fcoe_isr,
- (hw->intr_mode == CSIO_IM_MSI) ?
- 0 : IRQF_SHARED,
- KBUILD_MODNAME, hw);
+ rv = request_irq(pci_irq_vector(pdev, 0), csio_fcoe_isr,
+ hw->intr_mode == CSIO_IM_MSI ? 0 : IRQF_SHARED,
+ KBUILD_MODNAME, hw);
if (rv) {
- if (hw->intr_mode == CSIO_IM_MSI)
- pci_disable_msi(hw->pdev);
csio_err(hw, "Failed to allocate interrupt line.\n");
- return -EINVAL;
+ goto out_free_irqs;
}
goto out;
@@ -402,22 +400,22 @@ csio_request_irqs(struct csio_hw *hw)
/* Add the MSIX vector descriptions */
csio_add_msix_desc(hw);
- rv = request_irq(entryp[k].vector, csio_nondata_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_nondata_isr, 0,
entryp[k].desc, hw);
if (rv) {
csio_err(hw, "IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
- entryp[k++].dev_id = (void *)hw;
+ entryp[k++].dev_id = hw;
- rv = request_irq(entryp[k].vector, csio_fwevt_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_fwevt_isr, 0,
entryp[k].desc, hw);
if (rv) {
csio_err(hw, "IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
entryp[k++].dev_id = (void *)hw;
@@ -429,51 +427,31 @@ csio_request_irqs(struct csio_hw *hw)
struct csio_scsi_qset *sqset = &hw->sqset[i][j];
struct csio_q *q = hw->wrm.q_arr[sqset->iq_idx];
- rv = request_irq(entryp[k].vector, csio_scsi_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_scsi_isr, 0,
entryp[k].desc, q);
if (rv) {
csio_err(hw,
"IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
- entryp[k].dev_id = (void *)q;
+ entryp[k].dev_id = q;
} /* for all scsi cpus */
} /* for all ports */
out:
hw->flags |= CSIO_HWF_HOST_INTR_ENABLED;
-
return 0;
-err:
- for (i = 0; i < k; i++) {
- entryp = &hw->msix_entries[i];
- free_irq(entryp->vector, entryp->dev_id);
- }
- pci_disable_msix(hw->pdev);
-
+out_free_irqs:
+ for (i = 0; i < k; i++)
+ free_irq(pci_irq_vector(pdev, i), hw->msix_entries[i].dev_id);
+ pci_free_irq_vectors(hw->pdev);
return -EINVAL;
}
-static void
-csio_disable_msix(struct csio_hw *hw, bool free)
-{
- int i;
- struct csio_msix_entries *entryp;
- int cnt = hw->num_sqsets + CSIO_EXTRA_VECS;
-
- if (free) {
- for (i = 0; i < cnt; i++) {
- entryp = &hw->msix_entries[i];
- free_irq(entryp->vector, entryp->dev_id);
- }
- }
- pci_disable_msix(hw->pdev);
-}
-
/* Reduce per-port max possible CPUs */
static void
csio_reduce_sqsets(struct csio_hw *hw, int cnt)
@@ -500,10 +478,9 @@ static int
csio_enable_msix(struct csio_hw *hw)
{
int i, j, k, n, min, cnt;
- struct csio_msix_entries *entryp;
- struct msix_entry *entries;
int extra = CSIO_EXTRA_VECS;
struct csio_scsi_cpu_info *info;
+ struct irq_affinity desc = { .pre_vectors = 2 };
min = hw->num_pports + extra;
cnt = hw->num_sqsets + extra;
@@ -512,50 +489,35 @@ csio_enable_msix(struct csio_hw *hw)
if (hw->flags & CSIO_HWF_USING_SOFT_PARAMS || !csio_is_hw_master(hw))
cnt = min_t(uint8_t, hw->cfg_niq, cnt);
- entries = kzalloc(sizeof(struct msix_entry) * cnt, GFP_KERNEL);
- if (!entries)
- return -ENOMEM;
-
- for (i = 0; i < cnt; i++)
- entries[i].entry = (uint16_t)i;
-
csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
- cnt = pci_enable_msix_range(hw->pdev, entries, min, cnt);
- if (cnt < 0) {
- kfree(entries);
+ cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
+ PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, &desc);
+ if (cnt < 0)
return cnt;
- }
if (cnt < (hw->num_sqsets + extra)) {
csio_dbg(hw, "Reducing sqsets to %d\n", cnt - extra);
csio_reduce_sqsets(hw, cnt - extra);
}
- /* Save off vectors */
- for (i = 0; i < cnt; i++) {
- entryp = &hw->msix_entries[i];
- entryp->vector = entries[i].vector;
- }
-
/* Distribute vectors */
k = 0;
- csio_set_nondata_intr_idx(hw, entries[k].entry);
- csio_set_mb_intr_idx(csio_hw_to_mbm(hw), entries[k++].entry);
- csio_set_fwevt_intr_idx(hw, entries[k++].entry);
+ csio_set_nondata_intr_idx(hw, k);
+ csio_set_mb_intr_idx(csio_hw_to_mbm(hw), k++);
+ csio_set_fwevt_intr_idx(hw, k++);
for (i = 0; i < hw->num_pports; i++) {
info = &hw->scsi_cpu_info[i];
for (j = 0; j < hw->num_scsi_msix_cpus; j++) {
n = (j % info->max_cpus) + k;
- hw->sqset[i][j].intr_idx = entries[n].entry;
+ hw->sqset[i][j].intr_idx = n;
}
k += info->max_cpus;
}
- kfree(entries);
return 0;
}
@@ -593,26 +555,25 @@ csio_intr_enable(struct csio_hw *hw)
}
void
-csio_intr_disable(struct csio_hw *hw, bool free)
+csio_free_irqs(struct csio_hw *hw)
{
- csio_hw_intr_disable(hw);
+ if (hw->intr_mode == CSIO_IM_MSIX) {
+ int i;
- switch (hw->intr_mode) {
- case CSIO_IM_MSIX:
- csio_disable_msix(hw, free);
- break;
- case CSIO_IM_MSI:
- if (free)
- free_irq(hw->pdev->irq, hw);
- pci_disable_msi(hw->pdev);
- break;
- case CSIO_IM_INTX:
- if (free)
- free_irq(hw->pdev->irq, hw);
- break;
- default:
- break;
+ for (i = 0; i < hw->num_sqsets + CSIO_EXTRA_VECS; i++) {
+ free_irq(pci_irq_vector(hw->pdev, i),
+ hw->msix_entries[i].dev_id);
+ }
+ } else {
+ free_irq(pci_irq_vector(hw->pdev, 0), hw);
}
+}
+
+void
+csio_intr_disable(struct csio_hw *hw)
+{
+ csio_hw_intr_disable(hw);
+ pci_free_irq_vectors(hw->pdev);
hw->intr_mode = CSIO_IM_NONE;
hw->flags &= ~CSIO_HWF_HOST_INTR_ENABLED;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] csiostor: switch to pci_alloc_irq_vectors
2017-04-05 6:26 ` Christoph Hellwig
@ 2017-04-05 15:39 ` Varun Prakash
2017-04-06 7:58 ` Christoph Hellwig
0 siblings, 1 reply; 11+ messages in thread
From: Varun Prakash @ 2017-04-05 15:39 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Martin K. Petersen, linux-scsi
On Wed, Apr 05, 2017 at 08:26:57AM +0200, Christoph Hellwig wrote:
> On Tue, Apr 04, 2017 at 01:49:44PM +0530, Varun Prakash wrote:
> > On Tue, Apr 04, 2017 at 08:46:14AM +0200, Christoph Hellwig wrote:
> > > Does this one work better?
> > >
> >
> > csiostor driver is triggering following warning during module unload.
>
> Looks like we need to explicitly ignore the CSIO_IM_NONE case in
> csio_free_irqs. New patch below:
>
Yes, we have to ignore CSIO_IM_NONE case in csio_free_irqs(), but I don't
see any CSIO_IM_NONE specific code in csio_free_irqs().
There is one more issue - this patch changes the order in which
csio_hw_intr_disable() and free_irq() are called.
In the current code first csio_hw_intr_disable() is called then free_irq()
is called, with this patch free_irq() is called without disabling hw
interrupts, this can cause regressions.
> ---
> From e5a4178cb810be581b6d9b8f48f13b12e88eae74 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Thu, 12 Jan 2017 11:17:29 +0100
> Subject: csiostor: switch to pci_alloc_irq_vectors
>
> And get automatic MSI-X affinity for free.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/scsi/csiostor/csio_hw.h | 4 +-
> drivers/scsi/csiostor/csio_init.c | 9 +--
> drivers/scsi/csiostor/csio_isr.c | 127 +++++++++++++-------------------------
> 3 files changed, 51 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/scsi/csiostor/csio_hw.h b/drivers/scsi/csiostor/csio_hw.h
> index 029bef82c057..a084d83ce70f 100644
> --- a/drivers/scsi/csiostor/csio_hw.h
> +++ b/drivers/scsi/csiostor/csio_hw.h
> @@ -95,7 +95,6 @@ enum {
> };
>
> struct csio_msix_entries {
> - unsigned short vector; /* Assigned MSI-X vector */
> void *dev_id; /* Priv object associated w/ this msix*/
> char desc[24]; /* Description of this vector */
> };
> @@ -591,8 +590,9 @@ int csio_enqueue_evt(struct csio_hw *, enum csio_evt, void *, uint16_t);
> void csio_evtq_flush(struct csio_hw *hw);
>
> int csio_request_irqs(struct csio_hw *);
> +void csio_free_irqs(struct csio_hw *);
> void csio_intr_enable(struct csio_hw *);
> -void csio_intr_disable(struct csio_hw *, bool);
> +void csio_intr_disable(struct csio_hw *);
> void csio_hw_fatal_err(struct csio_hw *);
>
> struct csio_lnode *csio_lnode_alloc(struct csio_hw *);
> diff --git a/drivers/scsi/csiostor/csio_init.c b/drivers/scsi/csiostor/csio_init.c
> index dbe416ff46c2..7e60699c8b55 100644
> --- a/drivers/scsi/csiostor/csio_init.c
> +++ b/drivers/scsi/csiostor/csio_init.c
> @@ -461,8 +461,7 @@ csio_config_queues(struct csio_hw *hw)
> return 0;
>
> intr_disable:
> - csio_intr_disable(hw, false);
> -
> + csio_intr_disable(hw);
> return -EINVAL;
> }
>
> @@ -575,7 +574,8 @@ static struct csio_hw *csio_hw_alloc(struct pci_dev *pdev)
> static void
> csio_hw_free(struct csio_hw *hw)
> {
> - csio_intr_disable(hw, true);
> + csio_free_irqs(hw);
> + csio_intr_disable(hw);
This changes the order, free_irq() is called without disabling hw interrupts.
> csio_hw_exit_workers(hw);
> csio_hw_exit(hw);
> iounmap(hw->regstart);
> @@ -1068,7 +1068,8 @@ csio_pci_error_detected(struct pci_dev *pdev, pci_channel_state_t state)
> spin_unlock_irq(&hw->lock);
> csio_lnodes_unblock_request(hw);
> csio_lnodes_exit(hw, 0);
> - csio_intr_disable(hw, true);
> + csio_free_irqs(hw);
> + csio_intr_disable(hw);
Same here.
> pci_disable_device(pdev);
> return state == pci_channel_io_perm_failure ?
> PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET;
> diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
> index 2fb71c6c3b37..a4ad847e9c53 100644
> --- a/drivers/scsi/csiostor/csio_isr.c
> +++ b/drivers/scsi/csiostor/csio_isr.c
> @@ -383,17 +383,15 @@ csio_request_irqs(struct csio_hw *hw)
> int rv, i, j, k = 0;
> struct csio_msix_entries *entryp = &hw->msix_entries[0];
> struct csio_scsi_cpu_info *info;
> + struct pci_dev *pdev = hw->pdev;
>
> if (hw->intr_mode != CSIO_IM_MSIX) {
> - rv = request_irq(hw->pdev->irq, csio_fcoe_isr,
> - (hw->intr_mode == CSIO_IM_MSI) ?
> - 0 : IRQF_SHARED,
> - KBUILD_MODNAME, hw);
> + rv = request_irq(pci_irq_vector(pdev, 0), csio_fcoe_isr,
> + hw->intr_mode == CSIO_IM_MSI ? 0 : IRQF_SHARED,
> + KBUILD_MODNAME, hw);
> if (rv) {
> - if (hw->intr_mode == CSIO_IM_MSI)
> - pci_disable_msi(hw->pdev);
> csio_err(hw, "Failed to allocate interrupt line.\n");
> - return -EINVAL;
> + goto out_free_irqs;
> }
>
> goto out;
> @@ -402,22 +400,22 @@ csio_request_irqs(struct csio_hw *hw)
> /* Add the MSIX vector descriptions */
> csio_add_msix_desc(hw);
>
> - rv = request_irq(entryp[k].vector, csio_nondata_isr, 0,
> + rv = request_irq(pci_irq_vector(pdev, k), csio_nondata_isr, 0,
> entryp[k].desc, hw);
> if (rv) {
> csio_err(hw, "IRQ request failed for vec %d err:%d\n",
> - entryp[k].vector, rv);
> - goto err;
> + pci_irq_vector(pdev, k), rv);
> + goto out_free_irqs;
> }
>
> - entryp[k++].dev_id = (void *)hw;
> + entryp[k++].dev_id = hw;
>
> - rv = request_irq(entryp[k].vector, csio_fwevt_isr, 0,
> + rv = request_irq(pci_irq_vector(pdev, k), csio_fwevt_isr, 0,
> entryp[k].desc, hw);
> if (rv) {
> csio_err(hw, "IRQ request failed for vec %d err:%d\n",
> - entryp[k].vector, rv);
> - goto err;
> + pci_irq_vector(pdev, k), rv);
> + goto out_free_irqs;
> }
>
> entryp[k++].dev_id = (void *)hw;
> @@ -429,51 +427,31 @@ csio_request_irqs(struct csio_hw *hw)
> struct csio_scsi_qset *sqset = &hw->sqset[i][j];
> struct csio_q *q = hw->wrm.q_arr[sqset->iq_idx];
>
> - rv = request_irq(entryp[k].vector, csio_scsi_isr, 0,
> + rv = request_irq(pci_irq_vector(pdev, k), csio_scsi_isr, 0,
> entryp[k].desc, q);
> if (rv) {
> csio_err(hw,
> "IRQ request failed for vec %d err:%d\n",
> - entryp[k].vector, rv);
> - goto err;
> + pci_irq_vector(pdev, k), rv);
> + goto out_free_irqs;
> }
>
> - entryp[k].dev_id = (void *)q;
> + entryp[k].dev_id = q;
>
> } /* for all scsi cpus */
> } /* for all ports */
>
> out:
> hw->flags |= CSIO_HWF_HOST_INTR_ENABLED;
> -
> return 0;
>
> -err:
> - for (i = 0; i < k; i++) {
> - entryp = &hw->msix_entries[i];
> - free_irq(entryp->vector, entryp->dev_id);
> - }
> - pci_disable_msix(hw->pdev);
> -
> +out_free_irqs:
> + for (i = 0; i < k; i++)
> + free_irq(pci_irq_vector(pdev, i), hw->msix_entries[i].dev_id);
> + pci_free_irq_vectors(hw->pdev);
> return -EINVAL;
> }
>
> -static void
> -csio_disable_msix(struct csio_hw *hw, bool free)
> -{
> - int i;
> - struct csio_msix_entries *entryp;
> - int cnt = hw->num_sqsets + CSIO_EXTRA_VECS;
> -
> - if (free) {
> - for (i = 0; i < cnt; i++) {
> - entryp = &hw->msix_entries[i];
> - free_irq(entryp->vector, entryp->dev_id);
> - }
> - }
> - pci_disable_msix(hw->pdev);
> -}
> -
> /* Reduce per-port max possible CPUs */
> static void
> csio_reduce_sqsets(struct csio_hw *hw, int cnt)
> @@ -500,10 +478,9 @@ static int
> csio_enable_msix(struct csio_hw *hw)
> {
> int i, j, k, n, min, cnt;
> - struct csio_msix_entries *entryp;
> - struct msix_entry *entries;
> int extra = CSIO_EXTRA_VECS;
> struct csio_scsi_cpu_info *info;
> + struct irq_affinity desc = { .pre_vectors = 2 };
>
> min = hw->num_pports + extra;
> cnt = hw->num_sqsets + extra;
> @@ -512,50 +489,35 @@ csio_enable_msix(struct csio_hw *hw)
> if (hw->flags & CSIO_HWF_USING_SOFT_PARAMS || !csio_is_hw_master(hw))
> cnt = min_t(uint8_t, hw->cfg_niq, cnt);
>
> - entries = kzalloc(sizeof(struct msix_entry) * cnt, GFP_KERNEL);
> - if (!entries)
> - return -ENOMEM;
> -
> - for (i = 0; i < cnt; i++)
> - entries[i].entry = (uint16_t)i;
> -
> csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
>
> - cnt = pci_enable_msix_range(hw->pdev, entries, min, cnt);
> - if (cnt < 0) {
> - kfree(entries);
> + cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
> + PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, &desc);
> + if (cnt < 0)
> return cnt;
> - }
>
> if (cnt < (hw->num_sqsets + extra)) {
> csio_dbg(hw, "Reducing sqsets to %d\n", cnt - extra);
> csio_reduce_sqsets(hw, cnt - extra);
> }
>
> - /* Save off vectors */
> - for (i = 0; i < cnt; i++) {
> - entryp = &hw->msix_entries[i];
> - entryp->vector = entries[i].vector;
> - }
> -
> /* Distribute vectors */
> k = 0;
> - csio_set_nondata_intr_idx(hw, entries[k].entry);
> - csio_set_mb_intr_idx(csio_hw_to_mbm(hw), entries[k++].entry);
> - csio_set_fwevt_intr_idx(hw, entries[k++].entry);
> + csio_set_nondata_intr_idx(hw, k);
> + csio_set_mb_intr_idx(csio_hw_to_mbm(hw), k++);
> + csio_set_fwevt_intr_idx(hw, k++);
>
> for (i = 0; i < hw->num_pports; i++) {
> info = &hw->scsi_cpu_info[i];
>
> for (j = 0; j < hw->num_scsi_msix_cpus; j++) {
> n = (j % info->max_cpus) + k;
> - hw->sqset[i][j].intr_idx = entries[n].entry;
> + hw->sqset[i][j].intr_idx = n;
> }
>
> k += info->max_cpus;
> }
>
> - kfree(entries);
> return 0;
> }
>
> @@ -593,26 +555,25 @@ csio_intr_enable(struct csio_hw *hw)
> }
>
> void
> -csio_intr_disable(struct csio_hw *hw, bool free)
> +csio_free_irqs(struct csio_hw *hw)
> {
> - csio_hw_intr_disable(hw);
> + if (hw->intr_mode == CSIO_IM_MSIX) {
> + int i;
>
> - switch (hw->intr_mode) {
> - case CSIO_IM_MSIX:
> - csio_disable_msix(hw, free);
> - break;
> - case CSIO_IM_MSI:
> - if (free)
> - free_irq(hw->pdev->irq, hw);
> - pci_disable_msi(hw->pdev);
> - break;
> - case CSIO_IM_INTX:
> - if (free)
> - free_irq(hw->pdev->irq, hw);
> - break;
> - default:
> - break;
> + for (i = 0; i < hw->num_sqsets + CSIO_EXTRA_VECS; i++) {
> + free_irq(pci_irq_vector(hw->pdev, i),
> + hw->msix_entries[i].dev_id);
> + }
> + } else {
> + free_irq(pci_irq_vector(hw->pdev, 0), hw);
> }
> +}
> +
> +void
> +csio_intr_disable(struct csio_hw *hw)
> +{
> + csio_hw_intr_disable(hw);
> + pci_free_irq_vectors(hw->pdev);
> hw->intr_mode = CSIO_IM_NONE;
> hw->flags &= ~CSIO_HWF_HOST_INTR_ENABLED;
> }
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] csiostor: switch to pci_alloc_irq_vectors
2017-04-05 15:39 ` Varun Prakash
@ 2017-04-06 7:58 ` Christoph Hellwig
2017-04-06 15:26 ` Varun Prakash
2017-04-06 16:58 ` Martin K. Petersen
0 siblings, 2 replies; 11+ messages in thread
From: Christoph Hellwig @ 2017-04-06 7:58 UTC (permalink / raw)
To: Varun Prakash; +Cc: Christoph Hellwig, Martin K. Petersen, linux-scsi
Ok, the version below simplify skip the function split entirely:
---
>From 7c9ca58f1d8cf53b42f14a51e02d0f3d0f12ab45 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Thu, 12 Jan 2017 11:17:29 +0100
Subject: csiostor: switch to pci_alloc_irq_vectors
And get automatic MSI-X affinity for free.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/csiostor/csio_hw.h | 1 -
drivers/scsi/csiostor/csio_isr.c | 128 ++++++++++++++-------------------------
2 files changed, 47 insertions(+), 82 deletions(-)
diff --git a/drivers/scsi/csiostor/csio_hw.h b/drivers/scsi/csiostor/csio_hw.h
index 029bef82c057..62758e830d3b 100644
--- a/drivers/scsi/csiostor/csio_hw.h
+++ b/drivers/scsi/csiostor/csio_hw.h
@@ -95,7 +95,6 @@ enum {
};
struct csio_msix_entries {
- unsigned short vector; /* Assigned MSI-X vector */
void *dev_id; /* Priv object associated w/ this msix*/
char desc[24]; /* Description of this vector */
};
diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
index 2fb71c6c3b37..7c8814715711 100644
--- a/drivers/scsi/csiostor/csio_isr.c
+++ b/drivers/scsi/csiostor/csio_isr.c
@@ -383,17 +383,15 @@ csio_request_irqs(struct csio_hw *hw)
int rv, i, j, k = 0;
struct csio_msix_entries *entryp = &hw->msix_entries[0];
struct csio_scsi_cpu_info *info;
+ struct pci_dev *pdev = hw->pdev;
if (hw->intr_mode != CSIO_IM_MSIX) {
- rv = request_irq(hw->pdev->irq, csio_fcoe_isr,
- (hw->intr_mode == CSIO_IM_MSI) ?
- 0 : IRQF_SHARED,
- KBUILD_MODNAME, hw);
+ rv = request_irq(pci_irq_vector(pdev, 0), csio_fcoe_isr,
+ hw->intr_mode == CSIO_IM_MSI ? 0 : IRQF_SHARED,
+ KBUILD_MODNAME, hw);
if (rv) {
- if (hw->intr_mode == CSIO_IM_MSI)
- pci_disable_msi(hw->pdev);
csio_err(hw, "Failed to allocate interrupt line.\n");
- return -EINVAL;
+ goto out_free_irqs;
}
goto out;
@@ -402,22 +400,22 @@ csio_request_irqs(struct csio_hw *hw)
/* Add the MSIX vector descriptions */
csio_add_msix_desc(hw);
- rv = request_irq(entryp[k].vector, csio_nondata_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_nondata_isr, 0,
entryp[k].desc, hw);
if (rv) {
csio_err(hw, "IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
- entryp[k++].dev_id = (void *)hw;
+ entryp[k++].dev_id = hw;
- rv = request_irq(entryp[k].vector, csio_fwevt_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_fwevt_isr, 0,
entryp[k].desc, hw);
if (rv) {
csio_err(hw, "IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
entryp[k++].dev_id = (void *)hw;
@@ -429,51 +427,31 @@ csio_request_irqs(struct csio_hw *hw)
struct csio_scsi_qset *sqset = &hw->sqset[i][j];
struct csio_q *q = hw->wrm.q_arr[sqset->iq_idx];
- rv = request_irq(entryp[k].vector, csio_scsi_isr, 0,
+ rv = request_irq(pci_irq_vector(pdev, k), csio_scsi_isr, 0,
entryp[k].desc, q);
if (rv) {
csio_err(hw,
"IRQ request failed for vec %d err:%d\n",
- entryp[k].vector, rv);
- goto err;
+ pci_irq_vector(pdev, k), rv);
+ goto out_free_irqs;
}
- entryp[k].dev_id = (void *)q;
+ entryp[k].dev_id = q;
} /* for all scsi cpus */
} /* for all ports */
out:
hw->flags |= CSIO_HWF_HOST_INTR_ENABLED;
-
return 0;
-err:
- for (i = 0; i < k; i++) {
- entryp = &hw->msix_entries[i];
- free_irq(entryp->vector, entryp->dev_id);
- }
- pci_disable_msix(hw->pdev);
-
+out_free_irqs:
+ for (i = 0; i < k; i++)
+ free_irq(pci_irq_vector(pdev, i), hw->msix_entries[i].dev_id);
+ pci_free_irq_vectors(hw->pdev);
return -EINVAL;
}
-static void
-csio_disable_msix(struct csio_hw *hw, bool free)
-{
- int i;
- struct csio_msix_entries *entryp;
- int cnt = hw->num_sqsets + CSIO_EXTRA_VECS;
-
- if (free) {
- for (i = 0; i < cnt; i++) {
- entryp = &hw->msix_entries[i];
- free_irq(entryp->vector, entryp->dev_id);
- }
- }
- pci_disable_msix(hw->pdev);
-}
-
/* Reduce per-port max possible CPUs */
static void
csio_reduce_sqsets(struct csio_hw *hw, int cnt)
@@ -500,10 +478,9 @@ static int
csio_enable_msix(struct csio_hw *hw)
{
int i, j, k, n, min, cnt;
- struct csio_msix_entries *entryp;
- struct msix_entry *entries;
int extra = CSIO_EXTRA_VECS;
struct csio_scsi_cpu_info *info;
+ struct irq_affinity desc = { .pre_vectors = 2 };
min = hw->num_pports + extra;
cnt = hw->num_sqsets + extra;
@@ -512,50 +489,35 @@ csio_enable_msix(struct csio_hw *hw)
if (hw->flags & CSIO_HWF_USING_SOFT_PARAMS || !csio_is_hw_master(hw))
cnt = min_t(uint8_t, hw->cfg_niq, cnt);
- entries = kzalloc(sizeof(struct msix_entry) * cnt, GFP_KERNEL);
- if (!entries)
- return -ENOMEM;
-
- for (i = 0; i < cnt; i++)
- entries[i].entry = (uint16_t)i;
-
csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
- cnt = pci_enable_msix_range(hw->pdev, entries, min, cnt);
- if (cnt < 0) {
- kfree(entries);
+ cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
+ PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, &desc);
+ if (cnt < 0)
return cnt;
- }
if (cnt < (hw->num_sqsets + extra)) {
csio_dbg(hw, "Reducing sqsets to %d\n", cnt - extra);
csio_reduce_sqsets(hw, cnt - extra);
}
- /* Save off vectors */
- for (i = 0; i < cnt; i++) {
- entryp = &hw->msix_entries[i];
- entryp->vector = entries[i].vector;
- }
-
/* Distribute vectors */
k = 0;
- csio_set_nondata_intr_idx(hw, entries[k].entry);
- csio_set_mb_intr_idx(csio_hw_to_mbm(hw), entries[k++].entry);
- csio_set_fwevt_intr_idx(hw, entries[k++].entry);
+ csio_set_nondata_intr_idx(hw, k);
+ csio_set_mb_intr_idx(csio_hw_to_mbm(hw), k++);
+ csio_set_fwevt_intr_idx(hw, k++);
for (i = 0; i < hw->num_pports; i++) {
info = &hw->scsi_cpu_info[i];
for (j = 0; j < hw->num_scsi_msix_cpus; j++) {
n = (j % info->max_cpus) + k;
- hw->sqset[i][j].intr_idx = entries[n].entry;
+ hw->sqset[i][j].intr_idx = n;
}
k += info->max_cpus;
}
- kfree(entries);
return 0;
}
@@ -597,22 +559,26 @@ csio_intr_disable(struct csio_hw *hw, bool free)
{
csio_hw_intr_disable(hw);
- switch (hw->intr_mode) {
- case CSIO_IM_MSIX:
- csio_disable_msix(hw, free);
- break;
- case CSIO_IM_MSI:
- if (free)
- free_irq(hw->pdev->irq, hw);
- pci_disable_msi(hw->pdev);
- break;
- case CSIO_IM_INTX:
- if (free)
- free_irq(hw->pdev->irq, hw);
- break;
- default:
- break;
+ if (free) {
+ int i;
+
+ switch (hw->intr_mode) {
+ case CSIO_IM_MSIX:
+ for (i = 0; i < hw->num_sqsets + CSIO_EXTRA_VECS; i++) {
+ free_irq(pci_irq_vector(hw->pdev, i),
+ hw->msix_entries[i].dev_id);
+ }
+ break;
+ case CSIO_IM_MSI:
+ case CSIO_IM_INTX:
+ free_irq(pci_irq_vector(hw->pdev, 0), hw);
+ break;
+ default:
+ break;
+ }
}
+
+ pci_free_irq_vectors(hw->pdev);
hw->intr_mode = CSIO_IM_NONE;
hw->flags &= ~CSIO_HWF_HOST_INTR_ENABLED;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] csiostor: switch to pci_alloc_irq_vectors
2017-04-06 7:58 ` Christoph Hellwig
@ 2017-04-06 15:26 ` Varun Prakash
2017-04-06 16:58 ` Martin K. Petersen
1 sibling, 0 replies; 11+ messages in thread
From: Varun Prakash @ 2017-04-06 15:26 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Martin K. Petersen, linux-scsi
On Thu, Apr 06, 2017 at 09:58:58AM +0200, Christoph Hellwig wrote:
> Ok, the version below simplify skip the function split entirely:
>
> ---
> From 7c9ca58f1d8cf53b42f14a51e02d0f3d0f12ab45 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Thu, 12 Jan 2017 11:17:29 +0100
> Subject: csiostor: switch to pci_alloc_irq_vectors
>
> And get automatic MSI-X affinity for free.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/scsi/csiostor/csio_hw.h | 1 -
> drivers/scsi/csiostor/csio_isr.c | 128 ++++++++++++++-------------------------
> 2 files changed, 47 insertions(+), 82 deletions(-)
>
Looks good.
Acked-by: Varun Prakash <varun@chelsio.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] csiostor: switch to pci_alloc_irq_vectors
2017-04-06 7:58 ` Christoph Hellwig
2017-04-06 15:26 ` Varun Prakash
@ 2017-04-06 16:58 ` Martin K. Petersen
1 sibling, 0 replies; 11+ messages in thread
From: Martin K. Petersen @ 2017-04-06 16:58 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Varun Prakash, Martin K. Petersen, linux-scsi
Christoph Hellwig <hch@lst.de> writes:
> Ok, the version below simplify skip the function split entirely:
Applied to 4.12/scsi-queue.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-04-06 16:58 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-13 16:30 [PATCH] csiostor: switch to pci_alloc_irq_vectors Christoph Hellwig
2017-01-21 0:27 ` Martin K. Petersen
2017-03-31 6:55 ` Christoph Hellwig
2017-04-03 14:51 ` Varun Prakash
2017-04-04 6:46 ` Christoph Hellwig
2017-04-04 8:19 ` Varun Prakash
2017-04-05 6:26 ` Christoph Hellwig
2017-04-05 15:39 ` Varun Prakash
2017-04-06 7:58 ` Christoph Hellwig
2017-04-06 15:26 ` Varun Prakash
2017-04-06 16:58 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.