Re: [PATCH v3 6/8] riscv: stacktrace: switch to frame-pointer based unwinder

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Comment inaccurately describes the memory location of the metadata type field.
--

commit 0eadbda136e709b7b12de76094d4f29e825575fb
Author: Wang Han <wanghan@linux.alibaba.com>

riscv: stacktrace: switch to frame-pointer based unwinder

Replace the open-coded frame-pointer walker in arch_stack_walk() with a
robust kunwind state machine. The new walker tracks stack bounds, consumes
frame records monotonically, and integrates with function graph tracing and
perf callchains.

> diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
> index 2692d3a06afa2..8fcf23b046d59 100644
> --- a/arch/riscv/kernel/stacktrace.c
> +++ b/arch/riscv/kernel/stacktrace.c
[ ... ]
> +/*
> + * Unwind from one frame record to the next.
> + *
> + * On RISC-V, the frame record sits at fp - sizeof(struct frame_record),
> + * immediately below the address pointed to by fp/s0. This applies to both
> + * normal frame records and metadata frame records (embedded in pt_regs).
> + *
> + * A metadata record is identified by both fp and ra being zero in the
> + * frame_record portion, with a type value following at fp + 16.
> + */

[Severity: Low]
Is the comment describing the metadata type location accurate?

It states "with a type value following at fp + 16". However, since fp points
directly above the 16-byte frame_record, and the type field immediately
follows the record in struct frame_record_meta, wouldn't the type field be
located exactly at fp rather than fp + 16?

> +static __always_inline int
> +kunwind_next_frame_record(struct kunwind_state *state)
> +{
> +	unsigned long fp = state->common.fp;
> +	struct frame_record *record;
> +	struct stack_info *info;
> +	unsigned long new_fp, new_pc;
> +	unsigned long record_base;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com?part=6