Skip to contentSkip to conversation

csky: Fixup perf callchain unwind

PATCH New [email protected] 1 participant 4 messages May 13, 2020

Subsystems (4)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

PATCH 1/4 csky: Fixup perf callchain unwind

From: Mao Han <[email protected]>

 [ 5221.974084] Unable to handle kernel paging request at virtual address 0xfffff000, pc: 0x8002c18e
 [ 5221.985929] Oops: 00000000
 [ 5221.989488]
 [ 5221.989488] CURRENT PROCESS:
 [ 5221.989488]
 [ 5221.992877] COMM=callchain_test PID=11962
 [ 5221.995213] TEXT=00008000-000087e0 DATA=00009f1c-0000a018 BSS=0000a018-0000b000
 [ 5221.999037] USER-STACK=7fc18e20  KERNEL-STACK=be204680
 [ 5221.999037]
 [ 5222.003292] PC: 0x8002c18e (perf_callchain_kernel+0x3e/0xd4)
 [ 5222.007957] LR: 0x8002c198 (perf_callchain_kernel+0x48/0xd4)
 [ 5222.074873] Call Trace:
 [ 5222.074873] [<800a248e>] get_perf_callchain+0x20a/0x29c
 [ 5222.074873] [<8009d964>] perf_callchain+0x64/0x80
 [ 5222.074873] [<8009dc1c>] perf_prepare_sample+0x29c/0x4b8
 [ 5222.074873] [<8009de6e>] perf_event_output_forward+0x36/0x98
 [ 5222.074873] [<800497e0>] search_exception_tables+0x20/0x44
 [ 5222.074873] [<8002cbb6>] do_page_fault+0x92/0x378
 [ 5222.074873] [<80098608>] __perf_event_overflow+0x54/0xdc
 [ 5222.074873] [<80098778>] perf_swevent_hrtimer+0xe8/0x164
 [ 5222.074873] [<8002ddd0>] update_mmu_cache+0x0/0xd8
 [ 5222.074873] [<8002c014>] user_backtrace+0x58/0xc4
 [ 5222.074873] [<8002c0b4>] perf_callchain_user+0x34/0xd0
 [ 5222.074873] [<800a2442>] get_perf_callchain+0x1be/0x29c
 [ 5222.074873] [<8009d964>] perf_callchain+0x64/0x80
 [ 5222.074873] [<8009d834>] perf_output_sample+0x78c/0x858
 [ 5222.074873] [<8009dc1c>] perf_prepare_sample+0x29c/0x4b8
 [ 5222.074873] [<8009de94>] perf_event_output_forward+0x5c/0x98
 [ 5222.097846]
 [ 5222.097846] [<800a0300>] perf_event_exit_task+0x58/0x43c
 [ 5222.097846] [<8006c874>] hrtimer_interrupt+0x104/0x2ec
 [ 5222.097846] [<800a0300>] perf_event_exit_task+0x58/0x43c
 [ 5222.097846] [<80437bb6>] dw_apb_clockevent_irq+0x2a/0x4c
 [ 5222.097846] [<8006c770>] hrtimer_interrupt+0x0/0x2ec
 [ 5222.097846] [<8005f2e4>] __handle_irq_event_percpu+0xac/0x19c
 [ 5222.097846] [<80437bb6>] dw_apb_clockevent_irq+0x2a/0x4c
 [ 5222.097846] [<8005f408>] handle_irq_event_percpu+0x34/0x88
 [ 5222.097846] [<8005f480>] handle_irq_event+0x24/0x64
 [ 5222.097846] [<8006218c>] handle_level_irq+0x68/0xdc
 [ 5222.097846] [<8005ec76>] __handle_domain_irq+0x56/0xa8
 [ 5222.097846] [<80450e90>] ck_irq_handler+0xac/0xe4
 [ 5222.097846] [<80029012>] csky_do_IRQ+0x12/0x24
 [ 5222.097846] [<8002a3a0>] csky_irq+0x70/0x80
 [ 5222.097846] [<800ca612>] alloc_set_pte+0xd2/0x238
 [ 5222.097846] [<8002ddd0>] update_mmu_cache+0x0/0xd8
 [ 5222.097846] [<800a0340>] perf_event_exit_task+0x98/0x43c

The original fp check doesn't base on the real kernal stack region. Invalid fp address may cause kernel panic.

---
 arch/csky/kernel/perf_callchain.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
arch/csky/kernel/perf_callchain.c +7 -2
index e68ff375c8f8..ab55e98ee8f6 100644
@@ -12,12 +12,17 @@ struct stackframe {
 
 static int unwind_frame_kernel(struct stackframe *frame)
 {
-	if (kstack_end((void *)frame->fp))
+	unsigned long low = (unsigned long)task_stack_page(current);
+	unsigned long high = low + THREAD_SIZE;
+
+	if (unlikely(frame->fp < low || frame->fp > high))
 		return -EPERM;
-	if (frame->fp & 0x3 || frame->fp < TASK_SIZE)
+
+	if (kstack_end((void *)frame->fp) || frame->fp & 0x3)
 		return -EPERM;
 
 	*frame = *(struct stackframe *)frame->fp;
+
 	if (__kernel_text_address(frame->lr)) {
 		int graph = 0;
 
Signature · 1 line
2.17.0
author maintainer
#1 lore raw

PATCH 2/4 csky: Fixup calltrace panic

From: Guo Ren <[email protected]>

The implementation of show_stack will panic with wrong fp:

addr = *fp++;

because the fp isn't checked properly.

The current implementations of show_stack, wchan and stack_trace haven't been designed properly, so just deprecate them.

This patch is a reference to riscv's way, all codes are modified from arm's. The patch is passed with:

- cat /proc/<pid>/stack
- cat /proc/<pid>/wchan
- echo c > /proc/sysrq-trigger

---
 arch/csky/Kconfig                   |   2 +
 arch/csky/Makefile                  |   2 +-
 arch/csky/include/asm/ptrace.h      |  10 ++
 arch/csky/include/asm/thread_info.h |   6 +
 arch/csky/kernel/Makefile           |   2 +-
 arch/csky/kernel/dumpstack.c        |  49 --------
 arch/csky/kernel/process.c          |  31 -----
 arch/csky/kernel/stacktrace.c       | 176 ++++++++++++++++++++++------
 8 files changed, 159 insertions(+), 119 deletions(-)
 delete mode 100644 arch/csky/kernel/dumpstack.c
arch/csky/Kconfig +2 -0
index 94545d50d40f..bd31ab12f77d 100644
@@ -8,6 +8,7 @@ config CSKY
 	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
 	select ARCH_USE_BUILTIN_BSWAP
 	select ARCH_USE_QUEUED_RWLOCKS if NR_CPUS>2
+	select ARCH_WANT_FRAME_POINTERS if !CPU_CK610
 	select COMMON_CLK
 	select CLKSRC_MMIO
 	select CSKY_MPINTC if CPU_CK860
@@ -38,6 +39,7 @@ config CSKY
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_COPY_THREAD_TLS
+	select HAVE_DEBUG_BUGVERBOSE
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS
 	select HAVE_FUNCTION_TRACER
arch/csky/Makefile +1 -1
index fb1bbbd91954..37f593a4bf53 100644
@@ -47,7 +47,7 @@ ifeq ($(CSKYABI),abiv2)
 KBUILD_CFLAGS += -mno-stack-size
 endif
 
-ifdef CONFIG_STACKTRACE
+ifdef CONFIG_FRAME_POINTER
 KBUILD_CFLAGS += -mbacktrace
 endif
 
arch/csky/include/asm/ptrace.h +10 -0
index aae5aa96cf54..bcfb7070e48d 100644
@@ -58,6 +58,16 @@ static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
 	return regs->usp;
 }
 
+static inline unsigned long frame_pointer(struct pt_regs *regs)
+{
+	return regs->regs[4];
+}
+static inline void frame_pointer_set(struct pt_regs *regs,
+				     unsigned long val)
+{
+	regs->regs[4] = val;
+}
+
 extern int regs_query_register_offset(const char *name);
 extern unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
 						unsigned int n);
arch/csky/include/asm/thread_info.h +6 -0
index 5d33fcfd7f2a..d381a5752f0e 100644
@@ -40,6 +40,12 @@ struct thread_info {
 #define thread_saved_fp(tsk) \
 	((unsigned long)(((struct switch_stack *)(tsk->thread.ksp))->r8))
 
+#define thread_saved_sp(tsk) \
+	((unsigned long)(tsk->thread.ksp))
+
+#define thread_saved_lr(tsk) \
+	((unsigned long)(((struct switch_stack *)(tsk->thread.ksp))->r15))
+
 static inline struct thread_info *current_thread_info(void)
 {
 	unsigned long sp;
arch/csky/kernel/Makefile +1 -1
index fd6d9dc8b7f3..37f37c0e934a 100644
@@ -3,7 +3,7 @@ extra-y := head.o vmlinux.lds
 
 obj-y += entry.o atomic.o signal.o traps.o irq.o time.o vdso.o
 obj-y += power.o syscall.o syscall_table.o setup.o
-obj-y += process.o cpu-probe.o ptrace.o dumpstack.o
+obj-y += process.o cpu-probe.o ptrace.o stacktrace.o
 obj-y += probes/
 
 obj-$(CONFIG_MODULES)			+= module.o
arch/csky/kernel/dumpstack.c deleted +0 -49

49 lines not on this page · raw message

arch/csky/kernel/process.c +0 -31
index f7b231ca269a..4ad6db56c3cd 100644
@@ -98,37 +98,6 @@ int dump_task_regs(struct task_struct *tsk, elf_gregset_t *pr_regs)
 	return 1;
 }
 
-unsigned long get_wchan(struct task_struct *p)
-{
-	unsigned long lr;
-	unsigned long *fp, *stack_start, *stack_end;
-	int count = 0;
-
-	if (!p || p == current || p->state == TASK_RUNNING)
-		return 0;
-
-	stack_start = (unsigned long *)end_of_stack(p);
-	stack_end = (unsigned long *)(task_stack_page(p) + THREAD_SIZE);
-
-	fp = (unsigned long *) thread_saved_fp(p);
-	do {
-		if (fp < stack_start || fp > stack_end)
-			return 0;
-#ifdef CONFIG_STACKTRACE
-		lr = fp[1];
-		fp = (unsigned long *)fp[0];
-#else
-		lr = *fp++;
-#endif
-		if (!in_sched_functions(lr) &&
-		    __kernel_text_address(lr))
-			return lr;
-	} while (count++ < 16);
-
-	return 0;
-}
-EXPORT_SYMBOL(get_wchan);
-
 #ifndef CONFIG_CPU_PM_NONE
 void arch_cpu_idle(void)
 {
arch/csky/kernel/stacktrace.c +139 -37

196 lines not on this page · raw message

Signature · 1 line
2.17.0
author maintainer
#2 lore raw

PATCH 3/4 csky: Fixup remove duplicate irq_disable

In reply to [email protected]

From: Liu Yibin <[email protected]>

Interrupt has been disabled in __schedule() with local_irq_disable() and enabled in finish_task_switch->finish_lock_switch() with local_irq_enabled(), So needn't to disable irq here.

---
 arch/csky/kernel/entry.S | 2 --
 1 file changed, 2 deletions(-)
arch/csky/kernel/entry.S +0 -2
index 364819536f2e..6a468ff75432 100644
@@ -332,8 +332,6 @@ ENTRY(__switch_to)
 
 	mfcr	a2, psr			/* Save PSR value */
 	stw	a2, (a3, THREAD_SR)	/* Save PSR in task struct */
-	bclri	a2, 6			/* Disable interrupts */
-	mtcr	a2, psr
 
 	SAVE_SWITCH_STACK
 
Signature · 1 line
2.17.0
author maintainer
#3 lore raw

PATCH 4/4 csky: Fixup remove unnecessary save/restore PSR code

In reply to [email protected]

From: Guo Ren <[email protected]>

All processes' PSR could success from SETUP_MMU, so need set it in INIT_THREAD again.

And use a3 instead of r7 in __switch_to for code convention.

---
 arch/csky/include/asm/processor.h |  2 --
 arch/csky/kernel/asm-offsets.c    |  1 -
 arch/csky/kernel/entry.S          | 10 ++--------
 3 files changed, 2 insertions(+), 11 deletions(-)
arch/csky/include/asm/processor.h +0 -2
index c6bcd7f7c720..79eaaaed3d23 100644
@@ -42,7 +42,6 @@ extern struct cpuinfo_csky cpu_data[];
 
 struct thread_struct {
 	unsigned long  ksp;       /* kernel stack pointer */
-	unsigned long  sr;        /* saved status register */
 	unsigned long  trap_no;   /* saved status register */
 
 	/* FPU regs */
@@ -51,7 +50,6 @@ struct thread_struct {
 
 #define INIT_THREAD  { \
 	.ksp = sizeof(init_stack) + (unsigned long) &init_stack, \
-	.sr = DEFAULT_PSR_VALUE, \
 }
 
 /*
arch/csky/kernel/asm-offsets.c +0 -1
index f8be348df9e4..bbc259eed233 100644
@@ -19,7 +19,6 @@ int main(void)
 
 	/* offsets into the thread struct */
 	DEFINE(THREAD_KSP,        offsetof(struct thread_struct, ksp));
-	DEFINE(THREAD_SR,         offsetof(struct thread_struct, sr));
 	DEFINE(THREAD_FESR,       offsetof(struct thread_struct, user_fp.fesr));
 	DEFINE(THREAD_FCR,        offsetof(struct thread_struct, user_fp.fcr));
 	DEFINE(THREAD_FPREG,      offsetof(struct thread_struct, user_fp.vr));
arch/csky/kernel/entry.S +2 -8
index 6a468ff75432..3760397fdd3d 100644
@@ -330,9 +330,6 @@ ENTRY(__switch_to)
 	lrw	a3, TASK_THREAD
 	addu	a3, a0
 
-	mfcr	a2, psr			/* Save PSR value */
-	stw	a2, (a3, THREAD_SR)	/* Save PSR in task struct */
-
 	SAVE_SWITCH_STACK
 
 	stw	sp, (a3, THREAD_KSP)
@@ -343,12 +340,9 @@ ENTRY(__switch_to)
 
 	ldw	sp, (a3, THREAD_KSP)	/* Set next kernel sp */
 
-	ldw	a2, (a3, THREAD_SR)	/* Set next PSR */
-	mtcr	a2, psr
-
 #if  defined(__CSKYABIV2__)
-	addi	r7, a1, TASK_THREAD_INFO
-	ldw	tls, (r7, TINFO_TP_VALUE)
+	addi	a3, a1, TASK_THREAD_INFO
+	ldw	tls, (a3, TINFO_TP_VALUE)
 #endif
 
 	RESTORE_SWITCH_STACK
Signature · 1 line
2.17.0