[PATCH 1/6] powerpc: Move #ifdef'ed body of do_IRQ() into a separate function

[PATCH 1/6] powerpc: Move #ifdef'ed body of do_IRQ() into a separate function

[Michael Ellerman]

Rather than a giant ifdef in the body of do_IRQ(), including a
dangling else, move the irq stack logic into a separate routine and
do the ifdef there.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/kernel/irq.c |   96 ++++++++++++++++++++++++++-------------------
 1 files changed, 56 insertions(+), 40 deletions(-)

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 8c1a496..bc08827 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -248,13 +248,63 @@ void fixup_irqs(cpumask_t map)
 }
 #endif
 
+#ifdef CONFIG_IRQSTACKS
+static inline void handle_one_irq(unsigned int irq)
+{
+	struct thread_info *curtp, *irqtp;
+	unsigned long saved_sp_limit;
+	struct irq_desc *desc;
+	void *handler;
+
+	/* Switch to the irq stack to handle this */
+	curtp = current_thread_info();
+	irqtp = hardirq_ctx[smp_processor_id()];
+
+	if (curtp == irqtp) {
+		/* We're already on the irq stack, just handle it */
+		generic_handle_irq(irq);
+		return;
+	}
+
+	desc = irq_desc + irq;
+	saved_sp_limit = current->thread.ksp_limit;
+
+	handler = desc->handler;
+	if (handler == NULL)
+		handler = &__do_IRQ;
+
+	irqtp->task = curtp->task;
+	irqtp->flags = 0;
+
+	/* Copy the softirq bits in preempt_count so that the
+	 * softirq checks work in the hardirq context. */
+	irqtp->preempt_count = (irqtp->preempt_count & ~SOFTIRQ_MASK) |
+			       (curtp->preempt_count & SOFTIRQ_MASK);
+
+	current->thread.ksp_limit = (unsigned long)irqtp +
+		_ALIGN_UP(sizeof(struct thread_info), 16);
+
+	call_handle_irq(irq, desc, irqtp, handler);
+	current->thread.ksp_limit = saved_sp_limit;
+	irqtp->task = NULL;
+
+	/* Set any flag that may have been set on the
+	 * alternate stack
+	 */
+	if (irqtp->flags)
+		set_bits(irqtp->flags, &curtp->flags);
+}
+#else
+static inline void handle_one_irq(unsigned int irq)
+{
+	generic_handle_irq(irq);
+}
+#endif
+
 void do_IRQ(struct pt_regs *regs)
 {
 	struct pt_regs *old_regs = set_irq_regs(regs);
 	unsigned int irq;
-#ifdef CONFIG_IRQSTACKS
-	struct thread_info *curtp, *irqtp;
-#endif
 
 	irq_enter();
 
@@ -282,43 +332,9 @@ void do_IRQ(struct pt_regs *regs)
 	 */
 	irq = ppc_md.get_irq();
 
-	if (irq != NO_IRQ && irq != NO_IRQ_IGNORE) {
-#ifdef CONFIG_IRQSTACKS
-		/* Switch to the irq stack to handle this */
-		curtp = current_thread_info();
-		irqtp = hardirq_ctx[smp_processor_id()];
-		if (curtp != irqtp) {
-			struct irq_desc *desc = irq_desc + irq;
-			void *handler = desc->handle_irq;
-			unsigned long saved_sp_limit = current->thread.ksp_limit;
-			if (handler == NULL)
-				handler = &__do_IRQ;
-			irqtp->task = curtp->task;
-			irqtp->flags = 0;
-
-			/* Copy the softirq bits in preempt_count so that the
-			 * softirq checks work in the hardirq context.
-			 */
-			irqtp->preempt_count =
-				(irqtp->preempt_count & ~SOFTIRQ_MASK) |
-				(curtp->preempt_count & SOFTIRQ_MASK);
-
-			current->thread.ksp_limit = (unsigned long)irqtp +
-				_ALIGN_UP(sizeof(struct thread_info), 16);
-			call_handle_irq(irq, desc, irqtp, handler);
-			current->thread.ksp_limit = saved_sp_limit;
-			irqtp->task = NULL;
-
-
-			/* Set any flag that may have been set on the
-			 * alternate stack
-			 */
-			if (irqtp->flags)
-				set_bits(irqtp->flags, &curtp->flags);
-		} else
-#endif
-			generic_handle_irq(irq);
-	} else if (irq != NO_IRQ_IGNORE)
+	if (irq != NO_IRQ && irq != NO_IRQ_IGNORE)
+		handle_one_irq(irq);
+	else if (irq != NO_IRQ_IGNORE)
 		/* That's not SMP safe ... but who cares ? */
 		ppc_spurious_interrupts++;
 
-- 
1.6.2.1

[PATCH 2/6] powerpc: Move stack overflow check into a separate function

[Michael Ellerman]

Makes do_IRQ() shorter and clearer.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/kernel/irq.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index bc08827..f725610 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -301,6 +301,22 @@ static inline void handle_one_irq(unsigned int irq)
 }
 #endif
 
+static inline void check_stack_overflow(void)
+{
+#ifdef CONFIG_DEBUG_STACKOVERFLOW
+	long sp;
+
+	sp = __get_SP() & (THREAD_SIZE-1);
+
+	/* check for stack overflow: is there less than 2KB free? */
+	if (unlikely(sp < (sizeof(struct thread_info) + 2048))) {
+		printk("do_IRQ: stack overflow: %ld\n",
+			sp - sizeof(struct thread_info));
+		dump_stack();
+	}
+#endif
+}
+
 void do_IRQ(struct pt_regs *regs)
 {
 	struct pt_regs *old_regs = set_irq_regs(regs);
@@ -308,20 +324,7 @@ void do_IRQ(struct pt_regs *regs)
 
 	irq_enter();
 
-#ifdef CONFIG_DEBUG_STACKOVERFLOW
-	/* Debugging check for stack overflow: is there less than 2KB free? */
-	{
-		long sp;
-
-		sp = __get_SP() & (THREAD_SIZE-1);
-
-		if (unlikely(sp < (sizeof(struct thread_info) + 2048))) {
-			printk("do_IRQ: stack overflow: %ld\n",
-				sp - sizeof(struct thread_info));
-			dump_stack();
-		}
-	}
-#endif
+	check_stack_overflow();
 
 	/*
 	 * Every platform is required to implement ppc_md.get_irq.
-- 
1.6.2.1

[PATCH 3/6] powerpc: Move get_irq() comment into header

[Michael Ellerman]

The guts of do_IRQ() isn't really the right place to be documenting
the ppc_md.get_irq() interface. So move the comment into machdep.h

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/include/asm/machdep.h |    4 ++++
 arch/powerpc/kernel/irq.c          |    7 -------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 0efdb1d..11d1fc3 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -110,6 +110,10 @@ struct machdep_calls {
 	void		(*show_percpuinfo)(struct seq_file *m, int i);
 
 	void		(*init_IRQ)(void);
+
+	/* Return an irq, or NO_IRQ to indicate there are none pending.
+	 * If for some reason there is no irq, but the interrupt
+	 * shouldn't be counted as spurious, return NO_IRQ_IGNORE. */
 	unsigned int	(*get_irq)(void);
 #ifdef CONFIG_KEXEC
 	void		(*kexec_cpu_down)(int crash_shutdown, int secondary);
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index f725610..b8f09bd 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -326,13 +326,6 @@ void do_IRQ(struct pt_regs *regs)
 
 	check_stack_overflow();
 
-	/*
-	 * Every platform is required to implement ppc_md.get_irq.
-	 * This function will either return an irq number or NO_IRQ to
-	 * indicate there are no more pending.
-	 * The value NO_IRQ_IGNORE is for buggy hardware and means that this
-	 * IRQ has already been handled. -- Tom
-	 */
 	irq = ppc_md.get_irq();
 
 	if (irq != NO_IRQ && irq != NO_IRQ_IGNORE)
-- 
1.6.2.1

[PATCH 4/6] powerpc: Remove fallback to __do_IRQ()

[Michael Ellerman]

We should no longer have any irq code that needs __do_IRQ(), so
remove the fallback to __do_IRQ().

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/kernel/irq.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index b8f09bd..7d46e5d 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -254,7 +254,6 @@ static inline void handle_one_irq(unsigned int irq)
 	struct thread_info *curtp, *irqtp;
 	unsigned long saved_sp_limit;
 	struct irq_desc *desc;
-	void *handler;
 
 	/* Switch to the irq stack to handle this */
 	curtp = current_thread_info();
@@ -269,10 +268,6 @@ static inline void handle_one_irq(unsigned int irq)
 	desc = irq_desc + irq;
 	saved_sp_limit = current->thread.ksp_limit;
 
-	handler = desc->handler;
-	if (handler == NULL)
-		handler = &__do_IRQ;
-
 	irqtp->task = curtp->task;
 	irqtp->flags = 0;
 
@@ -284,7 +279,7 @@ static inline void handle_one_irq(unsigned int irq)
 	current->thread.ksp_limit = (unsigned long)irqtp +
 		_ALIGN_UP(sizeof(struct thread_info), 16);
 
-	call_handle_irq(irq, desc, irqtp, handler);
+	call_handle_irq(irq, desc, irqtp, desc->handle_irq);
 	current->thread.ksp_limit = saved_sp_limit;
 	irqtp->task = NULL;
 
-- 
1.6.2.1

[PATCH 5/6] powerpc/powermac: Use generic_handle_irq() in gatwick_action()

[Michael Ellerman]

Don't call __do_IRQ() directly in gatwick_action(), use
generic_handle_irq().

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/powermac/pic.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/pic.c b/arch/powerpc/platforms/powermac/pic.c
index 7039d8f..dce7363 100644
--- a/arch/powerpc/platforms/powermac/pic.c
+++ b/arch/powerpc/platforms/powermac/pic.c
@@ -221,7 +221,7 @@ static irqreturn_t gatwick_action(int cpl, void *dev_id)
 			continue;
 		irq += __ilog2(bits);
 		spin_unlock_irqrestore(&pmac_pic_lock, flags);
-		__do_IRQ(irq);
+		generic_handle_irq(irq);
 		spin_lock_irqsave(&pmac_pic_lock, flags);
 		rc = IRQ_HANDLED;
 	}
-- 
1.6.2.1

[PATCH 6/6] powerpc: We don't need __do_IRQ() anymore

[Michael Ellerman]

So select GENERIC_HARDIRQS_NO__DO_IRQ to disable it.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/Kconfig |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 4c78045..3ba167a 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -42,6 +42,10 @@ config GENERIC_HARDIRQS
 	bool
 	default y
 
+config GENERIC_HARDIRQS_NO__DO_IRQ
+	bool
+	default y
+
 config HAVE_SETUP_PER_CPU_AREA
 	def_bool PPC64
 
-- 
1.6.2.1

Re: [PATCH 1/6] powerpc: Move #ifdef'ed body of do_IRQ() into a separate function

[Scott Wood]

On Thu, Apr 23, 2009 at 11:31:37AM +1000, Michael Ellerman wrote:
> +	handler = desc->handler;

Should be desc->handle_irq.  It's fixed in a later patch, but this breaks
bisect.

-Scott

Re: [PATCH 1/6] powerpc: Move #ifdef'ed body of do_IRQ() into a separate function

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

On Thu, 2009-04-23 at 11:49 -0500, Scott Wood wrote:
> On Thu, Apr 23, 2009 at 11:31:37AM +1000, Michael Ellerman wrote:
> > +	handler = desc->handler;
> 
> Should be desc->handle_irq.  It's fixed in a later patch, but this breaks
> bisect.

Ah crud, thanks for spotting it. That's an artifact of me reordering the
patches. Will resend.

cheers


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/6] powerpc: Move #ifdef'ed body of do_IRQ() into a separate function

[Christoph Hellwig]

On Thu, Apr 23, 2009 at 11:31:37AM +1000, Michael Ellerman wrote:
> +#ifdef CONFIG_IRQSTACKS

Wasn't there a plan to make CONFIG_IRQSTACKS the unconditional default?


The actual patch looks good to me.

Re: [PATCH 1/6] powerpc: Move #ifdef'ed body of do_IRQ() into a separate function

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

On Sat, 2009-04-25 at 20:18 +0200, Christoph Hellwig wrote:
> On Thu, Apr 23, 2009 at 11:31:37AM +1000, Michael Ellerman wrote:
> > +#ifdef CONFIG_IRQSTACKS
> 
> Wasn't there a plan to make CONFIG_IRQSTACKS the unconditional default?

Not sure. Looks like the 64-bit configs all turn it on, and all but one
or two of the 32-bit configs don't.

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/6] powerpc: Move #ifdef'ed body of do_IRQ() into a separate function

[Christoph Hellwig]

On Tue, Apr 28, 2009 at 10:49:07AM +1000, Michael Ellerman wrote:
> On Sat, 2009-04-25 at 20:18 +0200, Christoph Hellwig wrote:
> > On Thu, Apr 23, 2009 at 11:31:37AM +1000, Michael Ellerman wrote:
> > > +#ifdef CONFIG_IRQSTACKS
> > 
> > Wasn't there a plan to make CONFIG_IRQSTACKS the unconditional default?
> 
> Not sure. Looks like the 64-bit configs all turn it on, and all but one
> or two of the 32-bit configs don't.

Yeah, but do they have a reason not to turn it on?  Having irqstacks
is a lot safer than no having it because the stack useage is a lot more
predictable.  And not having to maintain two codepathes is also a
benefit all by itself.

Re: [PATCH 1/6] powerpc: Move #ifdef'ed body of do_IRQ() into a separate function

[Kumar Gala]

On Apr 29, 2009, at 6:44 AM, Christoph Hellwig wrote:

> On Tue, Apr 28, 2009 at 10:49:07AM +1000, Michael Ellerman wrote:
>> On Sat, 2009-04-25 at 20:18 +0200, Christoph Hellwig wrote:
>>> On Thu, Apr 23, 2009 at 11:31:37AM +1000, Michael Ellerman wrote:
>>>> +#ifdef CONFIG_IRQSTACKS
>>>
>>> Wasn't there a plan to make CONFIG_IRQSTACKS the unconditional  
>>> default?
>>
>> Not sure. Looks like the 64-bit configs all turn it on, and all but  
>> one
>> or two of the 32-bit configs don't.
>
> Yeah, but do they have a reason not to turn it on?  Having irqstacks
> is a lot safer than no having it because the stack useage is a lot  
> more
> predictable.  And not having to maintain two codepathes is also a
> benefit all by itself.

I think Ben, Paul and I had discussed just universally enabling it.   
Can't remember why Ben hadn't done that yet.

- k

Re: [PATCH 1/6] powerpc: Move #ifdef'ed body of do_IRQ() into a separate function

[Benjamin Herrenschmidt]

On Wed, 2009-04-29 at 07:48 -0500, Kumar Gala wrote:
> 
> I think Ben, Paul and I had discussed just universally enabling it.   
> Can't remember why Ben hadn't done that yet.

Slipped between the cracks. Patch welcome.

Cheers,
Ben.