* [PATCH tip/core/rcu 0/4] Eliminate BUG_ON for v4.21/v5.0
@ 2018-11-11 19:31 Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 1/4] rcu: Eliminate BUG_ON() for sync.c Paul E. McKenney
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Paul E. McKenney @ 2018-11-11 19:31 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg, joel
Hello!
This series converts BUG_ON() and friends to WARN_ON() and friends,
split out by file: (1) kernel/rcu/sync.c, (2) kernel/rcu/tree.c,
(3) kernel/rcu/tree_plugin.h, and (4) kernel/rcu/update.c.
Thanx, Paul
------------------------------------------------------------------------
sync.c | 13 ++++++-------
tree.c | 5 +++--
tree_plugin.h | 9 ++++++---
update.c | 3 ++-
4 files changed, 17 insertions(+), 13 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH tip/core/rcu 1/4] rcu: Eliminate BUG_ON() for sync.c
2018-11-11 19:31 [PATCH tip/core/rcu 0/4] Eliminate BUG_ON for v4.21/v5.0 Paul E. McKenney
@ 2018-11-11 19:32 ` Paul E. McKenney
2018-11-12 2:07 ` Steven Rostedt
2018-11-11 19:32 ` [PATCH tip/core/rcu 2/4] rcu: Eliminate BUG_ON() for kernel/rcu/tree.c Paul E. McKenney
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Paul E. McKenney @ 2018-11-11 19:32 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg, joel,
Paul E. McKenney
The sync.c file has a number of calls to BUG_ON(), which panics the
kernel, which is not a good strategy for devices (like embedded) that
don't have a way to capture console output. This commit therefore
changes these BUG_ON() calls to WARN_ON_ONCE(), but does so quite naively.
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
kernel/rcu/sync.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/kernel/rcu/sync.c b/kernel/rcu/sync.c
index 3f943efcf61c..a6ba446a9693 100644
--- a/kernel/rcu/sync.c
+++ b/kernel/rcu/sync.c
@@ -125,8 +125,7 @@ void rcu_sync_enter(struct rcu_sync *rsp)
rsp->gp_state = GP_PENDING;
spin_unlock_irq(&rsp->rss_lock);
- BUG_ON(need_wait && need_sync);
-
+ WARN_ON_ONCE(need_wait && need_sync);
if (need_sync) {
gp_ops[rsp->gp_type].sync();
rsp->gp_state = GP_PASSED;
@@ -139,7 +138,7 @@ void rcu_sync_enter(struct rcu_sync *rsp)
* Nobody has yet been allowed the 'fast' path and thus we can
* avoid doing any sync(). The callback will get 'dropped'.
*/
- BUG_ON(rsp->gp_state != GP_PASSED);
+ WARN_ON_ONCE(rsp->gp_state != GP_PASSED);
}
}
@@ -166,8 +165,8 @@ static void rcu_sync_func(struct rcu_head *rhp)
struct rcu_sync *rsp = container_of(rhp, struct rcu_sync, cb_head);
unsigned long flags;
- BUG_ON(rsp->gp_state != GP_PASSED);
- BUG_ON(rsp->cb_state == CB_IDLE);
+ WARN_ON_ONCE(rsp->gp_state != GP_PASSED);
+ WARN_ON_ONCE(rsp->cb_state == CB_IDLE);
spin_lock_irqsave(&rsp->rss_lock, flags);
if (rsp->gp_count) {
@@ -225,7 +224,7 @@ void rcu_sync_dtor(struct rcu_sync *rsp)
{
int cb_state;
- BUG_ON(rsp->gp_count);
+ WARN_ON_ONCE(rsp->gp_count);
spin_lock_irq(&rsp->rss_lock);
if (rsp->cb_state == CB_REPLAY)
@@ -235,6 +234,6 @@ void rcu_sync_dtor(struct rcu_sync *rsp)
if (cb_state != CB_IDLE) {
gp_ops[rsp->gp_type].wait();
- BUG_ON(rsp->cb_state != CB_IDLE);
+ WARN_ON_ONCE(rsp->cb_state != CB_IDLE);
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH tip/core/rcu 2/4] rcu: Eliminate BUG_ON() for kernel/rcu/tree.c
2018-11-11 19:31 [PATCH tip/core/rcu 0/4] Eliminate BUG_ON for v4.21/v5.0 Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 1/4] rcu: Eliminate BUG_ON() for sync.c Paul E. McKenney
@ 2018-11-11 19:32 ` Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 3/4] rcu: Eliminate BUG_ON() for kernel/rcu/tree_plugin.h Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 4/4] rcu: Eliminate BUG_ON() for kernel/rcu/update.c Paul E. McKenney
3 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2018-11-11 19:32 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg, joel,
Paul E. McKenney
The tree.c file has a number of calls to BUG_ON(), which panics the
kernel, which is not a good strategy for devices (like embedded) that
don't have a way to capture console output. This commit therefore
converts these BUG_ON() calls to WARN_ON_ONCE() and WARN_ONCE().
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
---
kernel/rcu/tree.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 121f833acd04..bdb6659a8dbc 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2826,7 +2826,7 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func, int cpu, bool lazy)
* Very early boot, before rcu_init(). Initialize if needed
* and then drop through to queue the callback.
*/
- BUG_ON(cpu != -1);
+ WARN_ON_ONCE(cpu != -1);
WARN_ON_ONCE(!rcu_is_watching());
if (rcu_segcblist_empty(&rdp->cblist))
rcu_segcblist_init(&rdp->cblist);
@@ -3485,7 +3485,8 @@ static int __init rcu_spawn_gp_kthread(void)
rcu_scheduler_fully_active = 1;
t = kthread_create(rcu_gp_kthread, NULL, "%s", rcu_state.name);
- BUG_ON(IS_ERR(t));
+ if (WARN_ONCE(IS_ERR(t), "%s: Could not start grace-period kthread, OOM is now expected behavior\n", __func__))
+ return 0;
rnp = rcu_get_root();
raw_spin_lock_irqsave_rcu_node(rnp, flags);
rcu_state.gp_kthread = t;
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH tip/core/rcu 3/4] rcu: Eliminate BUG_ON() for kernel/rcu/tree_plugin.h
2018-11-11 19:31 [PATCH tip/core/rcu 0/4] Eliminate BUG_ON for v4.21/v5.0 Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 1/4] rcu: Eliminate BUG_ON() for sync.c Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 2/4] rcu: Eliminate BUG_ON() for kernel/rcu/tree.c Paul E. McKenney
@ 2018-11-11 19:32 ` Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 4/4] rcu: Eliminate BUG_ON() for kernel/rcu/update.c Paul E. McKenney
3 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2018-11-11 19:32 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg, joel,
Paul E. McKenney
The tree_plugin.h file has a number of calls to BUG_ON(), which panics
the kernel, which is not a good strategy for devices (like embedded)
that don't have a way to capture console output. This commit therefore
converts these BUG_ON() calls to WARN_ON_ONCE() and WARN_ONCE().
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
---
kernel/rcu/tree_plugin.h | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 05915e536336..105a19752ac2 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1464,7 +1464,8 @@ static void __init rcu_spawn_boost_kthreads(void)
for_each_possible_cpu(cpu)
per_cpu(rcu_cpu_has_work, cpu) = 0;
- BUG_ON(smpboot_register_percpu_thread(&rcu_cpu_thread_spec));
+ if (WARN_ONCE(smpboot_register_percpu_thread(&rcu_cpu_thread_spec), "%s: Could not start rcuo kthread, OOM is now expected behavior\n", __func__))
+ return;
rcu_for_each_leaf_node(rnp)
(void)rcu_spawn_one_boost_kthread(rnp);
}
@@ -2322,7 +2323,8 @@ static int rcu_nocb_kthread(void *arg)
tail = rdp->nocb_follower_tail;
rdp->nocb_follower_tail = &rdp->nocb_follower_head;
raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
- BUG_ON(!list);
+ if (WARN_ON_ONCE(!list))
+ continue;
trace_rcu_nocb_wake(rcu_state.name, rdp->cpu, TPS("WokeNonEmpty"));
/* Each pass through the following loop invokes a callback. */
@@ -2495,7 +2497,8 @@ static void rcu_spawn_one_nocb_kthread(int cpu)
/* Spawn the kthread for this CPU. */
t = kthread_run(rcu_nocb_kthread, rdp_spawn,
"rcuo%c/%d", rcu_state.abbr, cpu);
- BUG_ON(IS_ERR(t));
+ if (WARN_ONCE(IS_ERR(t), "%s: Could not start rcuo kthread, OOM is now expected behavior\n", __func__))
+ return;
WRITE_ONCE(rdp_spawn->nocb_kthread, t);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH tip/core/rcu 4/4] rcu: Eliminate BUG_ON() for kernel/rcu/update.c
2018-11-11 19:31 [PATCH tip/core/rcu 0/4] Eliminate BUG_ON for v4.21/v5.0 Paul E. McKenney
` (2 preceding siblings ...)
2018-11-11 19:32 ` [PATCH tip/core/rcu 3/4] rcu: Eliminate BUG_ON() for kernel/rcu/tree_plugin.h Paul E. McKenney
@ 2018-11-11 19:32 ` Paul E. McKenney
3 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2018-11-11 19:32 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg, joel,
Paul E. McKenney
The update.c file has a number of calls to BUG_ON(), which panics the
kernel, which is not a good strategy for devices (like embedded) that
don't have a way to capture console output. This commit therefore
converts these BUG_ON() calls to WARN_ON_ONCE() and WARN_ONCE().
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
---
kernel/rcu/update.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index f203b94f6b5b..cb26de25658a 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -822,7 +822,8 @@ static int __init rcu_spawn_tasks_kthread(void)
struct task_struct *t;
t = kthread_run(rcu_tasks_kthread, NULL, "rcu_tasks_kthread");
- BUG_ON(IS_ERR(t));
+ if (WARN_ONCE(IS_ERR(t), "%s: Could not start Tasks-RCU grace-period kthread, OOM is now expected behavior\n", __func__))
+ return 0;
smp_mb(); /* Ensure others see full kthread. */
WRITE_ONCE(rcu_tasks_kthread_ptr, t);
return 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH tip/core/rcu 1/4] rcu: Eliminate BUG_ON() for sync.c
2018-11-11 19:32 ` [PATCH tip/core/rcu 1/4] rcu: Eliminate BUG_ON() for sync.c Paul E. McKenney
@ 2018-11-12 2:07 ` Steven Rostedt
2018-11-12 2:20 ` Paul E. McKenney
0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2018-11-12 2:07 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
mathieu.desnoyers, josh, tglx, peterz, dhowells, edumazet,
fweisbec, oleg, joel
On Sun, 11 Nov 2018 11:32:14 -0800
"Paul E. McKenney" <paulmck@linux.ibm.com> wrote:
> The sync.c file has a number of calls to BUG_ON(), which panics the
> kernel, which is not a good strategy for devices (like embedded) that
> don't have a way to capture console output. This commit therefore
> changes these BUG_ON() calls to WARN_ON_ONCE(), but does so quite naively.
>
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
> Acked-by: Oleg Nesterov <oleg@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> ---
> kernel/rcu/sync.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/rcu/sync.c b/kernel/rcu/sync.c
> index 3f943efcf61c..a6ba446a9693 100644
> --- a/kernel/rcu/sync.c
> +++ b/kernel/rcu/sync.c
> @@ -125,8 +125,7 @@ void rcu_sync_enter(struct rcu_sync *rsp)
> rsp->gp_state = GP_PENDING;
> spin_unlock_irq(&rsp->rss_lock);
>
> - BUG_ON(need_wait && need_sync);
> -
> + WARN_ON_ONCE(need_wait && need_sync);
> if (need_sync) {
> gp_ops[rsp->gp_type].sync();
> rsp->gp_state = GP_PASSED;
> @@ -139,7 +138,7 @@ void rcu_sync_enter(struct rcu_sync *rsp)
> * Nobody has yet been allowed the 'fast' path and thus we can
> * avoid doing any sync(). The callback will get 'dropped'.
> */
> - BUG_ON(rsp->gp_state != GP_PASSED);
> + WARN_ON_ONCE(rsp->gp_state != GP_PASSED);
> }
> }
>
> @@ -166,8 +165,8 @@ static void rcu_sync_func(struct rcu_head *rhp)
> struct rcu_sync *rsp = container_of(rhp, struct rcu_sync, cb_head);
> unsigned long flags;
>
> - BUG_ON(rsp->gp_state != GP_PASSED);
> - BUG_ON(rsp->cb_state == CB_IDLE);
> + WARN_ON_ONCE(rsp->gp_state != GP_PASSED);
> + WARN_ON_ONCE(rsp->cb_state == CB_IDLE);
>
> spin_lock_irqsave(&rsp->rss_lock, flags);
> if (rsp->gp_count) {
> @@ -225,7 +224,7 @@ void rcu_sync_dtor(struct rcu_sync *rsp)
> {
> int cb_state;
>
> - BUG_ON(rsp->gp_count);
> + WARN_ON_ONCE(rsp->gp_count);
>
> spin_lock_irq(&rsp->rss_lock);
> if (rsp->cb_state == CB_REPLAY)
> @@ -235,6 +234,6 @@ void rcu_sync_dtor(struct rcu_sync *rsp)
>
> if (cb_state != CB_IDLE) {
> gp_ops[rsp->gp_type].wait();
> - BUG_ON(rsp->cb_state != CB_IDLE);
> + WARN_ON_ONCE(rsp->cb_state != CB_IDLE);
> }
> }
I take it that if any of these WARN_ON_ONCE() triggers, they wont cause
immediate catastrophe, and/or there's no gentle way out like you have
with the other patches exiting the function when one is hit.
-- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH tip/core/rcu 1/4] rcu: Eliminate BUG_ON() for sync.c
2018-11-12 2:07 ` Steven Rostedt
@ 2018-11-12 2:20 ` Paul E. McKenney
0 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2018-11-12 2:20 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
mathieu.desnoyers, josh, tglx, peterz, dhowells, edumazet,
fweisbec, oleg, joel
On Sun, Nov 11, 2018 at 09:07:04PM -0500, Steven Rostedt wrote:
> On Sun, 11 Nov 2018 11:32:14 -0800
> "Paul E. McKenney" <paulmck@linux.ibm.com> wrote:
>
> > The sync.c file has a number of calls to BUG_ON(), which panics the
> > kernel, which is not a good strategy for devices (like embedded) that
> > don't have a way to capture console output. This commit therefore
> > changes these BUG_ON() calls to WARN_ON_ONCE(), but does so quite naively.
> >
> > Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
> > Acked-by: Oleg Nesterov <oleg@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > ---
> > kernel/rcu/sync.c | 13 ++++++-------
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/kernel/rcu/sync.c b/kernel/rcu/sync.c
> > index 3f943efcf61c..a6ba446a9693 100644
> > --- a/kernel/rcu/sync.c
> > +++ b/kernel/rcu/sync.c
> > @@ -125,8 +125,7 @@ void rcu_sync_enter(struct rcu_sync *rsp)
> > rsp->gp_state = GP_PENDING;
> > spin_unlock_irq(&rsp->rss_lock);
> >
> > - BUG_ON(need_wait && need_sync);
> > -
> > + WARN_ON_ONCE(need_wait && need_sync);
> > if (need_sync) {
> > gp_ops[rsp->gp_type].sync();
> > rsp->gp_state = GP_PASSED;
> > @@ -139,7 +138,7 @@ void rcu_sync_enter(struct rcu_sync *rsp)
> > * Nobody has yet been allowed the 'fast' path and thus we can
> > * avoid doing any sync(). The callback will get 'dropped'.
> > */
> > - BUG_ON(rsp->gp_state != GP_PASSED);
> > + WARN_ON_ONCE(rsp->gp_state != GP_PASSED);
> > }
> > }
> >
> > @@ -166,8 +165,8 @@ static void rcu_sync_func(struct rcu_head *rhp)
> > struct rcu_sync *rsp = container_of(rhp, struct rcu_sync, cb_head);
> > unsigned long flags;
> >
> > - BUG_ON(rsp->gp_state != GP_PASSED);
> > - BUG_ON(rsp->cb_state == CB_IDLE);
> > + WARN_ON_ONCE(rsp->gp_state != GP_PASSED);
> > + WARN_ON_ONCE(rsp->cb_state == CB_IDLE);
> >
> > spin_lock_irqsave(&rsp->rss_lock, flags);
> > if (rsp->gp_count) {
> > @@ -225,7 +224,7 @@ void rcu_sync_dtor(struct rcu_sync *rsp)
> > {
> > int cb_state;
> >
> > - BUG_ON(rsp->gp_count);
> > + WARN_ON_ONCE(rsp->gp_count);
> >
> > spin_lock_irq(&rsp->rss_lock);
> > if (rsp->cb_state == CB_REPLAY)
> > @@ -235,6 +234,6 @@ void rcu_sync_dtor(struct rcu_sync *rsp)
> >
> > if (cb_state != CB_IDLE) {
> > gp_ops[rsp->gp_type].wait();
> > - BUG_ON(rsp->cb_state != CB_IDLE);
> > + WARN_ON_ONCE(rsp->cb_state != CB_IDLE);
> > }
> > }
>
> I take it that if any of these WARN_ON_ONCE() triggers, they wont cause
> immediate catastrophe, and/or there's no gentle way out like you have
> with the other patches exiting the function when one is hit.
Oleg was actually OK with removing them entirely:
"I added these BUG_ON's for documentation when I was prototyping
this code, perhaps we can simply remove them."
And they are "cannot happen" types of things (famous last words).
Oleg also has another approach that could rip-and-replace the current
implementation, which would render these WARN*()s moot.
Thanx, Paul
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-11-12 2:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-11 19:31 [PATCH tip/core/rcu 0/4] Eliminate BUG_ON for v4.21/v5.0 Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 1/4] rcu: Eliminate BUG_ON() for sync.c Paul E. McKenney
2018-11-12 2:07 ` Steven Rostedt
2018-11-12 2:20 ` Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 2/4] rcu: Eliminate BUG_ON() for kernel/rcu/tree.c Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 3/4] rcu: Eliminate BUG_ON() for kernel/rcu/tree_plugin.h Paul E. McKenney
2018-11-11 19:32 ` [PATCH tip/core/rcu 4/4] rcu: Eliminate BUG_ON() for kernel/rcu/update.c Paul E. McKenney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).