* [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@vger.kernel.org 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@vger.kernel.org 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@vger.kernel.org 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@vger.kernel.org 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@vger.kernel.org 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@vger.kernel.org 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@vger.kernel.org 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@vger.kernel.org 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox