| Message ID | 20240328184020.34278-2-puranjay12@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | riscv: ftrace: make stack walk more robust. | expand |
Puranjay Mohan <puranjay12@gmail.com> writes: > Currently, dump_backtrace(), __get_wchan(), and perf_callchain_kernel() > directly call walk_stackframe(). Make then call arch_stack_walk() which > is a wrapper around walk_stackframe() and make walk_stackframe() static. > > This allows making changes to walk_stackframe() without disturbing the > users of arch_stack_walk() which is the exposed function. > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > --- > arch/riscv/include/asm/stacktrace.h | 2 -- > arch/riscv/kernel/perf_callchain.c | 2 +- > arch/riscv/kernel/stacktrace.c | 26 ++++++++++++++------------ > 3 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/arch/riscv/include/asm/stacktrace.h b/arch/riscv/include/asm/stacktrace.h > index b1495a7e06ce6..32213e37c379f 100644 > --- a/arch/riscv/include/asm/stacktrace.h > +++ b/arch/riscv/include/asm/stacktrace.h > @@ -11,8 +11,6 @@ struct stackframe { > unsigned long ra; > }; > > -extern void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, > - bool (*fn)(void *, unsigned long), void *arg); > extern void dump_backtrace(struct pt_regs *regs, struct task_struct *task, > const char *loglvl); > > diff --git a/arch/riscv/kernel/perf_callchain.c b/arch/riscv/kernel/perf_callchain.c > index 3348a61de7d99..c023e0b1eb814 100644 > --- a/arch/riscv/kernel/perf_callchain.c > +++ b/arch/riscv/kernel/perf_callchain.c > @@ -74,5 +74,5 @@ static bool fill_callchain(void *entry, unsigned long pc) > void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, > struct pt_regs *regs) > { > - walk_stackframe(NULL, regs, fill_callchain, entry); > + arch_stack_walk(fill_callchain, entry, NULL, regs); > } > diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c > index 64a9c093aef93..e28f7b2e4b6a6 100644 > --- a/arch/riscv/kernel/stacktrace.c > +++ b/arch/riscv/kernel/stacktrace.c > @@ -18,8 +18,9 @@ > > 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) > +static __always_inline void > +walk_stackframe(struct task_struct *task, struct pt_regs *regs, > + bool (*fn)(void *, unsigned long), void *arg) Really a nit, but please try to keep the return value/linkage/function name on one line if possible. > { > unsigned long fp, sp, pc; > int level = 0; > @@ -76,8 +77,9 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, > > #else /* !CONFIG_FRAME_POINTER */ > > -void notrace walk_stackframe(struct task_struct *task, > - struct pt_regs *regs, bool (*fn)(void *, unsigned long), void *arg) > +static __always_inline void > +walk_stackframe(struct task_struct *task, struct pt_regs *regs, > + bool (*fn)(void *, unsigned long), void *arg) ...and here. Björn
diff --git a/arch/riscv/include/asm/stacktrace.h b/arch/riscv/include/asm/stacktrace.h index b1495a7e06ce6..32213e37c379f 100644 --- a/arch/riscv/include/asm/stacktrace.h +++ b/arch/riscv/include/asm/stacktrace.h @@ -11,8 +11,6 @@ struct stackframe { unsigned long ra; }; -extern void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, - bool (*fn)(void *, unsigned long), void *arg); extern void dump_backtrace(struct pt_regs *regs, struct task_struct *task, const char *loglvl); diff --git a/arch/riscv/kernel/perf_callchain.c b/arch/riscv/kernel/perf_callchain.c index 3348a61de7d99..c023e0b1eb814 100644 --- a/arch/riscv/kernel/perf_callchain.c +++ b/arch/riscv/kernel/perf_callchain.c @@ -74,5 +74,5 @@ static bool fill_callchain(void *entry, unsigned long pc) void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs) { - walk_stackframe(NULL, regs, fill_callchain, entry); + arch_stack_walk(fill_callchain, entry, NULL, regs); } diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c index 64a9c093aef93..e28f7b2e4b6a6 100644 --- a/arch/riscv/kernel/stacktrace.c +++ b/arch/riscv/kernel/stacktrace.c @@ -18,8 +18,9 @@ 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) +static __always_inline void +walk_stackframe(struct task_struct *task, struct pt_regs *regs, + bool (*fn)(void *, unsigned long), void *arg) { unsigned long fp, sp, pc; int level = 0; @@ -76,8 +77,9 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, #else /* !CONFIG_FRAME_POINTER */ -void notrace walk_stackframe(struct task_struct *task, - struct pt_regs *regs, bool (*fn)(void *, unsigned long), void *arg) +static __always_inline void +walk_stackframe(struct task_struct *task, struct pt_regs *regs, + bool (*fn)(void *, unsigned long), void *arg) { unsigned long sp, pc; unsigned long *ksp; @@ -107,6 +109,12 @@ void notrace walk_stackframe(struct task_struct *task, #endif /* CONFIG_FRAME_POINTER */ +noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, + struct task_struct *task, struct pt_regs *regs) +{ + walk_stackframe(task, regs, consume_entry, cookie); +} + static bool print_trace_address(void *arg, unsigned long pc) { const char *loglvl = arg; @@ -118,7 +126,7 @@ static bool print_trace_address(void *arg, unsigned long pc) noinline void dump_backtrace(struct pt_regs *regs, struct task_struct *task, const char *loglvl) { - walk_stackframe(task, regs, print_trace_address, (void *)loglvl); + arch_stack_walk(print_trace_address, (void *)loglvl, task, regs); } void show_stack(struct task_struct *task, unsigned long *sp, const char *loglvl) @@ -143,13 +151,7 @@ unsigned long __get_wchan(struct task_struct *task) if (!try_get_task_stack(task)) return 0; - walk_stackframe(task, NULL, save_wchan, &pc); + arch_stack_walk(save_wchan, &pc, task, NULL); put_task_stack(task); return pc; } - -noinline void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, - struct task_struct *task, struct pt_regs *regs) -{ - walk_stackframe(task, regs, consume_entry, cookie); -}
Currently, dump_backtrace(), __get_wchan(), and perf_callchain_kernel() directly call walk_stackframe(). Make then call arch_stack_walk() which is a wrapper around walk_stackframe() and make walk_stackframe() static. This allows making changes to walk_stackframe() without disturbing the users of arch_stack_walk() which is the exposed function. Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> --- arch/riscv/include/asm/stacktrace.h | 2 -- arch/riscv/kernel/perf_callchain.c | 2 +- arch/riscv/kernel/stacktrace.c | 26 ++++++++++++++------------ 3 files changed, 15 insertions(+), 15 deletions(-)