linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] powerpc/pseries: fix MSI/X IRQ affinity on pseries
@ 2020-11-25 15:09 Laurent Vivier
  2020-11-25 15:09 ` [PATCH v3 1/2] genirq/irqdomain: Add an irq_create_mapping_affinity() function Laurent Vivier
  2020-11-25 15:09 ` [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping() Laurent Vivier
  0 siblings, 2 replies; 9+ messages in thread
From: Laurent Vivier @ 2020-11-25 15:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pci, Christoph Hellwig, Paul Mackerras, Greg Kurz,
	linuxppc-dev, Thomas Gleixner, Benjamin Herrenschmidt,
	Michael Ellerman, linux-block, Michael S . Tsirkin, Marc Zyngier,
	Laurent Vivier

With virtio, in multiqueue case, each queue IRQ is normally\r
bound to a different CPU using the affinity mask.\r
\r
This works fine on x86_64 but totally ignored on pseries.\r
\r
This is not obvious at first look because irqbalance is doing\r
some balancing to improve that.\r
\r
It appears that the "managed" flag set in the MSI entry\r
is never copied to the system IRQ entry.\r
\r
This series passes the affinity mask from rtas_setup_msi_irqs()\r
to irq_domain_alloc_descs() by adding an affinity parameter to\r
irq_create_mapping().\r
\r
The first patch adds the parameter (no functional change), the\r
second patch passes the actual affinity mask to irq_create_mapping()\r
in rtas_setup_msi_irqs().\r
\r
For instance, with 32 CPUs VM and 32 queues virtio-scsi interface:\r
\r
... -smp 32 -device virtio-scsi-pci,id=virtio_scsi_pci0,num_queues=32\r
\r
for IRQ in $(grep virtio2-request /proc/interrupts |cut -d: -f1); do\r
    for file in /proc/irq/$IRQ/ ; do\r
        echo -n "IRQ: $(basename $file) CPU: " ; cat $file/smp_affinity_list\r
    done\r
done\r
\r
Without the patch (and without irqbalanced)\r
\r
IRQ: 268 CPU: 0-31\r
IRQ: 269 CPU: 0-31\r
IRQ: 270 CPU: 0-31\r
IRQ: 271 CPU: 0-31\r
IRQ: 272 CPU: 0-31\r
IRQ: 273 CPU: 0-31\r
IRQ: 274 CPU: 0-31\r
IRQ: 275 CPU: 0-31\r
IRQ: 276 CPU: 0-31\r
IRQ: 277 CPU: 0-31\r
IRQ: 278 CPU: 0-31\r
IRQ: 279 CPU: 0-31\r
IRQ: 280 CPU: 0-31\r
IRQ: 281 CPU: 0-31\r
IRQ: 282 CPU: 0-31\r
IRQ: 283 CPU: 0-31\r
IRQ: 284 CPU: 0-31\r
IRQ: 285 CPU: 0-31\r
IRQ: 286 CPU: 0-31\r
IRQ: 287 CPU: 0-31\r
IRQ: 288 CPU: 0-31\r
IRQ: 289 CPU: 0-31\r
IRQ: 290 CPU: 0-31\r
IRQ: 291 CPU: 0-31\r
IRQ: 292 CPU: 0-31\r
IRQ: 293 CPU: 0-31\r
IRQ: 294 CPU: 0-31\r
IRQ: 295 CPU: 0-31\r
IRQ: 296 CPU: 0-31\r
IRQ: 297 CPU: 0-31\r
IRQ: 298 CPU: 0-31\r
IRQ: 299 CPU: 0-31\r
\r
With the patch:\r
\r
IRQ: 265 CPU: 0\r
IRQ: 266 CPU: 1\r
IRQ: 267 CPU: 2\r
IRQ: 268 CPU: 3\r
IRQ: 269 CPU: 4\r
IRQ: 270 CPU: 5\r
IRQ: 271 CPU: 6\r
IRQ: 272 CPU: 7\r
IRQ: 273 CPU: 8\r
IRQ: 274 CPU: 9\r
IRQ: 275 CPU: 10\r
IRQ: 276 CPU: 11\r
IRQ: 277 CPU: 12\r
IRQ: 278 CPU: 13\r
IRQ: 279 CPU: 14\r
IRQ: 280 CPU: 15\r
IRQ: 281 CPU: 16\r
IRQ: 282 CPU: 17\r
IRQ: 283 CPU: 18\r
IRQ: 284 CPU: 19\r
IRQ: 285 CPU: 20\r
IRQ: 286 CPU: 21\r
IRQ: 287 CPU: 22\r
IRQ: 288 CPU: 23\r
IRQ: 289 CPU: 24\r
IRQ: 290 CPU: 25\r
IRQ: 291 CPU: 26\r
IRQ: 292 CPU: 27\r
IRQ: 293 CPU: 28\r
IRQ: 294 CPU: 29\r
IRQ: 295 CPU: 30\r
IRQ: 299 CPU: 31\r
\r
This matches what we have on an x86_64 system.\r
\r
v3: update changelog of PATCH 1 with comments from Thomas Gleixner and\r
    Marc Zyngier.\r
v2: add a wrapper around original irq_create_mapping() with the\r
    affinity parameter. Update comments\r
\r
Laurent Vivier (2):\r
  genirq/irqdomain: Add an irq_create_mapping_affinity() function\r
  powerpc/pseries: pass MSI affinity to irq_create_mapping()\r
\r
 arch/powerpc/platforms/pseries/msi.c |  3 ++-\r
 include/linux/irqdomain.h            | 12 ++++++++++--\r
 kernel/irq/irqdomain.c               | 13 ++++++++-----\r
 3 files changed, 20 insertions(+), 8 deletions(-)\r
\r
-- \r
2.28.0\r
\r


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/2] genirq/irqdomain: Add an irq_create_mapping_affinity() function
  2020-11-25 15:09 [PATCH v3 0/2] powerpc/pseries: fix MSI/X IRQ affinity on pseries Laurent Vivier
@ 2020-11-25 15:09 ` Laurent Vivier
  2020-11-25 15:09 ` [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping() Laurent Vivier
  1 sibling, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2020-11-25 15:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pci, Christoph Hellwig, Paul Mackerras, Greg Kurz,
	linuxppc-dev, Thomas Gleixner, Benjamin Herrenschmidt,
	Michael Ellerman, linux-block, Michael S . Tsirkin, Marc Zyngier,
	Laurent Vivier

There is currently no way to convey the affinity of an interrupt
via irq_create_mapping(), which creates issues for devices that
expect that affinity to be managed by the kernel.

In order to sort this out, rename irq_create_mapping() to
irq_create_mapping_affinity() with an additional affinity parameter
that can conveniently passed down to irq_domain_alloc_descs().

irq_create_mapping() is then re-implemented as a wrapper around
irq_create_mapping_affinity().

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 include/linux/irqdomain.h | 12 ++++++++++--
 kernel/irq/irqdomain.c    | 13 ++++++++-----
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 71535e87109f..ea5a337e0f8b 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -384,11 +384,19 @@ extern void irq_domain_associate_many(struct irq_domain *domain,
 extern void irq_domain_disassociate(struct irq_domain *domain,
 				    unsigned int irq);
 
-extern unsigned int irq_create_mapping(struct irq_domain *host,
-				       irq_hw_number_t hwirq);
+extern unsigned int irq_create_mapping_affinity(struct irq_domain *host,
+				      irq_hw_number_t hwirq,
+				      const struct irq_affinity_desc *affinity);
 extern unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec);
 extern void irq_dispose_mapping(unsigned int virq);
 
+static inline unsigned int irq_create_mapping(struct irq_domain *host,
+					      irq_hw_number_t hwirq)
+{
+	return irq_create_mapping_affinity(host, hwirq, NULL);
+}
+
+
 /**
  * irq_linear_revmap() - Find a linux irq from a hw irq number.
  * @domain: domain owning this hardware interrupt
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index cf8b374b892d..e4ca69608f3b 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -624,17 +624,19 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
 EXPORT_SYMBOL_GPL(irq_create_direct_mapping);
 
 /**
- * irq_create_mapping() - Map a hardware interrupt into linux irq space
+ * irq_create_mapping_affinity() - Map a hardware interrupt into linux irq space
  * @domain: domain owning this hardware interrupt or NULL for default domain
  * @hwirq: hardware irq number in that domain space
+ * @affinity: irq affinity
  *
  * Only one mapping per hardware interrupt is permitted. Returns a linux
  * irq number.
  * If the sense/trigger is to be specified, set_irq_type() should be called
  * on the number returned from that call.
  */
-unsigned int irq_create_mapping(struct irq_domain *domain,
-				irq_hw_number_t hwirq)
+unsigned int irq_create_mapping_affinity(struct irq_domain *domain,
+				       irq_hw_number_t hwirq,
+				       const struct irq_affinity_desc *affinity)
 {
 	struct device_node *of_node;
 	int virq;
@@ -660,7 +662,8 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
 	}
 
 	/* Allocate a virtual interrupt number */
-	virq = irq_domain_alloc_descs(-1, 1, hwirq, of_node_to_nid(of_node), NULL);
+	virq = irq_domain_alloc_descs(-1, 1, hwirq, of_node_to_nid(of_node),
+				      affinity);
 	if (virq <= 0) {
 		pr_debug("-> virq allocation failed\n");
 		return 0;
@@ -676,7 +679,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
 
 	return virq;
 }
-EXPORT_SYMBOL_GPL(irq_create_mapping);
+EXPORT_SYMBOL_GPL(irq_create_mapping_affinity);
 
 /**
  * irq_create_strict_mappings() - Map a range of hw irqs to fixed linux irqs
-- 
2.28.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping()
  2020-11-25 15:09 [PATCH v3 0/2] powerpc/pseries: fix MSI/X IRQ affinity on pseries Laurent Vivier
  2020-11-25 15:09 ` [PATCH v3 1/2] genirq/irqdomain: Add an irq_create_mapping_affinity() function Laurent Vivier
@ 2020-11-25 15:09 ` Laurent Vivier
  2020-11-25 16:05   ` Denis Kirjanov
  2020-11-26  3:22   ` Michael Ellerman
  1 sibling, 2 replies; 9+ messages in thread
From: Laurent Vivier @ 2020-11-25 15:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pci, Christoph Hellwig, Paul Mackerras, Greg Kurz,
	linuxppc-dev, Thomas Gleixner, Benjamin Herrenschmidt,
	Michael Ellerman, linux-block, Michael S . Tsirkin, Marc Zyngier,
	Laurent Vivier

With virtio multiqueue, normally each queue IRQ is mapped to a CPU.

But since commit 0d9f0a52c8b9f ("virtio_scsi: use virtio IRQ affinity")
this is broken on pseries.

The affinity is correctly computed in msi_desc but this is not applied
to the system IRQs.

It appears the affinity is correctly passed to rtas_setup_msi_irqs() but
lost at this point and never passed to irq_domain_alloc_descs()
(see commit 06ee6d571f0e ("genirq: Add affinity hint to irq allocation"))
because irq_create_mapping() doesn't take an affinity parameter.

As the previous patch has added the affinity parameter to
irq_create_mapping() we can forward the affinity from rtas_setup_msi_irqs()
to irq_domain_alloc_descs().

With this change, the virtqueues are correctly dispatched between the CPUs
on pseries.

BugId: https://bugzilla.redhat.com/show_bug.cgi?id=1702939
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 arch/powerpc/platforms/pseries/msi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 133f6adcb39c..b3ac2455faad 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -458,7 +458,8 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
 			return hwirq;
 		}
 
-		virq = irq_create_mapping(NULL, hwirq);
+		virq = irq_create_mapping_affinity(NULL, hwirq,
+						   entry->affinity);
 
 		if (!virq) {
 			pr_debug("rtas_msi: Failed mapping hwirq %d\n", hwirq);
-- 
2.28.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping()
  2020-11-25 15:09 ` [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping() Laurent Vivier
@ 2020-11-25 16:05   ` Denis Kirjanov
  2020-11-25 16:24     ` Laurent Vivier
  2020-11-26  3:22   ` Michael Ellerman
  1 sibling, 1 reply; 9+ messages in thread
From: Denis Kirjanov @ 2020-11-25 16:05 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: linux-kernel, linux-pci, Christoph Hellwig, Paul Mackerras,
	Greg Kurz, linuxppc-dev, Thomas Gleixner, Benjamin Herrenschmidt,
	Michael Ellerman, linux-block, Michael S . Tsirkin, Marc Zyngier

On 11/25/20, Laurent Vivier <lvivier@redhat.com> wrote:
> With virtio multiqueue, normally each queue IRQ is mapped to a CPU.
>
> But since commit 0d9f0a52c8b9f ("virtio_scsi: use virtio IRQ affinity")
> this is broken on pseries.

Please add "Fixes" tag.

Thanks!

>
> The affinity is correctly computed in msi_desc but this is not applied
> to the system IRQs.
>
> It appears the affinity is correctly passed to rtas_setup_msi_irqs() but
> lost at this point and never passed to irq_domain_alloc_descs()
> (see commit 06ee6d571f0e ("genirq: Add affinity hint to irq allocation"))
> because irq_create_mapping() doesn't take an affinity parameter.
>
> As the previous patch has added the affinity parameter to
> irq_create_mapping() we can forward the affinity from rtas_setup_msi_irqs()
> to irq_domain_alloc_descs().
>
> With this change, the virtqueues are correctly dispatched between the CPUs
> on pseries.
>
> BugId: https://bugzilla.redhat.com/show_bug.cgi?id=1702939
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
>  arch/powerpc/platforms/pseries/msi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/msi.c
> b/arch/powerpc/platforms/pseries/msi.c
> index 133f6adcb39c..b3ac2455faad 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -458,7 +458,8 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int
> nvec_in, int type)
>  			return hwirq;
>  		}
>
> -		virq = irq_create_mapping(NULL, hwirq);
> +		virq = irq_create_mapping_affinity(NULL, hwirq,
> +						   entry->affinity);
>
>  		if (!virq) {
>  			pr_debug("rtas_msi: Failed mapping hwirq %d\n", hwirq);
> --
> 2.28.0
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping()
  2020-11-25 16:05   ` Denis Kirjanov
@ 2020-11-25 16:24     ` Laurent Vivier
  2020-11-25 16:42       ` Marc Zyngier
  0 siblings, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2020-11-25 16:24 UTC (permalink / raw)
  To: Denis Kirjanov
  Cc: linux-kernel, linux-pci, Christoph Hellwig, Paul Mackerras,
	Greg Kurz, linuxppc-dev, Thomas Gleixner, Benjamin Herrenschmidt,
	Michael Ellerman, linux-block, Michael S . Tsirkin, Marc Zyngier

On 25/11/2020 17:05, Denis Kirjanov wrote:
> On 11/25/20, Laurent Vivier <lvivier@redhat.com> wrote:
>> With virtio multiqueue, normally each queue IRQ is mapped to a CPU.
>>
>> But since commit 0d9f0a52c8b9f ("virtio_scsi: use virtio IRQ affinity")
>> this is broken on pseries.
> 
> Please add "Fixes" tag.

In fact, the code in commit 0d9f0a52c8b9f is correct.

The problem is with MSI/X irq affinity and pseries. So this patch fixes more than
virtio_scsi. I put this information because this commit allows to clearly show the
problem. Perhaps I should remove this line in fact?

Thanks,
Laurent

> 
> Thanks!
> 
>>
>> The affinity is correctly computed in msi_desc but this is not applied
>> to the system IRQs.
>>
>> It appears the affinity is correctly passed to rtas_setup_msi_irqs() but
>> lost at this point and never passed to irq_domain_alloc_descs()
>> (see commit 06ee6d571f0e ("genirq: Add affinity hint to irq allocation"))
>> because irq_create_mapping() doesn't take an affinity parameter.
>>
>> As the previous patch has added the affinity parameter to
>> irq_create_mapping() we can forward the affinity from rtas_setup_msi_irqs()
>> to irq_domain_alloc_descs().
>>
>> With this change, the virtqueues are correctly dispatched between the CPUs
>> on pseries.
>>
>> BugId: https://bugzilla.redhat.com/show_bug.cgi?id=1702939
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> Reviewed-by: Greg Kurz <groug@kaod.org>
>> ---
>>  arch/powerpc/platforms/pseries/msi.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/msi.c
>> b/arch/powerpc/platforms/pseries/msi.c
>> index 133f6adcb39c..b3ac2455faad 100644
>> --- a/arch/powerpc/platforms/pseries/msi.c
>> +++ b/arch/powerpc/platforms/pseries/msi.c
>> @@ -458,7 +458,8 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int
>> nvec_in, int type)
>>  			return hwirq;
>>  		}
>>
>> -		virq = irq_create_mapping(NULL, hwirq);
>> +		virq = irq_create_mapping_affinity(NULL, hwirq,
>> +						   entry->affinity);
>>
>>  		if (!virq) {
>>  			pr_debug("rtas_msi: Failed mapping hwirq %d\n", hwirq);
>> --
>> 2.28.0
>>
>>
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping()
  2020-11-25 16:24     ` Laurent Vivier
@ 2020-11-25 16:42       ` Marc Zyngier
  2020-11-25 17:34         ` Greg Kurz
  2020-11-26  3:22         ` Michael Ellerman
  0 siblings, 2 replies; 9+ messages in thread
From: Marc Zyngier @ 2020-11-25 16:42 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Denis Kirjanov, linux-kernel, linux-pci, Christoph Hellwig,
	Paul Mackerras, Greg Kurz, linuxppc-dev, Thomas Gleixner,
	Benjamin Herrenschmidt, Michael Ellerman, linux-block,
	Michael S . Tsirkin

On 2020-11-25 16:24, Laurent Vivier wrote:
> On 25/11/2020 17:05, Denis Kirjanov wrote:
>> On 11/25/20, Laurent Vivier <lvivier@redhat.com> wrote:
>>> With virtio multiqueue, normally each queue IRQ is mapped to a CPU.
>>> 
>>> But since commit 0d9f0a52c8b9f ("virtio_scsi: use virtio IRQ 
>>> affinity")
>>> this is broken on pseries.
>> 
>> Please add "Fixes" tag.
> 
> In fact, the code in commit 0d9f0a52c8b9f is correct.
> 
> The problem is with MSI/X irq affinity and pseries. So this patch
> fixes more than virtio_scsi. I put this information because this
> commit allows to clearly show the problem. Perhaps I should remove
> this line in fact?

This patch does not fix virtio_scsi at all, which as you noticed, is
correct. It really fixes the PPC MSI setup, which is starting to show
its age. So getting rid of the reference seems like the right thing to 
do.

I'm also not keen on the BugId thing. It should really be a lore link.
I also cannot find any such tag in the kernel, nor is it a documented
practice. The last reference to a Bugzilla entry seems to have happened
with 786b5219081ff16 (five years ago).

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping()
  2020-11-25 16:42       ` Marc Zyngier
@ 2020-11-25 17:34         ` Greg Kurz
  2020-11-26  3:22         ` Michael Ellerman
  1 sibling, 0 replies; 9+ messages in thread
From: Greg Kurz @ 2020-11-25 17:34 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Laurent Vivier, Denis Kirjanov, linux-kernel, linux-pci,
	Christoph Hellwig, Paul Mackerras, linuxppc-dev, Thomas Gleixner,
	Benjamin Herrenschmidt, Michael Ellerman, linux-block,
	Michael S . Tsirkin

On Wed, 25 Nov 2020 16:42:30 +0000
Marc Zyngier <maz@kernel.org> wrote:

> On 2020-11-25 16:24, Laurent Vivier wrote:
> > On 25/11/2020 17:05, Denis Kirjanov wrote:
> >> On 11/25/20, Laurent Vivier <lvivier@redhat.com> wrote:
> >>> With virtio multiqueue, normally each queue IRQ is mapped to a CPU.
> >>> 
> >>> But since commit 0d9f0a52c8b9f ("virtio_scsi: use virtio IRQ 
> >>> affinity")
> >>> this is broken on pseries.
> >> 
> >> Please add "Fixes" tag.
> > 
> > In fact, the code in commit 0d9f0a52c8b9f is correct.
> > 
> > The problem is with MSI/X irq affinity and pseries. So this patch
> > fixes more than virtio_scsi. I put this information because this
> > commit allows to clearly show the problem. Perhaps I should remove
> > this line in fact?
> 
> This patch does not fix virtio_scsi at all, which as you noticed, is
> correct. It really fixes the PPC MSI setup, which is starting to show
> its age. So getting rid of the reference seems like the right thing to 
> do.
> 
> I'm also not keen on the BugId thing. It should really be a lore link.
> I also cannot find any such tag in the kernel, nor is it a documented
> practice. The last reference to a Bugzilla entry seems to have happened
> with 786b5219081ff16 (five years ago).
> 

My bad, I suggested BugId to Laurent but the intent was actually BugLink,
which seems to be commonly used in the kernel.

Cheers,

--
Greg

> Thanks,
> 
>          M.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping()
  2020-11-25 16:42       ` Marc Zyngier
  2020-11-25 17:34         ` Greg Kurz
@ 2020-11-26  3:22         ` Michael Ellerman
  1 sibling, 0 replies; 9+ messages in thread
From: Michael Ellerman @ 2020-11-26  3:22 UTC (permalink / raw)
  To: Marc Zyngier, Laurent Vivier
  Cc: Denis Kirjanov, linux-kernel, linux-pci, Christoph Hellwig,
	Paul Mackerras, Greg Kurz, linuxppc-dev, Thomas Gleixner,
	Benjamin Herrenschmidt, linux-block, Michael S . Tsirkin

Marc Zyngier <maz@kernel.org> writes:
> On 2020-11-25 16:24, Laurent Vivier wrote:
>> On 25/11/2020 17:05, Denis Kirjanov wrote:
>>> On 11/25/20, Laurent Vivier <lvivier@redhat.com> wrote:
>>>> With virtio multiqueue, normally each queue IRQ is mapped to a CPU.
>>>> 
>>>> But since commit 0d9f0a52c8b9f ("virtio_scsi: use virtio IRQ 
>>>> affinity")
>>>> this is broken on pseries.
>>> 
>>> Please add "Fixes" tag.
>> 
>> In fact, the code in commit 0d9f0a52c8b9f is correct.
>> 
>> The problem is with MSI/X irq affinity and pseries. So this patch
>> fixes more than virtio_scsi. I put this information because this
>> commit allows to clearly show the problem. Perhaps I should remove
>> this line in fact?
>
> This patch does not fix virtio_scsi at all, which as you noticed, is
> correct. It really fixes the PPC MSI setup, which is starting to show
> its age. So getting rid of the reference seems like the right thing to 
> do.

It's still useful to refer to that commit if the code worked prior to
that commit. But you should make it clearer that 0d9f0a52c8b9f wasn't in
error, it just exposed an existing shortcoming of the arch code.

cheers

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping()
  2020-11-25 15:09 ` [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping() Laurent Vivier
  2020-11-25 16:05   ` Denis Kirjanov
@ 2020-11-26  3:22   ` Michael Ellerman
  1 sibling, 0 replies; 9+ messages in thread
From: Michael Ellerman @ 2020-11-26  3:22 UTC (permalink / raw)
  To: Laurent Vivier, linux-kernel
  Cc: linux-pci, Christoph Hellwig, Paul Mackerras, Greg Kurz,
	linuxppc-dev, Thomas Gleixner, Benjamin Herrenschmidt,
	linux-block, Michael S . Tsirkin, Marc Zyngier, Laurent Vivier

Laurent Vivier <lvivier@redhat.com> writes:
> With virtio multiqueue, normally each queue IRQ is mapped to a CPU.
>
> But since commit 0d9f0a52c8b9f ("virtio_scsi: use virtio IRQ affinity")
> this is broken on pseries.
>
> The affinity is correctly computed in msi_desc but this is not applied
> to the system IRQs.
>
> It appears the affinity is correctly passed to rtas_setup_msi_irqs() but
> lost at this point and never passed to irq_domain_alloc_descs()
> (see commit 06ee6d571f0e ("genirq: Add affinity hint to irq allocation"))
> because irq_create_mapping() doesn't take an affinity parameter.
>
> As the previous patch has added the affinity parameter to
> irq_create_mapping() we can forward the affinity from rtas_setup_msi_irqs()
> to irq_domain_alloc_descs().
>
> With this change, the virtqueues are correctly dispatched between the CPUs
> on pseries.
>
> BugId: https://bugzilla.redhat.com/show_bug.cgi?id=1702939
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
>  arch/powerpc/platforms/pseries/msi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

cheers

> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index 133f6adcb39c..b3ac2455faad 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -458,7 +458,8 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
>  			return hwirq;
>  		}
>  
> -		virq = irq_create_mapping(NULL, hwirq);
> +		virq = irq_create_mapping_affinity(NULL, hwirq,
> +						   entry->affinity);
>  
>  		if (!virq) {
>  			pr_debug("rtas_msi: Failed mapping hwirq %d\n", hwirq);
> -- 
> 2.28.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-11-26  3:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-25 15:09 [PATCH v3 0/2] powerpc/pseries: fix MSI/X IRQ affinity on pseries Laurent Vivier
2020-11-25 15:09 ` [PATCH v3 1/2] genirq/irqdomain: Add an irq_create_mapping_affinity() function Laurent Vivier
2020-11-25 15:09 ` [PATCH v3 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping() Laurent Vivier
2020-11-25 16:05   ` Denis Kirjanov
2020-11-25 16:24     ` Laurent Vivier
2020-11-25 16:42       ` Marc Zyngier
2020-11-25 17:34         ` Greg Kurz
2020-11-26  3:22         ` Michael Ellerman
2020-11-26  3:22   ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).