Fix irqpoll on IA64 (timer interrupt != 0)

PATCH In review Bernhard Walle 3 participants 5 messages Mar 19, 2007 IRQ

Share & tools

Bernhard Walle

author

2007-03-19 18:13 UTC #0 lore raw

PATCH 1/1 Fix irqpoll on IA64 (timer interrupt != 0)

On IA64, the timer interrupt is not (always?) zero as it is on x86 platforms. Also, the timer interrupt is CPU-local. Two things need to be changed to make the irqpoll option make also working on IA64:

o Call note_interrupt() also on CPU-local interrupts in __do_IRQ().
o Set a variable timer_irq to the value of the timer interrupt after the timer interrupt has been registered and assigned.

That requires changes in Linux-generic files. The default of timer_irq is 0, so the patch doesn't break i386/x86_64. However, other platforms also may also have a timer interrupt non-equal to zero, so they can also use the new set_timer_interrupt() function.

The patch is against 2.6.21-rc4. Please give me your input how to improve the way it's done if you don't like the way I did the change. irqpoll is required to work with kdump in some situations and that's why I discovered that kdump doesn't work on that platform (HP rx2660).

Signed-off-by Bernhard Walle <[email protected]>

---
 arch/ia64/kernel/irq_ia64.c |    6 +++++-
 arch/ia64/kernel/time.c     |    6 +++++-
 include/asm-ia64/hw_irq.h   |    2 +-
 include/linux/irq.h         |    3 +++
 kernel/irq/handle.c         |    2 ++
 kernel/irq/spurious.c       |   10 +++++++++-
 6 files changed, 25 insertions(+), 4 deletions(-)

arch/ia64/kernel/irq_ia64.c +5 -1

Index: mainline-msi-init/arch/ia64/kernel/irq_ia64.c
===================================================================
@@ -280,11 +280,12 @@ static struct irqaction resched_irqactio
 };
 #endif
 
-void
+int
 register_percpu_irq (ia64_vector vec, struct irqaction *action)
 {
 	irq_desc_t *desc;
 	unsigned int irq;
+	int first_match = -1;
 
 	for (irq = 0; irq < NR_IRQS; ++irq)
 		if (irq_to_vector(irq) == vec) {

@@ -293,7 +294,10 @@ register_percpu_irq (ia64_vector vec, st
 			desc->chip = &irq_type_ia64_lsapic;
 			if (action)
 				setup_irq(irq, action);
+			first_match = irq;
 		}
+
+	return first_match;
 }
 
 void __init

arch/ia64/kernel/time.c +5 -1

Index: mainline-msi-init/arch/ia64/kernel/time.c
===================================================================
@@ -247,7 +247,11 @@ void __devinit ia64_disable_timer(void)
 void __init
 time_init (void)
 {
-	register_percpu_irq(IA64_TIMER_VECTOR, &timer_irqaction);
+	int timer_irq;
+
+	timer_irq = register_percpu_irq(IA64_TIMER_VECTOR, &timer_irqaction);
+	set_timer_interrupt(timer_irq);
+
 	efi_gettimeofday(&xtime);
 	ia64_init_itm();

include/linux/irq.h +3 -0

Index: mainline-msi-init/include/linux/irq.h
===================================================================
@@ -272,6 +272,9 @@ static inline int irq_balancing_disabled
 /* Handle irq action chains: */
 extern int handle_IRQ_event(unsigned int irq, struct irqaction *action);
 
+/* sets the timer interrupt number for irqpoll handling (kernel/irq/spurious.c) */
+extern void set_timer_interrupt(unsigned int irq);
+
 /*
  * Built-in IRQ handlers for various IRQ types,
  * callable via desc->chip->handle_irq()

kernel/irq/spurious.c +9 -1

Index: mainline-msi-init/kernel/irq/spurious.c
===================================================================
@@ -12,6 +12,7 @@
 #include <linux/interrupt.h>
 
 static int irqfixup __read_mostly;
+static unsigned int timer_irq __read_mostly;
 
 /*
  * Recovery handler for misrouted interrupts.

@@ -146,7 +147,7 @@ void note_interrupt(unsigned int irq, st
 
 	if (unlikely(irqfixup)) {
 		/* Don't punish working computers */
-		if ((irqfixup == 2 && irq == 0) || action_ret == IRQ_NONE) {
+		if ((irqfixup == 2 && irq == timer_irq) || action_ret == IRQ_NONE) {
 			int ok = misrouted_irq(irq);
 			if (action_ret == IRQ_NONE)
 				desc->irqs_unhandled -= ok;

@@ -174,6 +175,13 @@ void note_interrupt(unsigned int irq, st
 	desc->irqs_unhandled = 0;
 }
 
+
+void set_timer_interrupt(unsigned int irq)
+{
+	timer_irq = irq;
+}
+EXPORT_SYMBOL_GPL(set_timer_interrupt);
+
 int noirqdebug __read_mostly;
 
 int noirqdebug_setup(char *str)

include/asm-ia64/hw_irq.h +1 -1

Index: mainline-msi-init/include/asm-ia64/hw_irq.h
===================================================================
@@ -95,7 +95,7 @@ extern int assign_irq_vector (int irq);	
 extern void free_irq_vector (int vector);
 extern int reserve_irq_vector (int vector);
 extern void ia64_send_ipi (int cpu, int vector, int delivery_mode, int redirect);
-extern void register_percpu_irq (ia64_vector vec, struct irqaction *action);
+extern int register_percpu_irq (ia64_vector vec, struct irqaction *action);
 
 static inline void ia64_resend_irq(unsigned int vector)
 {

kernel/irq/handle.c +2 -0

Index: mainline-msi-init/kernel/irq/handle.c
===================================================================
@@ -180,6 +180,8 @@ fastcall unsigned int __do_IRQ(unsigned 
 		if (desc->chip->ack)
 			desc->chip->ack(irq);
 		action_ret = handle_IRQ_event(irq, desc->action);
+		if (!noirqdebug)
+			note_interrupt(irq, desc, action_ret);
 		desc->chip->end(irq);
 		return 1;
 	}

Jesper Juhl

2007-03-19 18:29 UTC #1 lore raw

Quoted 13 lines · Bernhard Walle

On IA64, the timer interrupt is not (always?) zero as it is on x86 platforms. Also, the timer interrupt is CPU-local. Two things need to be changed to make the irqpoll option make also working on IA64:
o Call note_interrupt() also on CPU-local interrupts in __do_IRQ().
o Set a variable timer_irq to the value of the timer interrupt after the timer interrupt has been registered and assigned.
That requires changes in Linux-generic files. The default of timer_irq is 0, so the patch doesn't break i386/x86_64. However, other platforms also may also have a timer interrupt non-equal to zero, so they can also use the new set_timer_interrupt() function.

Quoted 5 lines

The patch is against 2.6.21-rc4. Please give me your input how to improve the way it's done if you don't like the way I did the change. irqpoll is required to work with kdump in some situations and that's why I discovered that kdump doesn't work on that platform (HP rx2660).

A few style comments below.

Quoted 16 lines

Signed-off-by Bernhard Walle <[email protected]>
---
 arch/ia64/kernel/irq_ia64.c |    6 +++++-
 arch/ia64/kernel/time.c     |    6 +++++-
 include/asm-ia64/hw_irq.h   |    2 +-
 include/linux/irq.h         |    3 +++
 kernel/irq/handle.c         |    2 ++
 kernel/irq/spurious.c       |   10 +++++++++-
 6 files changed, 25 insertions(+), 4 deletions(-)
Index: mainline-msi-init/arch/ia64/kernel/irq_ia64.c =================================> --- mainline-msi-init.orig/arch/ia64/kernel/irq_ia64.c +++ mainline-msi-init/arch/ia64/kernel/irq_ia64.c

arch/ia64/kernel/irq_ia64.c static struct irqaction resched_irqactio · line 285

Quoted 7 lines

arch/ia64/kernel/irq_ia64.c +1 -1

@@ -280,11 +280,12 @@ static struct irqaction resched_irqactio
 };
 #endif
 
-void
+int
 register_percpu_irq (ia64_vector vec, struct irqaction *action)
 {

Each and every function example in Documentation/CodingStyle has the return type on the same line as the function name, so why not get these in-line while you are modifying them anyway?

int register_percpu_irq (ia64_vector vec, struct irqaction *action)

kernel/irq/spurious.c void note_interrupt(unsigned int irq, st · line 179

Quoted 6 lines

kernel/irq/spurious.c +2 -0

@@ -174,6 +175,13 @@ void note_interrupt(unsigned int irq, st
        desc->irqs_unhandled = 0;
 }
 
+
+void set_timer_interrupt(unsigned int irq)

Just one blank line between functions, as pr CodingStyle:
"In source files, separate functions with one blank line. ..."

Signature · 3 lines

Jesper Juhl <[email protected]>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Bernhard Walle

author

2007-03-19 18:37 UTC #2 lore raw

Quoted 9 lines · Jesper Juhl

arch/ia64/kernel/irq_ia64.c static struct irqaction resched_irqactio · line 285
Quoted 5 lines
arch/ia64/kernel/irq_ia64.c +1 -1
-void
+int
 register_percpu_irq (ia64_vector vec, struct irqaction *action)
 {
Each and every function example in Documentation/CodingStyle has the return type on the same line as the function name, so why not get these in-line while you are modifying them anyway?

Because all other functions in that file are coded this way. I think it looks better if you don't change the style in a particular file. Am I wrong?

Quoted 7 lines

int register_percpu_irq (ia64_vector vec, struct irqaction *action)
Quoted 4 lines
kernel/irq/spurious.c +0 -0
@@ -174,6 +175,13 @@ void note_interrupt(unsigned int irq, st
        desc->irqs_unhandled = 0;
 }
 

Quoted 5 lines

kernel/irq/spurious.c void note_interrupt(unsigned int irq, st · line 179
Quoted 2 lines
kernel/irq/spurious.c +2 -0
+
+void set_timer_interrupt(unsigned int irq)
Just one blank line between functions, as pr CodingStyle:
"In source files, separate functions with one blank line. ..."

Fixed (locally). I'll repost if I get feedback of the contents of the patch.

Regards,
Bernhard

Jesper Juhl

2007-03-19 18:38 UTC #3 lore raw

Quoted 11 lines · Bernhard Walle

Quoted 9 lines · Jesper Juhl
arch/ia64/kernel/irq_ia64.c static struct irqaction resched_irqactio · line 285
Quoted 5 lines
arch/ia64/kernel/irq_ia64.c +1 -1
-void
+int
 register_percpu_irq (ia64_vector vec, struct irqaction *action)
 {
Each and every function example in Documentation/CodingStyle has the return type on the same line as the function name, so why not get these in-line while you are modifying them anyway?

Quoted 4 lines

Because all other functions in that file are coded this way. I think it looks better if you don't change the style in a particular file. Am I wrong?

Consistency within the file certainly makes sense.

Signature · 3 lines

Jesper Juhl <[email protected]>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Thomas Gleixner

Acked-by on v1 1/1

2007-03-19 20:14 UTC #4 lore raw

In reply to Bernhard Walle

Quoted 5 lines · Bernhard Walle

That requires changes in Linux-generic files. The default of timer_irq is 0, so the patch doesn't break i386/x86_64. However, other platforms also may also have a timer interrupt non-equal to zero, so they can also use the new set_timer_interrupt() function.

Quoted 7 lines

The patch is against 2.6.21-rc4. Please give me your input how to improve the way it's done if you don't like the way I did the change. irqpoll is required to work with kdump in some situations and that's why I discovered that kdump doesn't work on that platform (HP rx2660).
Signed-off-by Bernhard Walle <[email protected]>

Acked-by Thomas Gleixner <[email protected]>