I reviewed the changes briefly. They look good. I will take a more detailed look this week.

Thanks for doing this!

Once this is part of v5.16-rc2, I will send out version 11 on top of that with the rest of
the patches in my series.

Madhavan

On 11/12/21 11:44 AM, Mark Rutland wrote:

On Fri, Oct 22, 2021 at 07:02:43PM +0100, Mark Rutland wrote:

quoted

On Thu, Oct 14, 2021 at 09:58:37PM -0500, madvenka@linux.microsoft.com wrote:

quoted

From: "Madhavan T. Venkataraman" <redacted>

Currently, there are multiple functions in ARM64 code that walk the
stack using start_backtrace() and unwind_frame() or start_backtrace()
and walk_stackframe(). They should all be converted to use
arch_stack_walk(). This makes maintenance easier.

To do that, arch_stack_walk() must always be defined. arch_stack_walk()
is within #ifdef CONFIG_STACKTRACE. So, select STACKTRACE in
arch/arm64/Kconfig.

I'd prefer if we could decouple ARCH_STACKWALK from STACKTRACE, so that
we don't have to expose /proc/*/stack unconditionally, which Peter
Zijlstra has a patch for:

  https://lore.kernel.org/lkml/20211022152104.356586621@infradead.org/ (local)

... but regardless the rest of the series looks pretty good, so I'll go
review that, and we can figure out how to queue the bits and pieces in
the right order.

FWIW, it looks like the direction of travel there is not go and unify
the various arch unwinders, but I would like to not depend on
STACKTRACE. Regardless, the initial arch_stack_walk() cleanup patches
all look good, so I reckon we should try to get those out of the way and
queue those for arm64 soon even if we need some more back-and-forth over
the later part of the series.

With that in mind, I've picked up Peter's patch decoupling
ARCH_STACKWALK from STACKTRACE, and rebased the initial patches from
this series atop. Since there's some subtltety in a few cases (and this
was easy to miss while reviewing), I've expanded the commit messages
with additional rationale as to why each transformation is safe.
I've pushed that to:

  https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/stacktrace/arch-stack-walk

There's a dependency on:

  https://lore.kernel.org/r/20211029162245.39761-1-mark.rutland@arm.com (local)

... which was queued for v5.16-rc1, but got dropped due to a conflict,
and I'm expecting it to be re-queued as a fix for v5.16-rc2 shortly
after v5.16-rc1 is tagged. Hopefully that means we have a table base by
v5.16-rc2.

I'll send the preparatory series as I've prepared it shortly after
v5.16-rc1 so that people can shout if I've messed something up.

Hopefully it's easy enough to use that as a base for the more involved
rework later in this series.

Thanks,
Mark.

quoted

Thanks,
Mark.

quoted

Signed-off-by: Madhavan T. Venkataraman <redacted>
---
 arch/arm64/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fdcd54d39c1e..bfb0ce60d820 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig

@@ -35,6 +35,7 @@ config ARM64
 	select ARCH_HAS_SET_DIRECT_MAP
 	select ARCH_HAS_SET_MEMORY
 	select ARCH_STACKWALK
+	select STACKTRACE
 	select ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_HAS_STRICT_MODULE_RWX
 	select ARCH_HAS_SYNC_DMA_FOR_DEVICE

-- 
2.25.1