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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJhHMCCe43QYjGo8gA-GHhj-DMbGbNcUajCwWVDx-woLpqixhA@mail.gmail.com>
Date:	Thu, 18 Sep 2014 21:03:43 -0400
From:	Pranith Kumar <bobby.prani@...il.com>
To:	Paul McKenney <paulmck@...ux.vnet.ibm.com>
Cc:	Josh Triplett <josh@...htriplett.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	"open list:READ-COPY UPDATE..." <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 1/3] rcu: Add early boot self tests

On Thu, Sep 18, 2014 at 5:29 PM, Paul E. McKenney
<paulmck@...ux.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@...r.kernel.org
>> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/