Re: [PATCH] scsi: csiostor: remove automatic irq affinity assignment
Christoph/Thomas: Please review. Varun has solicited feedback on this a
few times already.
Thanks!
> If number of interrupt vectors are more than num_online_cpus() then
> pci_alloc_irq_vectors_affinity() assigns cpumask based on
> num_possible_cpus() to the remaining vectors because of this
> interrupts are not generating for these vectors.
>
> This patch fixes this issue by using pci_alloc_irq_vectors() instead
> of pci_alloc_irq_vectors_affinity().
>
> Signed-off-by: Varun Prakash
> Cc: Christoph Hellwig
> Cc: Thomas Gleixner
> ---
> drivers/scsi/csiostor/csio_isr.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/csiostor/csio_isr.c
> b/drivers/scsi/csiostor/csio_isr.c
> index 7c88147..8b92c59 100644
> --- a/drivers/scsi/csiostor/csio_isr.c
> +++ b/drivers/scsi/csiostor/csio_isr.c
> @@ -480,7 +480,6 @@ csio_enable_msix(struct csio_hw *hw)
> int i, j, k, n, min, cnt;
> 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;
> @@ -491,8 +490,7 @@ csio_enable_msix(struct csio_hw *hw)
>
> csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
>
> - cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
> - PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, );
> + cnt = pci_alloc_irq_vectors(hw->pdev, min, cnt, PCI_IRQ_MSIX);
> if (cnt < 0)
> return cnt;
--
Martin K. Petersen Oracle Linux Engineering
[PATCH] scsi: csiostor: remove automatic irq affinity assignment
If number of interrupt vectors are more than num_online_cpus()
then pci_alloc_irq_vectors_affinity() assigns cpumask based
on num_possible_cpus() to the remaining vectors because of
this interrupts are not generating for these vectors.
This patch fixes this issue by using pci_alloc_irq_vectors()
instead of pci_alloc_irq_vectors_affinity().
Signed-off-by: Varun Prakash
Cc: Christoph Hellwig
Cc: Thomas Gleixner
---
drivers/scsi/csiostor/csio_isr.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
index 7c88147..8b92c59 100644
--- a/drivers/scsi/csiostor/csio_isr.c
+++ b/drivers/scsi/csiostor/csio_isr.c
@@ -480,7 +480,6 @@ csio_enable_msix(struct csio_hw *hw)
int i, j, k, n, min, cnt;
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;
@@ -491,8 +490,7 @@ csio_enable_msix(struct csio_hw *hw)
csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
- cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
- PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, );
+ cnt = pci_alloc_irq_vectors(hw->pdev, min, cnt, PCI_IRQ_MSIX);
if (cnt < 0)
return cnt;
--
2.0.2
[PATCH] scsi: csiostor: remove automatic irq affinity assignment
If number of interrupt vectors are more than num_online_cpus()
then pci_alloc_irq_vectors_affinity() assigns cpumask based
on num_possible_cpus() to the remaining vectors because of
this interrupts are not generating for these vectors.
This patch fixes this issue by using pci_alloc_irq_vectors()
instead of pci_alloc_irq_vectors_affinity().
Signed-off-by: Varun Prakash
---
drivers/scsi/csiostor/csio_isr.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
index 7c88147..8b92c59 100644
--- a/drivers/scsi/csiostor/csio_isr.c
+++ b/drivers/scsi/csiostor/csio_isr.c
@@ -480,7 +480,6 @@ csio_enable_msix(struct csio_hw *hw)
int i, j, k, n, min, cnt;
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;
@@ -491,8 +490,7 @@ csio_enable_msix(struct csio_hw *hw)
csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
- cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
- PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, );
+ cnt = pci_alloc_irq_vectors(hw->pdev, min, cnt, PCI_IRQ_MSIX);
if (cnt < 0)
return cnt;
--
2.0.2
Re: [PATCH] scsi: csiostor: remove automatic irq affinity assignment
On Wed, Aug 01, 2018 at 05:18:25PM +0530, Varun Prakash wrote:
> On Wed, Aug 01, 2018 at 08:33:23AM +0200, Hannes Reinecke wrote:
> > On 07/31/2018 05:07 PM, Varun Prakash wrote:
> > > If number of interrupt vectors are more than num_online_cpus()
> > > then pci_alloc_irq_vectors_affinity() assigns cpumask based
> > > on num_possible_cpus() to the remaining vectors because of
> > > this interrupt does not generate for these vectors.
> > >
> > > This patch fixes this issue by using pci_alloc_irq_vectors()
> > > instead of pci_alloc_irq_vectors_affinity().
> > >
> > > Signed-off-by: Varun Prakash
> > > ---
> > > drivers/scsi/csiostor/csio_isr.c | 4 +---
> > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > >
>
>
> > >
> > > - cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
> > > - PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, );
> > > + cnt = pci_alloc_irq_vectors(hw->pdev, min, cnt, PCI_IRQ_MSIX);
> > > if (cnt < 0)
> > > return cnt;
> > >
> > >
> > Hmm.
> > That patch (and the reasoning leading up to it) really sound dodgy.
> > I'd rather fix the interrupt affinity code to handle this case correctly.
>
> Yes, it is better to fix the affinity assignment code, I posted
> this patch to start a discussion on this issue.
>
> I can confirm that if number of interrupt vectors are more than
> num_online cpus() then pci_alloc_irq_vectors_affinity() assigns
> cpumask based on cpu_possible_mask to the remaining vectors.
>
> Following is the logic for creating affinity masks
>
> struct cpumask *
> irq_create_affinity_masks(int nvecs, const struct irq_affinity *affd)
> {
>...
> /* Spread on present CPUs starting from affd->pre_vectors */
> usedvecs = irq_build_affinity_masks(affd, curvec, affvecs,
> node_to_cpumask, cpu_present_mask,
> nmsk, masks);
>
> /*
> * Spread on non present CPUs starting from the next vector to be
> * handled. If the spreading of present CPUs already exhausted the
> * vector space, assign the non present CPUs to the already spread
> * out vectors.
> */
> if (usedvecs >= affvecs)
> curvec = affd->pre_vectors;
> else
> curvec = affd->pre_vectors + usedvecs;
> cpumask_andnot(npresmsk, cpu_possible_mask, cpu_present_mask);
> usedvecs += irq_build_affinity_masks(affd, curvec, affvecs,
> node_to_cpumask, npresmsk,
> nmsk, masks);
> ...
> }
>
> > Thomas, can you help here?
Any comments on this issue?
Re: [PATCH] scsi: csiostor: remove automatic irq affinity assignment
On Wed, Aug 01, 2018 at 08:33:23AM +0200, Hannes Reinecke wrote:
> On 07/31/2018 05:07 PM, Varun Prakash wrote:
> > If number of interrupt vectors are more than num_online_cpus()
> > then pci_alloc_irq_vectors_affinity() assigns cpumask based
> > on num_possible_cpus() to the remaining vectors because of
> > this interrupt does not generate for these vectors.
> >
> > This patch fixes this issue by using pci_alloc_irq_vectors()
> > instead of pci_alloc_irq_vectors_affinity().
> >
> > Signed-off-by: Varun Prakash
> > ---
> > drivers/scsi/csiostor/csio_isr.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> >
> > - cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
> > - PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, );
> > + cnt = pci_alloc_irq_vectors(hw->pdev, min, cnt, PCI_IRQ_MSIX);
> > if (cnt < 0)
> > return cnt;
> >
> >
> Hmm.
> That patch (and the reasoning leading up to it) really sound dodgy.
> I'd rather fix the interrupt affinity code to handle this case correctly.
Yes, it is better to fix the affinity assignment code, I posted
this patch to start a discussion on this issue.
I can confirm that if number of interrupt vectors are more than
num_online cpus() then pci_alloc_irq_vectors_affinity() assigns
cpumask based on cpu_possible_mask to the remaining vectors.
Following is the logic for creating affinity masks
struct cpumask *
irq_create_affinity_masks(int nvecs, const struct irq_affinity *affd)
{
...
/* Spread on present CPUs starting from affd->pre_vectors */
usedvecs = irq_build_affinity_masks(affd, curvec, affvecs,
node_to_cpumask, cpu_present_mask,
nmsk, masks);
/*
* Spread on non present CPUs starting from the next vector to be
* handled. If the spreading of present CPUs already exhausted the
* vector space, assign the non present CPUs to the already spread
* out vectors.
*/
if (usedvecs >= affvecs)
curvec = affd->pre_vectors;
else
curvec = affd->pre_vectors + usedvecs;
cpumask_andnot(npresmsk, cpu_possible_mask, cpu_present_mask);
usedvecs += irq_build_affinity_masks(affd, curvec, affvecs,
node_to_cpumask, npresmsk,
nmsk, masks);
...
}
> Thomas, can you help here?
>
Re: [PATCH] scsi: csiostor: remove automatic irq affinity assignment
On 07/31/2018 05:07 PM, Varun Prakash wrote:
> If number of interrupt vectors are more than num_online_cpus()
> then pci_alloc_irq_vectors_affinity() assigns cpumask based
> on num_possible_cpus() to the remaining vectors because of
> this interrupt does not generate for these vectors.
>
> This patch fixes this issue by using pci_alloc_irq_vectors()
> instead of pci_alloc_irq_vectors_affinity().
>
> Signed-off-by: Varun Prakash
> ---
> drivers/scsi/csiostor/csio_isr.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/csiostor/csio_isr.c
> b/drivers/scsi/csiostor/csio_isr.c
> index 7c88147..8b92c59 100644
> --- a/drivers/scsi/csiostor/csio_isr.c
> +++ b/drivers/scsi/csiostor/csio_isr.c
> @@ -480,7 +480,6 @@ csio_enable_msix(struct csio_hw *hw)
> int i, j, k, n, min, cnt;
> 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;
> @@ -491,8 +490,7 @@ csio_enable_msix(struct csio_hw *hw)
>
> csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
>
> - cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
> - PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, );
> + cnt = pci_alloc_irq_vectors(hw->pdev, min, cnt, PCI_IRQ_MSIX);
> if (cnt < 0)
> return cnt;
>
>
Hmm.
That patch (and the reasoning leading up to it) really sound dodgy.
I'd rather fix the interrupt affinity code to handle this case correctly.
Thomas, can you help here?
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
[PATCH] scsi: csiostor: remove automatic irq affinity assignment
If number of interrupt vectors are more than num_online_cpus()
then pci_alloc_irq_vectors_affinity() assigns cpumask based
on num_possible_cpus() to the remaining vectors because of
this interrupt does not generate for these vectors.
This patch fixes this issue by using pci_alloc_irq_vectors()
instead of pci_alloc_irq_vectors_affinity().
Signed-off-by: Varun Prakash
---
drivers/scsi/csiostor/csio_isr.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
index 7c88147..8b92c59 100644
--- a/drivers/scsi/csiostor/csio_isr.c
+++ b/drivers/scsi/csiostor/csio_isr.c
@@ -480,7 +480,6 @@ csio_enable_msix(struct csio_hw *hw)
int i, j, k, n, min, cnt;
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;
@@ -491,8 +490,7 @@ csio_enable_msix(struct csio_hw *hw)
csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
- cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
- PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, );
+ cnt = pci_alloc_irq_vectors(hw->pdev, min, cnt, PCI_IRQ_MSIX);
if (cnt < 0)
return cnt;
--
2.0.2
