Re: [bug report] rcuscale: Add laziness and kfree tests

[Uladzislau Rezki <urezki@gmail.com>]

On Tue, Oct 22, 2024 at 12:07:01PM +0300, Dan Carpenter wrote:
> Hello Joel Fernandes (Google),
> 
> Commit 084e04fff160 ("rcuscale: Add laziness and kfree tests") from
> Oct 16, 2022 (linux-next), leads to the following Smatch static
> checker warning:
> 
> 	kernel/rcu/rcuscale.c:1215 rcu_scale_init()
> 	warn: inconsistent returns 'global &fullstop_mutex'.
> 
> kernel/rcu/rcuscale.c
>    857  kfree_scale_init(void)
>    858  {
>    859          int firsterr = 0;
>    860          long i;
>    861          unsigned long jif_start;
>    862          unsigned long orig_jif;
>    863  
>    864          pr_alert("%s" SCALE_FLAG
>    865                   "--- kfree_rcu_test: kfree_mult=%d kfree_by_call_rcu=%d kfree_nthreads=%d kfree_alloc_num=%d kfree_loops=%d kfree_rcu_test_double=%d kfree_rcu_test_single=%d\n",
>    866                   scale_type, kfree_mult, kfree_by_call_rcu, kfree_nthreads, kfree_alloc_num, kfree_loops, kfree_rcu_test_double, kfree_rcu_test_single);
>    867  
>    868          // Also, do a quick self-test to ensure laziness is as much as
>    869          // expected.
>    870          if (kfree_by_call_rcu && !IS_ENABLED(CONFIG_RCU_LAZY)) {
>    871                  pr_alert("CONFIG_RCU_LAZY is disabled, falling back to kfree_rcu() for delayed RCU kfree'ing\n");
>    872                  kfree_by_call_rcu = 0;
>    873          }
>    874  
>    875          if (kfree_by_call_rcu) {
>    876                  /* do a test to check the timeout. */
>    877                  orig_jif = rcu_get_jiffies_lazy_flush();
>    878  
>    879                  rcu_set_jiffies_lazy_flush(2 * HZ);
>    880                  rcu_barrier();
>    881  
>    882                  jif_start = jiffies;
>    883                  jiffies_at_lazy_cb = 0;
>    884                  call_rcu(&lazy_test1_rh, call_rcu_lazy_test1);
>    885  
>    886                  smp_cond_load_relaxed(&rcu_lazy_test1_cb_called, VAL == 1);
>    887  
>    888                  rcu_set_jiffies_lazy_flush(orig_jif);
>    889  
>    890                  if (WARN_ON_ONCE(jiffies_at_lazy_cb - jif_start < 2 * HZ)) {
>    891                          pr_alert("ERROR: call_rcu() CBs are not being lazy as expected!\n");
>    892                          WARN_ON_ONCE(1);
>    893                          return -1;
>                                 ^^^^^^^^^^
>    894                  }
>    895  
>    896                  if (WARN_ON_ONCE(jiffies_at_lazy_cb - jif_start > 3 * HZ)) {
>    897                          pr_alert("ERROR: call_rcu() CBs are being too lazy!\n");
>    898                          WARN_ON_ONCE(1);
>    899                          return -1;
>                                 ^^^^^^^^^^
> The checker is complaining that we don't call torture_init_end().  Should these
> do a goto unwind?  Otherwise we're left holding the fullstop_mutex.
> 
Not really. At least if we want to "goto unwind", the following patch
should be applied:

<snip>
diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
index 6d37596deb1f..4eb872f9acd9 100644
--- a/kernel/rcu/rcuscale.c
+++ b/kernel/rcu/rcuscale.c
@@ -818,7 +818,8 @@ kfree_scale_cleanup(void)
 
 	if (kfree_reader_tasks) {
 		for (i = 0; i < kfree_nrealthreads; i++)
-			torture_stop_kthread(kfree_scale_thread,
+			if (kfree_reader_tasks[i])
+				torture_stop_kthread(kfree_scale_thread,
 					     kfree_reader_tasks[i]);
 		kfree(kfree_reader_tasks);
 		kfree_reader_tasks = NULL;
@@ -890,13 +891,15 @@ kfree_scale_init(void)
 		if (WARN_ON_ONCE(jiffies_at_lazy_cb - jif_start < 2 * HZ)) {
 			pr_alert("ERROR: call_rcu() CBs are not being lazy as expected!\n");
 			WARN_ON_ONCE(1);
-			return -1;
+			firsterr = -1;
+			goto unwind;
 		}
 
 		if (WARN_ON_ONCE(jiffies_at_lazy_cb - jif_start > 3 * HZ)) {
 			pr_alert("ERROR: call_rcu() CBs are being too lazy!\n");
 			WARN_ON_ONCE(1);
-			return -1;
+			firsterr = -1;
+			goto unwind;
 		}
 	}
<snip>

then it should address your "static checker warning" which looks correct
to me.

@Joel, @Paul, do you agree? If so i can prepare the patch.

--
Uladzislau Rezki