Messages in this thread |  | | Date | Fri, 22 Oct 2021 19:51:48 +0100 | From | Mark Rutland <> | Subject | Re: [PATCH v10 04/11] arm64: Make return_address() use arch_stack_walk() |
| |
On Thu, Oct 14, 2021 at 09:58:40PM -0500, madvenka@linux.microsoft.com wrote: > From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> > > Currently, return_address() in ARM64 code walks the stack using > start_backtrace() and walk_stackframe(). Make it use arch_stack_walk() > instead. This makes maintenance easier. > > Signed-off-by: Madhavan T. Venkataraman <madvenka@linux.microsoft.com> > --- > arch/arm64/kernel/return_address.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/arch/arm64/kernel/return_address.c b/arch/arm64/kernel/return_address.c > index a6d18755652f..92a0f4d434e4 100644 > --- a/arch/arm64/kernel/return_address.c > +++ b/arch/arm64/kernel/return_address.c > @@ -35,15 +35,11 @@ NOKPROBE_SYMBOL(save_return_addr); > void *return_address(unsigned int level) > { > struct return_address_data data; > - struct stackframe frame; > > data.level = level + 2; > data.addr = NULL; > > - start_backtrace(&frame, > - (unsigned long)__builtin_frame_address(0), > - (unsigned long)return_address); > - walk_stackframe(current, &frame, save_return_addr, &data); > + arch_stack_walk(save_return_addr, &data, current, NULL);
This looks equivalent to me. Previously the arguments to start_backtrace() meant that walk_stackframe would report return_address(), then the caller of return_address(), and so on. As arch_stack_walk() starts from its immediate caller (i.e. return_address()), that should result in the same trace.
It would be nice if we could note something to that effect in the commit message.
I had a play with ftrace, which uses return_address(), and that all looks sound.
> > if (!data.level) > return data.addr;
The end of this function currently does:
if (!data.level) return data.addr; else return NULL;
... but since we initialize data.addr to NULL, and save_return_addr() only writes to data.addr when called at the correct level, we can simplify that to:
return data.addr;
Regardles of that cleanup:
Reviewed-by: Mark Rutland <mark.rutland@arm.com> Tested-by: Mark Rutland <mark.rutland@arm.com>
I'll continue reviewing the series next week.
Thanks, Mark.
|  |