From: Mark Rutland <mark.rutland@arm.com>
To: madvenka@linux.microsoft.com
Cc: broonie@kernel.org, jpoimboe@redhat.com, ardb@kernel.org,
nobuta.keiya@fujitsu.com, sjitindarsingh@gmail.com,
catalin.marinas@arm.com, will@kernel.org, jmorris@namei.org,
linux-arm-kernel@lists.infradead.org,
live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v12 03/10] arm64: Rename stackframe to unwind_state
Date: Thu, 6 Jan 2022 16:11:31 +0000 [thread overview]
Message-ID: <YdcUsyNmALulzj3/@FVFF77S0Q05N> (raw)
In-Reply-To: <20220103165212.9303-4-madvenka@linux.microsoft.com>
On Mon, Jan 03, 2022 at 10:52:05AM -0600, madvenka@linux.microsoft.com wrote:
> From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>
>
> Rename "struct stackframe" to "struct unwind_state" for consistency and
> better naming. Accordingly, rename variable/argument "frame" to "state".
>
> Signed-off-by: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
Thanks for this!
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> arch/arm64/include/asm/stacktrace.h | 2 +-
> arch/arm64/kernel/stacktrace.c | 66 ++++++++++++++---------------
> 2 files changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
> index 3a15d376ab36..fc828c3c5dfd 100644
> --- a/arch/arm64/include/asm/stacktrace.h
> +++ b/arch/arm64/include/asm/stacktrace.h
> @@ -52,7 +52,7 @@ struct stack_info {
> * associated with the most recently encountered replacement lr
> * value.
> */
> -struct stackframe {
> +struct unwind_state {
> unsigned long fp;
> unsigned long pc;
> DECLARE_BITMAP(stacks_done, __NR_STACK_TYPES);
> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> index b980d96dccfc..a1a7ff93b84f 100644
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c
> @@ -33,13 +33,13 @@
> */
>
>
> -static void unwind_init(struct stackframe *frame, unsigned long fp,
> +static void unwind_init(struct unwind_state *state, unsigned long fp,
> unsigned long pc)
> {
> - frame->fp = fp;
> - frame->pc = pc;
> + state->fp = fp;
> + state->pc = pc;
> #ifdef CONFIG_KRETPROBES
> - frame->kr_cur = NULL;
> + state->kr_cur = NULL;
> #endif
>
> /*
> @@ -51,9 +51,9 @@ static void unwind_init(struct stackframe *frame, unsigned long fp,
> * prev_fp value won't be used, but we set it to 0 such that it is
> * definitely not an accessible stack address.
> */
> - bitmap_zero(frame->stacks_done, __NR_STACK_TYPES);
> - frame->prev_fp = 0;
> - frame->prev_type = STACK_TYPE_UNKNOWN;
> + bitmap_zero(state->stacks_done, __NR_STACK_TYPES);
> + state->prev_fp = 0;
> + state->prev_type = STACK_TYPE_UNKNOWN;
> }
>
> /*
> @@ -64,9 +64,9 @@ static void unwind_init(struct stackframe *frame, unsigned long fp,
> * and the location (but not the fp value) of B.
> */
> static int notrace unwind_next(struct task_struct *tsk,
> - struct stackframe *frame)
> + struct unwind_state *state)
> {
> - unsigned long fp = frame->fp;
> + unsigned long fp = state->fp;
> struct stack_info info;
>
> /* Final frame; nothing to unwind */
> @@ -79,7 +79,7 @@ static int notrace unwind_next(struct task_struct *tsk,
> if (!on_accessible_stack(tsk, fp, 16, &info))
> return -EINVAL;
>
> - if (test_bit(info.type, frame->stacks_done))
> + if (test_bit(info.type, state->stacks_done))
> return -EINVAL;
>
> /*
> @@ -95,27 +95,27 @@ static int notrace unwind_next(struct task_struct *tsk,
> * stack to another, it's never valid to unwind back to that first
> * stack.
> */
> - if (info.type == frame->prev_type) {
> - if (fp <= frame->prev_fp)
> + if (info.type == state->prev_type) {
> + if (fp <= state->prev_fp)
> return -EINVAL;
> } else {
> - set_bit(frame->prev_type, frame->stacks_done);
> + set_bit(state->prev_type, state->stacks_done);
> }
>
> /*
> * Record this frame record's values and location. The prev_fp and
> * prev_type are only meaningful to the next unwind_next() invocation.
> */
> - frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
> - frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
> - frame->prev_fp = fp;
> - frame->prev_type = info.type;
> + state->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
> + state->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
> + state->prev_fp = fp;
> + state->prev_type = info.type;
>
> - frame->pc = ptrauth_strip_insn_pac(frame->pc);
> + state->pc = ptrauth_strip_insn_pac(state->pc);
>
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> if (tsk->ret_stack &&
> - (frame->pc == (unsigned long)return_to_handler)) {
> + (state->pc == (unsigned long)return_to_handler)) {
> unsigned long orig_pc;
> /*
> * This is a case where function graph tracer has
> @@ -123,16 +123,16 @@ static int notrace unwind_next(struct task_struct *tsk,
> * to hook a function return.
> * So replace it to an original value.
> */
> - orig_pc = ftrace_graph_ret_addr(tsk, NULL, frame->pc,
> - (void *)frame->fp);
> - if (WARN_ON_ONCE(frame->pc == orig_pc))
> + orig_pc = ftrace_graph_ret_addr(tsk, NULL, state->pc,
> + (void *)state->fp);
> + if (WARN_ON_ONCE(state->pc == orig_pc))
> return -EINVAL;
> - frame->pc = orig_pc;
> + state->pc = orig_pc;
> }
> #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> #ifdef CONFIG_KRETPROBES
> - if (is_kretprobe_trampoline(frame->pc))
> - frame->pc = kretprobe_find_ret_addr(tsk, (void *)frame->fp, &frame->kr_cur);
> + if (is_kretprobe_trampoline(state->pc))
> + state->pc = kretprobe_find_ret_addr(tsk, (void *)state->fp, &state->kr_cur);
> #endif
>
> return 0;
> @@ -140,15 +140,15 @@ static int notrace unwind_next(struct task_struct *tsk,
> NOKPROBE_SYMBOL(unwind_next);
>
> static void notrace unwind(struct task_struct *tsk,
> - struct stackframe *frame,
> + struct unwind_state *state,
> bool (*fn)(void *, unsigned long), void *data)
> {
> while (1) {
> int ret;
>
> - if (!fn(data, frame->pc))
> + if (!fn(data, state->pc))
> break;
> - ret = unwind_next(tsk, frame);
> + ret = unwind_next(tsk, state);
> if (ret < 0)
> break;
> }
> @@ -192,17 +192,17 @@ noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry,
> void *cookie, struct task_struct *task,
> struct pt_regs *regs)
> {
> - struct stackframe frame;
> + struct unwind_state state;
>
> if (regs)
> - unwind_init(&frame, regs->regs[29], regs->pc);
> + unwind_init(&state, regs->regs[29], regs->pc);
> else if (task == current)
> - unwind_init(&frame,
> + unwind_init(&state,
> (unsigned long)__builtin_frame_address(1),
> (unsigned long)__builtin_return_address(0));
> else
> - unwind_init(&frame, thread_saved_fp(task),
> + unwind_init(&state, thread_saved_fp(task),
> thread_saved_pc(task));
>
> - unwind(task, &frame, consume_entry, cookie);
> + unwind(task, &state, consume_entry, cookie);
> }
> --
> 2.25.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-01-06 16:13 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <0d0eb36f348fb5a6af6eb592c0525f6e94007328>
2022-01-03 16:52 ` [PATCH v12 00/10] arm64: Reorganize the unwinder and implement stack trace reliability checks madvenka
2022-01-03 16:52 ` [PATCH v12 01/10] arm64: Remove NULL task check from unwind_frame() madvenka
2022-01-06 16:07 ` Mark Rutland
2022-01-03 16:52 ` [PATCH v12 02/10] arm64: Rename unwinder functions madvenka
2022-01-06 16:10 ` Mark Rutland
2022-01-03 16:52 ` [PATCH v12 03/10] arm64: Rename stackframe to unwind_state madvenka
2022-01-04 14:59 ` Mark Brown
2022-01-06 16:11 ` Mark Rutland [this message]
2022-01-03 16:52 ` [PATCH v12 04/10] arm64: Split unwind_init() madvenka
2022-01-06 16:31 ` Mark Rutland
2022-01-06 20:13 ` Madhavan T. Venkataraman
2022-01-03 16:52 ` [PATCH v12 05/10] arm64: Copy unwind arguments to unwind_state madvenka
2022-01-05 16:57 ` Mark Brown
2022-01-06 16:37 ` Mark Rutland
2022-01-06 20:17 ` Madhavan T. Venkataraman
2022-01-03 16:52 ` [PATCH v12 06/10] arm64: Make the unwind loop in unwind() similar to other architectures madvenka
2022-01-03 16:52 ` [PATCH v12 07/10] arm64: Introduce stack trace reliability checks in the unwinder madvenka
2022-01-05 16:58 ` Mark Brown
2022-01-05 23:58 ` Madhavan T. Venkataraman
2022-01-06 11:43 ` Mark Brown
2022-01-03 16:52 ` [PATCH v12 08/10] arm64: Create a list of SYM_CODE functions, check return PC against list madvenka
2022-01-03 16:52 ` [PATCH v12 09/10] arm64: Introduce arch_stack_walk_reliable() madvenka
2022-01-03 16:52 ` [PATCH v12 10/10] arm64: Select HAVE_RELIABLE_STACKTRACE madvenka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YdcUsyNmALulzj3/@FVFF77S0Q05N \
--to=mark.rutland@arm.com \
--cc=ardb@kernel.org \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=jmorris@namei.org \
--cc=jpoimboe@redhat.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=madvenka@linux.microsoft.com \
--cc=nobuta.keiya@fujitsu.com \
--cc=sjitindarsingh@gmail.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox