Messages in this thread | | | Subject | Re: [PATCH 1/6] LoongArch: Get frame info in unwind_start when regs is not supported | From | Jinyang He <> | Date | Thu, 15 Dec 2022 13:55:40 +0800 |
| |
On 2022-12-15 13:24, Qing Zhang wrote:
> Hi, Jinyang > > On 2022/12/15 下午12:01, Jinyang He wrote: >> At unwind_start, it is better to try to get its frame info even we not >> support regs rather than get them outside. So that we can simply use >> unwind_{start, next_frame, done} outside. >> >> Signed-off-by: Jinyang He <hejinyang@loongson.cn> >> --- >> arch/loongarch/kernel/process.c | 12 +++--------- >> arch/loongarch/kernel/unwind_guess.c | 6 ++++++ >> arch/loongarch/kernel/unwind_prologue.c | 16 +++++++++++++--- >> 3 files changed, 22 insertions(+), 12 deletions(-) >> >> diff --git a/arch/loongarch/kernel/process.c >> b/arch/loongarch/kernel/process.c >> index 502b8b950057..6ef45174ad35 100644 >> --- a/arch/loongarch/kernel/process.c >> +++ b/arch/loongarch/kernel/process.c >> @@ -197,20 +197,14 @@ int copy_thread(struct task_struct *p, const >> struct kernel_clone_args *args) >> unsigned long __get_wchan(struct task_struct *task) >> { >> - unsigned long pc; >> + unsigned long pc = 0; >> struct unwind_state state; >> if (!try_get_task_stack(task)) >> return 0; >> - unwind_start(&state, task, NULL); >> - state.sp = thread_saved_fp(task); >> - get_stack_info(state.sp, state.task, &state.stack_info); >> - state.pc = thread_saved_ra(task); >> -#ifdef CONFIG_UNWINDER_PROLOGUE >> - state.type = UNWINDER_PROLOGUE; >> -#endif >> - for (; !unwind_done(&state); unwind_next_frame(&state)) { >> + for (unwind_start(&state, task, NULL); !unwind_done(&state); >> + unwind_next_frame(&state)) { >> pc = unwind_get_return_address(&state); >> if (!pc) >> break; >> diff --git a/arch/loongarch/kernel/unwind_guess.c >> b/arch/loongarch/kernel/unwind_guess.c >> index e2d2e4f3001f..e03864511582 100644 >> --- a/arch/loongarch/kernel/unwind_guess.c >> +++ b/arch/loongarch/kernel/unwind_guess.c >> @@ -26,6 +26,12 @@ void unwind_start(struct unwind_state *state, >> struct task_struct *task, >> if (regs) { >> state->sp = regs->regs[3]; >> state->pc = regs->csr_era; >> + } else if (task == current) { >> + state->sp = (unsigned long)__builtin_frame_address(0); >> + state->pc = (unsigned long)__builtin_return_address(0); >> + } else { >> + state->sp = thread_saved_fp(task); >> + state->pc = thread_saved_ra(task); > The case for tasks is already handled in stacktrace.c, which is > reusable code: > void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, > struct task_struct *task, struct pt_regs *regs) > { > ... > > if (task == current) { > regs->regs[3] = (unsigned long)__builtin_frame_address(0); > regs->csr_era = (unsigned long)__builtin_return_address(0); > } else { > regs->regs[3] = thread_saved_fp(task); > regs->csr_era = thread_saved_ra(task); > } > ... > for (unwind_start(&state, task, regs); > !unwind_done(&state); unwind_next_frame(&state)) { > addr = unwind_get_return_address(&state); > ... > } > >> state->task = task; >> diff --git a/arch/loongarch/kernel/unwind_prologue.c >> b/arch/loongarch/kernel/unwind_prologue.c >> index 0f8d1451ebb8..9d51ea37782e 100644 >> --- a/arch/loongarch/kernel/unwind_prologue.c >> +++ b/arch/loongarch/kernel/unwind_prologue.c >> @@ -141,12 +141,22 @@ void unwind_start(struct unwind_state *state, >> struct task_struct *task, >> struct pt_regs *regs) >> { >> memset(state, 0, sizeof(*state)); >> + state->type = UNWINDER_PROLOGUE; >> - if (regs && __kernel_text_address(regs->csr_era)) { >> - state->pc = regs->csr_era; >> + if (regs) { >> state->sp = regs->regs[3]; >> + state->pc = regs->csr_era; >> state->ra = regs->regs[1]; >> - state->type = UNWINDER_PROLOGUE; >> + if (!__kernel_text_address(state->pc)) >> + state->type = UNWINDER_GUESS; >> + } else if (task == current) { >> + state->sp = (unsigned long)__builtin_frame_address(0); >> + state->pc = (unsigned long)__builtin_return_address(0); >> + state->ra = 0; >> + } else { >> + state->sp = thread_saved_fp(task); >> + state->pc = thread_saved_ra(task); >> + state->ra = 0; >> } > also... >> state->task = task; >> That looks similar, but stacktrace actually provides the regs parameter. Unwind_{start, next_frame, get_return_address} is API to others, such the GPL module. Btw, if we want to drop the redundant codes in arch_stack_walk and keep same result, use "cookie->skip += 1", I think, but beyond this series.
Thanks,
Jinyang
| |