2023-11-26 12:18:38

by Jinyang He

[permalink] [raw]
Subject: [PATCH] LoongArch: Set unwind stack type to unknown rather than set error flag

During the unwinding, unwind_done() is used as an end condition. Normally
it unwind to the user stack and then set the stack type to unknown, which
is a normal exit. When something unexpected happens in unwind process and
we cannot unwind anymore, we should set the error flag, and also set the
stack type to unknown to indicate that the unwind process cannot continue.
The error flag emphasizes that the unwind process produce an unexpected
error. There is no unexpected things when we unwind the PT_REGS in the
top of IRQ stack and find out that is an user mode PT_REGS. Thus, we
should not set error flag and just set stack type to unknown.

Reported-by: Hengqi Chen <[email protected]>
Signed-off-by: Jinyang He <[email protected]>
---
arch/loongarch/kernel/stacktrace.c | 2 +-
arch/loongarch/kernel/unwind.c | 1 -
arch/loongarch/kernel/unwind_prologue.c | 2 +-
3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/loongarch/kernel/stacktrace.c b/arch/loongarch/kernel/stacktrace.c
index 92270f14db94..f623feb2129f 100644
--- a/arch/loongarch/kernel/stacktrace.c
+++ b/arch/loongarch/kernel/stacktrace.c
@@ -32,7 +32,7 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
}

for (unwind_start(&state, task, regs);
- !unwind_done(&state) && !unwind_error(&state); unwind_next_frame(&state)) {
+ !unwind_done(&state); unwind_next_frame(&state)) {
addr = unwind_get_return_address(&state);
if (!addr || !consume_entry(cookie, addr))
break;
diff --git a/arch/loongarch/kernel/unwind.c b/arch/loongarch/kernel/unwind.c
index ba324ba76fa1..a463d6961344 100644
--- a/arch/loongarch/kernel/unwind.c
+++ b/arch/loongarch/kernel/unwind.c
@@ -28,6 +28,5 @@ bool default_next_frame(struct unwind_state *state)

} while (!get_stack_info(state->sp, state->task, info));

- state->error = true;
return false;
}
diff --git a/arch/loongarch/kernel/unwind_prologue.c b/arch/loongarch/kernel/unwind_prologue.c
index 55afc27320e1..929ae240280a 100644
--- a/arch/loongarch/kernel/unwind_prologue.c
+++ b/arch/loongarch/kernel/unwind_prologue.c
@@ -227,7 +227,7 @@ static bool next_frame(struct unwind_state *state)
} while (!get_stack_info(state->sp, state->task, info));

out:
- state->error = true;
+ state->stack_info.type = STACK_TYPE_UNKNOWN;
return false;
}

--
2.42.0


2023-11-27 02:41:58

by Hengqi Chen

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: Set unwind stack type to unknown rather than set error flag

On Sun, Nov 26, 2023 at 8:10 PM Jinyang He <[email protected]> wrote:
>
> During the unwinding, unwind_done() is used as an end condition. Normally
> it unwind to the user stack and then set the stack type to unknown, which
> is a normal exit. When something unexpected happens in unwind process and
> we cannot unwind anymore, we should set the error flag, and also set the
> stack type to unknown to indicate that the unwind process cannot continue.
> The error flag emphasizes that the unwind process produce an unexpected
> error. There is no unexpected things when we unwind the PT_REGS in the
> top of IRQ stack and find out that is an user mode PT_REGS. Thus, we
> should not set error flag and just set stack type to unknown.
>
> Reported-by: Hengqi Chen <[email protected]>
> Signed-off-by: Jinyang He <[email protected]>
> ---
> arch/loongarch/kernel/stacktrace.c | 2 +-
> arch/loongarch/kernel/unwind.c | 1 -
> arch/loongarch/kernel/unwind_prologue.c | 2 +-
> 3 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/loongarch/kernel/stacktrace.c b/arch/loongarch/kernel/stacktrace.c
> index 92270f14db94..f623feb2129f 100644
> --- a/arch/loongarch/kernel/stacktrace.c
> +++ b/arch/loongarch/kernel/stacktrace.c
> @@ -32,7 +32,7 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
> }
>
> for (unwind_start(&state, task, regs);
> - !unwind_done(&state) && !unwind_error(&state); unwind_next_frame(&state)) {
> + !unwind_done(&state); unwind_next_frame(&state)) {
> addr = unwind_get_return_address(&state);
> if (!addr || !consume_entry(cookie, addr))
> break;
> diff --git a/arch/loongarch/kernel/unwind.c b/arch/loongarch/kernel/unwind.c
> index ba324ba76fa1..a463d6961344 100644
> --- a/arch/loongarch/kernel/unwind.c
> +++ b/arch/loongarch/kernel/unwind.c
> @@ -28,6 +28,5 @@ bool default_next_frame(struct unwind_state *state)
>
> } while (!get_stack_info(state->sp, state->task, info));
>
> - state->error = true;
> return false;
> }
> diff --git a/arch/loongarch/kernel/unwind_prologue.c b/arch/loongarch/kernel/unwind_prologue.c
> index 55afc27320e1..929ae240280a 100644
> --- a/arch/loongarch/kernel/unwind_prologue.c
> +++ b/arch/loongarch/kernel/unwind_prologue.c
> @@ -227,7 +227,7 @@ static bool next_frame(struct unwind_state *state)
> } while (!get_stack_info(state->sp, state->task, info));
>
> out:
> - state->error = true;
> + state->stack_info.type = STACK_TYPE_UNKNOWN;
> return false;
> }
>
> --
> 2.42.0
>

Acked-by: Hengqi Chen <[email protected]>