Message ID | 3ec48fde01e4ee6505f77908ba351bad200ae3d1.1694763684.git.lukas@wunner.de |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp1006159vqi; Fri, 15 Sep 2023 05:29:34 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHfhNiT8Dtmbo96eAEml5mXlu5IQqHM9FV+3sNeWHiUPT2kICRDdn4V6/7Y0yvN6i6arUHf X-Received: by 2002:a05:6a20:1608:b0:154:fb34:5f23 with SMTP id l8-20020a056a20160800b00154fb345f23mr1463987pzj.8.1694780974150; Fri, 15 Sep 2023 05:29:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694780974; cv=none; d=google.com; s=arc-20160816; b=zrEJQwDG0B8T3ZxnnFsRLNp1PUkbmUdN66tudbH99MQRYCBqf6gp9afCO4QOIFeS4Z j21lKdSDAOHBl/dYQmzwUMQjC0cYgcDgcr8ARsRxVVdsf+X3ChJ+nd1M92Y22E+2gGSl /kAj7eDPEx9fWHf8XtBT6/xwDTAIsrBW4CNdjtjj28KIDHqM7hYIuIsPkfPCM2BLR92+ KUaFszezJzu8UofYfWPC3WfVRaGiupudye0C8PpPVzTzc6kbBsfoYyN3F+1tIHnCZ1uo CpTHxPZPi5o33QHLorP2EapqvD3XUga+n8BRgNse8rOWylWvernRhRpCR70UP0yaSKwG 63yw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:date:from:message-id; bh=43r8TCA2r2DvnU79FysC8gHK7bb7jzkxvEHWC7bKfWo=; fh=me6YpNhP2Ut7onWHBmPfB59KQni8zmhvPvJti8AMa7Q=; b=nXFn/4WRqu+mD0+pH5XbMDyXA9Ez8gZ85KHJzsgq+BjJma12LZCYkVVlIMydkg8lYD NuU2PyT/hSJaVkYIYQeDAyvBxfFkjJLq3xvSeK5+hnsKL6IGckEksJ4E8Tn3Z/IaflW8 mNcyduHmXn6m9SwGY2imfLbniq3uzjR+cRYnnZ7jRrTz2whe6rpH9shVENk3RgpGd20V 8Tb2dhRrvqU2OskX2iivAoNfVu/xMo7GkM0eV/GiR6HcUxoa315rGaF6EAe7hA98D5sf ApsM5Esf+TGLWk5+572RJHC5Wc27GPChsY+SGEyOwxVgm4M3bOvX6uhsFwJJ81ePuIbm TuSg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id w71-20020a63824a000000b00573f769d072si3164055pgd.462.2023.09.15.05.29.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Sep 2023 05:29:34 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 31E73825D124; Fri, 15 Sep 2023 01:05:08 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232462AbjIOIFF (ORCPT <rfc822;ruipengqi7@gmail.com> + 32 others); Fri, 15 Sep 2023 04:05:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33656 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230454AbjIOIFE (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 15 Sep 2023 04:05:04 -0400 X-Greylist: delayed 553 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Fri, 15 Sep 2023 01:04:59 PDT Received: from bmailout2.hostsharing.net (bmailout2.hostsharing.net [IPv6:2a01:37:3000::53df:4ef0:0]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4AD8D1998 for <linux-kernel@vger.kernel.org>; Fri, 15 Sep 2023 01:04:59 -0700 (PDT) Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "*.hostsharing.net", Issuer "RapidSSL Global TLS RSA4096 SHA256 2022 CA1" (verified OK)) by bmailout2.hostsharing.net (Postfix) with ESMTPS id 90A232800BC39; Fri, 15 Sep 2023 09:55:44 +0200 (CEST) Received: by h08.hostsharing.net (Postfix, from userid 100393) id 8356E50E339; Fri, 15 Sep 2023 09:55:44 +0200 (CEST) Message-Id: <3ec48fde01e4ee6505f77908ba351bad200ae3d1.1694763684.git.lukas@wunner.de> From: Lukas Wunner <lukas@wunner.de> Date: Wed, 15 Sep 2023 09:55:39 +0200 Subject: [PATCH] panic: Reenable preemption in WARN slowpath To: Peter Zijlstra <peterz@infradead.org>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org Cc: linux-kernel@vger.kernel.org X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Fri, 15 Sep 2023 01:05:08 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777106654984736561 X-GMAIL-MSGID: 1777106654984736561 |
Series |
panic: Reenable preemption in WARN slowpath
|
|
Commit Message
Lukas Wunner
Sept. 15, 2023, 7:55 a.m. UTC
Commit 5a5d7e9badd2 ("cpuidle: lib/bug: Disable rcu_is_watching() during
WARN/BUG") amended warn_slowpath_fmt() to disable preemption until the
WARN splat has been emitted.
However the commit neglected to reenable preemption in the !fmt codepath,
i.e. when a WARN splat is emitted without additional format string.
One consequence is that users may see more splats than intended. E.g. a
WARN splat emitted in a work item results in at least two extra splats:
BUG: workqueue leaked lock or atomic
(emitted by process_one_work())
BUG: scheduling while atomic
(emitted by worker_thread() -> schedule())
Ironically the point of the commit was to *avoid* extra splats. ;)
Fix it.
Fixes: 5a5d7e9badd2 ("cpuidle: lib/bug: Disable rcu_is_watching() during WARN/BUG")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v6.1+
Cc: Peter Zijlstra <peterz@infradead.org>
---
The original commit went in through the tip tree, hence submitting to
tip maintainers. The commit was backported to v6.1-stable (even though
it wasn't tagged for stable), hence this fix needs a stable designation.
kernel/panic.c | 1 +
1 file changed, 1 insertion(+)
Comments
On Fri, Sep 15, 2023 at 09:55:39AM +0200, Lukas Wunner wrote: > Commit 5a5d7e9badd2 ("cpuidle: lib/bug: Disable rcu_is_watching() during > WARN/BUG") amended warn_slowpath_fmt() to disable preemption until the > WARN splat has been emitted. > > However the commit neglected to reenable preemption in the !fmt codepath, > i.e. when a WARN splat is emitted without additional format string. > > One consequence is that users may see more splats than intended. E.g. a > WARN splat emitted in a work item results in at least two extra splats: > > BUG: workqueue leaked lock or atomic > (emitted by process_one_work()) > > BUG: scheduling while atomic > (emitted by worker_thread() -> schedule()) > > Ironically the point of the commit was to *avoid* extra splats. ;) > > Fix it. > > Fixes: 5a5d7e9badd2 ("cpuidle: lib/bug: Disable rcu_is_watching() during WARN/BUG") > Signed-off-by: Lukas Wunner <lukas@wunner.de> > Cc: stable@vger.kernel.org # v6.1+ > Cc: Peter Zijlstra <peterz@infradead.org> > --- > The original commit went in through the tip tree, hence submitting to > tip maintainers. The commit was backported to v6.1-stable (even though > it wasn't tagged for stable), hence this fix needs a stable designation. > > kernel/panic.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/panic.c b/kernel/panic.c > index 07239d4..ffa037fa 100644 > --- a/kernel/panic.c > +++ b/kernel/panic.c > @@ -697,6 +697,7 @@ void warn_slowpath_fmt(const char *file, int line, unsigned taint, > if (!fmt) { > __warn(file, line, __builtin_return_address(0), taint, > NULL, NULL); > + warn_rcu_exit(rcu); > return; > } Urgh, thanks!
* tip-bot2 for Lukas Wunner <tip-bot2@linutronix.de> wrote: > The following commit has been merged into the core/urgent branch of tip: > > Commit-ID: cccd32816506cbac3a4c65d9dff51b3125ef1a03 > Gitweb: https://git.kernel.org/tip/cccd32816506cbac3a4c65d9dff51b3125ef1a03 > Author: Lukas Wunner <lukas@wunner.de> > AuthorDate: Fri, 15 Sep 2023 09:55:39 +02:00 > Committer: Ingo Molnar <mingo@kernel.org> > CommitterDate: Fri, 15 Sep 2023 11:28:08 +02:00 > > panic: Reenable preemption in WARN slowpath > > Commit: > > 5a5d7e9badd2 ("cpuidle: lib/bug: Disable rcu_is_watching() during WARN/BUG") > > amended warn_slowpath_fmt() to disable preemption until the WARN splat > has been emitted. > > However the commit neglected to reenable preemption in the !fmt codepath, > i.e. when a WARN splat is emitted without additional format string. > > One consequence is that users may see more splats than intended. E.g. a > WARN splat emitted in a work item results in at least two extra splats: > > BUG: workqueue leaked lock or atomic > (emitted by process_one_work()) > > BUG: scheduling while atomic > (emitted by worker_thread() -> schedule()) > > Ironically the point of the commit was to *avoid* extra splats. ;) > > Fix it. > diff --git a/kernel/panic.c b/kernel/panic.c > index 07239d4..ffa037f 100644 > --- a/kernel/panic.c > +++ b/kernel/panic.c > @@ -697,6 +697,7 @@ void warn_slowpath_fmt(const char *file, int line, unsigned taint, > if (!fmt) { > __warn(file, line, __builtin_return_address(0), taint, > NULL, NULL); > + warn_rcu_exit(rcu); > return; BTW., one more thing we might want to consider here is to re-enable preemption in warn_rcu_exit() a bit more gently, without forcing a pending reschedule, ie. preempt_enable_no_resched() or so? Thanks, Ingo
On Fri, Sep 15, 2023 at 11:45:02AM +0200, Ingo Molnar wrote: > > * tip-bot2 for Lukas Wunner <tip-bot2@linutronix.de> wrote: > > > The following commit has been merged into the core/urgent branch of tip: > > > > Commit-ID: cccd32816506cbac3a4c65d9dff51b3125ef1a03 > > Gitweb: https://git.kernel.org/tip/cccd32816506cbac3a4c65d9dff51b3125ef1a03 > > Author: Lukas Wunner <lukas@wunner.de> > > AuthorDate: Fri, 15 Sep 2023 09:55:39 +02:00 > > Committer: Ingo Molnar <mingo@kernel.org> > > CommitterDate: Fri, 15 Sep 2023 11:28:08 +02:00 > > > > panic: Reenable preemption in WARN slowpath > > > > Commit: > > > > 5a5d7e9badd2 ("cpuidle: lib/bug: Disable rcu_is_watching() during WARN/BUG") > > > > amended warn_slowpath_fmt() to disable preemption until the WARN splat > > has been emitted. > > > > However the commit neglected to reenable preemption in the !fmt codepath, > > i.e. when a WARN splat is emitted without additional format string. > > > > One consequence is that users may see more splats than intended. E.g. a > > WARN splat emitted in a work item results in at least two extra splats: > > > > BUG: workqueue leaked lock or atomic > > (emitted by process_one_work()) > > > > BUG: scheduling while atomic > > (emitted by worker_thread() -> schedule()) > > > > Ironically the point of the commit was to *avoid* extra splats. ;) > > > > Fix it. > > > diff --git a/kernel/panic.c b/kernel/panic.c > > index 07239d4..ffa037f 100644 > > --- a/kernel/panic.c > > +++ b/kernel/panic.c > > @@ -697,6 +697,7 @@ void warn_slowpath_fmt(const char *file, int line, unsigned taint, > > if (!fmt) { > > __warn(file, line, __builtin_return_address(0), taint, > > NULL, NULL); > > + warn_rcu_exit(rcu); > > return; > > BTW., one more thing we might want to consider here is to re-enable > preemption in warn_rcu_exit() a bit more gently, without forcing a > pending reschedule, ie. preempt_enable_no_resched() or so? nah, it's a warn, if that triggers you get to keep the pieces. Also preempt_enable_no_resched() isn't exported because its a horribly dangerous function.
* Peter Zijlstra <peterz@infradead.org> wrote: > > > panic: Reenable preemption in WARN slowpath > > > > > > Commit: > > > > > > 5a5d7e9badd2 ("cpuidle: lib/bug: Disable rcu_is_watching() during WARN/BUG") > > > > > > amended warn_slowpath_fmt() to disable preemption until the WARN splat > > > has been emitted. > > > > > > However the commit neglected to reenable preemption in the !fmt codepath, > > > i.e. when a WARN splat is emitted without additional format string. > > > > > > One consequence is that users may see more splats than intended. E.g. a > > > WARN splat emitted in a work item results in at least two extra splats: > > > > > > BUG: workqueue leaked lock or atomic > > > (emitted by process_one_work()) > > > > > > BUG: scheduling while atomic > > > (emitted by worker_thread() -> schedule()) > > > > > > Ironically the point of the commit was to *avoid* extra splats. ;) > > > > > > Fix it. > > > > > diff --git a/kernel/panic.c b/kernel/panic.c > > > index 07239d4..ffa037f 100644 > > > --- a/kernel/panic.c > > > +++ b/kernel/panic.c > > > @@ -697,6 +697,7 @@ void warn_slowpath_fmt(const char *file, int line, unsigned taint, > > > if (!fmt) { > > > __warn(file, line, __builtin_return_address(0), taint, > > > NULL, NULL); > > > + warn_rcu_exit(rcu); > > > return; > > > > BTW., one more thing we might want to consider here is to re-enable > > preemption in warn_rcu_exit() a bit more gently, without forcing a > > pending reschedule, ie. preempt_enable_no_resched() or so? > > nah, it's a warn, if that triggers you get to keep the pieces. But but ... my overall point is that since we just WARN()ed, we are facing some sort of kernel bug, and scheduling policies are only a secondary concern, debuggability & getting the bug fixed is the primary concern. So the scheduler should switch to a debugging-friendlier behavior: 'Schedule tasks around as little as possible, to keep the debug output tidy & to keep things working a bit better even if it's all broken already'. ... or so. My suggestion was a small subset of that principle. > [...] Also preempt_enable_no_resched() isn't exported because its a > horribly dangerous function. Special exception for RCU debugging only, or so - it's a core kernel facility after all. No strong feelings either way though. Thanks, Ingo
diff --git a/kernel/panic.c b/kernel/panic.c index 07239d4..ffa037fa 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -697,6 +697,7 @@ void warn_slowpath_fmt(const char *file, int line, unsigned taint, if (!fmt) { __warn(file, line, __builtin_return_address(0), taint, NULL, NULL); + warn_rcu_exit(rcu); return; }