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

2009-04-29 Thread 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.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2009-04-29 Thread 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
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2009-04-29 Thread 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.


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2009-04-28 Thread 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 mich...@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..3d3658d 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-handle_irq;
+   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

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2009-04-27 Thread Michael Ellerman
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


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

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

2009-04-25 Thread 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.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2009-04-23 Thread 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
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2009-04-23 Thread Michael Ellerman
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



signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev