From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4665C433DB for ; Wed, 17 Mar 2021 18:38:20 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6D3E064E61 for ; Wed, 17 Mar 2021 18:38:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6D3E064E61 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=afKmCYQg9kzcL+q9XtRZ0S4l5qttFyyCVJjUAMH5UT8=; b=cn8ggfT2X4eK/shfDhSB3elZN gFclQQ0MmLa0TpUM2CaMViOG8VRgSEcRtN+ZQf/iNN+80sFRSVfsGCNvlLF3h2x+Xg7LtY63X1xb9 0xMJqQNJDAufOv7nQVMteKZk+2kCfzySfb+e8iBKUFRc4dT+y1oh0o4xzM1tXbypDlqozzgyOPyCe 7UPCJRy/37wKtt8MgzWQTo4HG4RaK9jY4ROa2JYiGVePqekq8BXX9Xmn/aGVHAVNp8mmlN+dPJSYW NXA2t4wSMfgWnpWpJ5CuH7hKEZlq4kBPleDrbOftSQf4JNAPwNKIEsKHuDbI9KOilky8G3gfeougj faq1Soteg==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lMb2H-003kmC-4C; Wed, 17 Mar 2021 18:36:49 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lMb2C-003kle-SS for linux-arm-kernel@lists.infradead.org; Wed, 17 Mar 2021 18:36:46 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 6611664E61; Wed, 17 Mar 2021 18:36:39 +0000 (UTC) Date: Wed, 17 Mar 2021 18:36:36 +0000 From: Catalin Marinas To: Chen Jun 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 Subject: Re: [PATCH 2/2] arm64: stacktrace: Add skip when task == current Message-ID: <20210317183636.GG12269@arm.com> References: <20210317142050.57712-1-chenjun102@huawei.com> <20210317142050.57712-3-chenjun102@huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210317142050.57712-3-chenjun102@huawei.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210317_183645_099036_230E3DA1 X-CRM114-Status: GOOD ( 21.91 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 > --- > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 38FF6C433DB for ; Wed, 17 Mar 2021 18:37:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DD08E64F51 for ; Wed, 17 Mar 2021 18:37:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232716AbhCQShM (ORCPT ); Wed, 17 Mar 2021 14:37:12 -0400 Received: from mail.kernel.org ([198.145.29.99]:58370 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229601AbhCQSgl (ORCPT ); Wed, 17 Mar 2021 14:36:41 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 6611664E61; Wed, 17 Mar 2021 18:36:39 +0000 (UTC) Date: Wed, 17 Mar 2021 18:36:36 +0000 From: Catalin Marinas To: Chen Jun 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 Subject: Re: [PATCH 2/2] arm64: stacktrace: Add skip when task == current Message-ID: <20210317183636.GG12269@arm.com> References: <20210317142050.57712-1-chenjun102@huawei.com> <20210317142050.57712-3-chenjun102@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210317142050.57712-3-chenjun102@huawei.com> User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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