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@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
Pranith
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/