public inbox for rcu@vger.kernel.org
 help / color / mirror / Atom feed
* [bug report] rcuscale: Add laziness and kfree tests
@ 2024-10-22  9:07 Dan Carpenter
  2024-10-22 16:32 ` Uladzislau Rezki
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2024-10-22  9:07 UTC (permalink / raw)
  To: Joel Fernandes; +Cc: rcu

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [bug report] rcuscale: Add laziness and kfree tests
  2024-10-22  9:07 [bug report] rcuscale: Add laziness and kfree tests Dan Carpenter
@ 2024-10-22 16:32 ` Uladzislau Rezki
  0 siblings, 0 replies; 2+ messages in thread
From: Uladzislau Rezki @ 2024-10-22 16:32 UTC (permalink / raw)
  To: Dan Carpenter, Joel Fernandes, Paul E. McKenney; +Cc: Joel Fernandes, rcu

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

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-10-22 16:32 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-22  9:07 [bug report] rcuscale: Add laziness and kfree tests Dan Carpenter
2024-10-22 16:32 ` Uladzislau Rezki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox