From: Lihao Liang <[email protected]>
Hi Paul,
Please find a patch series that adds rcutorture parameter usage warnings
against your git branch rcu/dev
commit e2c83bca7e80 ("EXP: rcu: Add ->boost_tasks to assertion")
Patch no.1 contains the actual changes. Patch no.2 is for testing purposes only
and is not intended to be merged. It seems that the scripts didn’t pick up the
lines in BUSTED02.boot that set the value of gp_normal and gp_exp. But when I
hard-coded gp_normal to true in rcutorture.c, it did print out
"rcu_torture_writer3: gp_sync without primitives" as expected.
It may be better to have more informative warning messages for the new if-else blocks
in patch no.1. Feel free to revise them.
Please let me know if you have any questions.
Many thanks,
Lihao.
Lihao Liang (2):
rcutorture: Add param usage warnings
rcutorture: Add tests of param usage warnings
kernel/rcu/rcutorture.c | 16 ++++++++++++++--
tools/testing/selftests/rcutorture/configs/rcu/BUSTED02 | 7 +++++++
.../selftests/rcutorture/configs/rcu/BUSTED02.boot | 1 +
tools/testing/selftests/rcutorture/configs/rcu/CFLIST | 2 ++
4 files changed, 24 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/BUSTED02
create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/BUSTED02.boot
--
2.7.4
From: Lihao Liang <[email protected]>
In rcu_torture_fakewriter(), when param gp_normal is equal to
gp_exp (even when they are false), both cur_ops->sync() and
cur_ops->exp_sync() may be invoked. This commit checks that
if any of them is NULL, it will print out a warning message.
It also emits a warning if gp_normal is true but cur_ops->sync()
is NULL.
Signed-off-by: Lihao Liang <[email protected]>
---
I think the last else-if block is redundant and I choose to include it
for completeness. In addition, it may be better to have more informative
warning messages for the new if-else blocks. Feel free to revise them.
kernel/rcu/rcutorture.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index d1b64dc..d2abebd 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -937,6 +937,16 @@ rcu_torture_writer(void *arg)
synctype[nsynctypes++] = RTWS_SYNC;
else if (gp_sync && !cur_ops->sync)
pr_alert("%s: gp_sync without primitives.\n", __func__);
+ if (gp_normal == gp_exp) {
+ if (!cur_ops->sync)
+ pr_alert("%s: gp_sync without primitives.\n", __func__);
+ if (!cur_ops->exp_sync)
+ pr_alert("%s: gp_exp without primitives.\n", __func__);
+ } else if (gp_normal && !cur_ops->sync) {
+ pr_alert("%s: gp_sync without primitives.\n", __func__);
+ } else if (gp_exp && !cur_ops->exp_sync) {
+ pr_alert("%s: gp_exp without primitives.\n", __func__);
+ }
if (WARN_ONCE(nsynctypes == 0,
"rcu_torture_writer: No update-side primitives.\n")) {
/*
--
2.7.4
From: Lihao Liang <[email protected]>
Test the first patch of this patch series
Signed-off-by: Lihao Liang <[email protected]>
---
This patch is for testing purposes only and is not intended to be merged.
It seems that the scripts didn’t pick up the lines in BUSTED02.boot that
set the value of gp_normal and gp_exp. But when I hard-coded gp_normal to
true in rcutorture.c, it did print out "rcu_torture_writer3: gp_sync
without primitives" as expected.
kernel/rcu/rcutorture.c | 14 ++++++++------
tools/testing/selftests/rcutorture/configs/rcu/BUSTED02 | 7 +++++++
.../testing/selftests/rcutorture/configs/rcu/BUSTED02.boot | 1 +
tools/testing/selftests/rcutorture/configs/rcu/CFLIST | 2 ++
4 files changed, 18 insertions(+), 6 deletions(-)
create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/BUSTED02
create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/BUSTED02.boot
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index d2abebd..233be41 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -465,10 +465,12 @@ static void rcu_busted_torture_deferred_free(struct rcu_torture *p)
rcu_torture_cb(&p->rtort_rcu);
}
+#if 0
static void synchronize_rcu_busted(void)
{
/* This is a deliberate bug for testing purposes only! */
}
+#endif
static void
call_rcu_busted(struct rcu_head *head, rcu_callback_t func)
@@ -486,8 +488,8 @@ static struct rcu_torture_ops rcu_busted_ops = {
.started = rcu_no_completed,
.completed = rcu_no_completed,
.deferred_free = rcu_busted_torture_deferred_free,
- .sync = synchronize_rcu_busted,
- .exp_sync = synchronize_rcu_busted,
+ .sync = NULL,
+ .exp_sync = NULL,
.call = call_rcu_busted,
.cb_barrier = NULL,
.fqs = NULL,
@@ -939,13 +941,13 @@ rcu_torture_writer(void *arg)
pr_alert("%s: gp_sync without primitives.\n", __func__);
if (gp_normal == gp_exp) {
if (!cur_ops->sync)
- pr_alert("%s: gp_sync without primitives.\n", __func__);
+ pr_alert("%s 1: gp_sync without primitives.\n", __func__);
if (!cur_ops->exp_sync)
- pr_alert("%s: gp_exp without primitives.\n", __func__);
+ pr_alert("%s 2: gp_exp without primitives.\n", __func__);
} else if (gp_normal && !cur_ops->sync) {
- pr_alert("%s: gp_sync without primitives.\n", __func__);
+ pr_alert("%s 3: gp_sync without primitives.\n", __func__);
} else if (gp_exp && !cur_ops->exp_sync) {
- pr_alert("%s: gp_exp without primitives.\n", __func__);
+ pr_alert("%s 4: gp_exp without primitives.\n", __func__);
}
if (WARN_ONCE(nsynctypes == 0,
"rcu_torture_writer: No update-side primitives.\n")) {
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/BUSTED02 b/tools/testing/selftests/rcutorture/configs/rcu/BUSTED02
new file mode 100644
index 0000000..48d8a24
--- /dev/null
+++ b/tools/testing/selftests/rcutorture/configs/rcu/BUSTED02
@@ -0,0 +1,7 @@
+CONFIG_RCU_TRACE=n
+CONFIG_SMP=y
+CONFIG_NR_CPUS=4
+CONFIG_HOTPLUG_CPU=y
+CONFIG_PREEMPT_NONE=n
+CONFIG_PREEMPT_VOLUNTARY=n
+CONFIG_PREEMPT=y
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/BUSTED02.boot b/tools/testing/selftests/rcutorture/configs/rcu/BUSTED02.boot
new file mode 100644
index 0000000..40cf978
--- /dev/null
+++ b/tools/testing/selftests/rcutorture/configs/rcu/BUSTED02.boot
@@ -0,0 +1 @@
+rcutorture.torture_type=busted rcutorture.gp_normal=true rcutorture.gp_sync=false
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/CFLIST b/tools/testing/selftests/rcutorture/configs/rcu/CFLIST
index 6a0b9f6..e91f8b5 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/CFLIST
+++ b/tools/testing/selftests/rcutorture/configs/rcu/CFLIST
@@ -16,3 +16,5 @@ TINY02
TASKS01
TASKS02
TASKS03
+BUSTED
+BUSTED02
--
2.7.4
On Wed, Dec 06, 2017 at 06:51:07PM +0800, [email protected] wrote:
> From: Lihao Liang <[email protected]>
>
> In rcu_torture_fakewriter(), when param gp_normal is equal to
> gp_exp (even when they are false), both cur_ops->sync() and
> cur_ops->exp_sync() may be invoked. This commit checks that
> if any of them is NULL, it will print out a warning message.
> It also emits a warning if gp_normal is true but cur_ops->sync()
> is NULL.
>
> Signed-off-by: Lihao Liang <[email protected]>
Queued for review and testing, thank you! Passes light rcutorture
testing.
Thanx, Paul
> ---
>
> I think the last else-if block is redundant and I choose to include it
> for completeness. In addition, it may be better to have more informative
> warning messages for the new if-else blocks. Feel free to revise them.
>
> kernel/rcu/rcutorture.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> index d1b64dc..d2abebd 100644
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -937,6 +937,16 @@ rcu_torture_writer(void *arg)
> synctype[nsynctypes++] = RTWS_SYNC;
> else if (gp_sync && !cur_ops->sync)
> pr_alert("%s: gp_sync without primitives.\n", __func__);
> + if (gp_normal == gp_exp) {
> + if (!cur_ops->sync)
> + pr_alert("%s: gp_sync without primitives.\n", __func__);
> + if (!cur_ops->exp_sync)
> + pr_alert("%s: gp_exp without primitives.\n", __func__);
> + } else if (gp_normal && !cur_ops->sync) {
> + pr_alert("%s: gp_sync without primitives.\n", __func__);
> + } else if (gp_exp && !cur_ops->exp_sync) {
> + pr_alert("%s: gp_exp without primitives.\n", __func__);
> + }
> if (WARN_ONCE(nsynctypes == 0,
> "rcu_torture_writer: No update-side primitives.\n")) {
> /*
> --
> 2.7.4
>