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=-9.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, INVALID_DATE_TZ_ABSURD,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 78644C636CA for ; Wed, 21 Jul 2021 13:25:36 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 473C161241 for ; Wed, 21 Jul 2021 13:25:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 473C161241 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linutronix.de 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=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References :In-Reply-To:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=2zrgxuHuenhTS+WFk3FGQVpd5uj0IMpLms1xq9GXNPI=; b=VqOtXGRhMv75oN Az6fi0gWGOZU5Tqm2PMQZz4Ugbbw+CnuF9jePP0XtgNXLKcDBNF+WwKigbmdFHaimYsiSnwiSTOdQ 4vWXwLidWRjHRPm3ZHhdAWFeiuOTTK8o85ZML78Pm/hHEYXhrMrEq4VCCfPO1dtNHI5bvwyCrtChG OasFEnkeijjNQ+sBgbk0T6qgKvF4LRcgqgXRkervvm/ZQU1luBFJN1+dV6JBFy/RyhUZtCYKFaUBz CniuKJXbMFUqWydqoLdJU3b55G/DWKdmeWFCgB8oYZ3pDEkYtvAlcdijyXijR0LoR6JblgwzOD9K1 60RFvTm2pgl33Y1ng+xQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m6CCR-00FjhK-TO; Wed, 21 Jul 2021 13:23:48 +0000 Received: from galois.linutronix.de ([2a0a:51c0:0:12e:550::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m6CCN-00Fjga-VN for linux-arm-kernel@lists.infradead.org; Wed, 21 Jul 2021 13:23:45 +0000 From: John Ogness DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1626873822; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=NgNIbyjTa8/w5GXhJRZhAY6J7D9A654ff12BUBUlpY0=; b=dZCndb66THqF2a7L26p9SkC46/o/4+QUVlNqN6U4Ys8mX/Hz4W7HFdTT9YyapOn+qLxWvH 3eG5Wh8vRde9gA+SQ6RAmpnLGCoJZmfbG9/qJsmRkjgvJdn8A/o3RI5mVsvCw/tQf9gWWR 71rPGNsUGQ3ertDnxklig3O26UaLwxHU7tcECzSTnhFuWjoaajzCn5DerEA9aIWF5dVRQi +NFJR2igfJh4yo9G2XT32uhhAzZJb8BVD6gCOB1ZfiVTfqaUADrg1QlyHsBobR4xaDEwjP Vi9JkRrHI+jB1iOw8HMZKWaTZK1pnTNRSWvc5JGm5xXN2tcN2qFSrH96anFxlg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1626873822; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=NgNIbyjTa8/w5GXhJRZhAY6J7D9A654ff12BUBUlpY0=; b=QnYzmJ8XiGMZ+TcBaOVzbBoFwtydvBugI5vwvBeM6CS9uutBONGZv5TxZ6FkidmhXy3r8e FYws43eAXTt28dCw== To: Petr Mladek Cc: Sergey Senozhatsky , Steven Rostedt , Thomas Gleixner , linux-kernel@vger.kernel.org, Russell King , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , Ingo Molnar , Marc Zyngier , Valentin Schneider , Geert Uytterhoeven , Mike Rapoport , "Wolfram Sang \(Renesas\)" , Anshuman Khandual , Xiongwei Song , Frederic Weisbecker , Peter Zijlstra , Masahiro Yamada , Kees Cook , Andrew Morton , Nathan Chancellor , Nick Desaulniers , Nick Terrell , Vipin Sharma , Rasmus Villemoes , Daniel Borkmann , Vlastimil Babka , linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH printk v4 4/6] printk: remove NMI tracking In-Reply-To: <20210721130852.zrjnti6b3fwjgdzj@pathway.suse.cz> References: <20210715193359.25946-1-john.ogness@linutronix.de> <20210715193359.25946-5-john.ogness@linutronix.de> <20210721120026.y3dqno24ahw4sazy@pathway.suse.cz> <877dhjygvc.fsf@jogness.linutronix.de> <20210721130852.zrjnti6b3fwjgdzj@pathway.suse.cz> Date: Wed, 21 Jul 2021 15:29:41 +0206 Message-ID: <874kcnyf4y.fsf@jogness.linutronix.de> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210721_062344_362628_7BF2CBA9 X-CRM114-Status: GOOD ( 27.77 ) 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 2021-07-21, Petr Mladek wrote: > On Wed 2021-07-21 14:52:15, John Ogness wrote: >> On 2021-07-21, Petr Mladek wrote: >> >> --- a/kernel/trace/trace.c >> >> +++ b/kernel/trace/trace.c >> >> @@ -9647,7 +9647,7 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) >> >> tracing_off(); >> >> >> >> local_irq_save(flags); >> >> - printk_nmi_direct_enter(); >> >> + printk_deferred_enter(); >> > >> > I would prefer to do not manipulate the printk context here anymore, >> > as it was done in v3. >> > >> > printk_nmi_direct_enter() was added here by the commit the commit >> > 03fc7f9c99c1e7ae2925d4 ("printk/nmi: Prevent deadlock when >> > accessing the main log buffer in NMI"). It was _not_ about console >> > handling. The reason was to modify the default behavior under NMI >> > and store the messages directly into the main log buffer. >> > >> > When I think about it. The original fix was not correct. We should have >> > modified the context only when ftrace_dump() was really called under NMI: >> > >> > if (in_nmi()) >> > printk_nmi_direct_enter(); >> > >> > By other words. We should try to show the messages on the console >> > when ftrace_dump()/panic() is not called from NMI. It will help >> > to see all messages even when the ftrace buffers are bigger >> > than printk() ones. >> > >> > And we do not need any special handling here for NMI. vprintk() >> > in printk/printk_safe.c will do the right thing for us. >> >> Agreed. We need to mention this behavior change in the commit >> message. Perhaps this as the commit message: >> >> All NMI contexts are handled the same as the safe context: store the >> message and defer printing. There is no need to have special NMI >> context tracking for this. Using in_nmi() is enough. >> >> There are several parts of the kernel that are manually calling into >> the printk NMI context tracking in order to cause general printk >> deferred printing: >> >> arch/arm/kernel/smp.c >> arch/powerpc/kexec/crash.c >> kernel/trace/trace.c >> >> For arm/kernel/smp.c and powerpc/kexec/crash.c, provide a new >> function pair printk_deferred_enter/exit that explicitly achieves the >> same objective. >> >> For ftrace, remove general printk deferring. This general deferrment >> was added in commit 03fc7f9c99c1 ("printk/nmi: Prevent deadlock when >> accessing the main log buffer in NMI"), but really should have only >> been deferred when in NMI context. Since vprintk() now checks for >> NMI context when deciding to defer, ftrace does not need any special >> handling. > > I would make it less focused on the deferring part and try to explain > the original purpose here, something like: > > "For ftrace, remove the printk context manipulation completely. It was > added in commit 03fc7f9c99c1 ("printk/nmi: Prevent deadlock when > accessing the main log buffer in NMI"). The purpose was to enforce > storing messages directly into the ring buffer even in NMI context. > It really should have only modified the behavior in NMI context. > There is no need for a special behavior any longer. All messages are > always stored directly now. The console deferring is handled > transparently in vprintk()." Your wording is OK for me. John Ogness _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel