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] [day] [month] [year] [list]

Message-ID: <20140919043206.GW4723@linux.vnet.ibm.com>
Date:	Thu, 18 Sep 2014 21:32:06 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Pranith Kumar <bobby.prani@...il.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 09:03:43PM -0400, Pranith Kumar wrote:
> 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?

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@...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/