* [PATCH] panic: Reenable preemption in WARN slowpath
@ 2023-09-15 7:55 Lukas Wunner
2023-09-15 9:36 ` [tip: core/urgent] " tip-bot2 for Lukas Wunner
2023-09-15 9:38 ` [PATCH] " Peter Zijlstra
0 siblings, 2 replies; 6+ messages in thread
From: Lukas Wunner @ 2023-09-15 7:55 UTC (permalink / raw)
To: Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86
Cc: linux-kernel
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;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [tip: core/urgent] panic: Reenable preemption in WARN slowpath
2023-09-15 7:55 [PATCH] panic: Reenable preemption in WARN slowpath Lukas Wunner
@ 2023-09-15 9:36 ` tip-bot2 for Lukas Wunner
2023-09-15 9:45 ` Ingo Molnar
2023-09-15 9:38 ` [PATCH] " Peter Zijlstra
1 sibling, 1 reply; 6+ messages in thread
From: tip-bot2 for Lukas Wunner @ 2023-09-15 9:36 UTC (permalink / raw)
To: linux-tip-commits
Cc: Lukas Wunner, Ingo Molnar, Linus Torvalds, Thomas Gleixner,
Paul E. McKenney, x86, linux-kernel
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.
Fixes: 5a5d7e9badd2 ("cpuidle: lib/bug: Disable rcu_is_watching() during WARN/BUG")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Paul E. McKenney <paulmck@kernel.org>
Link: https://lore.kernel.org/r/3ec48fde01e4ee6505f77908ba351bad200ae3d1.1694763684.git.lukas@wunner.de
---
kernel/panic.c | 1 +
1 file changed, 1 insertion(+)
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;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [tip: core/urgent] panic: Reenable preemption in WARN slowpath
2023-09-15 9:36 ` [tip: core/urgent] " tip-bot2 for Lukas Wunner
@ 2023-09-15 9:45 ` Ingo Molnar
2023-09-15 10:35 ` Peter Zijlstra
0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2023-09-15 9:45 UTC (permalink / raw)
To: linux-kernel
Cc: linux-tip-commits, Lukas Wunner, Linus Torvalds, Thomas Gleixner,
Paul E. McKenney, x86, Borislav Petkov, Andrew Morton,
Peter Zijlstra
* 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [tip: core/urgent] panic: Reenable preemption in WARN slowpath
2023-09-15 9:45 ` Ingo Molnar
@ 2023-09-15 10:35 ` Peter Zijlstra
2023-09-15 11:13 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2023-09-15 10:35 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, linux-tip-commits, Lukas Wunner, Linus Torvalds,
Thomas Gleixner, Paul E. McKenney, x86, Borislav Petkov,
Andrew Morton
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [tip: core/urgent] panic: Reenable preemption in WARN slowpath
2023-09-15 10:35 ` Peter Zijlstra
@ 2023-09-15 11:13 ` Ingo Molnar
0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2023-09-15 11:13 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-tip-commits, Lukas Wunner, Linus Torvalds,
Thomas Gleixner, Paul E. McKenney, x86, Borislav Petkov,
Andrew Morton
* 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] panic: Reenable preemption in WARN slowpath
2023-09-15 7:55 [PATCH] panic: Reenable preemption in WARN slowpath Lukas Wunner
2023-09-15 9:36 ` [tip: core/urgent] " tip-bot2 for Lukas Wunner
@ 2023-09-15 9:38 ` Peter Zijlstra
1 sibling, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2023-09-15 9:38 UTC (permalink / raw)
To: Lukas Wunner
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
linux-kernel
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!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-15 11:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-15 7:55 [PATCH] panic: Reenable preemption in WARN slowpath Lukas Wunner
2023-09-15 9:36 ` [tip: core/urgent] " tip-bot2 for Lukas Wunner
2023-09-15 9:45 ` Ingo Molnar
2023-09-15 10:35 ` Peter Zijlstra
2023-09-15 11:13 ` Ingo Molnar
2023-09-15 9:38 ` [PATCH] " Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox