[RFC PATCH 0/3] Early boot self tests for RCU

[RFC PATCH 0/3] Early boot self tests for RCU

[Pranith Kumar]

Hi Paul,

Please find the following patches which enable RCU early boot tests. For now
all we do in these tests are enqueue test callbacks and check if they are being
invoked or not.

I was able to reproduce the hang which Amit reported after reverting the fix. So
this should catch such errors if they occur in the future.

I enabled the selftests in a few rcutorture configurations.

Thanks!
--
Pranith

Pranith Kumar (3):
  rcu: Add early boot self tests
  doc: Document RCU self test boot params
  rcutorture: Enable RCU self test in configs

 Documentation/kernel-parameters.txt                | 12 +++
 kernel/rcu/rcu.h                                   |  2 +
 kernel/rcu/tiny.c                                  |  4 +-
 kernel/rcu/tree.c                                  |  2 +
 kernel/rcu/update.c                                | 91 ++++++++++++++++++++++
 .../selftests/rcutorture/configs/rcu/TINY02        |  2 +
 .../selftests/rcutorture/configs/rcu/TINY02.boot   |  2 +
 .../selftests/rcutorture/configs/rcu/TREE05.boot   |  1 +
 .../selftests/rcutorture/configs/rcu/TREE06.boot   |  4 +
 .../selftests/rcutorture/configs/rcu/TREE08        |  2 +
 .../selftests/rcutorture/configs/rcu/TREE08.boot   |  2 +
 11 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
 create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/TREE06.boot

-- 
2.1.0

[RFC PATCH 0/3] Early boot self tests for RCU

[Pranith Kumar]

Hi Paul,

Please find the following patches which enable RCU early boot tests. For now
all we do in these tests are enqueue test callbacks and check if they are being
invoked or not.

I was able to reproduce the hang which Amit reported after reverting the fix. So
this should catch such errors if they occur in the future.

I enabled the selftests in a few rcutorture configurations.

Thanks!
--
Pranith

Pranith Kumar (3):
  rcu: Add early boot self tests
  doc: Document RCU self test boot params
  rcutorture: Enable RCU self test in configs

 Documentation/kernel-parameters.txt                | 12 +++
 kernel/rcu/rcu.h                                   |  2 +
 kernel/rcu/tiny.c                                  |  4 +-
 kernel/rcu/tree.c                                  |  2 +
 kernel/rcu/update.c                                | 91 ++++++++++++++++++++++
 .../selftests/rcutorture/configs/rcu/TINY02        |  2 +
 .../selftests/rcutorture/configs/rcu/TINY02.boot   |  2 +
 .../selftests/rcutorture/configs/rcu/TREE05.boot   |  1 +
 .../selftests/rcutorture/configs/rcu/TREE06.boot   |  4 +
 .../selftests/rcutorture/configs/rcu/TREE08        |  2 +
 .../selftests/rcutorture/configs/rcu/TREE08.boot   |  2 +
 11 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
 create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/TREE06.boot

-- 
2.1.0

[RFC PATCH 1/3] rcu: Add early boot self tests

[Pranith Kumar]

Add early boot self tests for RCU under CONFIG_PROVE_RCU.

Currently the only test is adding a dummy callback which increments a counter
which we then later verify after calling rcu_barrier*().

Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
 kernel/rcu/rcu.h    |  2 ++
 kernel/rcu/tiny.c   |  4 ++-
 kernel/rcu/tree.c   |  2 ++
 kernel/rcu/update.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index ff1a6de..07bb02e 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -135,4 +135,6 @@ int rcu_jiffies_till_stall_check(void);
  */
 #define TPS(x)  tracepoint_string(x)
 
+void rcu_early_boot_tests(void);
+
 #endif /* __LINUX_RCU_H */
diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
index c0623fc..d3d44c5 100644
--- a/kernel/rcu/tiny.c
+++ b/kernel/rcu/tiny.c
@@ -380,7 +380,9 @@ void call_rcu_bh(struct rcu_head *head, void (*func)(struct rcu_head *rcu))
 }
 EXPORT_SYMBOL_GPL(call_rcu_bh);
 
-void rcu_init(void)
+void __init rcu_init(void)
 {
 	open_softirq(RCU_SOFTIRQ, rcu_process_callbacks);
+
+	rcu_early_boot_tests();
 }
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index e4c6d60..f93a62c 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3766,6 +3766,8 @@ void __init rcu_init(void)
 	pm_notifier(rcu_pm_notify, 0);
 	for_each_online_cpu(cpu)
 		rcu_cpu_notify(NULL, CPU_UP_PREPARE, (void *)(long)cpu);
+
+	rcu_early_boot_tests();
 }
 
 #include "tree_plugin.h"
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index 3ef8ba5..5929f0c 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -690,3 +690,94 @@ static void rcu_spawn_tasks_kthread(void)
 }
 
 #endif /* #ifdef CONFIG_TASKS_RCU */
+
+#ifdef CONFIG_PROVE_RCU
+
+/*
+ * Early boot self test parameters, one for each flavor
+ */
+static bool rcu_self_test;
+static bool rcu_self_test_bh;
+static bool rcu_self_test_sched;
+static bool rcu_self_test_srcu;
+
+module_param(rcu_self_test, bool, 0444);
+module_param(rcu_self_test_bh, bool, 0444);
+module_param(rcu_self_test_sched, bool, 0444);
+module_param(rcu_self_test_srcu, bool, 0444);
+
+static int rcu_self_test_counter;
+static struct rcu_head head;
+DEFINE_STATIC_SRCU(srcu_struct);
+
+static void test_callback(struct rcu_head *r)
+{
+	rcu_self_test_counter++;
+	pr_info("RCU test callback executed %d\n", rcu_self_test_counter);
+}
+
+static void early_boot_test_call_rcu(void)
+{
+	call_rcu(&head, test_callback);
+}
+
+static void early_boot_test_call_rcu_bh(void)
+{
+	call_rcu_bh(&head, test_callback);
+}
+
+static void early_boot_test_call_rcu_sched(void)
+{
+	call_rcu_sched(&head, test_callback);
+}
+
+static void early_boot_test_call_srcu(void)
+{
+	call_srcu(&srcu_struct, &head, test_callback);
+}
+
+void rcu_early_boot_tests(void)
+{
+	pr_info("Running RCU self tests\n");
+
+	if (rcu_self_test)
+		early_boot_test_call_rcu();
+	if (rcu_self_test_bh)
+		early_boot_test_call_rcu_bh();
+	if (rcu_self_test_sched)
+		early_boot_test_call_rcu_sched();
+	if (rcu_self_test_srcu)
+		early_boot_test_call_srcu();
+}
+
+static int rcu_verify_early_boot_tests(void)
+{
+	int ret = 0;
+	int early_boot_test_counter = 0;
+
+	if (rcu_self_test) {
+		early_boot_test_counter++;
+		rcu_barrier();
+	}
+	if (rcu_self_test_bh) {
+		early_boot_test_counter++;
+		rcu_barrier_bh();
+	}
+	if (rcu_self_test_sched) {
+		early_boot_test_counter++;
+		rcu_barrier_sched();
+	}
+	if (rcu_self_test_srcu) {
+		early_boot_test_counter++;
+		srcu_barrier(&srcu_struct);
+	}
+
+	if (rcu_self_test_counter != early_boot_test_counter)
+		ret = -1;
+
+	return ret;
+}
+late_initcall(rcu_verify_early_boot_tests);
+#else
+void rcu_early_boot_tests(void) {}
+#endif /* CONFIG_PROVE_RCU */
-- 
2.1.0

[RFC PATCH 2/3] doc: Document RCU self test boot params

[Pranith Kumar]

Document the RCU self test boot parameters in kernel-parameters.txt.

Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
 Documentation/kernel-parameters.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index c403957..6f5c01b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3054,6 +3054,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			messages.  Disable with a value less than or equal
 			to zero.
 
+	rcupdate.rcu_self_test= [KNL]
+			Run the RCU early boot self tests
+
+	rcupdate.rcu_self_test_bh= [KNL]
+			Run the RCU bh early boot self tests
+
+	rcupdate.rcu_self_test_sched= [KNL]
+			Run the RCU sched early boot self tests
+
+	rcupdate.rcu_self_test_srcu= [KNL]
+			Run the SRCU early boot self tests
+
 	rdinit=		[KNL]
 			Format: <full_path>
 			Run specified binary instead of /init from the ramdisk,
-- 
2.1.0

[RFC PATCH 3/3] rcutorture: Enable RCU self test in configs

[Pranith Kumar]

Add config and boot parameters to enable the self tests in rcutorture testing.

Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
 tools/testing/selftests/rcutorture/configs/rcu/TINY02      | 2 ++
 tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot | 2 ++
 tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot | 1 +
 tools/testing/selftests/rcutorture/configs/rcu/TREE06.boot | 4 ++++
 tools/testing/selftests/rcutorture/configs/rcu/TREE08      | 2 ++
 tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot | 2 ++
 6 files changed, 13 insertions(+)
 create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
 create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/TREE06.boot

diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TINY02 b/tools/testing/selftests/rcutorture/configs/rcu/TINY02
index f4feaee..36e41df 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/TINY02
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TINY02
@@ -7,6 +7,8 @@ CONFIG_HZ_PERIODIC=y
 CONFIG_NO_HZ_IDLE=n
 CONFIG_NO_HZ_FULL=n
 CONFIG_RCU_TRACE=y
+CONFIG_PROVE_LOCKING=y
+CONFIG_PROVE_RCU=y
 CONFIG_DEBUG_LOCK_ALLOC=y
 CONFIG_DEBUG_OBJECTS_RCU_HEAD=n
 CONFIG_PREEMPT_COUNT=y
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot b/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
new file mode 100644
index 0000000..0f08027
--- /dev/null
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
@@ -0,0 +1,2 @@
+rcupdate.rcu_self_test=1
+rcupdate.rcu_self_test_bh=1
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot b/tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot
index 3b42b8b..15b3e1a 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot
@@ -1 +1,2 @@
 rcutorture.torture_type=sched
+rcupdate.rcu_self_test_sched=1
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TREE06.boot b/tools/testing/selftests/rcutorture/configs/rcu/TREE06.boot
new file mode 100644
index 0000000..0436e07
--- /dev/null
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TREE06.boot
@@ -0,0 +1,4 @@
+rcupdate.rcu_self_test=1
+rcupdate.rcu_self_test_bh=1
+rcupdate.rcu_self_test_sched=1
+rcupdate.rcu_self_test_srcu=1
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TREE08 b/tools/testing/selftests/rcutorture/configs/rcu/TREE08
index 316aa6c..1a9303d 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/TREE08
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TREE08
@@ -18,6 +18,8 @@ CONFIG_RCU_FANOUT_LEAF=2
 CONFIG_RCU_NOCB_CPU=y
 CONFIG_RCU_NOCB_CPU_ALL=y
 CONFIG_DEBUG_LOCK_ALLOC=n
+CONFIG_PROVE_LOCKING=y
+CONFIG_PROVE_RCU=y
 CONFIG_RCU_CPU_STALL_INFO=n
 CONFIG_RCU_BOOST=n
 CONFIG_DEBUG_OBJECTS_RCU_HEAD=n
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot b/tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot
index 3b42b8b..2561daf 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot
@@ -1 +1,3 @@
 rcutorture.torture_type=sched
+rcupdate.rcu_self_test=1
+rcupdate.rcu_self_test_sched=1
-- 
2.1.0

Re: [RFC PATCH 1/3] rcu: Add early boot self tests

[Paul E. McKenney]

On Wed, Sep 17, 2014 at 11:21:47PM -0400, Pranith Kumar wrote:
> Add early boot self tests for RCU under CONFIG_PROVE_RCU.
> 
> Currently the only test is adding a dummy callback which increments a counter
> which we then later verify after calling rcu_barrier*().
> 
> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>

Great addition to RCU self-testing!

A couple of comments below.

> ---
>  kernel/rcu/rcu.h    |  2 ++
>  kernel/rcu/tiny.c   |  4 ++-
>  kernel/rcu/tree.c   |  2 ++
>  kernel/rcu/update.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> index ff1a6de..07bb02e 100644
> --- a/kernel/rcu/rcu.h
> +++ b/kernel/rcu/rcu.h
> @@ -135,4 +135,6 @@ int rcu_jiffies_till_stall_check(void);
>   */
>  #define TPS(x)  tracepoint_string(x)
>  
> +void rcu_early_boot_tests(void);
> +
>  #endif /* __LINUX_RCU_H */
> diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> index c0623fc..d3d44c5 100644
> --- a/kernel/rcu/tiny.c
> +++ b/kernel/rcu/tiny.c
> @@ -380,7 +380,9 @@ void call_rcu_bh(struct rcu_head *head, void (*func)(struct rcu_head *rcu))
>  }
>  EXPORT_SYMBOL_GPL(call_rcu_bh);
>  
> -void rcu_init(void)
> +void __init rcu_init(void)
>  {
>  	open_softirq(RCU_SOFTIRQ, rcu_process_callbacks);
> +
> +	rcu_early_boot_tests();
>  }
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index e4c6d60..f93a62c 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3766,6 +3766,8 @@ void __init rcu_init(void)
>  	pm_notifier(rcu_pm_notify, 0);
>  	for_each_online_cpu(cpu)
>  		rcu_cpu_notify(NULL, CPU_UP_PREPARE, (void *)(long)cpu);
> +
> +	rcu_early_boot_tests();
>  }
>  
>  #include "tree_plugin.h"
> diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> index 3ef8ba5..5929f0c 100644
> --- a/kernel/rcu/update.c
> +++ b/kernel/rcu/update.c
> @@ -690,3 +690,94 @@ static void rcu_spawn_tasks_kthread(void)
>  }
>  
>  #endif /* #ifdef CONFIG_TASKS_RCU */
> +
> +#ifdef CONFIG_PROVE_RCU
> +
> +/*
> + * Early boot self test parameters, one for each flavor
> + */
> +static bool rcu_self_test;
> +static bool rcu_self_test_bh;
> +static bool rcu_self_test_sched;
> +static bool rcu_self_test_srcu;
> +
> +module_param(rcu_self_test, bool, 0444);
> +module_param(rcu_self_test_bh, bool, 0444);
> +module_param(rcu_self_test_sched, bool, 0444);
> +module_param(rcu_self_test_srcu, bool, 0444);
> +
> +static int rcu_self_test_counter;
> +static struct rcu_head head;

This needs to be within the individual functions, because otherwise the
lists get messed up when you to multiple tests during the same boot...

> +DEFINE_STATIC_SRCU(srcu_struct);
> +
> +static void test_callback(struct rcu_head *r)
> +{
> +	rcu_self_test_counter++;
> +	pr_info("RCU test callback executed %d\n", rcu_self_test_counter);
> +}
> +
> +static void early_boot_test_call_rcu(void)
> +{

... as in:

	static struct rcu_head head;

> +	call_rcu(&head, test_callback);
> +}
> +
> +static void early_boot_test_call_rcu_bh(void)
> +{
> +	call_rcu_bh(&head, test_callback);
> +}
> +
> +static void early_boot_test_call_rcu_sched(void)
> +{
> +	call_rcu_sched(&head, test_callback);
> +}
> +
> +static void early_boot_test_call_srcu(void)
> +{
> +	call_srcu(&srcu_struct, &head, test_callback);

This looked like a great idea at first, but unfortunately call_srcu()
invokes queue_delayed_work(), which breaks horribly this early in boot.
Either this test has to be removed, or call_srcu() has to be updated
to handle early-boot invocation.  Given that no one is using call_srcu()
during early boot, it is probably best to just drop the test.

(In case you were wondering, TEST06 dies during boot.)

Could you please send an updated patch?

> +}
> +
> +void rcu_early_boot_tests(void)
> +{
> +	pr_info("Running RCU self tests\n");
> +
> +	if (rcu_self_test)
> +		early_boot_test_call_rcu();
> +	if (rcu_self_test_bh)
> +		early_boot_test_call_rcu_bh();
> +	if (rcu_self_test_sched)
> +		early_boot_test_call_rcu_sched();
> +	if (rcu_self_test_srcu)
> +		early_boot_test_call_srcu();
> +}
> +
> +static int rcu_verify_early_boot_tests(void)
> +{
> +	int ret = 0;
> +	int early_boot_test_counter = 0;
> +
> +	if (rcu_self_test) {
> +		early_boot_test_counter++;
> +		rcu_barrier();
> +	}
> +	if (rcu_self_test_bh) {
> +		early_boot_test_counter++;
> +		rcu_barrier_bh();
> +	}
> +	if (rcu_self_test_sched) {
> +		early_boot_test_counter++;
> +		rcu_barrier_sched();
> +	}
> +	if (rcu_self_test_srcu) {
> +		early_boot_test_counter++;
> +		srcu_barrier(&srcu_struct);
> +	}
> +
> +	if (rcu_self_test_counter != early_boot_test_counter)
> +		ret = -1;
> +
> +	return ret;
> +}
> +late_initcall(rcu_verify_early_boot_tests);
> +#else
> +void rcu_early_boot_tests(void) {}
> +#endif /* CONFIG_PROVE_RCU */
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH 1/3] rcu: Add early boot self tests

[Pranith Kumar]

On Thu, Sep 18, 2014 at 5:29 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:

>> +static int rcu_self_test_counter;
>> +static struct rcu_head head;
>
> This needs to be within the individual functions, because otherwise the
> lists get messed up when you to multiple tests during the same boot...
>

Hmm, I thought this was OK since we are not using this head anywhere.
What lists are getting messed up?

In any case, I will update this as you suggested.

>> +DEFINE_STATIC_SRCU(srcu_struct);
>> +
>> +static void test_callback(struct rcu_head *r)
>> +{
>> +     rcu_self_test_counter++;
>> +     pr_info("RCU test callback executed %d\n", rcu_self_test_counter);
>> +}
>> +
>> +static void early_boot_test_call_rcu(void)
>> +{
>
> ... as in:
>
>         static struct rcu_head head;
>
>> +     call_rcu(&head, test_callback);
>> +}
>> +
>> +static void early_boot_test_call_rcu_bh(void)
>> +{
>> +     call_rcu_bh(&head, test_callback);
>> +}
>> +
>> +static void early_boot_test_call_rcu_sched(void)
>> +{
>> +     call_rcu_sched(&head, test_callback);
>> +}
>> +
>> +static void early_boot_test_call_srcu(void)
>> +{
>> +     call_srcu(&srcu_struct, &head, test_callback);
>
> This looked like a great idea at first, but unfortunately call_srcu()
> invokes queue_delayed_work(), which breaks horribly this early in boot.
> Either this test has to be removed, or call_srcu() has to be updated
> to handle early-boot invocation.  Given that no one is using call_srcu()
> during early boot, it is probably best to just drop the test.
>
> (In case you were wondering, TEST06 dies during boot.)
>
> Could you please send an updated patch?


Yup, will do. Please see one question below:

<...>
>> +static int rcu_verify_early_boot_tests(void)
>> +{
>> +     int ret = 0;
>> +     int early_boot_test_counter = 0;
>> +
>> +     if (rcu_self_test) {
>> +             early_boot_test_counter++;
>> +             rcu_barrier();
>> +     }
>> +     if (rcu_self_test_bh) {
>> +             early_boot_test_counter++;
>> +             rcu_barrier_bh();
>> +     }
>> +     if (rcu_self_test_sched) {
>> +             early_boot_test_counter++;
>> +             rcu_barrier_sched();
>> +     }
>> +     if (rcu_self_test_srcu) {
>> +             early_boot_test_counter++;
>> +             srcu_barrier(&srcu_struct);
>> +     }
>> +
>> +     if (rcu_self_test_counter != early_boot_test_counter)
>> +             ret = -1;


So this basically does nothing when it does not match. All we see is
the return value when we pass initcall_debug. Should I add a WARN_ON()
or some such so that it is more explicit?

>> +
>> +     return ret;
>> +}
>> +late_initcall(rcu_verify_early_boot_tests);
>> +#else
>> +void rcu_early_boot_tests(void) {}
>> +#endif /* CONFIG_PROVE_RCU */
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
Pranith

Re: [RFC PATCH 1/3] rcu: Add early boot self tests

[Paul E. McKenney]

On Thu, Sep 18, 2014 at 09:03:43PM -0400, Pranith Kumar wrote:
> On Thu, Sep 18, 2014 at 5:29 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> 
> >> +static int rcu_self_test_counter;
> >> +static struct rcu_head head;
> >
> > This needs to be within the individual functions, because otherwise the
> > lists get messed up when you to multiple tests during the same boot...
> 
> Hmm, I thought this was OK since we are not using this head anywhere.
> What lists are getting messed up?

The problem is that the current code enqueues the same structure onto
up to four different lists, and we don't have a quantum computer, so
head.next can't point to four different places.  ;-)

Making head be static in all four functions allows four different
head.next pointer to point to the four different places, as required.

> In any case, I will update this as you suggested.

Very good!

> >> +DEFINE_STATIC_SRCU(srcu_struct);
> >> +
> >> +static void test_callback(struct rcu_head *r)
> >> +{
> >> +     rcu_self_test_counter++;
> >> +     pr_info("RCU test callback executed %d\n", rcu_self_test_counter);
> >> +}
> >> +
> >> +static void early_boot_test_call_rcu(void)
> >> +{
> >
> > ... as in:
> >
> >         static struct rcu_head head;
> >
> >> +     call_rcu(&head, test_callback);
> >> +}
> >> +
> >> +static void early_boot_test_call_rcu_bh(void)
> >> +{
> >> +     call_rcu_bh(&head, test_callback);
> >> +}
> >> +
> >> +static void early_boot_test_call_rcu_sched(void)
> >> +{
> >> +     call_rcu_sched(&head, test_callback);
> >> +}
> >> +
> >> +static void early_boot_test_call_srcu(void)
> >> +{
> >> +     call_srcu(&srcu_struct, &head, test_callback);
> >
> > This looked like a great idea at first, but unfortunately call_srcu()
> > invokes queue_delayed_work(), which breaks horribly this early in boot.
> > Either this test has to be removed, or call_srcu() has to be updated
> > to handle early-boot invocation.  Given that no one is using call_srcu()
> > during early boot, it is probably best to just drop the test.
> >
> > (In case you were wondering, TEST06 dies during boot.)
> >
> > Could you please send an updated patch?
> 
> 
> Yup, will do. Please see one question below:
> 
> <...>
> >> +static int rcu_verify_early_boot_tests(void)
> >> +{
> >> +     int ret = 0;
> >> +     int early_boot_test_counter = 0;
> >> +
> >> +     if (rcu_self_test) {
> >> +             early_boot_test_counter++;
> >> +             rcu_barrier();
> >> +     }
> >> +     if (rcu_self_test_bh) {
> >> +             early_boot_test_counter++;
> >> +             rcu_barrier_bh();
> >> +     }
> >> +     if (rcu_self_test_sched) {
> >> +             early_boot_test_counter++;
> >> +             rcu_barrier_sched();
> >> +     }
> >> +     if (rcu_self_test_srcu) {
> >> +             early_boot_test_counter++;
> >> +             srcu_barrier(&srcu_struct);
> >> +     }
> >> +
> >> +     if (rcu_self_test_counter != early_boot_test_counter)
> >> +             ret = -1;
> 
> 
> So this basically does nothing when it does not match. All we see is
> the return value when we pass initcall_debug. Should I add a WARN_ON()
> or some such so that it is more explicit?

Please do!

								Thanx, Paul

> >> +
> >> +     return ret;
> >> +}
> >> +late_initcall(rcu_verify_early_boot_tests);
> >> +#else
> >> +void rcu_early_boot_tests(void) {}
> >> +#endif /* CONFIG_PROVE_RCU */
> >> --
> >> 2.1.0
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> Please read the FAQ at  http://www.tux.org/lkml/
> >
> 
> 
> 
> -- 
> Pranith
>