Re: [PATCH 1/3] switch vector_irq[] from irq number to irq_desc pointerv2
From: Mike Travis
Date: Fri Sep 19 2008 - 16:25:15 EST
Hi Ingo,
Note that my recent patch from a couple of weeks ago, stripped almost all of
the cpumask_t's from the io_apic functions. Any chance we can apply them
before we add more?
I'd be willing to merge in Dean's patch over the top of mine (or vice-versa).
Thanks,
Mike
Dean Nelson wrote:
> Change per_cpu variable vector_irq[] from holding an 'int' irq number to
> holding a 'struct irq_desc' pointer.
>
> Signed-off-by: Dean Nelson <dcn@xxxxxxx>
>
> ---
>
> Note that this pre-allocates the irq_desc structure before we know whether
> a vector will be successfully found. And should it not, the irq_desc structure
> is left created with the irq still unallocated. Should someone in the future
> attempt to allocate a vector with that same irq and succeed, they will get
> the previously allocated irq_desc structure.
>
> Also, I won't claim the changes to arch/x86/xen/irq.c were correctly done, and
> I know they weren't tested.
>
> arch/x86/kernel/io_apic.c | 32 ++++++++++++++++++--------------
> arch/x86/kernel/irq_32.c | 14 ++++++--------
> arch/x86/kernel/irq_64.c | 10 ++++------
> arch/x86/kernel/irqinit_32.c | 29 +++++++++--------------------
> arch/x86/kernel/irqinit_64.c | 29 +++++++++--------------------
> arch/x86/kernel/vmiclock_32.c | 2 +-
> arch/x86/xen/irq.c | 3 ++-
> include/asm-x86/hw_irq.h | 2 +-
> 8 files changed, 50 insertions(+), 71 deletions(-)
>
> Index: linux/arch/x86/kernel/io_apic.c
> ===================================================================
> --- linux.orig/arch/x86/kernel/io_apic.c 2008-09-19 14:12:25.000000000 -0500
> +++ linux/arch/x86/kernel/io_apic.c 2008-09-19 14:13:09.000000000 -0500
> @@ -1226,6 +1226,7 @@ static int __assign_irq_vector(int irq,
> unsigned int old_vector;
> int cpu;
> struct irq_cfg *cfg;
> + struct irq_desc *desc;
>
> cfg = irq_cfg(irq);
>
> @@ -1243,6 +1244,8 @@ static int __assign_irq_vector(int irq,
> return 0;
> }
>
> + desc = irq_to_desc_alloc(irq);
> +
> for_each_cpu_mask_nr(cpu, mask) {
> cpumask_t domain, new_mask;
> int new_cpu;
> @@ -1270,7 +1273,7 @@ next:
> goto next;
> #endif
> for_each_cpu_mask_nr(new_cpu, new_mask)
> - if (per_cpu(vector_irq, new_cpu)[vector] != -1)
> + if (per_cpu(vector_irq, new_cpu)[vector] != NULL)
> goto next;
> /* Found one! */
> current_vector = vector;
> @@ -1280,7 +1283,7 @@ next:
> cfg->old_domain = cfg->domain;
> }
> for_each_cpu_mask_nr(new_cpu, new_mask)
> - per_cpu(vector_irq, new_cpu)[vector] = irq;
> + per_cpu(vector_irq, new_cpu)[vector] = desc;
> cfg->vector = vector;
> cfg->domain = domain;
> return 0;
> @@ -1311,7 +1314,7 @@ static void __clear_irq_vector(int irq)
> vector = cfg->vector;
> cpus_and(mask, cfg->domain, cpu_online_map);
> for_each_cpu_mask_nr(cpu, mask)
> - per_cpu(vector_irq, cpu)[vector] = -1;
> + per_cpu(vector_irq, cpu)[vector] = NULL;
>
> cfg->vector = 0;
> cpus_clear(cfg->domain);
> @@ -1323,23 +1326,26 @@ void __setup_vector_irq(int cpu)
> /* This function must be called with vector_lock held */
> int irq, vector;
> struct irq_cfg *cfg;
> + struct irq_desc *desc;
>
> /* Mark the inuse vectors */
> for_each_irq_cfg(irq, cfg) {
> if (!cpu_isset(cpu, cfg->domain))
> continue;
> vector = cfg->vector;
> - per_cpu(vector_irq, cpu)[vector] = irq;
> + desc = irq_to_desc(irq);
> + BUG_ON(desc == NULL);
> + per_cpu(vector_irq, cpu)[vector] = desc;
> }
> /* Mark the free vectors */
> for (vector = 0; vector < NR_VECTORS; ++vector) {
> - irq = per_cpu(vector_irq, cpu)[vector];
> - if (irq < 0)
> + desc = per_cpu(vector_irq, cpu)[vector];
> + if (desc == NULL)
> continue;
>
> - cfg = irq_cfg(irq);
> + cfg = irq_cfg(desc->irq);
> if (!cpu_isset(cpu, cfg->domain))
> - per_cpu(vector_irq, cpu)[vector] = -1;
> + per_cpu(vector_irq, cpu)[vector] = NULL;
> }
> }
>
> @@ -2389,16 +2395,14 @@ asmlinkage void smp_irq_move_cleanup_int
>
> me = smp_processor_id();
> for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
> - unsigned int irq;
> struct irq_desc *desc;
> struct irq_cfg *cfg;
> - irq = __get_cpu_var(vector_irq)[vector];
>
> - desc = irq_to_desc(irq);
> - if (!desc)
> + desc = __get_cpu_var(vector_irq)[vector];
> + if (desc == NULL)
> continue;
>
> - cfg = irq_cfg(irq);
> + cfg = irq_cfg(desc->irq);
> spin_lock(&desc->lock);
> if (!cfg->move_cleanup_count)
> goto unlock;
> @@ -2406,7 +2410,7 @@ asmlinkage void smp_irq_move_cleanup_int
> if ((vector == cfg->vector) && cpu_isset(me, cfg->domain))
> goto unlock;
>
> - __get_cpu_var(vector_irq)[vector] = -1;
> + __get_cpu_var(vector_irq)[vector] = NULL;
> cfg->move_cleanup_count--;
> unlock:
> spin_unlock(&desc->lock);
> Index: linux/include/asm-x86/hw_irq.h
> ===================================================================
> --- linux.orig/include/asm-x86/hw_irq.h 2008-09-19 14:12:25.000000000 -0500
> +++ linux/include/asm-x86/hw_irq.h 2008-09-19 14:13:09.000000000 -0500
> @@ -113,7 +113,7 @@ extern asmlinkage void smp_invalidate_in
> extern void (*const interrupt[NR_VECTORS])(void);
> #endif
>
> -typedef int vector_irq_t[NR_VECTORS];
> +typedef struct irq_desc *vector_irq_t[NR_VECTORS];
> DECLARE_PER_CPU(vector_irq_t, vector_irq);
>
> #ifdef CONFIG_X86_IO_APIC
> Index: linux/arch/x86/kernel/irq_32.c
> ===================================================================
> --- linux.orig/arch/x86/kernel/irq_32.c 2008-09-19 14:12:25.000000000 -0500
> +++ linux/arch/x86/kernel/irq_32.c 2008-09-19 14:13:09.000000000 -0500
> @@ -226,26 +226,24 @@ unsigned int do_IRQ(struct pt_regs *regs
> int overflow;
> unsigned vector = ~regs->orig_ax;
> struct irq_desc *desc;
> - unsigned irq;
>
>
> old_regs = set_irq_regs(regs);
> irq_enter();
> - irq = __get_cpu_var(vector_irq)[vector];
>
> overflow = check_stack_overflow();
>
> - desc = irq_to_desc(irq);
> - if (unlikely(!desc)) {
> - printk(KERN_EMERG "%s: cannot handle IRQ %d vector %#x cpu %d\n",
> - __func__, irq, vector, smp_processor_id());
> + desc = __get_cpu_var(vector_irq)[vector];
> + if (unlikely(desc == NULL)) {
> + printk(KERN_EMERG "%s: cannot handle IRQ vector %#x cpu %d\n",
> + __func__, vector, smp_processor_id());
> BUG();
> }
>
> - if (!execute_on_irq_stack(overflow, desc, irq)) {
> + if (!execute_on_irq_stack(overflow, desc, desc->irq)) {
> if (unlikely(overflow))
> print_stack_overflow();
> - desc->handle_irq(irq, desc);
> + desc->handle_irq(desc->irq, desc);
> }
>
> irq_exit();
> Index: linux/arch/x86/kernel/irq_64.c
> ===================================================================
> --- linux.orig/arch/x86/kernel/irq_64.c 2008-09-19 14:12:25.000000000 -0500
> +++ linux/arch/x86/kernel/irq_64.c 2008-09-19 14:13:09.000000000 -0500
> @@ -213,20 +213,18 @@ asmlinkage unsigned int do_IRQ(struct pt
>
> /* high bit used in ret_from_ code */
> unsigned vector = ~regs->orig_ax;
> - unsigned irq;
>
> exit_idle();
> irq_enter();
> - irq = __get_cpu_var(vector_irq)[vector];
>
> #ifdef CONFIG_DEBUG_STACKOVERFLOW
> stack_overflow_check(regs);
> #endif
>
> - desc = irq_to_desc(irq);
> - if (likely(desc))
> - generic_handle_irq_desc(irq, desc);
> - else {
> + desc = __get_cpu_var(vector_irq)[vector];
> + if (likely(desc != NULL)) {
> + generic_handle_irq_desc(desc->irq, desc);
> + } else {
> if (!disable_apic)
> ack_APIC_irq();
>
> Index: linux/arch/x86/kernel/vmiclock_32.c
> ===================================================================
> --- linux.orig/arch/x86/kernel/vmiclock_32.c 2008-09-19 14:12:25.000000000 -0500
> +++ linux/arch/x86/kernel/vmiclock_32.c 2008-09-19 14:13:09.000000000 -0500
> @@ -242,7 +242,7 @@ void __init vmi_time_init(void)
> vmi_time_init_clockevent();
> setup_irq(0, &vmi_clock_action);
> for_each_possible_cpu(cpu)
> - per_cpu(vector_irq, cpu)[vmi_get_timer_vector()] = 0;
> + per_cpu(vector_irq, cpu)[vmi_get_timer_vector()] = NULL;
> }
>
> #ifdef CONFIG_X86_LOCAL_APIC
> Index: linux/arch/x86/xen/irq.c
> ===================================================================
> --- linux.orig/arch/x86/xen/irq.c 2008-09-19 14:12:25.000000000 -0500
> +++ linux/arch/x86/xen/irq.c 2008-09-19 14:13:09.000000000 -0500
> @@ -27,8 +27,9 @@ static void __init __xen_init_IRQ(void)
> for(i = 0; i < NR_VECTORS; i++) {
> int cpu;
>
> + desc = irq_to_desc_alloc(i);
> for_each_possible_cpu(cpu)
> - per_cpu(vector_irq, cpu)[i] = i;
> + per_cpu(vector_irq, cpu)[i] = desc;
> }
>
> xen_init_IRQ();
> Index: linux/arch/x86/kernel/irqinit_32.c
> ===================================================================
> --- linux.orig/arch/x86/kernel/irqinit_32.c 2008-09-19 14:12:25.000000000 -0500
> +++ linux/arch/x86/kernel/irqinit_32.c 2008-09-19 14:13:09.000000000 -0500
> @@ -59,6 +59,8 @@ static struct irqaction fpu_irq = {
> void __init init_ISA_irqs (void)
> {
> int i;
> + int cpu;
> + unsigned int vector;
>
> #ifdef CONFIG_X86_LOCAL_APIC
> init_bsp_APIC();
> @@ -76,6 +78,12 @@ void __init init_ISA_irqs (void)
> desc->action = NULL;
> desc->depth = 1;
>
> + vector = IRQ0_VECTOR + i;
> + for_each_possible_cpu(cpu) {
> + BUG_ON(per_cpu(vector_irq, cpu)[vector] != NULL);
> + per_cpu(vector_irq, cpu)[vector] = desc;
> + }
> +
> set_irq_chip_and_handler_name(i, &i8259A_chip,
> handle_level_irq, "XT");
> }
> @@ -90,26 +98,7 @@ static struct irqaction irq2 = {
> .name = "cascade",
> };
>
> -DEFINE_PER_CPU(vector_irq_t, vector_irq) = {
> - [0 ... IRQ0_VECTOR - 1] = -1,
> - [IRQ0_VECTOR] = 0,
> - [IRQ1_VECTOR] = 1,
> - [IRQ2_VECTOR] = 2,
> - [IRQ3_VECTOR] = 3,
> - [IRQ4_VECTOR] = 4,
> - [IRQ5_VECTOR] = 5,
> - [IRQ6_VECTOR] = 6,
> - [IRQ7_VECTOR] = 7,
> - [IRQ8_VECTOR] = 8,
> - [IRQ9_VECTOR] = 9,
> - [IRQ10_VECTOR] = 10,
> - [IRQ11_VECTOR] = 11,
> - [IRQ12_VECTOR] = 12,
> - [IRQ13_VECTOR] = 13,
> - [IRQ14_VECTOR] = 14,
> - [IRQ15_VECTOR] = 15,
> - [IRQ15_VECTOR + 1 ... NR_VECTORS - 1] = -1
> -};
> +DEFINE_PER_CPU(vector_irq_t, vector_irq);
>
> /* Overridden in paravirt.c */
> void init_IRQ(void) __attribute__((weak, alias("native_init_IRQ")));
> Index: linux/arch/x86/kernel/irqinit_64.c
> ===================================================================
> --- linux.orig/arch/x86/kernel/irqinit_64.c 2008-09-19 14:12:25.000000000 -0500
> +++ linux/arch/x86/kernel/irqinit_64.c 2008-09-19 14:13:09.000000000 -0500
> @@ -114,30 +114,13 @@ static struct irqaction irq2 = {
> .mask = CPU_MASK_NONE,
> .name = "cascade",
> };
> -DEFINE_PER_CPU(vector_irq_t, vector_irq) = {
> - [0 ... IRQ0_VECTOR - 1] = -1,
> - [IRQ0_VECTOR] = 0,
> - [IRQ1_VECTOR] = 1,
> - [IRQ2_VECTOR] = 2,
> - [IRQ3_VECTOR] = 3,
> - [IRQ4_VECTOR] = 4,
> - [IRQ5_VECTOR] = 5,
> - [IRQ6_VECTOR] = 6,
> - [IRQ7_VECTOR] = 7,
> - [IRQ8_VECTOR] = 8,
> - [IRQ9_VECTOR] = 9,
> - [IRQ10_VECTOR] = 10,
> - [IRQ11_VECTOR] = 11,
> - [IRQ12_VECTOR] = 12,
> - [IRQ13_VECTOR] = 13,
> - [IRQ14_VECTOR] = 14,
> - [IRQ15_VECTOR] = 15,
> - [IRQ15_VECTOR + 1 ... NR_VECTORS - 1] = -1
> -};
> +DEFINE_PER_CPU(vector_irq_t, vector_irq);
>
> static void __init init_ISA_irqs (void)
> {
> int i;
> + int cpu;
> + unsigned int vector;
>
> init_bsp_APIC();
> init_8259A(0);
> @@ -150,6 +133,12 @@ static void __init init_ISA_irqs (void)
> desc->action = NULL;
> desc->depth = 1;
>
> + vector = IRQ0_VECTOR + i;
> + for_each_possible_cpu(cpu) {
> + BUG_ON(per_cpu(vector_irq, cpu)[vector] != NULL);
> + per_cpu(vector_irq, cpu)[vector] = desc;
> + }
> +
> /*
> * 16 old-style INTA-cycle interrupts:
> */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/