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

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

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/