Make the stacktrace more effective and the codes more clear.
Jinyang He (6):
LoongArch: Get frame info in unwind_start when regs is not supported
LoongArch: Use correct sp value to get graph addr in unwinder guess
LoongArch: Adjust PC value when unwind next frame in prologue unwinder
LoongArch: Strip guess_unwinder out from prologue_unwinder
LoongArch: Add raw_show_trace to enable guess unwinder default
LoongArch: Add generic ex-handler unwind in prologue unwinder
arch/loongarch/include/asm/ftrace.h | 2 -
arch/loongarch/include/asm/unwind.h | 33 +++-
arch/loongarch/kernel/Makefile | 3 +-
arch/loongarch/kernel/genex.S | 3 +
arch/loongarch/kernel/process.c | 12 +-
arch/loongarch/kernel/unwind.c | 52 +++++
arch/loongarch/kernel/unwind_guess.c | 45 ++---
arch/loongarch/kernel/unwind_prologue.c | 248 +++++++++++++++---------
arch/loongarch/mm/tlb.c | 2 +-
9 files changed, 256 insertions(+), 144 deletions(-)
create mode 100644 arch/loongarch/kernel/unwind.c
--
2.34.3
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 <[email protected]>
---
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);
}
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;
}
state->task = task;
--
2.34.3
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 <[email protected]>
> ---
> 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;
>
Thanks,
-Qing
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 <[email protected]>
>> ---
>> 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