2017-06-15 11:42:16

by Ralf Baechle

[permalink] [raw]
Subject: Re: [PATCH V5 6/9] MIPS: Loongson-3: support irq_set_affinity() in i8259 chip

On Thu, Jun 15, 2017 at 10:31:05AM +0800, Huacai Chen wrote:

> With this patch we can set irq affinity via procfs, so as to improve
> network performance.
>
> Signed-off-by: Huacai Chen <[email protected]>
> ---
> arch/mips/include/asm/irq.h | 3 ++
> arch/mips/loongson64/loongson-3/irq.c | 62 +++++++++++++++++++++++++++--------
> drivers/irqchip/irq-i8259.c | 3 ++
> 3 files changed, 55 insertions(+), 13 deletions(-)

You didn't cc the IRQCHIP maintainers:

IRQCHIP DRIVERS
M: Thomas Gleixner <[email protected]>
M: Jason Cooper <[email protected]>
M: Marc Zyngier <[email protected]>
L: [email protected]
S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core
T: git git://git.infradead.org/users/jcooper/linux.git irqchip/core
F: Documentation/devicetree/bindings/interrupt-controller/
F: drivers/irqchip/

Ralf


> diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
> index ddd1c91..47ff7c6 100644
> --- a/arch/mips/include/asm/irq.h
> +++ b/arch/mips/include/asm/irq.h
> @@ -53,6 +53,7 @@ static inline int irq_canonicalize(int irq)
> #define irq_canonicalize(irq) (irq) /* Sane hardware, sane code ... */
> #endif
>
> +struct irq_data;
> asmlinkage void plat_irq_dispatch(void);
>
> extern void do_IRQ(unsigned int irq);
> @@ -63,6 +64,8 @@ extern void spurious_interrupt(void);
> extern int allocate_irqno(void);
> extern void alloc_legacy_irqno(void);
> extern void free_irqno(unsigned int irq);
> +extern int plat_set_irq_affinity(struct irq_data *d,
> + const struct cpumask *affinity, bool force);
>
> /*
> * Before R2 the timer and performance counter interrupts were both fixed to
> diff --git a/arch/mips/loongson64/loongson-3/irq.c b/arch/mips/loongson64/loongson-3/irq.c
> index 2e6e205..e8b7a47 100644
> --- a/arch/mips/loongson64/loongson-3/irq.c
> +++ b/arch/mips/loongson64/loongson-3/irq.c
> @@ -10,32 +10,68 @@
> #include "smp.h"
>
> extern void loongson3_send_irq_by_ipi(int cpu, int irqs);
> +
> +unsigned int irq_cpu[16] = {[0 ... 15] = -1};
> unsigned int ht_irq[] = {0, 1, 3, 4, 5, 6, 7, 8, 12, 14, 15};
> unsigned int local_irq = 1<<0 | 1<<1 | 1<<2 | 1<<7 | 1<<8 | 1<<12;
>
> +int plat_set_irq_affinity(struct irq_data *d, const struct cpumask *affinity,
> + bool force)
> +{
> + unsigned int cpu;
> + struct cpumask new_affinity;
> +
> + /* I/O devices are connected on package-0 */
> + cpumask_copy(&new_affinity, affinity);
> + for_each_cpu(cpu, affinity)
> + if (cpu_data[cpu].package > 0)
> + cpumask_clear_cpu(cpu, &new_affinity);
> +
> + if (cpumask_empty(&new_affinity))
> + return -EINVAL;
> +
> + cpumask_copy(d->common->affinity, &new_affinity);
> +
> + return IRQ_SET_MASK_OK_NOCOPY;
> +}
> +
> static void ht_irqdispatch(void)
> {
> - unsigned int i, irq, irq0, irq1;
> - static unsigned int dest_cpu = 0;
> + unsigned int i, irq;
> + struct irq_data *irqd;
> + struct cpumask affinity;
>
> irq = LOONGSON_HT1_INT_VECTOR(0);
> LOONGSON_HT1_INT_VECTOR(0) = irq; /* Acknowledge the IRQs */
>
> - irq0 = irq & local_irq; /* handled by local core */
> - irq1 = irq & ~local_irq; /* balanced by other cores */
> + for (i = 0; i < ARRAY_SIZE(ht_irq); i++) {
> + if (!(irq & (0x1 << ht_irq[i])))
> + continue;
>
> - if (dest_cpu == 0 || !cpu_online(dest_cpu))
> - irq0 |= irq1;
> - else
> - loongson3_send_irq_by_ipi(dest_cpu, irq1);
> + /* handled by local core */
> + if (local_irq & (0x1 << ht_irq[i])) {
> + do_IRQ(ht_irq[i]);
> + continue;
> + }
>
> - dest_cpu = dest_cpu + 1;
> - if (dest_cpu >= num_possible_cpus() || cpu_data[dest_cpu].package > 0)
> - dest_cpu = 0;
> + irqd = irq_get_irq_data(ht_irq[i]);
> + cpumask_and(&affinity, irqd->common->affinity, cpu_active_mask);
> + if (cpumask_empty(&affinity)) {
> + do_IRQ(ht_irq[i]);
> + continue;
> + }
>
> - for (i = 0; i < ARRAY_SIZE(ht_irq); i++) {
> - if (irq0 & (0x1 << ht_irq[i]))
> + irq_cpu[ht_irq[i]] = cpumask_next(irq_cpu[ht_irq[i]], &affinity);
> + if (irq_cpu[ht_irq[i]] >= nr_cpu_ids)
> + irq_cpu[ht_irq[i]] = cpumask_first(&affinity);
> +
> + if (irq_cpu[ht_irq[i]] == 0) {
> do_IRQ(ht_irq[i]);
> + continue;
> + }
> +
> + /* balanced by other cores */
> + loongson3_send_irq_by_ipi(irq_cpu[ht_irq[i]], (0x1 << ht_irq[i]));
> }
> }
>
> diff --git a/drivers/irqchip/irq-i8259.c b/drivers/irqchip/irq-i8259.c
> index 1aec12c..95d21e3 100644
> --- a/drivers/irqchip/irq-i8259.c
> +++ b/drivers/irqchip/irq-i8259.c
> @@ -46,6 +46,9 @@ static struct irq_chip i8259A_chip = {
> .irq_disable = disable_8259A_irq,
> .irq_unmask = enable_8259A_irq,
> .irq_mask_ack = mask_and_ack_8259A,
> +#ifdef CONFIG_CPU_LOONGSON3
> + .irq_set_affinity = plat_set_irq_affinity,
> +#endif
> };
>
> /*
> --
> 2.7.0
>
>


2017-06-15 13:15:25

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH V5 6/9] MIPS: Loongson-3: support irq_set_affinity() in i8259 chip

Thanks Ralf.

On 15/06/17 12:41, Ralf Baechle wrote:
> On Thu, Jun 15, 2017 at 10:31:05AM +0800, Huacai Chen wrote:
>
>> With this patch we can set irq affinity via procfs, so as to improve
>> network performance.
>>
>> Signed-off-by: Huacai Chen <[email protected]>
>> ---
>> arch/mips/include/asm/irq.h | 3 ++
>> arch/mips/loongson64/loongson-3/irq.c | 62 +++++++++++++++++++++++++++--------
>> drivers/irqchip/irq-i8259.c | 3 ++
>> 3 files changed, 55 insertions(+), 13 deletions(-)
>
> You didn't cc the IRQCHIP maintainers:
>
> IRQCHIP DRIVERS
> M: Thomas Gleixner <[email protected]>
> M: Jason Cooper <[email protected]>
> M: Marc Zyngier <[email protected]>
> L: [email protected]
> S: Maintained
> T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core
> T: git git://git.infradead.org/users/jcooper/linux.git irqchip/core
> F: Documentation/devicetree/bindings/interrupt-controller/
> F: drivers/irqchip/
>
> Ralf
>
>
>> diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
>> index ddd1c91..47ff7c6 100644
>> --- a/arch/mips/include/asm/irq.h
>> +++ b/arch/mips/include/asm/irq.h
>> @@ -53,6 +53,7 @@ static inline int irq_canonicalize(int irq)
>> #define irq_canonicalize(irq) (irq) /* Sane hardware, sane code ... */
>> #endif
>>
>> +struct irq_data;
>> asmlinkage void plat_irq_dispatch(void);
>>
>> extern void do_IRQ(unsigned int irq);
>> @@ -63,6 +64,8 @@ extern void spurious_interrupt(void);
>> extern int allocate_irqno(void);
>> extern void alloc_legacy_irqno(void);
>> extern void free_irqno(unsigned int irq);
>> +extern int plat_set_irq_affinity(struct irq_data *d,
>> + const struct cpumask *affinity, bool force);
>>
>> /*
>> * Before R2 the timer and performance counter interrupts were both fixed to
>> diff --git a/arch/mips/loongson64/loongson-3/irq.c b/arch/mips/loongson64/loongson-3/irq.c
>> index 2e6e205..e8b7a47 100644
>> --- a/arch/mips/loongson64/loongson-3/irq.c
>> +++ b/arch/mips/loongson64/loongson-3/irq.c

[...]

Not going to comment on the Loongson-specific code which doesn't make
much sense to me (the patch doesn't explain anything about what it is
actually doing), but...

>> diff --git a/drivers/irqchip/irq-i8259.c b/drivers/irqchip/irq-i8259.c
>> index 1aec12c..95d21e3 100644
>> --- a/drivers/irqchip/irq-i8259.c
>> +++ b/drivers/irqchip/irq-i8259.c
>> @@ -46,6 +46,9 @@ static struct irq_chip i8259A_chip = {
>> .irq_disable = disable_8259A_irq,
>> .irq_unmask = enable_8259A_irq,
>> .irq_mask_ack = mask_and_ack_8259A,
>> +#ifdef CONFIG_CPU_LOONGSON3
>> + .irq_set_affinity = plat_set_irq_affinity,
>> +#endif
>> };

... that's a pretty horrible way of hooking up inside a random driver.

Doesn't MIPS have some form of multi-platform kernel? If you need to add
something like this, it'd be better to set it at runtime, once you've
made sure that you're on the relevant HW (and preferably using an accessor).

Thanks,

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

2017-06-15 15:39:26

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH V5 6/9] MIPS: Loongson-3: support irq_set_affinity() in i8259 chip

OK, I'll update my patch and set it at runtime.

Huacai

On Thu, Jun 15, 2017 at 9:15 PM, Marc Zyngier <[email protected]> wrote:
> Thanks Ralf.
>
> On 15/06/17 12:41, Ralf Baechle wrote:
>> On Thu, Jun 15, 2017 at 10:31:05AM +0800, Huacai Chen wrote:
>>
>>> With this patch we can set irq affinity via procfs, so as to improve
>>> network performance.
>>>
>>> Signed-off-by: Huacai Chen <[email protected]>
>>> ---
>>> arch/mips/include/asm/irq.h | 3 ++
>>> arch/mips/loongson64/loongson-3/irq.c | 62 +++++++++++++++++++++++++++--------
>>> drivers/irqchip/irq-i8259.c | 3 ++
>>> 3 files changed, 55 insertions(+), 13 deletions(-)
>>
>> You didn't cc the IRQCHIP maintainers:
>>
>> IRQCHIP DRIVERS
>> M: Thomas Gleixner <[email protected]>
>> M: Jason Cooper <[email protected]>
>> M: Marc Zyngier <[email protected]>
>> L: [email protected]
>> S: Maintained
>> T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core
>> T: git git://git.infradead.org/users/jcooper/linux.git irqchip/core
>> F: Documentation/devicetree/bindings/interrupt-controller/
>> F: drivers/irqchip/
>>
>> Ralf
>>
>>
>>> diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
>>> index ddd1c91..47ff7c6 100644
>>> --- a/arch/mips/include/asm/irq.h
>>> +++ b/arch/mips/include/asm/irq.h
>>> @@ -53,6 +53,7 @@ static inline int irq_canonicalize(int irq)
>>> #define irq_canonicalize(irq) (irq) /* Sane hardware, sane code ... */
>>> #endif
>>>
>>> +struct irq_data;
>>> asmlinkage void plat_irq_dispatch(void);
>>>
>>> extern void do_IRQ(unsigned int irq);
>>> @@ -63,6 +64,8 @@ extern void spurious_interrupt(void);
>>> extern int allocate_irqno(void);
>>> extern void alloc_legacy_irqno(void);
>>> extern void free_irqno(unsigned int irq);
>>> +extern int plat_set_irq_affinity(struct irq_data *d,
>>> + const struct cpumask *affinity, bool force);
>>>
>>> /*
>>> * Before R2 the timer and performance counter interrupts were both fixed to
>>> diff --git a/arch/mips/loongson64/loongson-3/irq.c b/arch/mips/loongson64/loongson-3/irq.c
>>> index 2e6e205..e8b7a47 100644
>>> --- a/arch/mips/loongson64/loongson-3/irq.c
>>> +++ b/arch/mips/loongson64/loongson-3/irq.c
>
> [...]
>
> Not going to comment on the Loongson-specific code which doesn't make
> much sense to me (the patch doesn't explain anything about what it is
> actually doing), but...
>
>>> diff --git a/drivers/irqchip/irq-i8259.c b/drivers/irqchip/irq-i8259.c
>>> index 1aec12c..95d21e3 100644
>>> --- a/drivers/irqchip/irq-i8259.c
>>> +++ b/drivers/irqchip/irq-i8259.c
>>> @@ -46,6 +46,9 @@ static struct irq_chip i8259A_chip = {
>>> .irq_disable = disable_8259A_irq,
>>> .irq_unmask = enable_8259A_irq,
>>> .irq_mask_ack = mask_and_ack_8259A,
>>> +#ifdef CONFIG_CPU_LOONGSON3
>>> + .irq_set_affinity = plat_set_irq_affinity,
>>> +#endif
>>> };
>
> ... that's a pretty horrible way of hooking up inside a random driver.
>
> Doesn't MIPS have some form of multi-platform kernel? If you need to add
> something like this, it'd be better to set it at runtime, once you've
> made sure that you're on the relevant HW (and preferably using an accessor).
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
>