On Wed, Aug 21, 2013 at 10:40:15AM +0800, Lai Jiangshan wrote: > On 08/21/2013 02:38 AM, Paul E. McKenney wrote: > > On Tue, Aug 20, 2013 at 06:02:39PM +0800, Lai Jiangshan wrote: > >> On 08/20/2013 10:51 AM, Paul E. McKenney wrote: > >>> From: "Paul E. McKenney" <paul...@linux.vnet.ibm.com> > >>> > >>> 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 <paul...@linux.vnet.ibm.com> > >>> Cc: Mathieu Desnoyers <mathieu.desnoy...@efficios.com> > >>> Cc: Sedat Dilek <sedat.di...@gmail.com> > >>> Cc: Davidlohr Bueso <davidlohr.bu...@hp.com> > >>> Cc: Rik van Riel <r...@surriel.com> > >>> Cc: Thomas Gleixner <t...@linutronix.de> > >>> Cc: Linus Torvalds <torva...@linux-foundation.org> > >>> Tested-by: Sedat Dilek <sedat.di...@gmail.com> > >>> [ paulmck: Banish mid-function ifdef, more or less per Josh Triplett. ] > >>> --- > >>> kernel/rcutorture.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 45 insertions(+) > >>> > >>> diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c > >>> index 3d936f0f..f5cf2bb 100644 > >>> --- a/kernel/rcutorture.c > >>> +++ b/kernel/rcutorture.c > >>> @@ -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. */ > >> > >> 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! > > > >>> + 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... > > > >>> + 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();
Fair point, no chance of the second rh2 callback being queued after the first one is invoked! I will leave the message. Whoever sees it with the current code will have something to tell their grandchildren. Thanx, Paul > > OK, a few more fixes, then! > > > >>> + 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 */ > > } > > > >>> + > >>> 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; > >> > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/