Skip to contentSkip to conversation

rcuscale: Add laziness and kfree tests

bug report Dan Carpenter 2 participants 2 messages Oct 22, 2024

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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.

900 }
901 }

[ snip ]

   938  unwind:
   939          torture_init_end();
   940          kfree_scale_cleanup();
   941          return firsterr;
   942  }

regards,
dan carpenter

maintainer
#1 lore raw
Quoted 55 lines · Dan Carpenter

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;
                                ^^^^^^^^^^
Quoted 3 lines

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>

kernel/rcu/rcuscale.c +6 -3
index 6d37596deb1f..4eb872f9acd9 100644
@@ -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