Re: [PATCH] powerpc/irq: Run softirqs off the top of the irq stack

2013-09-23 Thread Stephen Rothwell 
Hi Ben,

On Mon, 23 Sep 2013 14:35:58 +1000 Benjamin Herrenschmidt 
b...@kernel.crashing.org wrote:

 diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
 index c69440c..0c9646f 100644
 --- a/arch/powerpc/kernel/irq.c
 +++ b/arch/powerpc/kernel/irq.c
 @@ -443,46 +443,7 @@ void migrate_irqs(void)
  
  static inline void handle_one_irq(unsigned int irq)
  {
 - struct thread_info *curtp, *irqtp;
 - unsigned long saved_sp_limit;
 - struct irq_desc *desc;
 -
 - desc = irq_to_desc(irq);
 - if (!desc)
 - return;
 -
 - /* 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 */
 - desc-handle_irq(irq, desc);
 - return;
 - }
 -
 - saved_sp_limit = current-thread.ksp_limit;
 -
 - 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, desc-handle_irq);
 - 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);
  }

This function ends up as a single blank line ...

 @@ -519,18 +480,64 @@ void do_IRQ(struct pt_regs *regs)
*/
   irq = ppc_md.get_irq();
  
 - /* We can hard enable interrupts now */
 + /* We can hard enable interrupts now to allow perf interrupts */
   may_hard_irq_enable();
  
   /* And finally process it */
 - if (irq != NO_IRQ)
 - handle_one_irq(irq);

then you remove the only call, so why not just remove the function
completely?

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au


pgpeXLH2lyAd6.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/irq: Run softirqs off the top of the irq stack

2013-09-23 Thread Benjamin Herrenschmidt 
On Mon, 2013-09-23 at 17:56 +1000, Stephen Rothwell wrote:
 Hi Ben,
 
 On Mon, 23 Sep 2013 14:35:58 +1000 Benjamin Herrenschmidt 
 b...@kernel.crashing.org wrote:
 
  diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
  index c69440c..0c9646f 100644
  --- a/arch/powerpc/kernel/irq.c
  +++ b/arch/powerpc/kernel/irq.c
  @@ -443,46 +443,7 @@ void migrate_irqs(void)
   
   static inline void handle_one_irq(unsigned int irq)
   {
  -   struct thread_info *curtp, *irqtp;
  -   unsigned long saved_sp_limit;
  -   struct irq_desc *desc;
  -
  -   desc = irq_to_desc(irq);
  -   if (!desc)
  -   return;
  -
  -   /* 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 */
  -   desc-handle_irq(irq, desc);
  -   return;
  -   }
  -
  -   saved_sp_limit = current-thread.ksp_limit;
  -
  -   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, desc-handle_irq);
  -   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);
   }
 
 This function ends up as a single blank line ...
 
  @@ -519,18 +480,64 @@ void do_IRQ(struct pt_regs *regs)
   */
  irq = ppc_md.get_irq();
   
  -   /* We can hard enable interrupts now */
  +   /* We can hard enable interrupts now to allow perf interrupts */
  may_hard_irq_enable();
   
  /* And finally process it */
  -   if (irq != NO_IRQ)
  -   handle_one_irq(irq);
 
 then you remove the only call, so why not just remove the function
 completely?

Because I'm an idiot ? :-)

I moved bits and pieces to do_IRQ and forgot to remove the remainder
(and gcc didn't warn :-)

Cheers,
Ben.



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

Re: [PATCH] powerpc/irq: Run softirqs off the top of the irq stack

2013-09-23 Thread Linus Torvalds 
On Sun, Sep 22, 2013 at 9:35 PM, Benjamin Herrenschmidt
b...@kernel.crashing.org wrote:

 This is the band aid discussed so far for the stack overflow
 problem for powerpc.

I don't think it's a band-aid in any way, except perhaps in the
sense that there are certainly other things we can also do in this
series (ie I liked Frederic's cleanups etc).

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

Re: [PATCH] powerpc/irq: Run softirqs off the top of the irq stack

2013-09-23 Thread Benjamin Herrenschmidt 
On Mon, 2013-09-23 at 09:47 -0700, Linus Torvalds wrote:
 On Sun, Sep 22, 2013 at 9:35 PM, Benjamin Herrenschmidt
 b...@kernel.crashing.org wrote:
 
  This is the band aid discussed so far for the stack overflow
  problem for powerpc.
 
 I don't think it's a band-aid in any way, except perhaps in the
 sense that there are certainly other things we can also do in this
 series (ie I liked Frederic's cleanups etc).

Ah yes, I thought of it as a band-aid in the sense that a better
approach would be to switch to the irq stack earlier like x86_64 does
but that would be a lot more invasive. Definitely something I would look
into if I was to tackle changing how we do per-cpu and the PACA though.

Cheers,
Ben.


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

[PATCH] powerpc/irq: Run softirqs off the top of the irq stack

2013-09-22 Thread Benjamin Herrenschmidt 
Nowadays, irq_exit() calls __do_softirq() pretty much directly
instead of calling do_softirq() which switches to the decicated
softirq stack.

This has lead to observed stack overflows on powerpc since we call
irq_enter() and irq_exit() outside of the scope that switches to
the irq stack.

This fixes it by moving the stack switching up a level, making
irq_enter() and irq_exit() run off the irq stack.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---

This is the band aid discussed so far for the stack overflow
problem for powerpc. I assume sparc and i386 at least need
something similar (I had a quick look at ARM and it doesn't
seem to have irq stacks at all).

Unless objection in the next day or so, I intend to send that to
Linus and to -stable ASAP.

 arch/powerpc/include/asm/irq.h |  4 +-
 arch/powerpc/kernel/irq.c  | 99 ++
 arch/powerpc/kernel/misc_32.S  |  9 ++--
 arch/powerpc/kernel/misc_64.S  | 10 ++---
 4 files changed, 62 insertions(+), 60 deletions(-)

diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index 0e40843..41f13ce 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -69,9 +69,9 @@ extern struct thread_info *softirq_ctx[NR_CPUS];
 
 extern void irq_ctx_init(void);
 extern void call_do_softirq(struct thread_info *tp);
-extern int call_handle_irq(int irq, void *p1,
-  struct thread_info *tp, void *func);
+extern void call_do_irq(struct pt_regs *regs, struct thread_info *tp);
 extern void do_IRQ(struct pt_regs *regs);
+extern void __do_irq(struct pt_regs *regs);
 
 int irq_choose_cpu(const struct cpumask *mask);
 
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index c69440c..0c9646f 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -443,46 +443,7 @@ void migrate_irqs(void)
 
 static inline void handle_one_irq(unsigned int irq)
 {
-   struct thread_info *curtp, *irqtp;
-   unsigned long saved_sp_limit;
-   struct irq_desc *desc;
-
-   desc = irq_to_desc(irq);
-   if (!desc)
-   return;
-
-   /* 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 */
-   desc-handle_irq(irq, desc);
-   return;
-   }
-
-   saved_sp_limit = current-thread.ksp_limit;
-
-   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, desc-handle_irq);
-   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);
 }
 
 static inline void check_stack_overflow(void)
@@ -501,9 +462,9 @@ static inline void check_stack_overflow(void)
 #endif
 }
 
-void do_IRQ(struct pt_regs *regs)
+void __do_irq(struct pt_regs *regs)
 {
-   struct pt_regs *old_regs = set_irq_regs(regs);
+   struct irq_desc *desc;
unsigned int irq;
 
irq_enter();
@@ -519,18 +480,64 @@ void do_IRQ(struct pt_regs *regs)
 */
irq = ppc_md.get_irq();
 
-   /* We can hard enable interrupts now */
+   /* We can hard enable interrupts now to allow perf interrupts */
may_hard_irq_enable();
 
/* And finally process it */
-   if (irq != NO_IRQ)
-   handle_one_irq(irq);
-   else
+   if (unlikely(irq == NO_IRQ))
__get_cpu_var(irq_stat).spurious_irqs++;
+   else {
+   desc = irq_to_desc(irq);
+   if (likely(desc))
+   desc-handle_irq(irq, desc);
+   }
 
trace_irq_exit(regs);
 
irq_exit();
+}
+
+void do_IRQ(struct pt_regs *regs)
+{
+   struct pt_regs *old_regs = set_irq_regs(regs);
+   struct thread_info *curtp, *irqtp;
+   unsigned long saved_sp_limit;
+
+   /* Switch to the irq stack to handle this */
+   curtp = current_thread_info();
+   irqtp = hardirq_ctx[raw_smp_processor_id()];
+
+   /* Already there ? */
+   if (unlikely(curtp == irqtp)) {
+   __do_irq(regs);
+   set_irq_regs(old_regs);
+   return;
+   }
+
+   /* Adjust the stack limit */
+   saved_sp_limit = current-thread.ksp_limit;
+   current-thread.ksp_limit = (unsigned long)irqtp +
+   _ALIGN_UP(sizeof(struct thread_info),