From 9a8b4584065dd241d6c2bf818e349986bd900b8e Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Thu, 17 Nov 2005 16:24:52 -0500 Subject: [PARISC] Make sure timer and IPI execute with interrupts disabled Fix a longstanding smp bug The problem is that both the timer and ipi interrupts are being called with interrupts enabled, which isn't what anyone is expecting. The IPI issue has just started to show up by causing a BUG_ON in the slab debugging code. The timer issue never shows up because there's an eiem work around in our irq.c The fix is to label both these as SA_INTERRUPT which causes the generic irq code not to enable interrupts. I also suspect the smp_call_function timeouts we're seeing might be connected with the fact that we disable IPIs when handling any other type of interrupt. I've put a WARN_ON in the code for executing smp_call_function() with IPIs disabled. Signed-off-by: James Bottomley Signed-off-by: Kyle McMartin --- arch/parisc/kernel/irq.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'arch/parisc/kernel/irq.c') diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c index 006385dbee6..f7ae2bcd49a 100644 --- a/arch/parisc/kernel/irq.c +++ b/arch/parisc/kernel/irq.c @@ -291,12 +291,14 @@ void do_cpu_irq_mask(struct pt_regs *regs) static struct irqaction timer_action = { .handler = timer_interrupt, .name = "timer", + .flags = SA_INTERRUPT, }; #ifdef CONFIG_SMP static struct irqaction ipi_action = { .handler = ipi_interrupt, .name = "IPI", + .flags = SA_INTERRUPT, }; #endif -- cgit v1.2.3 From 3f902886a81c6d4e6c399760936b645b5c7a7342 Mon Sep 17 00:00:00 2001 From: Grant Grundler Date: Thu, 17 Nov 2005 16:26:20 -0500 Subject: [PARISC] Disable nesting of interrupts Disable nesting of interrupts - still has holes The offending sequence starts out like this: 1) take external interrupt 2) set_eiem() to only allow TIMER_IRQ; local interrupts still disabled 3) read the EIRR to get a "list" of pending interrupts 4) clear EIRR of pending interrupts we intend to handle 5) call __do_IRQ() to handle IRQ. 6) handle_IRQ_event() enables local interrupts (I-Bit) 7) take a timer interrupt 8) read EIRR to get a new list of pending interrupts 9) clear EIRR of pending interrupts we just read 10) handle pending interrupts found in (8) 11) set_eiem(cpu_eiem) and return [ TROUBLE! all enabled CPU IRQs are unmasked. } 12) handle remaining interrupts pending from (3) e.g. call __do_IRQ() -> handle_IRQ_event()..etc [ TROUBLE! call to handle_IRQ_event() can now enable *any* IRQ. } 13) set_eiem(cpu_eiem) and return The problem is we now get into ugly race conditions with Timer and IPI interrupts at this point. I'm not exactly sure what happens when things go wrong (perhaps nest calls to IPI or timer interrupt?). But I'm certain it's not good. This sequence will break sooner if (10) would accidentally leave interrupts enabled. I'm pretty sure the right answer is now to make cpu_eiem a per CPU variable since all external interrupts on parisc are per CPU. This means we will NOT need to send an IPI to every CPU in the system when enabling or disabling an IRQ since only one CPU needs to change it's EIEM. Thanks to James Bottomley for (once again) pointing out the problem. Signed-off-by: Grant Grundler Signed-off-by: Kyle McMartin --- arch/parisc/kernel/irq.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'arch/parisc/kernel/irq.c') diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c index f7ae2bcd49a..21a9c5ad580 100644 --- a/arch/parisc/kernel/irq.c +++ b/arch/parisc/kernel/irq.c @@ -250,10 +250,11 @@ void do_cpu_irq_mask(struct pt_regs *regs) irq_enter(); /* - * Only allow interrupt processing to be interrupted by the - * timer tick + * Don't allow TIMER or IPI nested interrupts. + * Allowing any single interrupt to nest can lead to that CPU + * handling interrupts with all enabled interrupts unmasked. */ - set_eiem(EIEM_MASK(TIMER_IRQ)); + set_eiem(0UL); /* 1) only process IRQs that are enabled/unmasked (cpu_eiem) * 2) We loop here on EIRR contents in order to avoid @@ -267,9 +268,6 @@ void do_cpu_irq_mask(struct pt_regs *regs) if (!eirr_val) break; - if (eirr_val & EIEM_MASK(TIMER_IRQ)) - set_eiem(0); - mtctl(eirr_val, 23); /* reset bits we are going to process */ /* Work our way from MSb to LSb...same order we alloc EIRs */ @@ -283,7 +281,8 @@ void do_cpu_irq_mask(struct pt_regs *regs) __do_IRQ(irq, regs); } } - set_eiem(cpu_eiem); + + set_eiem(cpu_eiem); /* restore original mask */ irq_exit(); } -- cgit v1.2.3 From d911aed8adf74e1fae88d082b8474b2175b7f1da Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Thu, 17 Nov 2005 16:27:02 -0500 Subject: [PARISC] Fix our interrupts not to use smp_call_function Fix our interrupts not to use smp_call_function On K and D class smp, the generic code calls this under an irq spinlock, which causes the WARN_ON() message in smp_call_function() (and is also illegal because it could deadlock). The fix is to use a new scheme based on the IPI_NOP. Signed-off-by: James Bottomley Signed-off-by: Kyle McMartin --- arch/parisc/kernel/irq.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) (limited to 'arch/parisc/kernel/irq.c') diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c index 21a9c5ad580..3998c0cb925 100644 --- a/arch/parisc/kernel/irq.c +++ b/arch/parisc/kernel/irq.c @@ -43,26 +43,34 @@ extern irqreturn_t ipi_interrupt(int, void *, struct pt_regs *); */ static volatile unsigned long cpu_eiem = 0; -static void cpu_set_eiem(void *info) -{ - set_eiem((unsigned long) info); -} - -static inline void cpu_disable_irq(unsigned int irq) +static void cpu_disable_irq(unsigned int irq) { unsigned long eirr_bit = EIEM_MASK(irq); cpu_eiem &= ~eirr_bit; - on_each_cpu(cpu_set_eiem, (void *) cpu_eiem, 1, 1); + /* Do nothing on the other CPUs. If they get this interrupt, + * The & cpu_eiem in the do_cpu_irq_mask() ensures they won't + * handle it, and the set_eiem() at the bottom will ensure it + * then gets disabled */ } static void cpu_enable_irq(unsigned int irq) { unsigned long eirr_bit = EIEM_MASK(irq); - mtctl(eirr_bit, 23); /* clear EIRR bit before unmasking */ cpu_eiem |= eirr_bit; - on_each_cpu(cpu_set_eiem, (void *) cpu_eiem, 1, 1); + + /* FIXME: while our interrupts aren't nested, we cannot reset + * the eiem mask if we're already in an interrupt. Once we + * implement nested interrupts, this can go away + */ + if (!in_interrupt()) + set_eiem(cpu_eiem); + + /* This is just a simple NOP IPI. But what it does is cause + * all the other CPUs to do a set_eiem(cpu_eiem) at the end + * of the interrupt handler */ + smp_send_all_nop(); } static unsigned int cpu_startup_irq(unsigned int irq) -- cgit v1.2.3 From 1d4c452a85503cdb4bca5925cf698b61d3aa43a0 Mon Sep 17 00:00:00 2001 From: Kyle McMartin Date: Thu, 17 Nov 2005 16:27:44 -0500 Subject: [PARISC] Fix uniprocessor build by dummying smp_send_all_nop() Since irq.c uses smp_send_all_nop, we must define it for UP builds as well. Make it a static inline so it gets optimized away. This forces irq.c to include though. Signed-off-by: Kyle McMartin --- arch/parisc/kernel/irq.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'arch/parisc/kernel/irq.c') diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c index 3998c0cb925..865611c1553 100644 --- a/arch/parisc/kernel/irq.c +++ b/arch/parisc/kernel/irq.c @@ -31,6 +31,8 @@ #include #include +#include + #undef PARISC_IRQ_CR16_COUNTS extern irqreturn_t timer_interrupt(int, void *, struct pt_regs *); -- cgit v1.2.3 From c2ab64d09815cc4d48347ee3679658f197455a2a Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Thu, 17 Nov 2005 16:28:37 -0500 Subject: [PARISC] Add IRQ affinities This really only adds them for the machines I can check SMP on, which is CPU interrupts and IOSAPIC (so not any of the GSC based machines). With this patch, irqbalanced can be used to maintain irq balancing. Unfortunately, irqbalanced is a bit x86 centric, so it doesn't do an incredibly good job, but it does work. Signed-off-by: James Bottomley Signed-off-by: Kyle McMartin --- arch/parisc/kernel/irq.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 58 insertions(+), 2 deletions(-) (limited to 'arch/parisc/kernel/irq.c') diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c index 865611c1553..2626405e70c 100644 --- a/arch/parisc/kernel/irq.c +++ b/arch/parisc/kernel/irq.c @@ -30,6 +30,7 @@ #include #include #include +#include #include @@ -84,6 +85,35 @@ static unsigned int cpu_startup_irq(unsigned int irq) void no_ack_irq(unsigned int irq) { } void no_end_irq(unsigned int irq) { } +#ifdef CONFIG_SMP +int cpu_check_affinity(unsigned int irq, cpumask_t *dest) +{ + int cpu_dest; + + /* timer and ipi have to always be received on all CPUs */ + if (irq == TIMER_IRQ || irq == IPI_IRQ) { + /* Bad linux design decision. The mask has already + * been set; we must reset it */ + irq_affinity[irq] = CPU_MASK_ALL; + return -EINVAL; + } + + /* whatever mask they set, we just allow one CPU */ + cpu_dest = first_cpu(*dest); + *dest = cpumask_of_cpu(cpu_dest); + + return 0; +} + +static void cpu_set_affinity_irq(unsigned int irq, cpumask_t dest) +{ + if (cpu_check_affinity(irq, &dest)) + return; + + irq_affinity[irq] = dest; +} +#endif + static struct hw_interrupt_type cpu_interrupt_type = { .typename = "CPU", .startup = cpu_startup_irq, @@ -92,7 +122,9 @@ static struct hw_interrupt_type cpu_interrupt_type = { .disable = cpu_disable_irq, .ack = no_ack_irq, .end = no_end_irq, -// .set_affinity = cpu_set_affinity_irq, +#ifdef CONFIG_SMP + .set_affinity = cpu_set_affinity_irq, +#endif }; int show_interrupts(struct seq_file *p, void *v) @@ -229,6 +261,13 @@ int txn_alloc_irq(unsigned int bits_wide) return -1; } +unsigned long txn_affinity_addr(unsigned int irq, int cpu) +{ + irq_affinity[irq] = cpumask_of_cpu(cpu); + + return cpu_data[cpu].txn_addr; +} + unsigned long txn_alloc_addr(unsigned int virt_irq) { static int next_cpu = -1; @@ -243,7 +282,7 @@ unsigned long txn_alloc_addr(unsigned int virt_irq) if (next_cpu >= NR_CPUS) next_cpu = 0; /* nothing else, assign monarch */ - return cpu_data[next_cpu].txn_addr; + return txn_affinity_addr(virt_irq, next_cpu); } @@ -282,12 +321,29 @@ void do_cpu_irq_mask(struct pt_regs *regs) /* Work our way from MSb to LSb...same order we alloc EIRs */ for (irq = TIMER_IRQ; eirr_val && bit; bit>>=1, irq++) { + cpumask_t dest = irq_affinity[irq]; + if (!(bit & eirr_val)) continue; /* clear bit in mask - can exit loop sooner */ eirr_val &= ~bit; + /* FIXME: because generic set affinity mucks + * with the affinity before sending it to us + * we can get the situation where the affinity is + * wrong for our CPU type interrupts */ + if (irq != TIMER_IRQ && irq != IPI_IRQ && + !cpu_isset(smp_processor_id(), dest)) { + int cpu = first_cpu(dest); + + printk("rethrowing irq %d from %d to %d\n", + irq, smp_processor_id(), cpu); + gsc_writel(irq + CPU_IRQ_BASE, + cpu_data[cpu].hpa); + continue; + } + __do_IRQ(irq, regs); } } -- cgit v1.2.3 From 03afe22f074231196dcf3298f962cfc787ebbc60 Mon Sep 17 00:00:00 2001 From: Grant Grundler Date: Thu, 17 Nov 2005 16:29:16 -0500 Subject: [PARISC] irq_affinityp[] only available for SMP builds irq_affinityp[] only available for SMP builds, make code that uses it conditional on CONFIG_SMP. Signed-off-by: Grant Grundler Signed-off-by: Kyle McMartin --- arch/parisc/kernel/irq.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'arch/parisc/kernel/irq.c') diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c index 2626405e70c..144fc25b387 100644 --- a/arch/parisc/kernel/irq.c +++ b/arch/parisc/kernel/irq.c @@ -261,13 +261,17 @@ int txn_alloc_irq(unsigned int bits_wide) return -1; } + unsigned long txn_affinity_addr(unsigned int irq, int cpu) { +#ifdef CONFIG_SMP irq_affinity[irq] = cpumask_of_cpu(cpu); +#endif return cpu_data[cpu].txn_addr; } + unsigned long txn_alloc_addr(unsigned int virt_irq) { static int next_cpu = -1; @@ -321,14 +325,16 @@ void do_cpu_irq_mask(struct pt_regs *regs) /* Work our way from MSb to LSb...same order we alloc EIRs */ for (irq = TIMER_IRQ; eirr_val && bit; bit>>=1, irq++) { +#ifdef CONFIG_SMP cpumask_t dest = irq_affinity[irq]; - +#endif if (!(bit & eirr_val)) continue; /* clear bit in mask - can exit loop sooner */ eirr_val &= ~bit; +#ifdef CONFIG_SMP /* FIXME: because generic set affinity mucks * with the affinity before sending it to us * we can get the situation where the affinity is @@ -337,12 +343,13 @@ void do_cpu_irq_mask(struct pt_regs *regs) !cpu_isset(smp_processor_id(), dest)) { int cpu = first_cpu(dest); - printk("rethrowing irq %d from %d to %d\n", + printk("redirecting irq %d from CPU %d to %d\n", irq, smp_processor_id(), cpu); gsc_writel(irq + CPU_IRQ_BASE, cpu_data[cpu].hpa); continue; } +#endif __do_IRQ(irq, regs); } -- cgit v1.2.3 From 75be99a8c597aaebf82802109cdfd1249eea951e Mon Sep 17 00:00:00 2001 From: Ryan Bradetich Date: Thu, 17 Nov 2005 16:29:50 -0500 Subject: [PARISC] Make redirecting irq messages less noisy Make the "redirecting irq" message to not display on the console by setting the severity to KERN_DEBUG. The console was basically unusable. Signed-off-by: Ryan Bradetich Signed-off-by: Kyle McMartin --- arch/parisc/kernel/irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch/parisc/kernel/irq.c') diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c index 144fc25b387..197936d9359 100644 --- a/arch/parisc/kernel/irq.c +++ b/arch/parisc/kernel/irq.c @@ -343,7 +343,7 @@ void do_cpu_irq_mask(struct pt_regs *regs) !cpu_isset(smp_processor_id(), dest)) { int cpu = first_cpu(dest); - printk("redirecting irq %d from CPU %d to %d\n", + printk(KERN_DEBUG "redirecting irq %d from CPU %d to %d\n", irq, smp_processor_id(), cpu); gsc_writel(irq + CPU_IRQ_BASE, cpu_data[cpu].hpa); -- cgit v1.2.3