* 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