linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] powerpc/pseries: fix MSI/X IRQ affinity on pseries
@ 2020-11-25 11:16 Laurent Vivier
  2020-11-25 11:16 ` [PATCH v2 1/2] genirq: add an irq_create_mapping_affinity() function Laurent Vivier
  2020-11-25 11:16 ` [PATCH v2 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 11:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-block, Paul Mackerras, linux-pci, Thomas Gleixner,
	Marc Zyngier, linuxppc-dev, Benjamin Herrenschmidt,
	Michael S . Tsirkin, Greg Kurz, Christoph Hellwig,
	Michael Ellerman, 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
v2: add a wrapper around original irq_create_mapping() with the\r
    affinity parameter. Update comments\r
\r
Laurent Vivier (2):\r
  genirq: 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 v2 1/2] genirq: add an irq_create_mapping_affinity() function
  2020-11-25 11:16 [PATCH v2 0/2] powerpc/pseries: fix MSI/X IRQ affinity on pseries Laurent Vivier
@ 2020-11-25 11:16 ` Laurent Vivier
  2020-11-25 12:45   ` Greg Kurz
  2020-11-25 13:20   ` Thomas Gleixner
  2020-11-25 11:16 ` [PATCH v2 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping() Laurent Vivier
  1 sibling, 2 replies; 9+ messages in thread
From: Laurent Vivier @ 2020-11-25 11:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-block, Paul Mackerras, linux-pci, Thomas Gleixner,
	Marc Zyngier, linuxppc-dev, Benjamin Herrenschmidt,
	Michael S . Tsirkin, Greg Kurz, Christoph Hellwig,
	Michael Ellerman, Laurent Vivier

This function adds an affinity parameter to irq_create_mapping().
This parameter is needed to pass it to irq_domain_alloc_descs().

irq_create_mapping() is a wrapper around irq_create_mapping_affinity()
to pass NULL for the affinity parameter.

No functional change.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 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 v2 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping()
  2020-11-25 11:16 [PATCH v2 0/2] powerpc/pseries: fix MSI/X IRQ affinity on pseries Laurent Vivier
  2020-11-25 11:16 ` [PATCH v2 1/2] genirq: add an irq_create_mapping_affinity() function Laurent Vivier
@ 2020-11-25 11:16 ` Laurent Vivier
  2020-11-25 12:51   ` Greg Kurz
  1 sibling, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2020-11-25 11:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-block, Paul Mackerras, linux-pci, Thomas Gleixner,
	Marc Zyngier, linuxppc-dev, Benjamin Herrenschmidt,
	Michael S . Tsirkin, Greg Kurz, Christoph Hellwig,
	Michael Ellerman, 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.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 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 v2 1/2] genirq: add an irq_create_mapping_affinity() function
  2020-11-25 11:16 ` [PATCH v2 1/2] genirq: add an irq_create_mapping_affinity() function Laurent Vivier
@ 2020-11-25 12:45   ` Greg Kurz
  2020-11-25 13:20   ` Thomas Gleixner
  1 sibling, 0 replies; 9+ messages in thread
From: Greg Kurz @ 2020-11-25 12:45 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: linux-kernel, linux-block, Paul Mackerras, linux-pci,
	Thomas Gleixner, Marc Zyngier, linuxppc-dev,
	Benjamin Herrenschmidt, Michael S . Tsirkin, Christoph Hellwig,
	Michael Ellerman

On Wed, 25 Nov 2020 12:16:56 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> This function adds an affinity parameter to irq_create_mapping().
> This parameter is needed to pass it to irq_domain_alloc_descs().
> 
> irq_create_mapping() is a wrapper around irq_create_mapping_affinity()
> to pass NULL for the affinity parameter.
> 
> No functional change.
> 
> 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


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

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

On Wed, 25 Nov 2020 12:16:57 +0100
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.
> 
> 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.
> 

Since it is public, maybe add:

BugId: https://bugzilla.redhat.com/show_bug.cgi?id=1702939

?

> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

Anyway,

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);


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

* Re: [PATCH v2 1/2] genirq: add an irq_create_mapping_affinity() function
  2020-11-25 11:16 ` [PATCH v2 1/2] genirq: add an irq_create_mapping_affinity() function Laurent Vivier
  2020-11-25 12:45   ` Greg Kurz
@ 2020-11-25 13:20   ` Thomas Gleixner
  2020-11-25 14:09     ` Laurent Vivier
  1 sibling, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2020-11-25 13:20 UTC (permalink / raw)
  To: Laurent Vivier, linux-kernel
  Cc: linux-block, Paul Mackerras, linux-pci, Marc Zyngier,
	linuxppc-dev, Benjamin Herrenschmidt, Michael S . Tsirkin,
	Greg Kurz, Christoph Hellwig, Michael Ellerman, Laurent Vivier

Laurent,

On Wed, Nov 25 2020 at 12:16, Laurent Vivier wrote:

The proper subsystem prefix is: 'genirq/irqdomain:' and the first letter
after the colon wants to be uppercase.

> This function adds an affinity parameter to irq_create_mapping().
> This parameter is needed to pass it to irq_domain_alloc_descs().

A changelog has to explain the WHY. 'The parameter is needed' is not
really useful information.

Thanks,

        tglx


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

* Re: [PATCH v2 1/2] genirq: add an irq_create_mapping_affinity() function
  2020-11-25 13:20   ` Thomas Gleixner
@ 2020-11-25 14:09     ` Laurent Vivier
  2020-11-25 14:54       ` Marc Zyngier
  0 siblings, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2020-11-25 14:09 UTC (permalink / raw)
  To: Thomas Gleixner, linux-kernel
  Cc: linux-block, Paul Mackerras, linux-pci, Marc Zyngier,
	linuxppc-dev, Benjamin Herrenschmidt, Michael S . Tsirkin,
	Greg Kurz, Christoph Hellwig, Michael Ellerman

On 25/11/2020 14:20, Thomas Gleixner wrote:
> Laurent,
> 
> On Wed, Nov 25 2020 at 12:16, Laurent Vivier wrote:
> 
> The proper subsystem prefix is: 'genirq/irqdomain:' and the first letter
> after the colon wants to be uppercase.

Ok.

>> This function adds an affinity parameter to irq_create_mapping().
>> This parameter is needed to pass it to irq_domain_alloc_descs().
> 
> A changelog has to explain the WHY. 'The parameter is needed' is not
> really useful information.
> 

The reason of this change is explained in PATCH 2.

I have two patches, one to change the interface with no functional change (PATCH 1) and
one to fix the problem (PATCH 2). Moreover they don't cover the same subsystems.

I can either:
- merge the two patches
- or make a reference in the changelog of PATCH 1 to PATCH 2
  (something like "(see folowing patch "powerpc/pseries: pass MSI affinity to
   irq_create_mapping()")")
- or copy some information from PATCH 2
  (something like "this parameter is needed by rtas_setup_msi_irqs() to pass the affinity
   to irq_domain_alloc_descs() to fix multiqueue affinity")

What do you prefer?

Thanks,
Laurent


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

* Re: [PATCH v2 1/2] genirq: add an irq_create_mapping_affinity() function
  2020-11-25 14:09     ` Laurent Vivier
@ 2020-11-25 14:54       ` Marc Zyngier
  2020-11-25 15:00         ` Laurent Vivier
  0 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2020-11-25 14:54 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Thomas Gleixner, linux-kernel, linux-block, Paul Mackerras,
	linux-pci, linuxppc-dev, Benjamin Herrenschmidt,
	Michael S . Tsirkin, Greg Kurz, Christoph Hellwig,
	Michael Ellerman

On 2020-11-25 14:09, Laurent Vivier wrote:
> On 25/11/2020 14:20, Thomas Gleixner wrote:
>> Laurent,
>> 
>> On Wed, Nov 25 2020 at 12:16, Laurent Vivier wrote:
>> 
>> The proper subsystem prefix is: 'genirq/irqdomain:' and the first 
>> letter
>> after the colon wants to be uppercase.
> 
> Ok.
> 
>>> This function adds an affinity parameter to irq_create_mapping().
>>> This parameter is needed to pass it to irq_domain_alloc_descs().
>> 
>> A changelog has to explain the WHY. 'The parameter is needed' is not
>> really useful information.
>> 
> 
> The reason of this change is explained in PATCH 2.
> 
> I have two patches, one to change the interface with no functional
> change (PATCH 1) and
> one to fix the problem (PATCH 2). Moreover they don't cover the same 
> subsystems.
> 
> I can either:
> - merge the two patches
> - or make a reference in the changelog of PATCH 1 to PATCH 2
>   (something like "(see folowing patch "powerpc/pseries: pass MSI 
> affinity to
>    irq_create_mapping()")")
> - or copy some information from PATCH 2
>   (something like "this parameter is needed by rtas_setup_msi_irqs()
> to pass the affinity
>    to irq_domain_alloc_descs() to fix multiqueue affinity")
> 
> What do you prefer?

How about something like this for the first patch:

"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()."

Thanks,

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

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

* Re: [PATCH v2 1/2] genirq: add an irq_create_mapping_affinity() function
  2020-11-25 14:54       ` Marc Zyngier
@ 2020-11-25 15:00         ` Laurent Vivier
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2020-11-25 15:00 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Thomas Gleixner, linux-kernel, linux-block, Paul Mackerras,
	linux-pci, linuxppc-dev, Benjamin Herrenschmidt,
	Michael S . Tsirkin, Greg Kurz, Christoph Hellwig,
	Michael Ellerman

On 25/11/2020 15:54, Marc Zyngier wrote:
> On 2020-11-25 14:09, Laurent Vivier wrote:
>> On 25/11/2020 14:20, Thomas Gleixner wrote:
>>> Laurent,
>>>
>>> On Wed, Nov 25 2020 at 12:16, Laurent Vivier wrote:
>>>
>>> The proper subsystem prefix is: 'genirq/irqdomain:' and the first letter
>>> after the colon wants to be uppercase.
>>
>> Ok.
>>
>>>> This function adds an affinity parameter to irq_create_mapping().
>>>> This parameter is needed to pass it to irq_domain_alloc_descs().
>>>
>>> A changelog has to explain the WHY. 'The parameter is needed' is not
>>> really useful information.
>>>
>>
>> The reason of this change is explained in PATCH 2.
>>
>> I have two patches, one to change the interface with no functional
>> change (PATCH 1) and
>> one to fix the problem (PATCH 2). Moreover they don't cover the same subsystems.
>>
>> I can either:
>> - merge the two patches
>> - or make a reference in the changelog of PATCH 1 to PATCH 2
>>   (something like "(see folowing patch "powerpc/pseries: pass MSI affinity to
>>    irq_create_mapping()")")
>> - or copy some information from PATCH 2
>>   (something like "this parameter is needed by rtas_setup_msi_irqs()
>> to pass the affinity
>>    to irq_domain_alloc_descs() to fix multiqueue affinity")
>>
>> What do you prefer?
> 
> How about something like this for the first patch:
> 
> "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()."

It looks perfect. I update the changelog with that.

Thanks,
Laurent


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

end of thread, other threads:[~2020-11-25 15:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-25 11:16 [PATCH v2 0/2] powerpc/pseries: fix MSI/X IRQ affinity on pseries Laurent Vivier
2020-11-25 11:16 ` [PATCH v2 1/2] genirq: add an irq_create_mapping_affinity() function Laurent Vivier
2020-11-25 12:45   ` Greg Kurz
2020-11-25 13:20   ` Thomas Gleixner
2020-11-25 14:09     ` Laurent Vivier
2020-11-25 14:54       ` Marc Zyngier
2020-11-25 15:00         ` Laurent Vivier
2020-11-25 11:16 ` [PATCH v2 2/2] powerpc/pseries: pass MSI affinity to irq_create_mapping() Laurent Vivier
2020-11-25 12:51   ` Greg Kurz

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).