* [PATCH v4 0/2] ARM: Convert to ARCH_STACKWALK
@ 2022-10-18 12:43 Li Huafei
2022-10-18 12:43 ` [PATCH v4 1/2] ARM: stacktrace: Make stack walk callback consistent with generic code Li Huafei
2022-10-18 12:43 ` [PATCH v4 2/2] ARM: stacktrace: Convert stacktrace to generic ARCH_STACKWALK Li Huafei
0 siblings, 2 replies; 3+ messages in thread
From: Li Huafei @ 2022-10-18 12:43 UTC (permalink / raw
To: linux, linus.walleij
Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, will, arnd, ardb, rmk+kernel, geert+renesas,
nick.hawkins, john, mhiramat, lihuafei1, ast, thunder.leizhen,
rostedt, linyujun809, linux-arm-kernel, linux-kernel,
linux-perf-users
This series mainly updates the ARM stack trace code to use the newer and
simpler arch_stack_walk() interface.
When Russell applied patch 1, the patch tracker complained of a minor
conflict with Lei Zhen's patch (see [1]). Lei Zhen's patch has been
merged into v6.1-rc1. Now rebase to v6.1-rc1 and resend this patch set.
Thanks!
[1] https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9235/1
v4:
- Rebase to v6.1-rc1.
- Fix commit log for patch 2: 'arch _stack_walk()' -> 'arch_stack_walk()'.
- Remove the two bugfix patches that have been merged in:
ARM: stacktrace: Skip frame pointer boundary check for call_with_stack()
ARM: stacktrace: Avoid duplicate saving of exception PC value
v3 resend: https://lore.kernel.org/lkml/20220826074047.107357-1-lihuafei1@huawei.com/
It's been a month since I sent the v3. Linus told me I needed to add the
patch to Russell's patch tracker myself, rebase to v6.0-rc1 before that.
Thanks Linus for the tip!
Since commit af6f23b88e95 ("ARM/dma-mapping: use the generic versions of
dma_to_phys/phys_to_dma by default") and commit ae626eb97376
("ARM/dma-mapping: use dma-direct unconditionally") modified
arch/arm/Kconfig, there is a minor conflict with patch 4 when rebase.
v3: https://lore.kernel.org/lkml/20220727040022.139387-1-lihuafei1@huawei.com/
- According to the discussion with Linus and Russell in v1:
- Add a comment about "regs[1]" in patch 2, and remove the
unnecessary ternary operator in the initialization of
"frame->ex_frame".
- Remove the patch "ARM: stacktrace: Allow stack trace saving for
non-current tasks", and keep the check for not being able to
unwind non-current tasks (including tasks running on other CPUs)
when CONFIG_SMP=y in patch 4.
- Rebase to linux-5.19-rc8.
v2: https://lore.kernel.org/lkml/20220713110020.85511-1-lihuafei1@huawei.com/
- As suggested by Mark, the commit logs for patch 4 and 5 were
refined for easy review.
v1: https://lore.kernel.org/lkml/20220712021527.109921-1-lihuafei1@huawei.com/
Li Huafei (2):
ARM: stacktrace: Make stack walk callback consistent with generic code
ARM: stacktrace: Convert stacktrace to generic ARCH_STACKWALK
arch/arm/Kconfig | 1 +
arch/arm/include/asm/stacktrace.h | 2 +-
arch/arm/kernel/perf_callchain.c | 9 +--
arch/arm/kernel/return_address.c | 8 +-
arch/arm/kernel/stacktrace.c | 119 +++++++++---------------------
5 files changed, 44 insertions(+), 95 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v4 1/2] ARM: stacktrace: Make stack walk callback consistent with generic code
2022-10-18 12:43 [PATCH v4 0/2] ARM: Convert to ARCH_STACKWALK Li Huafei
@ 2022-10-18 12:43 ` Li Huafei
2022-10-18 12:43 ` [PATCH v4 2/2] ARM: stacktrace: Convert stacktrace to generic ARCH_STACKWALK Li Huafei
1 sibling, 0 replies; 3+ messages in thread
From: Li Huafei @ 2022-10-18 12:43 UTC (permalink / raw
To: linux, linus.walleij
Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, will, arnd, ardb, rmk+kernel, geert+renesas,
nick.hawkins, john, mhiramat, lihuafei1, ast, thunder.leizhen,
rostedt, linyujun809, linux-arm-kernel, linux-kernel,
linux-perf-users
As with the generic arch_stack_walk() code the ARM stack walk code takes
a callback that is called per stack frame. Currently the ARM code always
passes a struct stackframe to the callback and the generic code just
passes the pc, however none of the users ever reference anything in the
struct other than the pc value. The ARM code also uses a return type of
int while the generic code uses a return type of bool though in both
cases the return value is a boolean value and the sense is inverted
between the two.
In order to reduce code duplication when ARM is converted to use
arch_stack_walk() change the signature and return sense of the ARM
specific callback to match that of the generic code.
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Linus Waleij <linus.walleij@linaro.org>
---
arch/arm/include/asm/stacktrace.h | 2 +-
arch/arm/kernel/perf_callchain.c | 9 ++++-----
arch/arm/kernel/return_address.c | 8 ++++----
arch/arm/kernel/stacktrace.c | 13 ++++++-------
4 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/arch/arm/include/asm/stacktrace.h b/arch/arm/include/asm/stacktrace.h
index 36b2ff44fcbb..360f0d2406bf 100644
--- a/arch/arm/include/asm/stacktrace.h
+++ b/arch/arm/include/asm/stacktrace.h
@@ -44,7 +44,7 @@ void arm_get_current_stackframe(struct pt_regs *regs, struct stackframe *frame)
extern int unwind_frame(struct stackframe *frame);
extern void walk_stackframe(struct stackframe *frame,
- int (*fn)(struct stackframe *, void *), void *data);
+ bool (*fn)(void *, unsigned long), void *data);
extern void dump_mem(const char *lvl, const char *str, unsigned long bottom,
unsigned long top);
extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk,
diff --git a/arch/arm/kernel/perf_callchain.c b/arch/arm/kernel/perf_callchain.c
index bc6b246ab55e..7147edbe56c6 100644
--- a/arch/arm/kernel/perf_callchain.c
+++ b/arch/arm/kernel/perf_callchain.c
@@ -81,13 +81,12 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs
* whist unwinding the stackframe and is like a subroutine return so we use
* the PC.
*/
-static int
-callchain_trace(struct stackframe *fr,
- void *data)
+static bool
+callchain_trace(void *data, unsigned long pc)
{
struct perf_callchain_entry_ctx *entry = data;
- perf_callchain_store(entry, fr->pc);
- return 0;
+ perf_callchain_store(entry, pc);
+ return true;
}
void
diff --git a/arch/arm/kernel/return_address.c b/arch/arm/kernel/return_address.c
index 38f1ea9c724d..ac15db66df4c 100644
--- a/arch/arm/kernel/return_address.c
+++ b/arch/arm/kernel/return_address.c
@@ -16,17 +16,17 @@ struct return_address_data {
void *addr;
};
-static int save_return_addr(struct stackframe *frame, void *d)
+static bool save_return_addr(void *d, unsigned long pc)
{
struct return_address_data *data = d;
if (!data->level) {
- data->addr = (void *)frame->pc;
+ data->addr = (void *)pc;
- return 1;
+ return false;
} else {
--data->level;
- return 0;
+ return true;
}
}
diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
index 85443b5d1922..d05968bc7812 100644
--- a/arch/arm/kernel/stacktrace.c
+++ b/arch/arm/kernel/stacktrace.c
@@ -127,12 +127,12 @@ int notrace unwind_frame(struct stackframe *frame)
#endif
void notrace walk_stackframe(struct stackframe *frame,
- int (*fn)(struct stackframe *, void *), void *data)
+ bool (*fn)(void *, unsigned long), void *data)
{
while (1) {
int ret;
- if (fn(frame, data))
+ if (!fn(data, frame->pc))
break;
ret = unwind_frame(frame);
if (ret < 0)
@@ -148,21 +148,20 @@ struct stack_trace_data {
unsigned int skip;
};
-static int save_trace(struct stackframe *frame, void *d)
+static bool save_trace(void *d, unsigned long addr)
{
struct stack_trace_data *data = d;
struct stack_trace *trace = data->trace;
- unsigned long addr = frame->pc;
if (data->no_sched_functions && in_sched_functions(addr))
- return 0;
+ return true;
if (data->skip) {
data->skip--;
- return 0;
+ return true;
}
trace->entries[trace->nr_entries++] = addr;
- return trace->nr_entries >= trace->max_entries;
+ return trace->nr_entries < trace->max_entries;
}
/* This must be noinline to so that our skip calculation works correctly */
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v4 2/2] ARM: stacktrace: Convert stacktrace to generic ARCH_STACKWALK
2022-10-18 12:43 [PATCH v4 0/2] ARM: Convert to ARCH_STACKWALK Li Huafei
2022-10-18 12:43 ` [PATCH v4 1/2] ARM: stacktrace: Make stack walk callback consistent with generic code Li Huafei
@ 2022-10-18 12:43 ` Li Huafei
1 sibling, 0 replies; 3+ messages in thread
From: Li Huafei @ 2022-10-18 12:43 UTC (permalink / raw
To: linux, linus.walleij
Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, will, arnd, ardb, rmk+kernel, geert+renesas,
nick.hawkins, john, mhiramat, lihuafei1, ast, thunder.leizhen,
rostedt, linyujun809, linux-arm-kernel, linux-kernel,
linux-perf-users
Historically architectures have had duplicated code in their stack trace
implementations for filtering what gets traced. In order to avoid this
duplication some generic code has been provided using a new interface
arch_stack_walk(), enabled by selecting ARCH_STACKWALK in Kconfig, which
factors all this out into the generic stack trace code. Convert ARM to
use this common infrastructure.
When initializing the stack frame of the current task, arm64 uses
__builtin_frame_address(1) to initialize the frame pointer, skipping
arch_stack_walk(), see the commit c607ab4f916d ("arm64: stacktrace:
don't trace arch_stack_walk()"). Since __builtin_frame_address(1) does
not work on ARM, unwind_frame() is used to unwind the stack one layer
forward before calling walk_stackframe().
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
arch/arm/Kconfig | 1 +
arch/arm/kernel/stacktrace.c | 114 ++++++++++-------------------------
2 files changed, 33 insertions(+), 82 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a08c9d092a33..16920f2215f2 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -17,6 +17,7 @@ config ARM
select ARCH_HAS_PTE_SPECIAL if ARM_LPAE
select ARCH_HAS_SETUP_DMA_OPS
select ARCH_HAS_SET_MEMORY
+ select ARCH_STACKWALK
select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
select ARCH_HAS_STRICT_MODULE_RWX if MMU
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
index d05968bc7812..620aa82e3bdd 100644
--- a/arch/arm/kernel/stacktrace.c
+++ b/arch/arm/kernel/stacktrace.c
@@ -142,40 +142,32 @@ void notrace walk_stackframe(struct stackframe *frame,
EXPORT_SYMBOL(walk_stackframe);
#ifdef CONFIG_STACKTRACE
-struct stack_trace_data {
- struct stack_trace *trace;
- unsigned int no_sched_functions;
- unsigned int skip;
-};
-
-static bool save_trace(void *d, unsigned long addr)
+static void start_stack_trace(struct stackframe *frame, struct task_struct *task,
+ unsigned long fp, unsigned long sp,
+ unsigned long lr, unsigned long pc)
{
- struct stack_trace_data *data = d;
- struct stack_trace *trace = data->trace;
-
- if (data->no_sched_functions && in_sched_functions(addr))
- return true;
- if (data->skip) {
- data->skip--;
- return true;
- }
-
- trace->entries[trace->nr_entries++] = addr;
- return trace->nr_entries < trace->max_entries;
+ frame->fp = fp;
+ frame->sp = sp;
+ frame->lr = lr;
+ frame->pc = pc;
+#ifdef CONFIG_KRETPROBES
+ frame->kr_cur = NULL;
+ frame->tsk = task;
+#endif
+#ifdef CONFIG_UNWINDER_FRAME_POINTER
+ frame->ex_frame = in_entry_text(frame->pc);
+#endif
}
-/* This must be noinline to so that our skip calculation works correctly */
-static noinline void __save_stack_trace(struct task_struct *tsk,
- struct stack_trace *trace, unsigned int nosched)
+void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
+ struct task_struct *task, struct pt_regs *regs)
{
- struct stack_trace_data data;
struct stackframe frame;
- data.trace = trace;
- data.skip = trace->skip;
- data.no_sched_functions = nosched;
-
- if (tsk != current) {
+ if (regs) {
+ start_stack_trace(&frame, NULL, regs->ARM_fp, regs->ARM_sp,
+ regs->ARM_lr, regs->ARM_pc);
+ } else if (task != current) {
#ifdef CONFIG_SMP
/*
* What guarantees do we have here that 'tsk' is not
@@ -184,64 +176,22 @@ static noinline void __save_stack_trace(struct task_struct *tsk,
*/
return;
#else
- frame.fp = thread_saved_fp(tsk);
- frame.sp = thread_saved_sp(tsk);
- frame.lr = 0; /* recovered from the stack */
- frame.pc = thread_saved_pc(tsk);
+ start_stack_trace(&frame, task, thread_saved_fp(task),
+ thread_saved_sp(task), 0,
+ thread_saved_pc(task));
#endif
} else {
- /* We don't want this function nor the caller */
- data.skip += 2;
- frame.fp = (unsigned long)__builtin_frame_address(0);
- frame.sp = current_stack_pointer;
- frame.lr = (unsigned long)__builtin_return_address(0);
here:
- frame.pc = (unsigned long)&&here;
+ start_stack_trace(&frame, task,
+ (unsigned long)__builtin_frame_address(0),
+ current_stack_pointer,
+ (unsigned long)__builtin_return_address(0),
+ (unsigned long)&&here);
+ /* skip this function */
+ if (unwind_frame(&frame))
+ return;
}
-#ifdef CONFIG_KRETPROBES
- frame.kr_cur = NULL;
- frame.tsk = tsk;
-#endif
-#ifdef CONFIG_UNWINDER_FRAME_POINTER
- frame.ex_frame = false;
-#endif
- walk_stackframe(&frame, save_trace, &data);
-}
-
-void save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
-{
- struct stack_trace_data data;
- struct stackframe frame;
-
- data.trace = trace;
- data.skip = trace->skip;
- data.no_sched_functions = 0;
-
- frame.fp = regs->ARM_fp;
- frame.sp = regs->ARM_sp;
- frame.lr = regs->ARM_lr;
- frame.pc = regs->ARM_pc;
-#ifdef CONFIG_KRETPROBES
- frame.kr_cur = NULL;
- frame.tsk = current;
-#endif
-#ifdef CONFIG_UNWINDER_FRAME_POINTER
- frame.ex_frame = in_entry_text(frame.pc);
-#endif
-
- walk_stackframe(&frame, save_trace, &data);
-}
-
-void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
-{
- __save_stack_trace(tsk, trace, 1);
-}
-EXPORT_SYMBOL(save_stack_trace_tsk);
-
-void save_stack_trace(struct stack_trace *trace)
-{
- __save_stack_trace(current, trace, 0);
+ walk_stackframe(&frame, consume_entry, cookie);
}
-EXPORT_SYMBOL_GPL(save_stack_trace);
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-18 12:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-18 12:43 [PATCH v4 0/2] ARM: Convert to ARCH_STACKWALK Li Huafei
2022-10-18 12:43 ` [PATCH v4 1/2] ARM: stacktrace: Make stack walk callback consistent with generic code Li Huafei
2022-10-18 12:43 ` [PATCH v4 2/2] ARM: stacktrace: Convert stacktrace to generic ARCH_STACKWALK Li Huafei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).