On Wed, Mar 19, 2025 at 11:38 AM Weinan Liu <wn...@google.com> wrote: > > On Tue, Mar 18, 2025 at 10:39 PM Josh Poimboeuf <jpoim...@kernel.org> wrote: > > > > On Tue, Mar 18, 2025 at 08:58:52PM -0700, Song Liu wrote: > > > On a closer look, I think we also need some logic in unwind_find_stack() > > > so that we can see when the unwinder hits the exception boundary. For > > > this reason, we may still need unwind_state.unreliable. I will look into > > > fixing this and send v2. > > > > Isn't that what FRAME_META_TYPE_PT_REGS is for? > > > > Maybe it can just tell kunwind_stack_walk() to set a bit in > > kunwind_state which tells kunwind_next_frame_record_meta() to quit the > > unwind early for the FRAME_META_TYPE_PT_REGS case. That also has the > > benefit of stopping the unwind as soon as the exception is encounterd. > > > > After reviewing the code flow, it seems like we should treat all -EINVALID > cases or `FRAME_META_TYPE_PT_REGS` cases as unreliable unwinds.
Agreed with this: all -EINVALID cases or `FRAME_META_TYPE_PT_REGS` should be unreliable, IIUC. > > Would a simplification like the one below work? > Or we can return a special value for success cases in kunwind_next_regs_pc() > > ``` > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c > index 69d0567a0c38..0eb69fa6161a 100644 > --- a/arch/arm64/kernel/stacktrace.c > +++ b/arch/arm64/kernel/stacktrace.c > @@ -296,7 +296,8 @@ do_kunwind(struct kunwind_state *state, > kunwind_consume_fn consume_state, > if (!consume_state(state, cookie)) > break; > ret = kunwind_next(state); > - if (ret < 0) > + if (ret < 0 || state->source == KUNWIND_SOURCE_REGS_PC) > + state->common.unreliable = true; I am current leaning toward not using common.unreliable. It seems to add unnecessary complexity here. But I may change my mind later on. Thanks, Song > break; > } > } > ``` > > -- > Weinan
