Re: [PATCH] arm64: kernel: Use a separate stack for irq interrupts.
From: AKASHI Takahiro
Date: Tue Sep 08 2015 - 03:51:17 EST
On 09/07/2015 11:36 PM, James Morse wrote:
Having to handle interrupts on top of an existing kernel stack means the
kernel stack must be large enough to accomodate both the maximum kernel
usage, and the maximum irq handler usage. Switching to a different stack
when processing irqs allows us to make the stack size smaller.
Maximum kernel stack usage (running ltp and generating usb+ethernet
interrupts) was 7256 bytes. With this patch, the same workload gives
a maximum stack usage of 5816 bytes.
With stack tracer on, the kernel with this patch ran into BUG() in check_stack():
if (task_stack_end_corrupted(current))
BUG();
This is because a check for an interrupt stack with "object_is_on_stack(stack)"
has passed, while your irq stack doesn't have a STACK_END_MAGIC.
+-+
Signed-off-by: James Morse <james.morse@xxxxxxx>
---
arch/arm64/include/asm/irq.h | 12 +++++++++
arch/arm64/include/asm/thread_info.h | 8 ++++--
arch/arm64/kernel/entry.S | 33 ++++++++++++++++++++---
arch/arm64/kernel/irq.c | 52 ++++++++++++++++++++++++++++++++++++
arch/arm64/kernel/smp.c | 4 +++
arch/arm64/kernel/stacktrace.c | 4 ++-
6 files changed, 107 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index bbb251b14746..050d4196c736 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -2,14 +2,20 @@
#define __ASM_IRQ_H
#include <linux/irqchip/arm-gic-acpi.h>
+#include <linux/percpu.h>
#include <asm-generic/irq.h>
+#include <asm/thread_info.h>
+
+DECLARE_PER_CPU(unsigned long, irq_sp);
struct pt_regs;
extern void migrate_irqs(void);
extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
+extern int alloc_irq_stack(unsigned int cpu);
+
static inline void acpi_irq_init(void)
{
/*
@@ -21,4 +27,10 @@ static inline void acpi_irq_init(void)
}
#define acpi_irq_init acpi_irq_init
+static inline bool is_irq_stack(unsigned long sp)
+{
+ struct thread_info *ti = get_thread_info(sp);
+ return (get_thread_info(per_cpu(irq_sp, ti->cpu)) == ti);
+}
+
#endif
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index dcd06d18a42a..b906254fc400 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -69,12 +69,16 @@ register unsigned long current_stack_pointer asm ("sp");
/*
* how to get the thread information struct from C
*/
+static inline struct thread_info *get_thread_info(unsigned long sp)
+{
+ return (struct thread_info *)(sp & ~(THREAD_SIZE - 1));
+}
+
static inline struct thread_info *current_thread_info(void) __attribute_const__;
static inline struct thread_info *current_thread_info(void)
{
- return (struct thread_info *)
- (current_stack_pointer & ~(THREAD_SIZE - 1));
+ return get_thread_info(current_stack_pointer);
}
#define thread_saved_pc(tsk) \
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index e16351819fed..d42371f3f5a1 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -190,10 +190,37 @@ tsk .req x28 // current thread_info
* Interrupt handling.
*/
.macro irq_handler
- adrp x1, handle_arch_irq
- ldr x1, [x1, #:lo12:handle_arch_irq]
- mov x0, sp
+ mrs x21, tpidr_el1
+ adr_l x20, irq_sp
+ add x20, x20, x21
+
+ ldr x21, [x20]
+ mov x20, sp
+
+ mov x0, x21
+ mov x1, x20
+ bl irq_copy_thread_info
+
+ /* test for recursive use of irq_sp */
+ cbz w0, 1f
+ mrs x30, elr_el1
+ mov sp, x21
+
+ /*
+ * Create a fake stack frame to bump unwind_frame() onto the original
+ * stack. This relies on x29 not being clobbered by kernel_entry().
+ */
+ push x29, x30
I don't think that it works. Since unwind_frame() doesn't care about sp, but
only does fp, a pushed stack frame will never be dereferenced.
-Takahiro AKASHI
+
+1: ldr_l x1, handle_arch_irq
+ mov x0, x20
blr x1
+
+ mov x0, x20
+ mov x1, x21
+ bl irq_copy_thread_info
+ mov sp, x20
+
.endm
.text
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 463fa2e7e34c..10b57a006da8 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -26,11 +26,14 @@
#include <linux/smp.h>
#include <linux/init.h>
#include <linux/irqchip.h>
+#include <linux/percpu.h>
#include <linux/seq_file.h>
#include <linux/ratelimit.h>
unsigned long irq_err_count;
+DEFINE_PER_CPU(unsigned long, irq_sp) = 0;
+
int arch_show_interrupts(struct seq_file *p, int prec)
{
#ifdef CONFIG_SMP
@@ -55,6 +58,10 @@ void __init init_IRQ(void)
irqchip_init();
if (!handle_arch_irq)
panic("No interrupt controller found.");
+
+ /* Allocate an irq stack for the boot cpu */
+ if (alloc_irq_stack(smp_processor_id()))
+ panic("Failed to allocate irq stack for boot cpu.");
}
#ifdef CONFIG_HOTPLUG_CPU
@@ -117,3 +124,48 @@ void migrate_irqs(void)
local_irq_restore(flags);
}
#endif /* CONFIG_HOTPLUG_CPU */
+
+/* Allocate an irq_stack for a cpu that is about to be brought up. */
+int alloc_irq_stack(unsigned int cpu)
+{
+ struct page *irq_stack_page;
+ union thread_union *irq_stack;
+
+ /* reuse stack allocated previously */
+ if (per_cpu(irq_sp, cpu))
+ return 0;
+
+ irq_stack_page = alloc_kmem_pages(THREADINFO_GFP, THREAD_SIZE_ORDER);
+ if (!irq_stack_page) {
+ pr_crit("CPU%u: failed to allocate irq stack for cpu %u\n",
+ smp_processor_id(), cpu);
+ return -ENOMEM;
+ }
+ irq_stack = page_address(irq_stack_page);
+
+ per_cpu(irq_sp, cpu) = (unsigned long)irq_stack->stack
+ + THREAD_START_SP;
+
+ return 0;
+}
+
+/*
+ * Copy struct thread_info between two stacks, and update current->stack.
+ * This is used when moving to the irq exception stack.
+ * Changing current->stack is necessary so that non-arch thread_info writers
+ * don't use the new thread_info->task->stack to find the old thread_info when
+ * setting flags like TIF_NEED_RESCHED...
+ */
+asmlinkage int irq_copy_thread_info(unsigned long dst_sp, unsigned long src_sp)
+{
+ struct thread_info *src = get_thread_info(src_sp);
+ struct thread_info *dst = get_thread_info(dst_sp);
+
+ if (dst == src)
+ return 0;
+
+ *dst = *src;
+ current->stack = dst;
+
+ return 1;
+}
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 50fb4696654e..5283dc5629e4 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -89,6 +89,10 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
{
int ret;
+ ret = alloc_irq_stack(cpu);
+ if (ret)
+ return ret;
+
/*
* We need to tell the secondary core where to find its stack and the
* page tables.
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 407991bf79f5..3d6d5b45aa4b 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -20,6 +20,7 @@
#include <linux/sched.h>
#include <linux/stacktrace.h>
+#include <asm/irq.h>
#include <asm/stacktrace.h>
/*
@@ -43,7 +44,8 @@ int notrace unwind_frame(struct stackframe *frame)
low = frame->sp;
high = ALIGN(low, THREAD_SIZE);
- if (fp < low || fp > high - 0x18 || fp & 0xf)
+ if ((fp < low || fp > high - 0x18 || fp & 0xf) &&
+ !is_irq_stack(frame->sp))
return -EINVAL;
frame->sp = fp + 0x10;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/