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.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,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 2B9B1C433DB for ; Wed, 3 Mar 2021 22:51:33 +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 C93F764EEE for ; Wed, 3 Mar 2021 22:51:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C93F764EEE Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.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=u46/ZXbrQuDVxk5iDYK6li0RfpJk7pmch+eab5z7yqE=; b=Lq2+IUoquH6IUxfn3Hq2FkWTV wFZnoiyoirdNzElVzJ/YJmlG2WFPDiHR6l0WQUPb+N3ILpC2HDAtploG8ckoB6KP7TfW0vMQfcbqT +v3N70+sJjlCSNfTFRdBjcP4ZTIUfzoVShpuHIdL2u0nkIw9Az5BvN/wv0dF0JGUGmFrnujp1UxxQ dkvY2SrIAp7Dcsjhb7/RIqrGYUA8cUAkPdVbnZH8qs8xdpBVI06qJV5o9ph1E44nkGeqzgJgSaNTp o0mMiXOAhy+pXZdMNn0x5VWu7OkJhWubx3D7RXV/PJfvRQ/oPFrns2tiHV9PBLSoGp9+HoRV2Cku0 QPv4wfBMg==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lHaGO-006nVo-84; Wed, 03 Mar 2021 22:46:40 +0000 Received: from casper.infradead.org ([2001:8b0:10b:1236::1]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lHTK3-005Mcy-2K for linux-arm-kernel@desiato.infradead.org; Wed, 03 Mar 2021 15:21:59 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=kayPbw8A7iB7X1IwkMYnahJ/vHZk8i/lpZLyVsXqadA=; b=hV78nFaq8YIGuwT4ehNzZgk82y LP308FNYVYcGXHkCcKzirRPRaNqYyvQ1OoHluCl/10QlwwA9OBdnA74mpmqjoDH8ExRll2KtVagXM Mlt808opJpkwAHttdGQWXgBP595b94Z/j3vw6o64adcMHbE8p+vQKKkcVApB+rzHKIls0qOJEkH4K 72TynRR2XXwYJiSFJB28+7//Lc6krqPKJ7lbDJcfzdUwoFdTq78ejun1gPkyP3XMGQBUwdhHau11O IbrLLQIPYbUHUXd0yUyqoTSr15XEUPnkz/lmmfRjQkSWQA1IRFfEtl7xFwW48QmELH+n2wQWJiiNM 2dvOi+Jw==; Received: from mail-wr1-x42f.google.com ([2a00:1450:4864:20::42f]) by casper.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lHTJx-003C9M-Kx for linux-arm-kernel@lists.infradead.org; Wed, 03 Mar 2021 15:21:55 +0000 Received: by mail-wr1-x42f.google.com with SMTP id a18so15894134wrc.13 for ; Wed, 03 Mar 2021 07:21:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=kayPbw8A7iB7X1IwkMYnahJ/vHZk8i/lpZLyVsXqadA=; b=Ki76JYgQVoIG2qdRj46qe9zGPIt1dFW3VmcoWPhPLJol6teZDpGdkRtGVRsg1NONMo /nzZlCiLuZEmd5NeMXOm9/EWJMouz+HxwCpoIJ4t19npSVB6kuW6lefopzni0T7NewTZ UhJLToGAcqUhB+mYzJDk8G8UrGWhTAqPfJT+yomO3PTOOxQbceuuh7113nfCaYGHTOmm kOyCGkay0k5PH0zy5zrdWlwbKx5R7wIMkKLnDeQ3zvz173nT0p3PrbRaAbWgt1aYMQKE WjGxGcwUi3qFZFajvMA0g4hPhCbokx8p32FAtzAg9ReGcgP2jftet/uiMBZ2EITeLHVC VuqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=kayPbw8A7iB7X1IwkMYnahJ/vHZk8i/lpZLyVsXqadA=; b=a0l2zx/+9irnJwK/WraD8lUuvjDvmLGQIto7XZhC1bK+Lz01ago4RDcwYkvS8cl71i k4hNMZQwKtid0FagjTfxogZnPEqHGv8o9SW79KYQmQrUtYFJCHvPiYzp3Z+74+EMBGLj SlHN52U6oS0a6LZugf2gX0imwoyLX21LhLPmBdAYMMUgxYzSO4B8+OSW8/Jj+jb5FejE vdcYfRVpOdZaQGF0pD1sDCyuB7kcRSrw/HnGdhDZJye7g76ABqQc5sV6h4RYDV40TcLD VjNvBB0cpYwfHDP7xWNcM8N5DHd+4CC8z7wUHUDYoTDCjELeHb5/eeJF8TWlJTBP/8Qb Vq7A== X-Gm-Message-State: AOAM531Y8HchjxcfjvyMMcCpNOmo05pjCfIQnM3uYz0BUWNLU/MxO36z Q5U/IfWOy9F4lxfhrypx69on5VCWTiZnNw== X-Google-Smtp-Source: ABdhPJwpE9naZxZIaRqiO69zCJbnioXYpQz9cNXCdFzrUs7TnwrWJjq6RN+kUn8bHstSCC3mq+o5tQ== X-Received: by 2002:adf:fecc:: with SMTP id q12mr27405465wrs.317.1614784849639; Wed, 03 Mar 2021 07:20:49 -0800 (PST) Received: from elver.google.com ([2a00:79e0:15:13:811:228c:e84:3381]) by smtp.gmail.com with ESMTPSA id m6sm32306902wrv.73.2021.03.03.07.20.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 03 Mar 2021 07:20:49 -0800 (PST) Date: Wed, 3 Mar 2021 16:20:43 +0100 From: Marco Elver To: Christophe Leroy Cc: Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , LKML , linuxppc-dev@lists.ozlabs.org, kasan-dev , Catalin Marinas , Will Deacon , Mark Rutland , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v1] powerpc: Include running function as first entry in save_stack_trace() and friends Message-ID: References: <1802be3e-dc1a-52e0-1754-a40f0ea39658@csgroup.eu> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1802be3e-dc1a-52e0-1754-a40f0ea39658@csgroup.eu> User-Agent: Mutt/2.0.5 (2021-01-21) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210303_152154_992619_44120476 X-CRM114-Status: GOOD ( 27.34 ) 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="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Mar 03, 2021 at 03:52PM +0100, Christophe Leroy wrote: > Le 03/03/2021 =E0 15:38, Marco Elver a =E9crit=A0: > > On Wed, 3 Mar 2021 at 15:09, Christophe Leroy > > wrote: > > > = > > > It seems like all other sane architectures, namely x86 and arm64 > > > at least, include the running function as top entry when saving > > > stack trace. > > > = > > > Functionnalities like KFENCE expect it. > > > = > > > Do the same on powerpc, it allows KFENCE to properly identify the fau= lting > > > function as depicted below. Before the patch KFENCE was identifying > > > finish_task_switch.isra as the faulting function. > > > = > > > [ 14.937370] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > [ 14.948692] BUG: KFENCE: invalid read in test_invalid_access+0x54/= 0x108 > > > [ 14.948692] > > > [ 14.956814] Invalid read at 0xdf98800a: > > > [ 14.960664] test_invalid_access+0x54/0x108 > > > [ 14.964876] finish_task_switch.isra.0+0x54/0x23c > > > [ 14.969606] kunit_try_run_case+0x5c/0xd0 > > > [ 14.973658] kunit_generic_run_threadfn_adapter+0x24/0x30 > > > [ 14.979079] kthread+0x15c/0x174 > > > [ 14.982342] ret_from_kernel_thread+0x14/0x1c > > > [ 14.986731] > > > [ 14.988236] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: G B = 5.12.0-rc1-01537-g95f6e2088d7e-dirty #4682 > > > [ 14.999795] NIP: c016ec2c LR: c02f517c CTR: c016ebd8 > > > [ 15.004851] REGS: e2449d90 TRAP: 0301 Tainted: G B = (5.12.0-rc1-01537-g95f6e2088d7e-dirty) > > > [ 15.015274] MSR: 00009032 CR: 22000004 XER: 00= 000000 > > > [ 15.022043] DAR: df98800a DSISR: 20000000 > > > [ 15.022043] GPR00: c02f517c e2449e50 c1142080 e100dd24 c084b13c 00= 000008 c084b32b c016ebd8 > > > [ 15.022043] GPR08: c0850000 df988000 c0d10000 e2449eb0 22000288 > > > [ 15.040581] NIP [c016ec2c] test_invalid_access+0x54/0x108 > > > [ 15.046010] LR [c02f517c] kunit_try_run_case+0x5c/0xd0 > > > [ 15.051181] Call Trace: > > > [ 15.053637] [e2449e50] [c005a68c] finish_task_switch.isra.0+0x54/0= x23c (unreliable) > > > [ 15.061338] [e2449eb0] [c02f517c] kunit_try_run_case+0x5c/0xd0 > > > [ 15.067215] [e2449ed0] [c02f648c] kunit_generic_run_threadfn_adapt= er+0x24/0x30 > > > [ 15.074472] [e2449ef0] [c004e7b0] kthread+0x15c/0x174 > > > [ 15.079571] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c > > > [ 15.085798] Instruction dump: > > > [ 15.088784] 8129d608 38e7ebd8 81020280 911f004c 39000000 995f0024 = 907f0028 90ff001c > > > [ 15.096613] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb= 0 812a4b98 3d40c02f > > > [ 15.104612] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > = > > > Signed-off-by: Christophe Leroy > > = > > Acked-by: Marco Elver > > = > > Thank you, I think this looks like the right solution. Just a question = below: > > = > ... > = > > > @@ -59,23 +70,26 @@ void save_stack_trace(struct stack_trace *trace) > > > = > > > sp =3D current_stack_frame(); > > > = > > > - save_context_stack(trace, sp, current, 1); > > > + save_context_stack(trace, sp, (unsigned long)save_stack_trace= , current, 1); > > = > > This causes ip =3D=3D save_stack_trace and also below for > > save_stack_trace_tsk. Does this mean save_stack_trace() is included in > > the trace? Looking at kernel/stacktrace.c, I think the library wants > > to exclude itself from the trace, as it does '.skip =3D skipnr + 1' (and > > '.skip =3D skipnr + (current =3D=3D tsk)' for the _tsk variant). > > = > > If the arch-helper here is included, should this use _RET_IP_ instead? > > = > = > Don't really know, I was inspired by arm64 which has: > = > void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, > struct task_struct *task, struct pt_regs *regs) > { > struct stackframe frame; > = > if (regs) > start_backtrace(&frame, regs->regs[29], regs->pc); > else if (task =3D=3D current) > start_backtrace(&frame, > (unsigned long)__builtin_frame_address(0), > (unsigned long)arch_stack_walk); > else > start_backtrace(&frame, thread_saved_fp(task), > thread_saved_pc(task)); > = > walk_stackframe(task, &frame, consume_entry, cookie); > } > = > But looking at x86 you may be right, so what should be done really ? x86: [ 2.843292] calling stack_trace_save: [ 2.843705] test_func+0x6c/0x118 [ 2.844184] do_one_initcall+0x58/0x270 [ 2.844618] kernel_init_freeable+0x1da/0x23a [ 2.845110] kernel_init+0xc/0x166 [ 2.845494] ret_from_fork+0x22/0x30 [ 2.867525] calling stack_trace_save_tsk: [ 2.868017] test_func+0xa9/0x118 [ 2.868530] do_one_initcall+0x58/0x270 [ 2.869003] kernel_init_freeable+0x1da/0x23a [ 2.869535] kernel_init+0xc/0x166 [ 2.869957] ret_from_fork+0x22/0x30 arm64: [ 3.786911] calling stack_trace_save: [ 3.787147] stack_trace_save+0x50/0x78 [ 3.787443] test_func+0x84/0x13c [ 3.787738] do_one_initcall+0x5c/0x310 [ 3.788099] kernel_init_freeable+0x214/0x294 [ 3.788363] kernel_init+0x18/0x164 [ 3.788585] ret_from_fork+0x10/0x30 [ 3.803615] calling stack_trace_save_tsk: [ 3.804266] stack_trace_save_tsk+0x9c/0x100 [ 3.804541] test_func+0xc4/0x13c [ 3.804803] do_one_initcall+0x5c/0x310 [ 3.805031] kernel_init_freeable+0x214/0x294 [ 3.805284] kernel_init+0x18/0x164 [ 3.805505] ret_from_fork+0x10/0x30 +Cc arm64 folks. So I think the arm64 version also has a bug, because I think a user of really doesn't care about the library function itself. And from reading kernel/stacktrace.c I think it wants to exclude itself entirely. It's a shame that isn't better documented, but I'm pretty sure that including the library functions in the trace is not useful. For the ppc version, let's do what x86 does and start with the caller. Thanks, -- Marco _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel