| Message ID | 20211015025847.17694-4-madvenka@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Reorganize the unwinder and implement stack trace reliability checks | expand |
On Thu, Oct 14, 2021 at 09:58:39PM -0500, madvenka@linux.microsoft.com wrote: > From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> > > Currently, get_wchan() in ARM64 code walks the stack using start_backtrace() > and unwind_frame(). Make it use arch_stack_walk() instead. This makes > maintenance easier. This overlaps with some very similar updates that Peter Zijlstra is working on which addresses some existing problems with wchan: https://lore.kernel.org/all/20211008111527.438276127@infradead.org/ It probably makes sense for you to coordinate with Peter here, some of that series is already merged up to his patch 6 which looks very similar to what you've got here. In that thread you'll see that Mark Rutland spotted an issue with the handling of __switch_to() on arm64 which probably also applies to your change.
I will take a look at what Peter has done and will coordinate with him. Thanks. Madhavan On 10/20/21 11:10 AM, Mark Brown wrote: > On Thu, Oct 14, 2021 at 09:58:39PM -0500, madvenka@linux.microsoft.com wrote: >> From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> >> >> Currently, get_wchan() in ARM64 code walks the stack using start_backtrace() >> and unwind_frame(). Make it use arch_stack_walk() instead. This makes >> maintenance easier. > > This overlaps with some very similar updates that Peter Zijlstra is > working on which addresses some existing problems with wchan: > > https://lore.kernel.org/all/20211008111527.438276127@infradead.org/ > > It probably makes sense for you to coordinate with Peter here, some of > that series is already merged up to his patch 6 which looks very > similar to what you've got here. In that thread you'll see that Mark > Rutland spotted an issue with the handling of __switch_to() on arm64 > which probably also applies to your change. > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index c8989b999250..48ed89acce0d 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -544,11 +544,27 @@ __notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev, return last; } +struct wchan_info { + unsigned long pc; + int count; +}; + +static bool get_wchan_cb(void *arg, unsigned long pc) +{ + struct wchan_info *wchan_info = arg; + + if (!in_sched_functions(pc)) { + wchan_info->pc = pc; + return false; + } + return wchan_info->count++ < 16; +} + unsigned long get_wchan(struct task_struct *p) { - struct stackframe frame; - unsigned long stack_page, ret = 0; - int count = 0; + unsigned long stack_page; + struct wchan_info wchan_info; + if (!p || p == current || task_is_running(p)) return 0; @@ -556,20 +572,12 @@ unsigned long get_wchan(struct task_struct *p) if (!stack_page) return 0; - start_backtrace(&frame, thread_saved_fp(p), thread_saved_pc(p)); + wchan_info.pc = 0; + wchan_info.count = 0; + arch_stack_walk(get_wchan_cb, &wchan_info, p, NULL); - do { - if (unwind_frame(p, &frame)) - goto out; - if (!in_sched_functions(frame.pc)) { - ret = frame.pc; - goto out; - } - } while (count++ < 16); - -out: put_task_stack(p); - return ret; + return wchan_info.pc; } unsigned long arch_align_stack(unsigned long sp)