Re: [PATCH] parisc: increase kernel stack size to 32k

Helge Deller
2013-04-27T21:53:43Z

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

On 04/27/2013 12:30 AM, Helge Deller wrote:
> * James Bottomley <James.Bottomley@HansenPartnership.com>:
>> On Wed, 2013-04-24 at 09:33 +0200, Helge Deller wrote:
>>> On 04/23/2013 10:22 PM, Helge Deller wrote:
>>>> commit e4e1e78facf7565cada909a69c7fb6415b6e7b83
>>>> Author: Helge Deller <deller@gmx.de>
>>>> Date: Tue Apr 23 17:19:37 2013 +0200
>>>>
>>>> parisc: increase kernel stack size to 32k
>>>>
>>>> --- a/arch/parisc/include/asm/thread_info.h
>>>> +++ b/arch/parisc/include/asm/thread_info.h
>>>> -#define THREAD_SIZE_ORDER 2
>>>> +#define THREAD_SIZE_ORDER 3 /* 32k stack */
>>>
>>> I tested again, and it actually needs to be 64k stacks to not crash any longer.
>>> So, the right temporary fix is:
>>>
>>>> +#define THREAD_SIZE_ORDER 4 /* 64k stack */
>>>
>>> Will send updated patch soon.
>>
>> This is an indicator of something seriously wrong somewhere. We've
>> always had the 16k stack just because of our large frames. In theory,
>> the IRQ stack should only be the same size as the kernel stack, so if we
>> have both in the same place, we should only need at max 32k ... if we're
>> still seeing problems related to stack overrun, then it might be we have
>> an IRQ recursion where we shouldn't have. To be honest, I have a hard
>> time explaining why our stacks should be over 8k.

Attached is a new version of my irq-stack-patch, which made my system really stable :-)
As test I did used "hackbench 300" (from the LTP project) which created 12000 threads:
uptime: 23:44:09 up 17 min, 2 users, load average: 1232.23, 2966.09, 2466.39

My findings so far:
* kernel stack: THREAD_SIZE_ORDER needs to be at least 2 (=16k). x86 has 1 (8k).
With 8k kernel stacks and DEBUG_STACKOVERFLOW enabled, I get directly after bootup:
stackcheck: swapper/0 has overflown a kernel stack (sp:bfc52030, stk bottom-top:bfc50000-bfc52000)

* IRQ stack: 16k seems sufficient as well.

So, the combination of 16k kernel stack and 16k irq stacks seems OK.

I still need to clean up my patch, test if backtraces still work (with which I currently
have problems) and prepare a final patch.

Helge

[-- Attachment #2: irqstack.patch6 --]
[-- Type: text/plain, Size: 6520 bytes --]

diff --git a/arch/parisc/Kconfig.debug b/arch/parisc/Kconfig.debug
index 7305ac8..ed88c37 100644
--- a/arch/parisc/Kconfig.debug
+++ b/arch/parisc/Kconfig.debug
@@ -12,6 +12,17 @@ config DEBUG_RODATA
portion of the kernel code won't be covered by a TLB anymore.
If in doubt, say "N".

+config DEBUG_STACKOVERFLOW
+ bool "Check for stack overflows"
+ depends on DEBUG_KERNEL
+ ---help---
+ Say Y here if you want to check the overflows of kernel, IRQ
+ and exception stacks. This option will cause messages of the
+ stacks in detail when free stack space drops below a certain
+ limit.
+ If in doubt, say "N".
+
+
config DEBUG_STRICT_USER_COPY_CHECKS
bool "Strict copy size checks"
depends on DEBUG_KERNEL && !TRACE_BRANCH_PROFILING
diff --git a/arch/parisc/include/asm/irq.h b/arch/parisc/include/asm/irq.h
index 1073599..0417ebf 100644
--- a/arch/parisc/include/asm/irq.h
+++ b/arch/parisc/include/asm/irq.h
@@ -10,6 +10,8 @@
#include <linux/cpumask.h>
#include <asm/types.h>

+#define __ARCH_HAS_DO_SOFTIRQ
+
#define NO_IRQ (-1)

#ifdef CONFIG_GSC
diff --git a/arch/parisc/include/asm/processor.h b/arch/parisc/include/asm/processor.h
index 09b54a5..d725591 100644
--- a/arch/parisc/include/asm/processor.h
+++ b/arch/parisc/include/asm/processor.h
@@ -20,8 +20,6 @@

#endif /* __ASSEMBLY__ */

-#define KERNEL_STACK_SIZE (4*PAGE_SIZE)
-
/*
* Default implementation of macro that returns current
* instruction pointer ("program counter").
@@ -33,6 +31,8 @@
#endif
#define current_text_addr() ({ void *pc; current_ia(pc); pc; })

+#define get_current_sp(sp) __asm__("copy %%r30, %0" : "=r"(sp))
+
#define TASK_SIZE_OF(tsk) ((tsk)->thread.task_size)
#define TASK_SIZE TASK_SIZE_OF(current)
#define TASK_UNMAPPED_BASE (current->thread.map_base)
@@ -61,6 +61,20 @@
#ifndef __ASSEMBLY__

/*
+ * IRQ STACK - used for irq and irq bh handler
+ */
+#ifdef __KERNEL__
+
+#define IRQ_STACK_SIZE (4096 << 2) /* = 16k, todo: use: PAGE_SIZE instead of 4096 */
+
+union irq_stack_union {
+ unsigned long irq_stack[IRQ_STACK_SIZE/sizeof(unsigned long)];
+};
+
+DECLARE_PER_CPU(union irq_stack_union, irq_stack_union);
+#endif /* __KERNEL__ */
+
+/*
* Data detected about CPUs at boot time which is the same for all CPU's.
* HP boxes are SMP - ie identical processors.
*
diff --git a/arch/parisc/include/asm/thread_info.h b/arch/parisc/include/asm/thread_info.h
index d1fb79a..85568cc 100644
--- a/arch/parisc/include/asm/thread_info.h
+++ b/arch/parisc/include/asm/thread_info.h
@@ -40,7 +40,7 @@ struct thread_info {

/* thread information allocation */

-#define THREAD_SIZE_ORDER 2
+#define THREAD_SIZE_ORDER 2 /* keep value 2 for 16k, use 3 for 32k */
/* Be sure to hunt all references to this down when you change the size of
* the kernel stack */
#define THREAD_SIZE (PAGE_SIZE << THREAD_SIZE_ORDER)
diff --git a/arch/parisc/kernel/entry.S b/arch/parisc/kernel/entry.S
index f33201b..117d516 100644
--- a/arch/parisc/kernel/entry.S
+++ b/arch/parisc/kernel/entry.S
@@ -1997,6 +1997,28 @@ ftrace_stub:
ENDPROC(return_to_handler)
#endif /* CONFIG_FUNCTION_TRACER */

+/* void call_on_stack(unsigned long param1, void *func, unsigned long new_stack) */
+ENTRY(call_on_stack)
+ STREG %sp, 8(%arg2)
+ STREG %rp, 16(%arg2)
+
+ /* HPPA calling convention for function pointers */
+#ifdef CONFIG_64BIT
+ LDREG 16(%arg1), %arg1
+ bve,l (%arg1), %rp
+ addi 0x40, %arg2, %sp
+#else
+ addi 0x40, %arg2, %sp
+ be,l 0(%sr4,%arg1), %sr0, %r31
+ copy %r31, %rp
+#endif
+
+ addi -0x40, %sp, %sp
+ LDREG 16(%sp),%rp
+ bv (%rp)
+ LDREG 8(%sp),%sp
+ENDPROC(call_on_stack)
+

get_register:
/*
diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
index 8094d3e..6c23120 100644
--- a/arch/parisc/kernel/irq.c
+++ b/arch/parisc/kernel/irq.c
@@ -330,6 +330,60 @@ static inline int eirr_to_irq(unsigned long eirr)
return (BITS_PER_LONG - bit) + TIMER_IRQ;
}

+
+int sysctl_panic_on_stackoverflow __read_mostly;
+
+/*
+ * Stack overflow check:
+ */
+static inline void stack_overflow_check(unsigned long sp, unsigned long stack_start,
+ unsigned long stack_size, const char *stackname)
+{
+#ifdef CONFIG_DEBUG_STACKOVERFLOW
+#define STACK_MARGIN 128
+ if (likely((sp - stack_start) < (stack_size - STACK_MARGIN)))
+ return;
+
+ WARN("stackcheck: %s has overflown the %s stack (sp:%lx, stk bottom-top:%lx-%lx)\n",
+ current->comm, stackname, sp,
+ stack_start, stack_start + stack_size);
+
+ if (sysctl_panic_on_stackoverflow)
+ panic("low stack detected by irq handler - check messages\n");
+#endif
+}
+
+extern void call_on_stack(unsigned long param1, void *func, unsigned long new_stack); /* in entry.S */
+
+
+static void noinline execute_on_irq_stack(void *func, unsigned long param1)
+{
+ int cpu = smp_processor_id();
+ unsigned long sp, irq_stack;
+ void (*direct_call)(unsigned long param1) = func;
+
+ irq_stack = (unsigned long) &per_cpu(irq_stack_union, cpu);
+ get_current_sp(sp);
+
+ /*
+ * this is where we try to switch to the IRQ stack. However, if we are
+ * already using the IRQ stack (because we interrupted a hardirq
+ * handler) we can't do that and just have to keep using the
+ * current stack (which is the irq stack already after all)
+ */
+
+ if ((sp - irq_stack) >= IRQ_STACK_SIZE) {
+ stack_overflow_check(sp, (unsigned long)task_stack_page(current), THREAD_SIZE, "kernel");
+ call_on_stack(param1, func, irq_stack);
+ // WARN_ON_ONCE(1); /* enable to check if irq stack is being used. */
+ // TODO: check if backtrace works from irq stack
+ // TODO: use get_current_sp() macro in other code as well.
+ } else {
+ stack_overflow_check(sp, irq_stack, IRQ_STACK_SIZE, "irq");
+ direct_call(param1);
+ }
+}
+
/* ONLY called from entry.S:intr_extint() */
void do_cpu_irq_mask(struct pt_regs *regs)
{
@@ -364,7 +418,7 @@ void do_cpu_irq_mask(struct pt_regs *regs)
goto set_out;
}
#endif
- generic_handle_irq(irq);
+ execute_on_irq_stack(&generic_handle_irq, irq);

out:
irq_exit();
@@ -423,3 +477,24 @@ void __init init_IRQ(void)

}

+DEFINE_PER_CPU(union irq_stack_union, irq_stack_union);
+
+asmlinkage void do_softirq(void)
+{
+ __u32 pending;
+ unsigned long flags;
+
+ if (in_interrupt())
+ return;
+
+ local_irq_save(flags);
+
+ pending = local_softirq_pending();
+
+ /* Switch to interrupt stack */
+ if (pending) {
+ execute_on_irq_stack(&__do_softirq, 0);
+ WARN_ON_ONCE(softirq_count());
+ }
+ local_irq_restore(flags);
+}