From: Catalin Marinas <catalin.marinas@arm.com> To: Chen Jun <chenjun102@huawei.com> Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, akpm@linux-foundation.org, will@kernel.org, rui.xiang@huawei.com, Mark Brown <broonie@kernel.org> Subject: Re: [PATCH 2/2] arm64: stacktrace: Add skip when task == current Date: Wed, 17 Mar 2021 18:36:36 +0000 [thread overview] Message-ID: <20210317183636.GG12269@arm.com> (raw) In-Reply-To: <20210317142050.57712-3-chenjun102@huawei.com> On Wed, Mar 17, 2021 at 02:20:50PM +0000, Chen Jun wrote: > On ARM64, cat /sys/kernel/debug/page_owner, all pages return the same > stack: > stack_trace_save+0x4c/0x78 > register_early_stack+0x34/0x70 > init_page_owner+0x34/0x230 > page_ext_init+0x1bc/0x1dc > > The reason is that: > check_recursive_alloc always return 1 because that > entries[0] is always equal to ip (__set_page_owner+0x3c/0x60). > > The root cause is that: > commit 5fc57df2f6fd ("arm64: stacktrace: Convert to ARCH_STACKWALK") > make the save_trace save 2 more entries. > > Add skip in arch_stack_walk when task == current. > > Fixes: 5fc57df2f6fd ("arm64: stacktrace: Convert to ARCH_STACKWALK") > Signed-off-by: Chen Jun <chenjun102@huawei.com> > --- > arch/arm64/kernel/stacktrace.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c > index ad20981..c26b0ac 100644 > --- a/arch/arm64/kernel/stacktrace.c > +++ b/arch/arm64/kernel/stacktrace.c > @@ -201,11 +201,12 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, > > if (regs) > start_backtrace(&frame, regs->regs[29], regs->pc); > - else if (task == current) > + else if (task == current) { > + ((struct stacktrace_cookie *)cookie)->skip += 2; > start_backtrace(&frame, > (unsigned long)__builtin_frame_address(0), > (unsigned long)arch_stack_walk); > - else > + } else > start_backtrace(&frame, thread_saved_fp(task), > thread_saved_pc(task)); I don't like abusing the cookie here. It's void * as it's meant to be an opaque type. I'd rather skip the first two frames in walk_stackframe() instead before invoking fn(). Prior to the conversion to ARCH_STACKWALK, we were indeed skipping two more entries in __save_stack_trace() if tsk == current. Something like below, completely untested: diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c index ad20981dfda4..2a9f759aa41a 100644 --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -115,10 +115,15 @@ NOKPROBE_SYMBOL(unwind_frame); void notrace walk_stackframe(struct task_struct *tsk, struct stackframe *frame, bool (*fn)(void *, unsigned long), void *data) { + /* for the current task, we don't want this function nor its caller */ + int skip = tsk == current ? 2 : 0; + while (1) { int ret; - if (!fn(data, frame->pc)) + if (skip) + skip--; + else if (!fn(data, frame->pc)) break; ret = unwind_frame(tsk, frame); if (ret < 0) -- Catalin
WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com> To: Chen Jun <chenjun102@huawei.com> Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, akpm@linux-foundation.org, will@kernel.org, rui.xiang@huawei.com, Mark Brown <broonie@kernel.org> Subject: Re: [PATCH 2/2] arm64: stacktrace: Add skip when task == current Date: Wed, 17 Mar 2021 18:36:36 +0000 [thread overview] Message-ID: <20210317183636.GG12269@arm.com> (raw) In-Reply-To: <20210317142050.57712-3-chenjun102@huawei.com> On Wed, Mar 17, 2021 at 02:20:50PM +0000, Chen Jun wrote: > On ARM64, cat /sys/kernel/debug/page_owner, all pages return the same > stack: > stack_trace_save+0x4c/0x78 > register_early_stack+0x34/0x70 > init_page_owner+0x34/0x230 > page_ext_init+0x1bc/0x1dc > > The reason is that: > check_recursive_alloc always return 1 because that > entries[0] is always equal to ip (__set_page_owner+0x3c/0x60). > > The root cause is that: > commit 5fc57df2f6fd ("arm64: stacktrace: Convert to ARCH_STACKWALK") > make the save_trace save 2 more entries. > > Add skip in arch_stack_walk when task == current. > > Fixes: 5fc57df2f6fd ("arm64: stacktrace: Convert to ARCH_STACKWALK") > Signed-off-by: Chen Jun <chenjun102@huawei.com> > --- > arch/arm64/kernel/stacktrace.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c > index ad20981..c26b0ac 100644 > --- a/arch/arm64/kernel/stacktrace.c > +++ b/arch/arm64/kernel/stacktrace.c > @@ -201,11 +201,12 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, > > if (regs) > start_backtrace(&frame, regs->regs[29], regs->pc); > - else if (task == current) > + else if (task == current) { > + ((struct stacktrace_cookie *)cookie)->skip += 2; > start_backtrace(&frame, > (unsigned long)__builtin_frame_address(0), > (unsigned long)arch_stack_walk); > - else > + } else > start_backtrace(&frame, thread_saved_fp(task), > thread_saved_pc(task)); I don't like abusing the cookie here. It's void * as it's meant to be an opaque type. I'd rather skip the first two frames in walk_stackframe() instead before invoking fn(). Prior to the conversion to ARCH_STACKWALK, we were indeed skipping two more entries in __save_stack_trace() if tsk == current. Something like below, completely untested: diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c index ad20981dfda4..2a9f759aa41a 100644 --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -115,10 +115,15 @@ NOKPROBE_SYMBOL(unwind_frame); void notrace walk_stackframe(struct task_struct *tsk, struct stackframe *frame, bool (*fn)(void *, unsigned long), void *data) { + /* for the current task, we don't want this function nor its caller */ + int skip = tsk == current ? 2 : 0; + while (1) { int ret; - if (!fn(data, frame->pc)) + if (skip) + skip--; + else if (!fn(data, frame->pc)) break; ret = unwind_frame(tsk, frame); if (ret < 0) -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-03-17 18:37 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-17 14:20 [PATCH 0/2] Fix page_owner broken on arm64 Chen Jun 2021-03-17 14:20 ` Chen Jun 2021-03-17 14:20 ` [PATCH 1/2] stacktrace: Move struct stacktrace_cookie to stacktrace.h Chen Jun 2021-03-17 14:20 ` Chen Jun 2021-03-17 14:20 ` [PATCH 2/2] arm64: stacktrace: Add skip when task == current Chen Jun 2021-03-17 14:20 ` Chen Jun 2021-03-17 18:36 ` Catalin Marinas [this message] 2021-03-17 18:36 ` Catalin Marinas 2021-03-17 19:34 ` Mark Rutland 2021-03-17 19:34 ` Mark Rutland 2021-03-18 3:24 ` chenjun (AM) 2021-03-18 3:24 ` chenjun (AM) 2021-03-18 13:22 ` chenjun (AM) 2021-03-18 13:22 ` chenjun (AM) 2021-03-18 16:17 ` Catalin Marinas 2021-03-18 16:17 ` Catalin Marinas 2021-03-18 17:12 ` Mark Rutland 2021-03-18 17:12 ` Mark Rutland 2021-03-18 18:36 ` Catalin Marinas 2021-03-18 18:36 ` Catalin Marinas 2021-03-17 22:23 ` [PATCH 0/2] Fix page_owner broken on arm64 Andrew Morton 2021-03-17 22:23 ` Andrew Morton
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210317183636.GG12269@arm.com \ --to=catalin.marinas@arm.com \ --cc=akpm@linux-foundation.org \ --cc=broonie@kernel.org \ --cc=chenjun102@huawei.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=rui.xiang@huawei.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.