* [bug report] rcutorture: Perform more frequent testing of ->gpwrap
@ 2025-04-23 7:17 Dan Carpenter
2025-04-23 7:19 ` Dan Carpenter
0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2025-04-23 7:17 UTC (permalink / raw)
To: Joel Fernandes; +Cc: rcu
Hello Joel Fernandes,
Commit bd57ec707441 ("rcutorture: Perform more frequent testing of
->gpwrap") from Feb 16, 2025 (linux-next), leads to the following
Smatch static checker warning:
kernel/rcu/rcutorture.c:4586 rcu_torture_init()
warn: missing error code 'firsterr'
kernel/rcu/rcutorture.c
4576 if (torture_init_error(firsterr))
4577 goto unwind;
4578 }
4579 if (object_debug)
4580 rcu_test_debug_objects();
4581 torture_init_end();
4582 if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister))
4583 cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay);
4584
4585 if (cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init())
--> 4586 goto unwind;
set an error code?
4587
4588 return 0;
4589
4590 unwind:
4591 torture_init_end();
4592 rcu_torture_cleanup();
4593 if (shutdown_secs) {
4594 WARN_ON(!IS_MODULE(CONFIG_RCU_TORTURE_TEST));
4595 kernel_power_off();
4596 }
4597 return firsterr;
4598 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [bug report] rcutorture: Perform more frequent testing of ->gpwrap 2025-04-23 7:17 [bug report] rcutorture: Perform more frequent testing of ->gpwrap Dan Carpenter @ 2025-04-23 7:19 ` Dan Carpenter 2025-04-24 0:41 ` Paul E. McKenney 0 siblings, 1 reply; 7+ messages in thread From: Dan Carpenter @ 2025-04-23 7:19 UTC (permalink / raw) To: Joel Fernandes; +Cc: rcu On Wed, Apr 23, 2025 at 10:17:16AM +0300, Dan Carpenter wrote: > Hello Joel Fernandes, > > Commit bd57ec707441 ("rcutorture: Perform more frequent testing of > ->gpwrap") from Feb 16, 2025 (linux-next), leads to the following > Smatch static checker warning: > > kernel/rcu/rcutorture.c:4586 rcu_torture_init() > warn: missing error code 'firsterr' > > kernel/rcu/rcutorture.c > 4576 if (torture_init_error(firsterr)) > 4577 goto unwind; > 4578 } > 4579 if (object_debug) > 4580 rcu_test_debug_objects(); > 4581 torture_init_end(); ^^^^^^^^^^^^^^^^^^^ Also: kernel/rcu/rcutorture.c:4591 rcu_torture_init() warn: double unlock 'global &fullstop_mutex' (orig line 4581) > 4582 if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister)) > 4583 cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay); > 4584 > 4585 if (cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init()) > --> 4586 goto unwind; > > set an error code? > > 4587 > 4588 return 0; > 4589 > 4590 unwind: > 4591 torture_init_end(); ^^^^^^^^^^^^^^^^^^^ regards, dan carpenter > 4592 rcu_torture_cleanup(); > 4593 if (shutdown_secs) { > 4594 WARN_ON(!IS_MODULE(CONFIG_RCU_TORTURE_TEST)); > 4595 kernel_power_off(); > 4596 } > 4597 return firsterr; > 4598 } > > regards, > dan carpenter ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [bug report] rcutorture: Perform more frequent testing of ->gpwrap 2025-04-23 7:19 ` Dan Carpenter @ 2025-04-24 0:41 ` Paul E. McKenney 2025-04-24 6:54 ` Dan Carpenter 0 siblings, 1 reply; 7+ messages in thread From: Paul E. McKenney @ 2025-04-24 0:41 UTC (permalink / raw) To: Dan Carpenter; +Cc: Joel Fernandes, rcu On Wed, Apr 23, 2025 at 10:19:56AM +0300, Dan Carpenter wrote: > On Wed, Apr 23, 2025 at 10:17:16AM +0300, Dan Carpenter wrote: > > Hello Joel Fernandes, > > > > Commit bd57ec707441 ("rcutorture: Perform more frequent testing of > > ->gpwrap") from Feb 16, 2025 (linux-next), leads to the following > > Smatch static checker warning: > > > > kernel/rcu/rcutorture.c:4586 rcu_torture_init() > > warn: missing error code 'firsterr' > > > > kernel/rcu/rcutorture.c > > 4576 if (torture_init_error(firsterr)) > > 4577 goto unwind; > > 4578 } > > 4579 if (object_debug) > > 4580 rcu_test_debug_objects(); > > 4581 torture_init_end(); > ^^^^^^^^^^^^^^^^^^^ > Also: > > kernel/rcu/rcutorture.c:4591 rcu_torture_init() warn: double unlock 'global &fullstop_mutex' (orig line 4581) You lost me on this one. This mutex is acquired by the earlier call to torture_init_begin(), but only if that function returns true. If that function returns false, yes, it releases fullstop_mutex, but in that case rcu_torture_init() returns immediately, thus avoiding this invocation of torture_init_end(). Or am I missing something subtle here? Or missing something obvious, for that matter! ;-) Thanx, Paul > > 4582 if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister)) > > 4583 cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay); > > 4584 > > 4585 if (cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init()) > > --> 4586 goto unwind; > > > > set an error code? > > > > 4587 > > 4588 return 0; > > 4589 > > 4590 unwind: > > 4591 torture_init_end(); > ^^^^^^^^^^^^^^^^^^^ > > regards, > dan carpenter > > > 4592 rcu_torture_cleanup(); > > 4593 if (shutdown_secs) { > > 4594 WARN_ON(!IS_MODULE(CONFIG_RCU_TORTURE_TEST)); > > 4595 kernel_power_off(); > > 4596 } > > 4597 return firsterr; > > 4598 } > > > > regards, > > dan carpenter > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [bug report] rcutorture: Perform more frequent testing of ->gpwrap 2025-04-24 0:41 ` Paul E. McKenney @ 2025-04-24 6:54 ` Dan Carpenter 2025-04-24 14:44 ` Paul E. McKenney 0 siblings, 1 reply; 7+ messages in thread From: Dan Carpenter @ 2025-04-24 6:54 UTC (permalink / raw) To: Paul E. McKenney; +Cc: Joel Fernandes, rcu On Wed, Apr 23, 2025 at 05:41:37PM -0700, Paul E. McKenney wrote: > On Wed, Apr 23, 2025 at 10:19:56AM +0300, Dan Carpenter wrote: > > On Wed, Apr 23, 2025 at 10:17:16AM +0300, Dan Carpenter wrote: > > > Hello Joel Fernandes, > > > > > > Commit bd57ec707441 ("rcutorture: Perform more frequent testing of > > > ->gpwrap") from Feb 16, 2025 (linux-next), leads to the following > > > Smatch static checker warning: > > > > > > kernel/rcu/rcutorture.c:4586 rcu_torture_init() > > > warn: missing error code 'firsterr' > > > > > > kernel/rcu/rcutorture.c > > > 4576 if (torture_init_error(firsterr)) > > > 4577 goto unwind; > > > 4578 } > > > 4579 if (object_debug) > > > 4580 rcu_test_debug_objects(); > > > 4581 torture_init_end(); > > ^^^^^^^^^^^^^^^^^^^ > > Also: > > > > kernel/rcu/rcutorture.c:4591 rcu_torture_init() warn: double unlock 'global &fullstop_mutex' (orig line 4581) > > You lost me on this one. This mutex is acquired by the earlier call to > torture_init_begin(), but only if that function returns true. If that > function returns false, yes, it releases fullstop_mutex, but in that case > rcu_torture_init() returns immediately, thus avoiding this invocation > of torture_init_end(). > > Or am I missing something subtle here? Or missing something obvious, > for that matter! ;-) > I explained it badly. Joel's patch added a goto so now we call torture_init_end() twice in a row. > Thanx, Paul > > > > 4582 if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister)) > > > 4583 cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay); > > > 4584 > > > 4585 if (cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init()) > > > --> 4586 goto unwind; > > > > > > set an error code? Here is the goto. > > > > > > 4587 > > > 4588 return 0; > > > 4589 > > > 4590 unwind: > > > 4591 torture_init_end(); > > ^^^^^^^^^^^^^^^^^^^ Here is the second torture_init_end(). regards, dan carpenter ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [bug report] rcutorture: Perform more frequent testing of ->gpwrap 2025-04-24 6:54 ` Dan Carpenter @ 2025-04-24 14:44 ` Paul E. McKenney 2025-04-24 15:17 ` Joel Fernandes 0 siblings, 1 reply; 7+ messages in thread From: Paul E. McKenney @ 2025-04-24 14:44 UTC (permalink / raw) To: Dan Carpenter; +Cc: Joel Fernandes, rcu On Thu, Apr 24, 2025 at 09:54:14AM +0300, Dan Carpenter wrote: > On Wed, Apr 23, 2025 at 05:41:37PM -0700, Paul E. McKenney wrote: > > On Wed, Apr 23, 2025 at 10:19:56AM +0300, Dan Carpenter wrote: > > > On Wed, Apr 23, 2025 at 10:17:16AM +0300, Dan Carpenter wrote: > > > > Hello Joel Fernandes, > > > > > > > > Commit bd57ec707441 ("rcutorture: Perform more frequent testing of > > > > ->gpwrap") from Feb 16, 2025 (linux-next), leads to the following > > > > Smatch static checker warning: > > > > > > > > kernel/rcu/rcutorture.c:4586 rcu_torture_init() > > > > warn: missing error code 'firsterr' > > > > > > > > kernel/rcu/rcutorture.c > > > > 4576 if (torture_init_error(firsterr)) > > > > 4577 goto unwind; > > > > 4578 } > > > > 4579 if (object_debug) > > > > 4580 rcu_test_debug_objects(); > > > > 4581 torture_init_end(); > > > ^^^^^^^^^^^^^^^^^^^ > > > Also: > > > > > > kernel/rcu/rcutorture.c:4591 rcu_torture_init() warn: double unlock 'global &fullstop_mutex' (orig line 4581) > > > > You lost me on this one. This mutex is acquired by the earlier call to > > torture_init_begin(), but only if that function returns true. If that > > function returns false, yes, it releases fullstop_mutex, but in that case > > rcu_torture_init() returns immediately, thus avoiding this invocation > > of torture_init_end(). > > > > Or am I missing something subtle here? Or missing something obvious, > > for that matter! ;-) > > > > I explained it badly. Joel's patch added a goto so now we call > torture_init_end() twice in a row. > > > Thanx, Paul > > > > > > 4582 if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister)) > > > > 4583 cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay); > > > > 4584 > > > > 4585 if (cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init()) > > > > --> 4586 goto unwind; > > > > > > > > set an error code? > > Here is the goto. > > > > > > > > > 4587 > > > > 4588 return 0; > > > > 4589 > > > > 4590 unwind: > > > > 4591 torture_init_end(); > > > ^^^^^^^^^^^^^^^^^^^ > > Here is the second torture_init_end(). Ah! Thank you for persisting. Over to you, Joel! Thanx, Paul ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [bug report] rcutorture: Perform more frequent testing of ->gpwrap 2025-04-24 14:44 ` Paul E. McKenney @ 2025-04-24 15:17 ` Joel Fernandes 2025-04-24 18:14 ` Joel Fernandes 0 siblings, 1 reply; 7+ messages in thread From: Joel Fernandes @ 2025-04-24 15:17 UTC (permalink / raw) To: paulmck, Dan Carpenter; +Cc: rcu On 4/24/2025 10:44 AM, Paul E. McKenney wrote: > On Thu, Apr 24, 2025 at 09:54:14AM +0300, Dan Carpenter wrote: >> On Wed, Apr 23, 2025 at 05:41:37PM -0700, Paul E. McKenney wrote: >>> On Wed, Apr 23, 2025 at 10:19:56AM +0300, Dan Carpenter wrote: >>>> On Wed, Apr 23, 2025 at 10:17:16AM +0300, Dan Carpenter wrote: >>>>> Hello Joel Fernandes, >>>>> >>>>> Commit bd57ec707441 ("rcutorture: Perform more frequent testing of >>>>> ->gpwrap") from Feb 16, 2025 (linux-next), leads to the following >>>>> Smatch static checker warning: >>>>> >>>>> kernel/rcu/rcutorture.c:4586 rcu_torture_init() >>>>> warn: missing error code 'firsterr' >>>>> >>>>> kernel/rcu/rcutorture.c >>>>> 4576 if (torture_init_error(firsterr)) >>>>> 4577 goto unwind; >>>>> 4578 } >>>>> 4579 if (object_debug) >>>>> 4580 rcu_test_debug_objects(); >>>>> 4581 torture_init_end(); >>>> ^^^^^^^^^^^^^^^^^^^ >>>> Also: >>>> >>>> kernel/rcu/rcutorture.c:4591 rcu_torture_init() warn: double unlock 'global &fullstop_mutex' (orig line 4581) >>> >>> You lost me on this one. This mutex is acquired by the earlier call to >>> torture_init_begin(), but only if that function returns true. If that >>> function returns false, yes, it releases fullstop_mutex, but in that case >>> rcu_torture_init() returns immediately, thus avoiding this invocation >>> of torture_init_end(). >>> >>> Or am I missing something subtle here? Or missing something obvious, >>> for that matter! ;-) >>> >> >> I explained it badly. Joel's patch added a goto so now we call >> torture_init_end() twice in a row. >> >>> Thanx, Paul >>> >>>>> 4582 if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister)) >>>>> 4583 cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay); >>>>> 4584 >>>>> 4585 if (cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init()) >>>>> --> 4586 goto unwind; >>>>> >>>>> set an error code? >> >> Here is the goto. >> >>>>> >>>>> 4587 >>>>> 4588 return 0; >>>>> 4589 >>>>> 4590 unwind: >>>>> 4591 torture_init_end(); >>>> ^^^^^^^^^^^^^^^^^^^ >> >> Here is the second torture_init_end(). > > Ah! Thank you for persisting. Over to you, Joel! Oops, thanks for finding both the issues. I will move the torture_init_end() further down and run some tests. At first look, that seems like the right thing to do: diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c index 92e2686a4795..bb9a8e718533 100644 --- a/kernel/rcu/rcutorture.c +++ b/kernel/rcu/rcutorture.c @@ -4582,13 +4582,17 @@ rcu_torture_init(void) } if (object_debug) rcu_test_debug_objects(); - torture_init_end(); + if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister)) cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay); - if (gpwrap_lag && cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init()) - goto unwind; + if (gpwrap_lag && cur_ops->set_gpwrap_lag) { + firsterr = rcu_gpwrap_lag_init(); + if (torture_init_error(firsterr)) + goto unwind; + } + torture_init_end(); return 0; unwind: ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [bug report] rcutorture: Perform more frequent testing of ->gpwrap 2025-04-24 15:17 ` Joel Fernandes @ 2025-04-24 18:14 ` Joel Fernandes 0 siblings, 0 replies; 7+ messages in thread From: Joel Fernandes @ 2025-04-24 18:14 UTC (permalink / raw) To: paulmck, Dan Carpenter; +Cc: rcu On 4/24/2025 11:17 AM, Joel Fernandes wrote: > > > On 4/24/2025 10:44 AM, Paul E. McKenney wrote: >> On Thu, Apr 24, 2025 at 09:54:14AM +0300, Dan Carpenter wrote: >>> On Wed, Apr 23, 2025 at 05:41:37PM -0700, Paul E. McKenney wrote: >>>> On Wed, Apr 23, 2025 at 10:19:56AM +0300, Dan Carpenter wrote: >>>>> On Wed, Apr 23, 2025 at 10:17:16AM +0300, Dan Carpenter wrote: >>>>>> Hello Joel Fernandes, >>>>>> >>>>>> Commit bd57ec707441 ("rcutorture: Perform more frequent testing of >>>>>> ->gpwrap") from Feb 16, 2025 (linux-next), leads to the following >>>>>> Smatch static checker warning: >>>>>> >>>>>> kernel/rcu/rcutorture.c:4586 rcu_torture_init() >>>>>> warn: missing error code 'firsterr' >>>>>> >>>>>> kernel/rcu/rcutorture.c >>>>>> 4576 if (torture_init_error(firsterr)) >>>>>> 4577 goto unwind; >>>>>> 4578 } >>>>>> 4579 if (object_debug) >>>>>> 4580 rcu_test_debug_objects(); >>>>>> 4581 torture_init_end(); >>>>> ^^^^^^^^^^^^^^^^^^^ >>>>> Also: >>>>> >>>>> kernel/rcu/rcutorture.c:4591 rcu_torture_init() warn: double unlock 'global &fullstop_mutex' (orig line 4581) >>>> >>>> You lost me on this one. This mutex is acquired by the earlier call to >>>> torture_init_begin(), but only if that function returns true. If that >>>> function returns false, yes, it releases fullstop_mutex, but in that case >>>> rcu_torture_init() returns immediately, thus avoiding this invocation >>>> of torture_init_end(). >>>> >>>> Or am I missing something subtle here? Or missing something obvious, >>>> for that matter! ;-) >>>> >>> >>> I explained it badly. Joel's patch added a goto so now we call >>> torture_init_end() twice in a row. >>> >>>> Thanx, Paul >>>> >>>>>> 4582 if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister)) >>>>>> 4583 cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay); >>>>>> 4584 >>>>>> 4585 if (cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init()) >>>>>> --> 4586 goto unwind; >>>>>> >>>>>> set an error code? >>> >>> Here is the goto. >>> >>>>>> >>>>>> 4587 >>>>>> 4588 return 0; >>>>>> 4589 >>>>>> 4590 unwind: >>>>>> 4591 torture_init_end(); >>>>> ^^^^^^^^^^^^^^^^^^^ >>> >>> Here is the second torture_init_end(). >> >> Ah! Thank you for persisting. Over to you, Joel! > > Oops, thanks for finding both the issues. I will move the torture_init_end() > further down and run some tests. At first look, that seems like the right thing > to do: > > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > index 92e2686a4795..bb9a8e718533 100644 > --- a/kernel/rcu/rcutorture.c > +++ b/kernel/rcu/rcutorture.c > @@ -4582,13 +4582,17 @@ rcu_torture_init(void) > } > if (object_debug) > rcu_test_debug_objects(); > - torture_init_end(); > + > if (cur_ops->gp_slow_register && > !WARN_ON_ONCE(!cur_ops->gp_slow_unregister)) > cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay); > > - if (gpwrap_lag && cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init()) > - goto unwind; > + if (gpwrap_lag && cur_ops->set_gpwrap_lag) { > + firsterr = rcu_gpwrap_lag_init(); > + if (torture_init_error(firsterr)) > + goto unwind; > + } > > + torture_init_end(); > return 0; Passes basic testing, fixed and pushed for more testing with attribution: git://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git (next.2025.04.24b) Thanks! - Joel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-04-24 18:15 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-23 7:17 [bug report] rcutorture: Perform more frequent testing of ->gpwrap Dan Carpenter 2025-04-23 7:19 ` Dan Carpenter 2025-04-24 0:41 ` Paul E. McKenney 2025-04-24 6:54 ` Dan Carpenter 2025-04-24 14:44 ` Paul E. McKenney 2025-04-24 15:17 ` Joel Fernandes 2025-04-24 18:14 ` Joel Fernandes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox