Skip to contentSkip to conversation

riscv: stacktrace: A fixup and an optimization

PATCH Applied [email protected] 4 participants 9 messages Nov 9, 2022 – Dec 6, 2022

Subsystems (2)
RISC-V Generic include/asm header files
lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover riscv: stacktrace: A fixup and an optimization

From: Guo Ren <[email protected]>

First is a fixup for the return address pointer. The second makes walk_stackframe could cross the pt_regs frame.

Guo Ren (2):
riscv: stacktrace: Fixup ftrace_graph_ret_addr retp argument
riscv: stacktrace: Make walk_stackframe cross pt_regs frame

 arch/riscv/kernel/entry.S      |  2 +-
 arch/riscv/kernel/stacktrace.c | 11 ++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)
Signature · 1 line
2.36.1
author
#1 lore raw

PATCH 1/2 riscv: stacktrace: Fixup ftrace_graph_ret_addr retp argument

From: Guo Ren <[email protected]>

The 'retp' is a pointer to the return address on the stack, so we must pass the current return address pointer as the 'retp' argument to ftrace_push_return_trace(). Not parent function's return address on the stack.

Fixes b785ec129bd9 ("riscv/ftrace: Add HAVE_FUNCTION_GRAPH_RET_ADDR_PTR support")
Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
Cc Alan Kao <[email protected]>
Cc Palmer Dabbelt <[email protected]>
---
 arch/riscv/kernel/stacktrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/kernel/stacktrace.c +1 -1
index 08d11a53f39e..bcfe9eb55f80 100644
@@ -58,7 +58,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 		} else {
 			fp = frame->fp;
 			pc = ftrace_graph_ret_addr(current, NULL, frame->ra,
-						   (unsigned long *)(fp - 8));
+						   &frame->ra);
 		}
 
 	}
Signature · 1 line
2.36.1
author
#2 lore raw

PATCH 2/2 riscv: stacktrace: Make walk_stackframe cross pt_regs frame

In reply to [email protected]

From: Guo Ren <[email protected]>

The current walk_stackframe with FRAME_POINTER would stop unwinding at ret_from_exception:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [<ffffffe0002038c8>] walk_stackframe+0x0/0xee
  [<ffffffe000aecf48>] show_stack+0x32/0x4a
  [<ffffffe000af1618>] dump_stack_lvl+0x72/0x8e
  [<ffffffe000af1648>] dump_stack+0x14/0x1c
  [<ffffffe000239ad2>] ___might_sleep+0x12e/0x138
  [<ffffffe000239aec>] __might_sleep+0x10/0x18
  [<ffffffe000afe3fe>] down_read+0x22/0xa4
  [<ffffffe000207588>] do_page_fault+0xb0/0x2fe
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc

The optimization would help walk_stackframe cross the pt_regs frame and get more backtrace of debug info:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [<ffffffe0002038c8>] walk_stackframe+0x0/0xee
  [<ffffffe000aecf48>] show_stack+0x32/0x4a
  [<ffffffe000af1618>] dump_stack_lvl+0x72/0x8e
  [<ffffffe000af1648>] dump_stack+0x14/0x1c
  [<ffffffe000239ad2>] ___might_sleep+0x12e/0x138
  [<ffffffe000239aec>] __might_sleep+0x10/0x18
  [<ffffffe000afe3fe>] down_read+0x22/0xa4
  [<ffffffe000207588>] do_page_fault+0xb0/0x2fe
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc
  [<ffffffe000613c06>] riscv_intc_irq+0x1a/0x72
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc
  [<ffffffe00033f44a>] vma_link+0x54/0x160
  [<ffffffe000341d7a>] mmap_region+0x2cc/0x4d0
  [<ffffffe000342256>] do_mmap+0x2d8/0x3ac
  [<ffffffe000326318>] vm_mmap_pgoff+0x70/0xb8
  [<ffffffe00032638a>] vm_mmap+0x2a/0x36
  [<ffffffe0003cfdde>] elf_map+0x72/0x84
  [<ffffffe0003d05f8>] load_elf_binary+0x69a/0xec8
  [<ffffffe000376240>] bprm_execve+0x246/0x53a
  [<ffffffe00037786c>] kernel_execve+0xe8/0x124
  [<ffffffe000aecdf2>] run_init_process+0xfa/0x10c
  [<ffffffe000aece16>] try_to_run_init_process+0x12/0x3c
  [<ffffffe000afa920>] kernel_init+0xb4/0xf8
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc

Here is the error injection test code for the above output:
drivers/irqchip/irq-riscv-intc.c:
static asmlinkage void riscv_intc_irq(struct pt_regs *regs)

 {
        unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;

+ u32 tmp; __get_user(tmp, (u32 *)0);

Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Changbin Du <[email protected]>
---
 arch/riscv/kernel/entry.S      | 2 +-
 arch/riscv/kernel/stacktrace.c | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
arch/riscv/kernel/entry.S +1 -1
index b9eda3fcbd6d..329cf51fcd4d 100644
@@ -248,7 +248,7 @@ ret_from_syscall_rejected:
 	andi t0, t0, _TIF_SYSCALL_WORK
 	bnez t0, handle_syscall_trace_exit
 
-ret_from_exception:
+ENTRY(ret_from_exception)
 	REG_L s0, PT_STATUS(sp)
 	csrc CSR_STATUS, SR_IE
 #ifdef CONFIG_TRACE_IRQFLAGS
arch/riscv/kernel/stacktrace.c +9 -0
index bcfe9eb55f80..75c8dd64fc48 100644
@@ -16,6 +16,8 @@
 
 #ifdef CONFIG_FRAME_POINTER
 
+extern asmlinkage void ret_from_exception(void);
+
 void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 			     bool (*fn)(void *, unsigned long), void *arg)
 {

@@ -59,6 +61,13 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 			fp = frame->fp;
 			pc = ftrace_graph_ret_addr(current, NULL, frame->ra,
 						   &frame->ra);
+			if (pc == (unsigned long)ret_from_exception) {
+				if (unlikely(!__kernel_text_address(pc) || !fn(arg, pc)))
+					break;
+
+				pc = ((struct pt_regs *)sp)->epc;
+				fp = ((struct pt_regs *)sp)->s0;
+			}
 		}
 
 	}
Signature · 1 line
2.36.1
author
#3 lore raw

Re: [PATCH 2/2] riscv: stacktrace: Make walk_stackframe cross pt_regs frame

Quoted 93 lines · On Wed, Nov 9, 2022 at 2:50 PM <[email protected]> wrote:

From: Guo Ren <[email protected]>

The current walk_stackframe with FRAME_POINTER would stop unwinding at ret_from_exception:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [<ffffffe0002038c8>] walk_stackframe+0x0/0xee
  [<ffffffe000aecf48>] show_stack+0x32/0x4a
  [<ffffffe000af1618>] dump_stack_lvl+0x72/0x8e
  [<ffffffe000af1648>] dump_stack+0x14/0x1c
  [<ffffffe000239ad2>] ___might_sleep+0x12e/0x138
  [<ffffffe000239aec>] __might_sleep+0x10/0x18
  [<ffffffe000afe3fe>] down_read+0x22/0xa4
  [<ffffffe000207588>] do_page_fault+0xb0/0x2fe
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc

The optimization would help walk_stackframe cross the pt_regs frame and get more backtrace of debug info:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [<ffffffe0002038c8>] walk_stackframe+0x0/0xee
  [<ffffffe000aecf48>] show_stack+0x32/0x4a
  [<ffffffe000af1618>] dump_stack_lvl+0x72/0x8e
  [<ffffffe000af1648>] dump_stack+0x14/0x1c
  [<ffffffe000239ad2>] ___might_sleep+0x12e/0x138
  [<ffffffe000239aec>] __might_sleep+0x10/0x18
  [<ffffffe000afe3fe>] down_read+0x22/0xa4
  [<ffffffe000207588>] do_page_fault+0xb0/0x2fe
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc
  [<ffffffe000613c06>] riscv_intc_irq+0x1a/0x72
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc
  [<ffffffe00033f44a>] vma_link+0x54/0x160
  [<ffffffe000341d7a>] mmap_region+0x2cc/0x4d0
  [<ffffffe000342256>] do_mmap+0x2d8/0x3ac
  [<ffffffe000326318>] vm_mmap_pgoff+0x70/0xb8
  [<ffffffe00032638a>] vm_mmap+0x2a/0x36
  [<ffffffe0003cfdde>] elf_map+0x72/0x84
  [<ffffffe0003d05f8>] load_elf_binary+0x69a/0xec8
  [<ffffffe000376240>] bprm_execve+0x246/0x53a
  [<ffffffe00037786c>] kernel_execve+0xe8/0x124
  [<ffffffe000aecdf2>] run_init_process+0xfa/0x10c
  [<ffffffe000aece16>] try_to_run_init_process+0x12/0x3c
  [<ffffffe000afa920>] kernel_init+0xb4/0xf8
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc

Here is the error injection test code for the above output:
drivers/irqchip/irq-riscv-intc.c:
static asmlinkage void riscv_intc_irq(struct pt_regs *regs)

 {
        unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;

+ u32 tmp; __get_user(tmp, (u32 *)0);

Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Changbin Du <[email protected]>
---
 arch/riscv/kernel/entry.S      | 2 +-
 arch/riscv/kernel/stacktrace.c | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
arch/riscv/kernel/entry.S +1 -1
index b9eda3fcbd6d..329cf51fcd4d 100644
@@ -248,7 +248,7 @@ ret_from_syscall_rejected:
        andi t0, t0, _TIF_SYSCALL_WORK
        bnez t0, handle_syscall_trace_exit
 
-ret_from_exception:
+ENTRY(ret_from_exception)
        REG_L s0, PT_STATUS(sp)
        csrc CSR_STATUS, SR_IE
 #ifdef CONFIG_TRACE_IRQFLAGS
arch/riscv/kernel/stacktrace.c +2 -0
index bcfe9eb55f80..75c8dd64fc48 100644
@@ -16,6 +16,8 @@
 
 #ifdef CONFIG_FRAME_POINTER
 
+extern asmlinkage void ret_from_exception(void);
+
 void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
                             bool (*fn)(void *, unsigned long), void *arg)
 {

arch/riscv/kernel/stacktrace.c void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, · line 64

Quoted 4 lines
arch/riscv/kernel/stacktrace.c +1 -0
@@ -59,6 +61,13 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
                        fp = frame->fp;
                        pc = ftrace_graph_ret_addr(current, NULL, frame->ra,
                                                   &frame->ra);
+                       if (pc == (unsigned long) ret_from_exception) {

I forgot ret_from_syscall because I tested it on the generic_entry series base. I would merge this patch into the generic_entry series as an optimization.

Quoted 12 lines
arch/riscv/kernel/stacktrace.c +6 -1
+                               if (unlikely(!__kernel_text_address(pc) || !fn(arg, pc)))
+                                       break;
+
+                               pc = ((struct pt_regs *)sp)->epc;
+                               fp = ((struct pt_regs *)sp)->s0;
+                       }
                }
 
        }
--

2.36.1

Signature · 7 lines
Best Regards
 Guo Ren

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
author
#4 lore raw

Re: [PATCH 2/2] riscv: stacktrace: Make walk_stackframe cross pt_regs frame

Quoted 89 lines · Guo Ren
Quoted 87 lines · On Wed, Nov 9, 2022 at 2:50 PM <[email protected]> wrote:

From: Guo Ren <[email protected]>

The current walk_stackframe with FRAME_POINTER would stop unwinding at ret_from_exception:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [<ffffffe0002038c8>] walk_stackframe+0x0/0xee
  [<ffffffe000aecf48>] show_stack+0x32/0x4a
  [<ffffffe000af1618>] dump_stack_lvl+0x72/0x8e
  [<ffffffe000af1648>] dump_stack+0x14/0x1c
  [<ffffffe000239ad2>] ___might_sleep+0x12e/0x138
  [<ffffffe000239aec>] __might_sleep+0x10/0x18
  [<ffffffe000afe3fe>] down_read+0x22/0xa4
  [<ffffffe000207588>] do_page_fault+0xb0/0x2fe
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc

The optimization would help walk_stackframe cross the pt_regs frame and get more backtrace of debug info:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [<ffffffe0002038c8>] walk_stackframe+0x0/0xee
  [<ffffffe000aecf48>] show_stack+0x32/0x4a
  [<ffffffe000af1618>] dump_stack_lvl+0x72/0x8e
  [<ffffffe000af1648>] dump_stack+0x14/0x1c
  [<ffffffe000239ad2>] ___might_sleep+0x12e/0x138
  [<ffffffe000239aec>] __might_sleep+0x10/0x18
  [<ffffffe000afe3fe>] down_read+0x22/0xa4
  [<ffffffe000207588>] do_page_fault+0xb0/0x2fe
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc
  [<ffffffe000613c06>] riscv_intc_irq+0x1a/0x72
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc
  [<ffffffe00033f44a>] vma_link+0x54/0x160
  [<ffffffe000341d7a>] mmap_region+0x2cc/0x4d0
  [<ffffffe000342256>] do_mmap+0x2d8/0x3ac
  [<ffffffe000326318>] vm_mmap_pgoff+0x70/0xb8
  [<ffffffe00032638a>] vm_mmap+0x2a/0x36
  [<ffffffe0003cfdde>] elf_map+0x72/0x84
  [<ffffffe0003d05f8>] load_elf_binary+0x69a/0xec8
  [<ffffffe000376240>] bprm_execve+0x246/0x53a
  [<ffffffe00037786c>] kernel_execve+0xe8/0x124
  [<ffffffe000aecdf2>] run_init_process+0xfa/0x10c
  [<ffffffe000aece16>] try_to_run_init_process+0x12/0x3c
  [<ffffffe000afa920>] kernel_init+0xb4/0xf8
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc

Here is the error injection test code for the above output:
drivers/irqchip/irq-riscv-intc.c:
static asmlinkage void riscv_intc_irq(struct pt_regs *regs)

 {
        unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;

+ u32 tmp; __get_user(tmp, (u32 *)0);

Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Changbin Du <[email protected]>
---
 arch/riscv/kernel/entry.S      | 2 +-
 arch/riscv/kernel/stacktrace.c | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
arch/riscv/kernel/entry.S +1 -1
index b9eda3fcbd6d..329cf51fcd4d 100644
@@ -248,7 +248,7 @@ ret_from_syscall_rejected:
        andi t0, t0, _TIF_SYSCALL_WORK
        bnez t0, handle_syscall_trace_exit
 
-ret_from_exception:
+ENTRY(ret_from_exception)
        REG_L s0, PT_STATUS(sp)
        csrc CSR_STATUS, SR_IE
 #ifdef CONFIG_TRACE_IRQFLAGS
arch/riscv/kernel/stacktrace.c +0 -0
index bcfe9eb55f80..75c8dd64fc48 100644
@@ -16,6 +16,8 @@
 
 #ifdef CONFIG_FRAME_POINTER
Quoted 13 lines

arch/riscv/kernel/stacktrace.c void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, · line 64

Quoted 10 lines
arch/riscv/kernel/stacktrace.c +3 -0
+extern asmlinkage void ret_from_exception(void);
+
 void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
                             bool (*fn)(void *, unsigned long), void *arg)
 {

@@ -59,6 +61,13 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
                        fp = frame->fp;
                        pc = ftrace_graph_ret_addr(current, NULL, frame->ra,
                                                   &frame->ra);
+                       if (pc == (unsigned long) ret_from_exception) {

I forgot ret_from_syscall because I tested it on the generic_entry series base. I would merge this patch into the generic_entry series as an optimization.

No, the patch is correct. The ret_from_syscall is unnecessary because it's from user space.

Quoted 18 lines
Quoted 11 lines
arch/riscv/kernel/stacktrace.c +6 -1
+                               if (unlikely(!__kernel_text_address(pc) || !fn(arg, pc)))
+                                       break;
+
+                               pc = ((struct pt_regs *)sp)->epc;
+                               fp = ((struct pt_regs *)sp)->s0;
+                       }
                }
 
        }
--
Quoted 2 lines

2.36.1

--
Best Regards
Guo Ren

Signature · 2 lines
Best Regards
 Guo Ren
maintainer
#5 lore raw

Re: [PATCH 2/2] riscv: stacktrace: Make walk_stackframe cross pt_regs frame

In reply to [email protected]
Quoted 70 lines · On Tue, 08 Nov 2022 22:49:37 PST (-0800), [email protected] wrote:

From: Guo Ren <[email protected]>

The current walk_stackframe with FRAME_POINTER would stop unwinding at ret_from_exception:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [<ffffffe0002038c8>] walk_stackframe+0x0/0xee
  [<ffffffe000aecf48>] show_stack+0x32/0x4a
  [<ffffffe000af1618>] dump_stack_lvl+0x72/0x8e
  [<ffffffe000af1648>] dump_stack+0x14/0x1c
  [<ffffffe000239ad2>] ___might_sleep+0x12e/0x138
  [<ffffffe000239aec>] __might_sleep+0x10/0x18
  [<ffffffe000afe3fe>] down_read+0x22/0xa4
  [<ffffffe000207588>] do_page_fault+0xb0/0x2fe
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc

The optimization would help walk_stackframe cross the pt_regs frame and get more backtrace of debug info:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [<ffffffe0002038c8>] walk_stackframe+0x0/0xee
  [<ffffffe000aecf48>] show_stack+0x32/0x4a
  [<ffffffe000af1618>] dump_stack_lvl+0x72/0x8e
  [<ffffffe000af1648>] dump_stack+0x14/0x1c
  [<ffffffe000239ad2>] ___might_sleep+0x12e/0x138
  [<ffffffe000239aec>] __might_sleep+0x10/0x18
  [<ffffffe000afe3fe>] down_read+0x22/0xa4
  [<ffffffe000207588>] do_page_fault+0xb0/0x2fe
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc
  [<ffffffe000613c06>] riscv_intc_irq+0x1a/0x72
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc
  [<ffffffe00033f44a>] vma_link+0x54/0x160
  [<ffffffe000341d7a>] mmap_region+0x2cc/0x4d0
  [<ffffffe000342256>] do_mmap+0x2d8/0x3ac
  [<ffffffe000326318>] vm_mmap_pgoff+0x70/0xb8
  [<ffffffe00032638a>] vm_mmap+0x2a/0x36
  [<ffffffe0003cfdde>] elf_map+0x72/0x84
  [<ffffffe0003d05f8>] load_elf_binary+0x69a/0xec8
  [<ffffffe000376240>] bprm_execve+0x246/0x53a
  [<ffffffe00037786c>] kernel_execve+0xe8/0x124
  [<ffffffe000aecdf2>] run_init_process+0xfa/0x10c
  [<ffffffe000aece16>] try_to_run_init_process+0x12/0x3c
  [<ffffffe000afa920>] kernel_init+0xb4/0xf8
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc

Here is the error injection test code for the above output:
drivers/irqchip/irq-riscv-intc.c:
static asmlinkage void riscv_intc_irq(struct pt_regs *regs)

 {
        unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;

+ u32 tmp; __get_user(tmp, (u32 *)0);

Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Changbin Du <[email protected]>
---
 arch/riscv/kernel/entry.S      | 2 +-
 arch/riscv/kernel/stacktrace.c | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
arch/riscv/kernel/entry.S +0 -0
index b9eda3fcbd6d..329cf51fcd4d 100644

arch/riscv/kernel/entry.S ret_from_syscall_rejected: · line 251

Quoted 5 lines
arch/riscv/kernel/entry.S +1 -1
@@ -248,7 +248,7 @@ ret_from_syscall_rejected:
 	andi t0, t0, _TIF_SYSCALL_WORK
 	bnez t0, handle_syscall_trace_exit
 
-ret_from_exception:
+ENTRY(ret_from_exception)

This at least needs an END(), but it should also be converted over to some non-function entry flavor. I converted it over to SYM_CODE_START_NOALIGN(), with the cooresponding SYM_CODE_END(), and put it on for-next.

Quoted 30 lines
arch/riscv/kernel/entry.S +0 -0
 	REG_L s0, PT_STATUS(sp)
 	csrc CSR_STATUS, SR_IE
 #ifdef CONFIG_TRACE_IRQFLAGS
arch/riscv/kernel/stacktrace.c +9 -0
index bcfe9eb55f80..75c8dd64fc48 100644
@@ -16,6 +16,8 @@
 
 #ifdef CONFIG_FRAME_POINTER
 
+extern asmlinkage void ret_from_exception(void);
+
 void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 			     bool (*fn)(void *, unsigned long), void *arg)
 {

@@ -59,6 +61,13 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 			fp = frame->fp;
 			pc = ftrace_graph_ret_addr(current, NULL, frame->ra,
 						   &frame->ra);
+			if (pc == (unsigned long)ret_from_exception) {
+				if (unlikely(!__kernel_text_address(pc) || !fn(arg, pc)))
+					break;
+
+				pc = ((struct pt_regs *)sp)->epc;
+				fp = ((struct pt_regs *)sp)->s0;
+			}
 		}
 
 	}
author
#6 lore raw

Re: [PATCH 2/2] riscv: stacktrace: Make walk_stackframe cross pt_regs frame

Quoted 75 lines · Palmer Dabbelt
Quoted 73 lines · On Tue, 08 Nov 2022 22:49:37 PST (-0800), [email protected] wrote:

From: Guo Ren <[email protected]>

The current walk_stackframe with FRAME_POINTER would stop unwinding at ret_from_exception:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [<ffffffe0002038c8>] walk_stackframe+0x0/0xee
  [<ffffffe000aecf48>] show_stack+0x32/0x4a
  [<ffffffe000af1618>] dump_stack_lvl+0x72/0x8e
  [<ffffffe000af1648>] dump_stack+0x14/0x1c
  [<ffffffe000239ad2>] ___might_sleep+0x12e/0x138
  [<ffffffe000239aec>] __might_sleep+0x10/0x18
  [<ffffffe000afe3fe>] down_read+0x22/0xa4
  [<ffffffe000207588>] do_page_fault+0xb0/0x2fe
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc

The optimization would help walk_stackframe cross the pt_regs frame and get more backtrace of debug info:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [<ffffffe0002038c8>] walk_stackframe+0x0/0xee
  [<ffffffe000aecf48>] show_stack+0x32/0x4a
  [<ffffffe000af1618>] dump_stack_lvl+0x72/0x8e
  [<ffffffe000af1648>] dump_stack+0x14/0x1c
  [<ffffffe000239ad2>] ___might_sleep+0x12e/0x138
  [<ffffffe000239aec>] __might_sleep+0x10/0x18
  [<ffffffe000afe3fe>] down_read+0x22/0xa4
  [<ffffffe000207588>] do_page_fault+0xb0/0x2fe
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc
  [<ffffffe000613c06>] riscv_intc_irq+0x1a/0x72
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc
  [<ffffffe00033f44a>] vma_link+0x54/0x160
  [<ffffffe000341d7a>] mmap_region+0x2cc/0x4d0
  [<ffffffe000342256>] do_mmap+0x2d8/0x3ac
  [<ffffffe000326318>] vm_mmap_pgoff+0x70/0xb8
  [<ffffffe00032638a>] vm_mmap+0x2a/0x36
  [<ffffffe0003cfdde>] elf_map+0x72/0x84
  [<ffffffe0003d05f8>] load_elf_binary+0x69a/0xec8
  [<ffffffe000376240>] bprm_execve+0x246/0x53a
  [<ffffffe00037786c>] kernel_execve+0xe8/0x124
  [<ffffffe000aecdf2>] run_init_process+0xfa/0x10c
  [<ffffffe000aece16>] try_to_run_init_process+0x12/0x3c
  [<ffffffe000afa920>] kernel_init+0xb4/0xf8
  [<ffffffe000201b80>] ret_from_exception+0x0/0xc

Here is the error injection test code for the above output:
drivers/irqchip/irq-riscv-intc.c:
static asmlinkage void riscv_intc_irq(struct pt_regs *regs)

 {
        unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;

+ u32 tmp; __get_user(tmp, (u32 *)0);

Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Changbin Du <[email protected]>
---
 arch/riscv/kernel/entry.S      | 2 +-
 arch/riscv/kernel/stacktrace.c | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
arch/riscv/kernel/entry.S +0 -0
index b9eda3fcbd6d..329cf51fcd4d 100644
@@ -248,7 +248,7 @@ ret_from_syscall_rejected:
      andi t0, t0, _TIF_SYSCALL_WORK
      bnez t0, handle_syscall_trace_exit
Quoted 4 lines

arch/riscv/kernel/entry.S ret_from_syscall_rejected: · line 251

Quoted 2 lines
arch/riscv/kernel/entry.S +1 -1
-ret_from_exception:
+ENTRY(ret_from_exception)

This at least needs an END(), but it should also be converted over to

Yes, I missed END() here.

Quoted 3 lines

some non-function entry flavor. I converted it over to SYM_CODE_START_NOALIGN(), with the cooresponding SYM_CODE_END(), and put it on for-next.

Is that also for ret_from_fork & __switch_to?

Quoted 31 lines
Quoted 31 lines
arch/riscv/kernel/entry.S +0 -0
      REG_L s0, PT_STATUS(sp)
      csrc CSR_STATUS, SR_IE
 #ifdef CONFIG_TRACE_IRQFLAGS
arch/riscv/kernel/stacktrace.c +9 -0
index bcfe9eb55f80..75c8dd64fc48 100644
@@ -16,6 +16,8 @@
 
 #ifdef CONFIG_FRAME_POINTER
 
+extern asmlinkage void ret_from_exception(void);
+
 void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
                           bool (*fn)(void *, unsigned long), void *arg)
 {

@@ -59,6 +61,13 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
                      fp = frame->fp;
                      pc = ftrace_graph_ret_addr(current, NULL, frame->ra,
                                                 &frame->ra);
+                     if (pc == (unsigned long)ret_from_exception) {
+                             if (unlikely(!__kernel_text_address(pc) || !fn(arg, pc)))
+                                     break;
+
+                             pc = ((struct pt_regs *)sp)->epc;
+                             fp = ((struct pt_regs *)sp)->s0;
+                     }
              }
 
      }
Signature · 2 lines
Best Regards
 Guo Ren
Applied
#7 lore raw
In reply to [email protected]
Quoted 10 lines · On Wed, 9 Nov 2022 01:49:35 -0500, [email protected] wrote:

From: Guo Ren <[email protected]>

First is a fixup for the return address pointer. The second makes walk_stackframe could cross the pt_regs frame.

Guo Ren (2):
riscv: stacktrace: Fixup ftrace_graph_ret_addr retp argument
riscv: stacktrace: Make walk_stackframe cross pt_regs frame

[...]

Applied, thanks!

[1/2] riscv: stacktrace: Fixup ftrace_graph_ret_addr retp argument

      https://git.kernel.org/palmer/c/5c3022e4a616
[2/2] riscv: stacktrace: Make walk_stackframe cross pt_regs frame
      https://git.kernel.org/palmer/c/7ecdadf7f8c6

Best regards,

Signature · 1 line
Palmer Dabbelt <[email protected]>
CI Applied
#8 lore raw
In reply to [email protected]
This series was applied to riscv/linux.git (for-next) by Palmer Dabbelt <[email protected]>:

Hello:

This series was applied to riscv/linux.git (for-next) by Palmer Dabbelt <[email protected]>:

Quoted 10 lines · On Wed, 9 Nov 2022 01:49:35 -0500 you wrote:

From: Guo Ren <[email protected]>

First is a fixup for the return address pointer. The second makes walk_stackframe could cross the pt_regs frame.

Guo Ren (2):
riscv: stacktrace: Fixup ftrace_graph_ret_addr retp argument
riscv: stacktrace: Make walk_stackframe cross pt_regs frame

[...]

Here is the summary with links:
- [1/2] riscv: stacktrace: Fixup ftrace_graph_ret_addr retp argument

    https://git.kernel.org/riscv/c/5c3022e4a616
  - [2/2] riscv: stacktrace: Make walk_stackframe cross pt_regs frame
    https://git.kernel.org/riscv/c/7ecdadf7f8c6

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html