1. Patchew
  2. linux
  3. [PATCH 0/6] LoongArch: Some fix and new features for unwinders
  4. View patch

[PATCH 1/6] LoongArch: Get frame info in unwind_start when regs is not supported

Jinyang He posted 6 patches 2 years, 9 months ago
There is a newer version of this series
[PATCH 1/6] LoongArch: Get frame info in unwind_start when regs is not supported
Posted by Jinyang He 2 years, 9 months ago 
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);
 	}
 
 	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
Re: [PATCH 1/6] LoongArch: Get frame info in unwind_start when regs is not supported
Posted by Qing Zhang 2 years, 9 months ago 
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;
> 
Thanks,
-Qing
Re: [PATCH 1/6] LoongArch: Get frame info in unwind_start when regs is not supported
Posted by Jinyang He 2 years, 9 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.