From: Uladzislau Rezki <urezki@gmail.com>
To: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Uladzislau Rezki <urezki@gmail.com>,
LKML <linux-kernel@vger.kernel.org>, RCU <rcu@vger.kernel.org>,
Michael Ellerman <mpe@ellerman.id.au>,
Andrew Morton <akpm@linux-foundation.org>,
Daniel Axtens <dja@axtens.net>,
Frederic Weisbecker <frederic@kernel.org>,
Neeraj Upadhyay <neeraju@codeaurora.org>,
Joel Fernandes <joel@joelfernandes.org>,
Peter Zijlstra <peterz@infradead.org>,
Michal Hocko <mhocko@suse.com>,
Thomas Gleixner <tglx@linutronix.de>,
"Theodore Y . Ts'o" <tytso@mit.edu>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
Oleksiy Avramchenko <oleksiy.avramchenko@sonymobile.com>
Subject: Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests
Date: Mon, 21 Dec 2020 16:38:09 +0100 [thread overview]
Message-ID: <20201221153809.GA24756@pc638.lan> (raw)
In-Reply-To: <20201216232955.GO2657@paulmck-ThinkPad-P72>
On Wed, Dec 16, 2020 at 03:29:55PM -0800, Paul E. McKenney wrote:
> On Wed, Dec 16, 2020 at 04:49:59PM +0100, Uladzislau Rezki wrote:
> > > Add self tests for checking of RCU-tasks API functionality.
> > > It covers:
> > > - wait API functions;
> > > - invoking/completion call_rcu_tasks*().
> > >
> > > Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.
> > >
> > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > > ---
> > > kernel/rcu/tasks.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 44 insertions(+)
> > >
> > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > index 67a162949763..9407772780c1 100644
> > > --- a/kernel/rcu/tasks.h
> > > +++ b/kernel/rcu/tasks.h
> > > @@ -1225,6 +1225,16 @@ void show_rcu_tasks_gp_kthreads(void)
> > > }
> > > #endif /* #ifndef CONFIG_TINY_RCU */
> > >
> > > +static struct rcu_head rhp;
> > > +static int rcu_execurted_test_counter;
> > > +static int rcu_run_test_counter;
> > > +
> > > +static void test_rcu_tasks_callback(struct rcu_head *r)
> > > +{
> > > + pr_info("RCU-tasks test callback executed %d\n",
> > > + ++rcu_execurted_test_counter);
> > > +}
> > > +
> > > void __init rcu_init_tasks_generic(void)
> > > {
> > > #ifdef CONFIG_TASKS_RCU
> > > @@ -1238,7 +1248,41 @@ void __init rcu_init_tasks_generic(void)
> > > #ifdef CONFIG_TASKS_TRACE_RCU
> > > rcu_spawn_tasks_trace_kthread();
> > > #endif
> > > +
> > > + if (IS_ENABLED(CONFIG_PROVE_RCU)) {
> > > + pr_info("Running RCU-tasks wait API self tests\n");
> > > +#ifdef CONFIG_TASKS_RCU
> > > + rcu_run_test_counter++;
> > > + call_rcu_tasks(&rhp, test_rcu_tasks_callback);
> > > + synchronize_rcu_tasks();
> > > +#endif
> > > +
> > > +#ifdef CONFIG_TASKS_RUDE_RCU
> > > + rcu_run_test_counter++;
> > > + call_rcu_tasks_trace(&rhp, test_rcu_tasks_callback);
> > > + synchronize_rcu_tasks_rude();
> > > +#endif
> > > +
> > > +#ifdef CONFIG_TASKS_TRACE_RCU
> > > + rcu_run_test_counter++;
> > > + call_rcu_tasks_trace(&rhp, test_rcu_tasks_callback);
> > > + synchronize_rcu_tasks_trace();
> > > +#endif
> > > + }
> > > +}
> > > +
> > > +static int rcu_tasks_verify_self_tests(void)
> > > +{
> > > + int ret = 0;
> > > +
> > > + if (rcu_run_test_counter != rcu_execurted_test_counter) {
> > > + WARN_ON(1);
> > > + ret = -1;
> > > + }
> > > +
> > > + return ret;
> > > }
> > > +late_initcall(rcu_tasks_verify_self_tests);
> > >
> > > #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> > > static inline void rcu_tasks_bootup_oddness(void) {}
> > Please find a v2 of the patch that is in question. First version
> > uses the same rhp for all RCU flavors what is wrong. Initially
> > i had three different one per one flavor. But for some reason
> > end up with only one.
> >
> >
> > >From e7c6096af5a7916f29c0b4b05e1644b3b3a6c589 Mon Sep 17 00:00:00 2001
> > From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
> > Date: Wed, 9 Dec 2020 21:27:32 +0100
> > Subject: [PATCH v2 1/1] rcu-tasks: Add RCU-tasks self tests
> >
> > This commit adds self tests for early-boot use of RCU-tasks grace periods.
> > It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
> > call_rcu_tasks()) grace-period APIs.
> >
> > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> >
> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
>
> Much improved, thank you! A few more comments below.
>
> > ---
> > kernel/rcu/tasks.h | 69 ++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 69 insertions(+)
> >
> > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > index 36607551f966..7478d912734a 100644
> > --- a/kernel/rcu/tasks.h
> > +++ b/kernel/rcu/tasks.h
> > @@ -1224,6 +1224,35 @@ void show_rcu_tasks_gp_kthreads(void)
> > }
> > #endif /* #ifndef CONFIG_TINY_RCU */
> >
> > +struct test_desc {
>
> Please use something like "struct rcu_tasks_test_desc" to help the poor
> people who might need to grep for this. Feel free to shorten it, but
> please make it descriptive and thus more likely to stay unique.
>
> > + struct rcu_head rh;
> > + const char *name;
> > + bool run;
>
> If you make this "bool notrun" you don't need to initialize.
>
> > +};
> > +
> > +static struct test_desc tests[] = {
> > + { .name = "call_rcu_tasks()" },
> > + { .name = "call_rcu_rude()" },
> > + { .name = "call_rcu_trace()" },
> > +};
> > +
> > +static int rcu_executed_test_counter;
> > +
> > +static void test_rcu_tasks_callback(struct rcu_head *rhp)
> > +{
>
> struct rcu_tasks_test_desc *rttdp;
>
> > + int i;
> > +
> > + pr_info("RCU-tasks test callback executed %d\n",
> > + ++rcu_executed_test_counter);
>
> rttdp = container_of(rhp, rh, struct rcu_tasks_test_desc);
> rttdp->notrun = true;
>
> Or I suppose:
>
> container_of(rhp, rh, struct rcu_tasks_test_desc)->notrun = true;
>
> Then the loop below can go away.
>
> > +
> > + for (i = 0; i < ARRAY_SIZE(tests); i++) {
> > + if (rhp == &tests[i].rh) {
> > + tests[i].run = false;
> > + break;
> > + }
> > + }
> > +}
> > +
> > void __init rcu_init_tasks_generic(void)
> > {
> > #ifdef CONFIG_TASKS_RCU
> > @@ -1237,7 +1266,47 @@ void __init rcu_init_tasks_generic(void)
> > #ifdef CONFIG_TASKS_TRACE_RCU
> > rcu_spawn_tasks_trace_kthread();
> > #endif
> > +
> > + // Run the self-tests.
> > + if (IS_ENABLED(CONFIG_PROVE_RCU)) {
> > + pr_info("Running RCU-tasks wait API self tests\n");
> > +#ifdef CONFIG_TASKS_RCU
> > + tests[0].run = true;
>
> The s/run/notrun/ allows the three initializations of .run to go away.
>
> > + call_rcu_tasks(&tests[0].rh, test_rcu_tasks_callback);
> > + synchronize_rcu_tasks();
>
> Why not reverse the order of these two statements? That would test
> call_rcu_tasks*()'s ability to do a grace period on their own, without
> help from the corresponding synchronize_rcu_tasks*().
>
> > +#endif
> > +
> > +#ifdef CONFIG_TASKS_RUDE_RCU
> > + tests[1].run = true;
> > + call_rcu_tasks_rude(&tests[1].rh, test_rcu_tasks_callback);
> > + synchronize_rcu_tasks_rude();
> > +#endif
> > +
> > +#ifdef CONFIG_TASKS_TRACE_RCU
> > + tests[2].run = true;
> > + call_rcu_tasks_trace(&tests[2].rh, test_rcu_tasks_callback);
> > + synchronize_rcu_tasks_trace();
> > +#endif
> > + }
> > +}
> > +
> > +static int rcu_tasks_verify_self_tests(void)
> > +{
> > + int ret, i;
>
> Why not initialize "ret" in the declaration?
>
> > +
> > + for (i = 0, ret = 0; i < ARRAY_SIZE(tests); i++) {
> > + if (tests[i].run) { // still hanging.
> > + pr_err("%s has been failed.\n", tests[i].name);
> > + ret = -1;
> > + }
> > + }
> > +
> > + if (ret)
> > + WARN_ON(1);
> > +
> > + return ret;
> > }
> > +late_initcall(rcu_tasks_verify_self_tests);
> >
> > #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> > static inline void rcu_tasks_bootup_oddness(void) {}
> > --
> > 2.20.1
>
> Again, much improved!
>
See below the v3 version. I hope i fixed all comments :)
From 06f7adfd84cbb1994d0e2693ee9dcdfd272a9bd0 Mon Sep 17 00:00:00 2001
From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
Date: Wed, 9 Dec 2020 21:27:32 +0100
Subject: [PATCH v3 1/1] rcu-tasks: Add RCU-tasks self tests
This commit adds self tests for early-boot use of RCU-tasks grace periods.
It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
call_rcu_tasks()) grace-period APIs.
Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
kernel/rcu/tasks.h | 75 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index 36607551f966..670212ed9648 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -1224,6 +1224,43 @@ void show_rcu_tasks_gp_kthreads(void)
}
#endif /* #ifndef CONFIG_TINY_RCU */
+struct rcu_tasks_test_desc {
+ struct rcu_head rh;
+ const char *name;
+ bool notrun;
+};
+
+static struct rcu_tasks_test_desc tests[] = {
+ {
+ .name = "call_rcu_tasks()",
+ /* If not defined, the test is skipped. */
+ .notrun = !IS_ENABLED(CONFIG_TASKS_RCU),
+ },
+ {
+ .name = "call_rcu_tasks_rude()",
+ /* If not defined, the test is skipped. */
+ .notrun = !IS_ENABLED(CONFIG_TASKS_RUDE_RCU),
+ },
+ {
+ .name = "call_rcu_tasks_trace()",
+ /* If not defined, the test is skipped. */
+ .notrun = !IS_ENABLED(CONFIG_TASKS_TRACE_RCU)
+ }
+};
+
+static int rcu_executed_test_counter;
+
+static void test_rcu_tasks_callback(struct rcu_head *rhp)
+{
+ struct rcu_tasks_test_desc *rttd =
+ container_of(rhp, struct rcu_tasks_test_desc, rh);
+
+ pr_info("RCU-tasks test callback executed %d\n",
+ ++rcu_executed_test_counter);
+
+ rttd->notrun = true;
+}
+
void __init rcu_init_tasks_generic(void)
{
#ifdef CONFIG_TASKS_RCU
@@ -1237,7 +1274,45 @@ void __init rcu_init_tasks_generic(void)
#ifdef CONFIG_TASKS_TRACE_RCU
rcu_spawn_tasks_trace_kthread();
#endif
+
+ // Run the self-tests.
+ if (IS_ENABLED(CONFIG_PROVE_RCU)) {
+ pr_info("Running RCU-tasks wait API self tests\n");
+#ifdef CONFIG_TASKS_RCU
+ synchronize_rcu_tasks();
+ call_rcu_tasks(&tests[0].rh, test_rcu_tasks_callback);
+#endif
+
+#ifdef CONFIG_TASKS_RUDE_RCU
+ synchronize_rcu_tasks_rude();
+ call_rcu_tasks_rude(&tests[1].rh, test_rcu_tasks_callback);
+#endif
+
+#ifdef CONFIG_TASKS_TRACE_RCU
+ synchronize_rcu_tasks_trace();
+ call_rcu_tasks_trace(&tests[2].rh, test_rcu_tasks_callback);
+#endif
+ }
+}
+
+static int rcu_tasks_verify_self_tests(void)
+{
+ int ret = 0;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ if (!tests[i].notrun) { // still hanging.
+ pr_err("%s has been failed.\n", tests[i].name);
+ ret = -1;
+ }
+ }
+
+ if (ret)
+ WARN_ON(1);
+
+ return ret;
}
+late_initcall(rcu_tasks_verify_self_tests);
#else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
static inline void rcu_tasks_bootup_oddness(void) {}
--
2.20.1
--
Vlad Rezki
next prev parent reply other threads:[~2020-12-21 18:50 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-09 20:27 [PATCH 1/2] rcu-tasks: move RCU-tasks initialization out of core_initcall() Uladzislau Rezki (Sony)
2020-12-09 20:27 ` [PATCH 2/2] rcu-tasks: add RCU-tasks self tests Uladzislau Rezki (Sony)
2020-12-16 15:49 ` Uladzislau Rezki
2020-12-16 23:29 ` Paul E. McKenney
2020-12-21 15:38 ` Uladzislau Rezki [this message]
2020-12-21 17:18 ` Paul E. McKenney
2020-12-21 18:45 ` Uladzislau Rezki
2020-12-21 19:29 ` Paul E. McKenney
2020-12-21 19:48 ` Uladzislau Rezki
2020-12-21 20:45 ` Paul E. McKenney
2020-12-21 21:28 ` Uladzislau Rezki
2021-02-12 19:20 ` Sebastian Andrzej Siewior
2021-02-12 21:12 ` Uladzislau Rezki
2021-02-12 23:48 ` Paul E. McKenney
2021-02-13 0:37 ` Paul E. McKenney
2021-02-13 0:43 ` Paul E. McKenney
2021-02-13 11:30 ` Uladzislau Rezki
2021-02-13 16:45 ` Paul E. McKenney
2021-02-13 20:00 ` Uladzislau Rezki
2021-02-15 11:28 ` Sebastian Andrzej Siewior
2021-02-16 17:30 ` Paul E. McKenney
2021-02-17 14:47 ` Masami Hiramatsu
2021-02-17 18:17 ` Paul E. McKenney
2021-02-18 5:03 ` Masami Hiramatsu
2021-02-18 8:36 ` Uladzislau Rezki
2021-02-18 14:29 ` Masami Hiramatsu
2020-12-09 20:37 ` [PATCH 1/2] rcu-tasks: move RCU-tasks initialization out of core_initcall() Uladzislau Rezki
2020-12-10 13:39 ` Daniel Axtens
2020-12-10 17:32 ` Paul E. McKenney
2020-12-10 18:17 ` Uladzislau Rezki
2020-12-10 3:26 ` Paul E. McKenney
2020-12-10 13:04 ` Uladzislau Rezki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201221153809.GA24756@pc638.lan \
--to=urezki@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=bigeasy@linutronix.de \
--cc=dja@axtens.net \
--cc=frederic@kernel.org \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhocko@suse.com \
--cc=mpe@ellerman.id.au \
--cc=neeraju@codeaurora.org \
--cc=oleksiy.avramchenko@sonymobile.com \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=rcu@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.