rcutorture updates for 3.12

From: "Paul E. McKenney" <[email protected]>

There are getting to be too many module parameters to permit the current semi-random order, so this patch orders them.

Signed-off-by Paul E. McKenney <[email protected]>

Quoted 6 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

There are getting to be too many module parameters to permit the current semi-random order, so this patch orders them.

Signed-off-by Paul E. McKenney <[email protected]>

As long as you're reordering them anyway, how about grouping each of the variables together with the corresponding module parameter macros, and then dropping the comments that duplicate the module parameter documentation?

With that change:

Reviewed-by Josh Triplett <[email protected]>

From: "Paul E. McKenney" <[email protected]>

Although rcutorture counts CPU-hotplug online failures, it does not explicitly record which CPUs were having trouble coming online. This commit therefore emits a console message when online failure occurs.

Signed-off-by Paul E. McKenney <[email protected]>

 kernel/rcutorture.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

kernel/rcutorture.c +0 -0

index 7d42d13..15bec39 100644

Quoted 7 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

Although rcutorture counts CPU-hotplug online failures, it does not explicitly record which CPUs were having trouble coming online. This commit therefore emits a console message when online failure occurs.

Signed-off-by Paul E. McKenney <[email protected]>

One completely optional note below; with or without that change,

Reviewed-by Josh Triplett <[email protected]>

Quoted 3 lines

 kernel/rcutorture.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

kernel/rcutorture.c rcu_torture_onoff(void *arg) · line 1441

Quoted 11 lines

kernel/rcutorture.c +2 -1

index 7d42d13..15bec39 100644
@@ -1437,7 +1437,13 @@ rcu_torture_onoff(void *arg)
 					 torture_type, cpu);
 			starttime = jiffies;
 			n_online_attempts++;
-			if (cpu_up(cpu) == 0) {
+			ret = cpu_up(cpu);
+			if (ret != 0) {

Or just "if (ret) {"

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

kernel/rcutorture.c +10 -0

@@ -2163,6 +2178,28 @@ rcu_torture_init(void)
 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug) {
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+		struct rcu_head rh1;
+		struct rcu_head rh2;
+
+		init_rcu_head_on_stack(&rh1);
+		init_rcu_head_on_stack(&rh2);
+		pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
+		local_irq_disable(); /* Make it hard to finish grace period. */
+		call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */

Quoted 6 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Quoted 8 lines

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

Two comments below; with those fixed,

Reviewed-by Josh Triplett <[email protected]>

kernel/rcutorture.c module_param(fqs_stutter, int, 0444); · line 105

Quoted 7 lines

---

kernel/rcutorture.c +2 -0

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");

modules-next has a change to ignore and warn about
unknown module parameters. Thus, I'd suggest wrapping the ifdef around this module parameter, so it doesn't exist at all without CONFIG_DEBUG_OBJECTS_RCU_HEAD.

Alternatively, consider providing the test unconditionally, and just printing a big warning message saying that it's going to cause corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.

Quoted 20 lines

kernel/rcutorture.c +16 -0

@@ -2163,6 +2178,28 @@ rcu_torture_init(void)
 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug) {
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+		struct rcu_head rh1;
+		struct rcu_head rh2;
+
+		init_rcu_head_on_stack(&rh1);
+		init_rcu_head_on_stack(&rh2);
+		pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
+		local_irq_disable(); /* Make it hard to finish grace period. */
+		call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
+		call_rcu(&rh2, rcu_torture_err_cb);
+		call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
+		local_irq_enable();
+		rcu_barrier();
+		pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
+		destroy_rcu_head_on_stack(&rh1);

kernel/rcutorture.c rcu_torture_init(void) · line 2200

Quoted 4 lines

kernel/rcutorture.c +4 -0

+		destroy_rcu_head_on_stack(&rh2);
+#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+		pr_alert("rcutorture: !%s, not testing duplicate call_rcu()\n",
+			 "CONFIG_DEBUG_OBJECTS_RCU_HEAD");

Why put this parameter in a separate string? That makes it harder to grep for the full error message. (That's assuming you keep the error message, given the comment above.)

Quoted 32 lines · Josh Triplett

Quoted 6 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Quoted 8 lines

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

Two comments below; with those fixed,

Reviewed-by Josh Triplett <[email protected]>

kernel/rcutorture.c module_param(fqs_stutter, int, 0444); · line 105

Quoted 7 lines

---

kernel/rcutorture.c +2 -0

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");

modules-next has a change to ignore and warn about
unknown module parameters. Thus, I'd suggest wrapping the ifdef around this module parameter, so it doesn't exist at all without CONFIG_DEBUG_OBJECTS_RCU_HEAD.

Quoted 3 lines

Alternatively, consider providing the test unconditionally, and just printing a big warning message saying that it's going to cause corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.

I currently do something like the above. The module parameter is defined unconditionally, but the actual tests are under #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD. If you specify object_debug for a !CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens, and the test is omitted, thus avoiding the list corruption.

Seem reasonable?

Quoted 37 lines · Paul E. McKenney

Quoted 32 lines · Josh Triplett

Quoted 6 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Quoted 8 lines

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

Two comments below; with those fixed,

Reviewed-by Josh Triplett <[email protected]>

kernel/rcutorture.c module_param(fqs_stutter, int, 0444); · line 105

Quoted 7 lines

---

kernel/rcutorture.c +2 -0

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");

modules-next has a change to ignore and warn about
unknown module parameters. Thus, I'd suggest wrapping the ifdef around this module parameter, so it doesn't exist at all without CONFIG_DEBUG_OBJECTS_RCU_HEAD.

Quoted 3 lines

Alternatively, consider providing the test unconditionally, and just printing a big warning message saying that it's going to cause corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.

Quoted 7 lines

I currently do something like the above. The module parameter is defined unconditionally, but the actual tests are under #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD. If you specify object_debug for a !CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens, and the test is omitted, thus avoiding the list corruption.

Seem reasonable?

That's exactly the bit I was commenting on. I'm saying that you should either make the test unconditional (perhaps with a warning saying it's about to cause list corruption), or you should compile out the module parameter as well and then you don't need the pr_alert (since current kernels will emit a warning when you pass a non-existent module parameter).

Personally, I'd go with the latter.

Quoted 46 lines · Josh Triplett

Quoted 37 lines · Paul E. McKenney

Quoted 32 lines · Josh Triplett

Quoted 6 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Quoted 8 lines

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

Two comments below; with those fixed,

Reviewed-by Josh Triplett <[email protected]>

kernel/rcutorture.c module_param(fqs_stutter, int, 0444); · line 105

Quoted 7 lines

---

kernel/rcutorture.c +2 -0

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");

modules-next has a change to ignore and warn about
unknown module parameters. Thus, I'd suggest wrapping the ifdef around this module parameter, so it doesn't exist at all without CONFIG_DEBUG_OBJECTS_RCU_HEAD.

Quoted 3 lines

Alternatively, consider providing the test unconditionally, and just printing a big warning message saying that it's going to cause corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.

Quoted 7 lines

I currently do something like the above. The module parameter is defined unconditionally, but the actual tests are under #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD. If you specify object_debug for a !CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens, and the test is omitted, thus avoiding the list corruption.

Seem reasonable?

Quoted 8 lines

That's exactly the bit I was commenting on. I'm saying that you should either make the test unconditional (perhaps with a warning saying it's about to cause list corruption), or you should compile out the module parameter as well and then you don't need the pr_alert (since current kernels will emit a warning when you pass a non-existent module parameter).

Personally, I'd go with the latter.

Ah, the problem is the ugly ifdef in the middle of a function. Yeah, that is a problem in need of fixing. No idea what I was thinking...

How about if I pull that block of code out into its own function, and #ifdef the function body. For example, something like that shown below.

static void rcu_test_debug_objects(void)
{

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
	struct rcu_head rh1;
	struct rcu_head rh2;

	init_rcu_head_on_stack(&rh1);
	init_rcu_head_on_stack(&rh2);
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
	local_irq_disable(); /* Make it hard to finish grace period. */
	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
	call_rcu(&rh2, rcu_torture_err_cb);
	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
	local_irq_enable();
	rcu_barrier();
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
	destroy_rcu_head_on_stack(&rh1);
	destroy_rcu_head_on_stack(&rh2);
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");

#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */ }

Quoted 61 lines · Paul E. McKenney

Quoted 46 lines · Josh Triplett

Quoted 37 lines · Paul E. McKenney

Quoted 32 lines · Josh Triplett

Quoted 6 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Quoted 8 lines

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

Two comments below; with those fixed,

Reviewed-by Josh Triplett <[email protected]>

kernel/rcutorture.c module_param(fqs_stutter, int, 0444); · line 105

Quoted 7 lines

---

kernel/rcutorture.c +2 -0

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");

modules-next has a change to ignore and warn about
unknown module parameters. Thus, I'd suggest wrapping the ifdef around this module parameter, so it doesn't exist at all without CONFIG_DEBUG_OBJECTS_RCU_HEAD.

Quoted 3 lines

Alternatively, consider providing the test unconditionally, and just printing a big warning message saying that it's going to cause corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.

Quoted 7 lines

I currently do something like the above. The module parameter is defined unconditionally, but the actual tests are under #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD. If you specify object_debug for a !CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens, and the test is omitted, thus avoiding the list corruption.

Seem reasonable?

Quoted 8 lines

That's exactly the bit I was commenting on. I'm saying that you should either make the test unconditional (perhaps with a warning saying it's about to cause list corruption), or you should compile out the module parameter as well and then you don't need the pr_alert (since current kernels will emit a warning when you pass a non-existent module parameter).

Personally, I'd go with the latter.

Ah, the problem is the ugly ifdef in the middle of a function. Yeah, that is a problem in need of fixing. No idea what I was thinking...

How about if I pull that block of code out into its own function, and #ifdef the function body. For example, something like that shown below.

[...]

Quoted 20 lines

static void rcu_test_debug_objects(void)
{

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
	struct rcu_head rh1;
	struct rcu_head rh2;

	init_rcu_head_on_stack(&rh1);
	init_rcu_head_on_stack(&rh2);
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
	local_irq_disable(); /* Make it hard to finish grace period. */
	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
	call_rcu(&rh2, rcu_torture_err_cb);
	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
	local_irq_enable();
	rcu_barrier();
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
	destroy_rcu_head_on_stack(&rh1);
	destroy_rcu_head_on_stack(&rh2);
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");

Quoted 2 lines

#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */ }

That's a major improvement, but I'd still suggest a couple more changes:
move the if for the config option into that function inside the ifdef, wrap the config parameter itself in an ifdef so it doesn't exist without CONFIG_DEBUG_OBJECTS_RCU_HEAD, and drop the #else branch since the kernel will already emit a warning if you use the module parameter with !CONFIG_DEBUG_OBJECTS_RCU_HEAD.

Quoted 86 lines · Josh Triplett

Quoted 61 lines · Paul E. McKenney

Quoted 46 lines · Josh Triplett

Quoted 37 lines · Paul E. McKenney

Quoted 32 lines · Josh Triplett

Quoted 6 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Quoted 8 lines

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

Two comments below; with those fixed,

Reviewed-by Josh Triplett <[email protected]>

Review module_param(fqs_stutter, int, 0444); · line 105

Quoted 7 lines

---

(quoted diff) +2 -0

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");

modules-next has a change to ignore and warn about
unknown module parameters. Thus, I'd suggest wrapping the ifdef around this module parameter, so it doesn't exist at all without CONFIG_DEBUG_OBJECTS_RCU_HEAD.

Quoted 3 lines

Alternatively, consider providing the test unconditionally, and just printing a big warning message saying that it's going to cause corruption in the !CONFIG_DEBUG_OBJECTS_RCU_HEAD case.

Quoted 7 lines

I currently do something like the above. The module parameter is defined unconditionally, but the actual tests are under #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD. If you specify object_debug for a !CONFIG_DEBUG_OBJECTS_RCU_HEAD kernel, the pr_alert() below happens, and the test is omitted, thus avoiding the list corruption.

Seem reasonable?

Quoted 8 lines

That's exactly the bit I was commenting on. I'm saying that you should either make the test unconditional (perhaps with a warning saying it's about to cause list corruption), or you should compile out the module parameter as well and then you don't need the pr_alert (since current kernels will emit a warning when you pass a non-existent module parameter).

Personally, I'd go with the latter.

Ah, the problem is the ugly ifdef in the middle of a function. Yeah, that is a problem in need of fixing. No idea what I was thinking...

How about if I pull that block of code out into its own function, and #ifdef the function body. For example, something like that shown below.

[...]

Quoted 20 lines

static void rcu_test_debug_objects(void)
{

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
	struct rcu_head rh1;
	struct rcu_head rh2;

	init_rcu_head_on_stack(&rh1);
	init_rcu_head_on_stack(&rh2);
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
	local_irq_disable(); /* Make it hard to finish grace period. */
	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
	call_rcu(&rh2, rcu_torture_err_cb);
	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
	local_irq_enable();
	rcu_barrier();
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
	destroy_rcu_head_on_stack(&rh1);
	destroy_rcu_head_on_stack(&rh2);
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");

Quoted 2 lines

#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */ }

Quoted 6 lines

That's a major improvement, but I'd still suggest a couple more changes:
move the if for the config option into that function inside the ifdef, wrap the config parameter itself in an ifdef so it doesn't exist without CONFIG_DEBUG_OBJECTS_RCU_HEAD, and drop the #else branch since the kernel will already emit a warning if you use the module parameter with !CONFIG_DEBUG_OBJECTS_RCU_HEAD.

I really feel the need to distinguish between "that is never a valid parameter to rcutorture" (given by the module-parameter parser) and "that parameter is valid, but cannot be used in this case, and here are the consequences", so I will keep the pr_alert(). But definitely fix the mid-function #ifdef!

Hello!

This series provides some rcutorture updates:

1. Add a debug_object option to rcutorture to allow testing
of RCU's debug-objects facilities.

2. Increase rcutorture test coverage by concurrently testing
synchronous, asynchronous, and expedited grace periods.

3. Sort the extensive list of rcutorture module parameters.

4. Remove the unused "oldbatch" variable.

5. Emit CPU-online failure message to the console.

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

[ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]

---
 kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

kernel/rcutorture.c +45 -0

index 3d936f0f..f5cf2bb 100644
@@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
 static int fqs_holdoff;		/* Hold time within burst (us). */
 static int fqs_stutter = 3;	/* Wait time between bursts (s). */
 static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
+static int object_debug;	/* Test object-debug double call_rcu()?. */
 static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
 static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
 static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
 module_param(onoff_interval, int, 0444);
 MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
 module_param(onoff_holdoff, int, 0444);

@@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
 		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
 }
 
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+static void rcu_torture_leak_cb(struct rcu_head *rhp)
+{
+}
+
+static void rcu_torture_err_cb(struct rcu_head *rhp)
+{
+	/* This -might- happen due to race conditions, but is unlikely. */
+	pr_alert("rcutorture: duplicated callback was invoked.\n");
+}
+#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+
+/*
+ * Verify that double-free causes debug-objects to complain, but only
+ * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
+ * cannot be carried out.
+ */
+static void rcu_test_debug_objects(void)
+{
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+	struct rcu_head rh1;
+	struct rcu_head rh2;
+
+	init_rcu_head_on_stack(&rh1);
+	init_rcu_head_on_stack(&rh2);
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
+	local_irq_disable(); /* Make it hard to finish grace period. */
+	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
+	call_rcu(&rh2, rcu_torture_err_cb);
+	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
+	local_irq_enable();
+	rcu_barrier();
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
+	destroy_rcu_head_on_stack(&rh1);
+	destroy_rcu_head_on_stack(&rh2);
+#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
+#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+}
+
 static int __init
 rcu_torture_init(void)
 {

@@ -2163,6 +2206,8 @@ rcu_torture_init(void)
 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug)
+		rcu_test_debug_objects();
 	rcutorture_record_test_transition();
 	mutex_unlock(&fullstop_mutex);
 	return 0;

Signature · 1 line

1.8.1.5

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

[ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]

---
 kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

kernel/rcutorture.c +20 -0

index 3d936f0f..f5cf2bb 100644
@@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
 static int fqs_holdoff;		/* Hold time within burst (us). */
 static int fqs_stutter = 3;	/* Wait time between bursts (s). */
 static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
+static int object_debug;	/* Test object-debug double call_rcu()?. */
 static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
 static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
 static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
 module_param(onoff_interval, int, 0444);
 MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
 module_param(onoff_holdoff, int, 0444);

@@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
 		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
 }
 
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+static void rcu_torture_leak_cb(struct rcu_head *rhp)
+{
+}
+
+static void rcu_torture_err_cb(struct rcu_head *rhp)
+{
+	/* This -might- happen due to race conditions, but is unlikely. */
+	pr_alert("rcutorture: duplicated callback was invoked.\n");
+}
+#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+
+/*
+ * Verify that double-free causes debug-objects to complain, but only
+ * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
+ * cannot be carried out.
+ */

kernel/rcutorture.c +15 -0

+	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */
+	call_rcu(&rh2, rcu_torture_err_cb);
+	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
+	local_irq_enable();
+	rcu_barrier();
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
+	destroy_rcu_head_on_stack(&rh1);
+	destroy_rcu_head_on_stack(&rh2);
+#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
+#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+}
+
 static int __init
 rcu_torture_init(void)
 {

@@ -2163,6 +2206,8 @@ rcu_torture_init(void)
 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug)
+		rcu_test_debug_objects();
 	rcutorture_record_test_transition();
 	mutex_unlock(&fullstop_mutex);
 	return 0;

Quoted 65 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

[ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]

---
 kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

kernel/rcutorture.c +24 -0

index 3d936f0f..f5cf2bb 100644
@@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
 static int fqs_holdoff;		/* Hold time within burst (us). */
 static int fqs_stutter = 3;	/* Wait time between bursts (s). */
 static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
+static int object_debug;	/* Test object-debug double call_rcu()?. */
 static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
 static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
 static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
 module_param(onoff_interval, int, 0444);
 MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
 module_param(onoff_holdoff, int, 0444);

@@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
 		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
 }
 
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+static void rcu_torture_leak_cb(struct rcu_head *rhp)
+{
+}
+
+static void rcu_torture_err_cb(struct rcu_head *rhp)
+{
+	/* This -might- happen due to race conditions, but is unlikely. */
+	pr_alert("rcutorture: duplicated callback was invoked.\n");
+}
+#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+
+/*
+ * Verify that double-free causes debug-objects to complain, but only
+ * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
+ * cannot be carried out.
+ */
+static void rcu_test_debug_objects(void)
+{
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+	struct rcu_head rh1;

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1966

Quoted 6 lines

kernel/rcutorture.c +6 -0

+	struct rcu_head rh2;
+
+	init_rcu_head_on_stack(&rh1);
+	init_rcu_head_on_stack(&rh2);
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
+	local_irq_disable(); /* Make it hard to finish grace period. */

you can use rcu_read_lock() directly.

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1967

Quoted 1 line

kernel/rcutorture.c +1 -0

+	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1968

Quoted 1 line

kernel/rcutorture.c +1 -0

+	call_rcu(&rh2, rcu_torture_err_cb);

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1978

Quoted 10 lines

kernel/rcutorture.c +10 -0

+	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
+	local_irq_enable();
+	rcu_barrier();
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
+	destroy_rcu_head_on_stack(&rh1);
+	destroy_rcu_head_on_stack(&rh2);
+#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
+#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+}

Quoted 13 lines

kernel/rcutorture.c +3 -0

+
 static int __init
 rcu_torture_init(void)
 {

@@ -2163,6 +2206,8 @@ rcu_torture_init(void)
 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug)
+		rcu_test_debug_objects();
 	rcutorture_record_test_transition();
 	mutex_unlock(&fullstop_mutex);
 	return 0;

Quoted 66 lines · Lai Jiangshan

Quoted 65 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

[ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]

---
 kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

kernel/rcutorture.c +24 -0

index 3d936f0f..f5cf2bb 100644
@@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
 static int fqs_holdoff;		/* Hold time within burst (us). */
 static int fqs_stutter = 3;	/* Wait time between bursts (s). */
 static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
+static int object_debug;	/* Test object-debug double call_rcu()?. */
 static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
 static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
 static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
 module_param(onoff_interval, int, 0444);
 MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
 module_param(onoff_holdoff, int, 0444);

@@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
 		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
 }
 
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+static void rcu_torture_leak_cb(struct rcu_head *rhp)
+{
+}
+
+static void rcu_torture_err_cb(struct rcu_head *rhp)
+{
+	/* This -might- happen due to race conditions, but is unlikely. */
+	pr_alert("rcutorture: duplicated callback was invoked.\n");
+}
+#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+
+/*
+ * Verify that double-free causes debug-objects to complain, but only
+ * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
+ * cannot be carried out.
+ */
+static void rcu_test_debug_objects(void)
+{
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+	struct rcu_head rh1;

Quoted 8 lines

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1966

Quoted 6 lines

kernel/rcutorture.c +6 -0

+	struct rcu_head rh2;
+
+	init_rcu_head_on_stack(&rh1);
+	init_rcu_head_on_stack(&rh2);
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
+	local_irq_disable(); /* Make it hard to finish grace period. */

you can use rcu_read_lock() directly.

I could do that as well, but it doesn't do everything that local_irq_disable() does.

Right, which means that my comment is bad. Fixing both, thank you!

Quoted 1 line

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1967

Quoted 1 line

kernel/rcutorture.c +1 -0

+	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */

And the one above cannot start a grace period due to irqs being enabled. Which is -almost- always OK, but...

Quoted 1 line

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1968

Quoted 1 line

kernel/rcutorture.c +1 -0

+	call_rcu(&rh2, rcu_torture_err_cb);

And this one should invoke rcu_torture_leak_cb instead of rcu_torture_err_cb(). Just results in a confusing error message, but...

OK, a few more fixes, then!

Quoted 10 lines

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1978

Quoted 10 lines

kernel/rcutorture.c +10 -0

+	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
+	local_irq_enable();
+	rcu_barrier();
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
+	destroy_rcu_head_on_stack(&rh1);
+	destroy_rcu_head_on_stack(&rh2);
+#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
+#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+}

The result is as follows. Better?

Thanx, Paul

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
static void rcu_torture_leak_cb(struct rcu_head *rhp) {
}

static void rcu_torture_err_cb(struct rcu_head *rhp)
{
/*
* This -might- happen due to race conditions, but is unlikely.
* The scenario that leads to this happening is that the
* first of the pair of duplicate callbacks is queued,
* someone else starts a grace period that includes that
* callback, then the second of the pair must wait for the
* next grace period. Unlikely, but can happen. If it
* does happen, the debug-objects subsystem won't have splatted. */
pr_alert("rcutorture: duplicated callback was invoked.\n");
}
#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */

/*
* Verify that double-free causes debug-objects to complain, but only
* if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y. Otherwise, say that the test
* cannot be carried out.
*/
static void rcu_test_debug_objects(void)
{

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
	struct rcu_head rh1;
	struct rcu_head rh2;

	init_rcu_head_on_stack(&rh1);
	init_rcu_head_on_stack(&rh2);
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
	preempt_disable(); /* Prevent preemption from interrupting test. */
	rcu_read_lock(); /* Make it impossible to finish a grace period. */
	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */
	local_irq_disable(); /* Make it harder to start a new grace period. */
	call_rcu(&rh2, rcu_torture_leak_cb);
	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
	local_irq_enable();
	rcu_read_unlock();
	preempt_enable();
	rcu_barrier();
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
	destroy_rcu_head_on_stack(&rh1);
	destroy_rcu_head_on_stack(&rh2);
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");

#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */ }

Quoted 5 lines

Quoted 5 lines

kernel/rcutorture.c +1 -0

+
 static int __init
 rcu_torture_init(void)
 {

@@ -2163,6 +2206,8 @@ rcu_torture_init(void)

Quoted 9 lines

kernel/rcutorture.c rcu_torture_init(void) · line 2213

Quoted 8 lines

kernel/rcutorture.c +2 -0

 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug)
+		rcu_test_debug_objects();
 	rcutorture_record_test_transition();
 	mutex_unlock(&fullstop_mutex);
 	return 0;

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

Quoted 86 lines · Paul E. McKenney

Quoted 66 lines · Lai Jiangshan

Quoted 65 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

This commit adds a object_debug option to rcutorture to allow the debug-object-based checks for duplicate call_rcu() invocations to be deterministically tested.

Signed-off-by Paul E. McKenney <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Sedat Dilek <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Linus Torvalds <[email protected]>

Tested-by Sedat Dilek <[email protected]>

[ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ]

---
 kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

kernel/rcutorture.c +24 -0

index 3d936f0f..f5cf2bb 100644
@@ -66,6 +66,7 @@ static int fqs_duration;	/* Duration of bursts (us), 0 to disable. */
 static int fqs_holdoff;		/* Hold time within burst (us). */
 static int fqs_stutter = 3;	/* Wait time between bursts (s). */
 static int n_barrier_cbs;	/* Number of callbacks to test RCU barriers. */
+static int object_debug;	/* Test object-debug double call_rcu()?. */
 static int onoff_interval;	/* Wait time between CPU hotplugs, 0=disable. */
 static int onoff_holdoff;	/* Seconds after boot before CPU hotplugs. */
 static int shutdown_secs;	/* Shutdown time (s).  <=0 for no shutdown. */

@@ -100,6 +101,8 @@ module_param(fqs_stutter, int, 0444);
 MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(n_barrier_cbs, int, 0444);
 MODULE_PARM_DESC(n_barrier_cbs, "# of callbacks/kthreads for barrier testing");
+module_param(object_debug, int, 0444);
+MODULE_PARM_DESC(object_debug, "Enable debug-object double call_rcu() testing");
 module_param(onoff_interval, int, 0444);
 MODULE_PARM_DESC(onoff_interval, "Time between CPU hotplugs (s), 0=disable");
 module_param(onoff_holdoff, int, 0444);

@@ -1934,6 +1937,46 @@ rcu_torture_cleanup(void)
 		rcu_torture_print_module_parms(cur_ops, "End of test: SUCCESS");
 }
 
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+static void rcu_torture_leak_cb(struct rcu_head *rhp)
+{
+}
+
+static void rcu_torture_err_cb(struct rcu_head *rhp)
+{
+	/* This -might- happen due to race conditions, but is unlikely. */
+	pr_alert("rcutorture: duplicated callback was invoked.\n");
+}
+#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+
+/*
+ * Verify that double-free causes debug-objects to complain, but only
+ * if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y.  Otherwise, say that the test
+ * cannot be carried out.
+ */
+static void rcu_test_debug_objects(void)
+{
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+	struct rcu_head rh1;

Quoted 8 lines

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1966

Quoted 6 lines

kernel/rcutorture.c +6 -0

+	struct rcu_head rh2;
+
+	init_rcu_head_on_stack(&rh1);
+	init_rcu_head_on_stack(&rh2);
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
+	local_irq_disable(); /* Make it hard to finish grace period. */

you can use rcu_read_lock() directly.

I could do that as well, but it doesn't do everything that local_irq_disable() does.

Right, which means that my comment is bad. Fixing both, thank you!

Quoted 1 line

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1967

Quoted 1 line

kernel/rcutorture.c +1 -0

+	call_rcu(&rh1, rcu_torture_leak_cb); /* start grace period. */

And the one above cannot start a grace period due to irqs being enabled. Which is -almost- always OK, but...

Quoted 4 lines

Quoted 1 line

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1968

Quoted 1 line

kernel/rcutorture.c +1 -0

+	call_rcu(&rh2, rcu_torture_err_cb);

And this one should invoke rcu_torture_leak_cb instead of rcu_torture_err_cb(). Just results in a confusing error message, but...

I still don't understand why rcu_torture_err_cb() will be called when:

rcu_read_lock();
call_rcu(&rh2, rcu_torture_leak_cb);
call_rcu(&rh2, rcu_torture_err_cb); // rh2 will be still queued here,

				    // debug-objects will find it and
				    // change it to rcu_leak_callback()

rcu_read_unlock();

Quoted 90 lines

OK, a few more fixes, then!

Quoted 10 lines

kernel/rcutorture.c rcu_torture_cleanup(void) · line 1978

Quoted 10 lines

kernel/rcutorture.c +10 -0

+	call_rcu(&rh2, rcu_torture_err_cb); /* duplicate callback. */
+	local_irq_enable();
+	rcu_barrier();
+	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
+	destroy_rcu_head_on_stack(&rh1);
+	destroy_rcu_head_on_stack(&rh2);
+#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
+#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+}

The result is as follows. Better?

Thanx, Paul

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
static void rcu_torture_leak_cb(struct rcu_head *rhp) {
}

static void rcu_torture_err_cb(struct rcu_head *rhp)
{
/*
* This -might- happen due to race conditions, but is unlikely.
* The scenario that leads to this happening is that the
* first of the pair of duplicate callbacks is queued,
* someone else starts a grace period that includes that
* callback, then the second of the pair must wait for the
* next grace period. Unlikely, but can happen. If it
* does happen, the debug-objects subsystem won't have splatted. */
pr_alert("rcutorture: duplicated callback was invoked.\n");
}
#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */

/*
* Verify that double-free causes debug-objects to complain, but only
* if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y. Otherwise, say that the test
* cannot be carried out.
*/
static void rcu_test_debug_objects(void)
{

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
	struct rcu_head rh1;
	struct rcu_head rh2;

	init_rcu_head_on_stack(&rh1);
	init_rcu_head_on_stack(&rh2);
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
	preempt_disable(); /* Prevent preemption from interrupting test. */
	rcu_read_lock(); /* Make it impossible to finish a grace period. */
	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */
	local_irq_disable(); /* Make it harder to start a new grace period. */
	call_rcu(&rh2, rcu_torture_leak_cb);
	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
	local_irq_enable();
	rcu_read_unlock();
	preempt_enable();
	rcu_barrier();
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
	destroy_rcu_head_on_stack(&rh1);
	destroy_rcu_head_on_stack(&rh2);
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");

#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */ }

Quoted 5 lines

Quoted 5 lines

kernel/rcutorture.c +1 -0

+
 static int __init
 rcu_torture_init(void)
 {

@@ -2163,6 +2206,8 @@ rcu_torture_init(void)

Quoted 9 lines

kernel/rcutorture.c rcu_torture_init(void) · line 2213

Quoted 8 lines

kernel/rcutorture.c +2 -0

 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug)
+		rcu_test_debug_objects();
 	rcutorture_record_test_transition();
 	mutex_unlock(&fullstop_mutex);
 	return 0;

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

The result is as follows. Better?

Thanx, Paul

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
static void rcu_torture_leak_cb(struct rcu_head *rhp) {
}

static void rcu_torture_err_cb(struct rcu_head *rhp)
{
/*
* This -might- happen due to race conditions, but is unlikely.
* The scenario that leads to this happening is that the
* first of the pair of duplicate callbacks is queued,
* someone else starts a grace period that includes that
* callback, then the second of the pair must wait for the
* next grace period. Unlikely, but can happen. If it
* does happen, the debug-objects subsystem won't have splatted. */
pr_alert("rcutorture: duplicated callback was invoked.\n");
}
#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */

/*
* Verify that double-free causes debug-objects to complain, but only
* if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y. Otherwise, say that the test
* cannot be carried out.
*/
static void rcu_test_debug_objects(void)
{

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
	struct rcu_head rh1;
	struct rcu_head rh2;

	init_rcu_head_on_stack(&rh1);
	init_rcu_head_on_stack(&rh2);
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
	preempt_disable(); /* Prevent preemption from interrupting test. */
	rcu_read_lock(); /* Make it impossible to finish a grace period. */
	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */

	local_irq_disable(); /* Make it harder to start a new grace period. */
	call_rcu(&rh2, rcu_torture_leak_cb);
	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
	local_irq_enable();
	rcu_read_unlock();
	preempt_enable();
	rcu_barrier();
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
	destroy_rcu_head_on_stack(&rh1);
	destroy_rcu_head_on_stack(&rh2);
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");

#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */ }

Quoted 14 lines

Quoted 13 lines

kernel/rcutorture.c +3 -0

+
 static int __init
 rcu_torture_init(void)
 {

@@ -2163,6 +2206,8 @@ rcu_torture_init(void)
 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug)
+		rcu_test_debug_objects();
 	rcutorture_record_test_transition();
 	mutex_unlock(&fullstop_mutex);
 	return 0;

* Paul E. McKenney ([email protected]) wrote:
[...]

Quoted 1 line

The result is as follows. Better?

Hi Paul,

Pitching in late in the thread, so that I can get a share of the fun ;-)

Quoted 7 lines

Thanx, Paul

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
static void rcu_torture_leak_cb(struct rcu_head *rhp) {
}

Quoted 15 lines

static void rcu_torture_err_cb(struct rcu_head *rhp)
{
/*
* This -might- happen due to race conditions, but is unlikely.
* The scenario that leads to this happening is that the
* first of the pair of duplicate callbacks is queued,
* someone else starts a grace period that includes that
* callback, then the second of the pair must wait for the
* next grace period. Unlikely, but can happen. If it
* does happen, the debug-objects subsystem won't have splatted. */
pr_alert("rcutorture: duplicated callback was invoked.\n");
}
#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */

Hrm. Putting an #ifdef within a function when not utterly needed is usually a bad idea. How about:

/*
* Verify that double-free causes debug-objects to complain, but only
* if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y. Otherwise, say that the test
* cannot be carried out.
*/
#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
static void rcu_test_debug_objects(void)
{

(quoted diff) +0 -0

 	struct rcu_head rh1;
 	struct rcu_head rh2;
 
 	init_rcu_head_on_stack(&rh1);
 	init_rcu_head_on_stack(&rh2);
 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
 	preempt_disable(); /* Prevent preemption from interrupting test. */
 	rcu_read_lock(); /* Make it impossible to finish a grace period. */
 	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */
 	local_irq_disable(); /* Make it harder to start a new grace period. */
 	call_rcu(&rh2, rcu_torture_leak_cb);
 	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
 	local_irq_enable();
 	rcu_read_unlock();
 	preempt_enable();
 	rcu_barrier();
 	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
 	destroy_rcu_head_on_stack(&rh1);
 	destroy_rcu_head_on_stack(&rh2);

}
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
static void rcu_test_debug_objects(void)
{
pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");
}
#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */

More comments inlined in the code below,

Quoted 7 lines

/*
* Verify that double-free causes debug-objects to complain, but only
* if CONFIG_DEBUG_OBJECTS_RCU_HEAD=y. Otherwise, say that the test
* cannot be carried out.
*/
static void rcu_test_debug_objects(void)
{

Quoted 10 lines

#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
	struct rcu_head rh1;
	struct rcu_head rh2;

	init_rcu_head_on_stack(&rh1);
	init_rcu_head_on_stack(&rh2);
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
	preempt_disable(); /* Prevent preemption from interrupting test. */
	rcu_read_lock(); /* Make it impossible to finish a grace period. */
	call_rcu(&rh1, rcu_torture_leak_cb); /* Start grace period. */

Are we really "starting" a grace period ? If rcu_test_debug_objects() is executed after some callbacks are already queued, are we, by definition, "starting" the grace period ?

Also, I find it weird to have, in that order:

1) preempt_disable()
2) rcu_read_lock()
3) local_irq_disable()

I would rather expect:

1) rcu_read_lock()
2) preempt_disable()
3) local_irq_disable()

So they come in increasing order of impact on the system: with non-preemptable RCU, the read-lock and preempt disable mean the same thing, however, with preemptable RCU, the impact of preempt disable seems larger than the impact of RCU read lock: preemption is still enabled when within a RCU critical section. Both will work, but I find this call order slightly weird.

Also, if your goal is to increase the chances that call_rcu() enqueues both callbacks into the same grace period, you might want to issue a rcu_barrier() early in this function, so that call_rcu() has even more chances to enqueue the callbacks into the same grace period.

However, if you care about testing enqueue into same _and_ different grace periods, you might want to turn this single-shot test into a stress-test by calling it repeatedly.

Thanks!

Mathieu

Quoted 30 lines

	local_irq_disable(); /* Make it harder to start a new grace period. */
	call_rcu(&rh2, rcu_torture_leak_cb);
	call_rcu(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
	local_irq_enable();
	rcu_read_unlock();
	preempt_enable();
	rcu_barrier();
	pr_alert("rcutorture: WARN: Duplicate call_rcu() test complete.\n");
	destroy_rcu_head_on_stack(&rh1);
	destroy_rcu_head_on_stack(&rh2);
#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
	pr_alert("rcutorture: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n");

#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */ }

Quoted 14 lines

Quoted 13 lines

kernel/rcutorture.c +3 -0

+
 static int __init
 rcu_torture_init(void)
 {

@@ -2163,6 +2206,8 @@ rcu_torture_init(void)
 		firsterr = retval;
 		goto unwind;
 	}
+	if (object_debug)
+		rcu_test_debug_objects();
 	rcutorture_record_test_transition();
 	mutex_unlock(&fullstop_mutex);
 	return 0;

Signature · 3 lines

Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com