From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752131Ab3HUCgB (ORCPT ); Tue, 20 Aug 2013 22:36:01 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:16242 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1751502Ab3HUCgA (ORCPT ); Tue, 20 Aug 2013 22:36:00 -0400 X-IronPort-AV: E=Sophos;i="4.89,924,1367942400"; d="scan'208";a="8256103" Message-ID: <5214288F.7040308@cn.fujitsu.com> Date: Wed, 21 Aug 2013 10:40:15 +0800 From: Lai Jiangshan User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.9) Gecko/20100921 Fedora/3.1.4-1.fc14 Thunderbird/3.1.4 MIME-Version: 1.0 To: paulmck@linux.vnet.ibm.com CC: linux-kernel@vger.kernel.org, mingo@elte.hu, dipankar@in.ibm.com, akpm@linux-foundation.org, mathieu.desnoyers@polymtl.ca, josh@joshtriplett.org, niv@us.ibm.com, tglx@linutronix.de, peterz@infradead.org, rostedt@goodmis.org, dhowells@redhat.com, edumazet@google.com, darren@dvhart.com, fweisbec@gmail.com, sbw@mit.edu, Mathieu Desnoyers , Sedat Dilek , Davidlohr Bueso , Rik van Riel , Linus Torvalds Subject: Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture References: <20130818022453.GA3475@linux.vnet.ibm.com> <1376967074-32530-1-git-send-email-paulmck@linux.vnet.ibm.com> <52133EBF.3020605@cn.fujitsu.com> <20130820183843.GK29406@linux.vnet.ibm.com> In-Reply-To: <20130820183843.GK29406@linux.vnet.ibm.com> X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/08/21 10:34:03, Serialize by Router on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/08/21 10:34:13, Serialize complete at 2013/08/21 10:34:13 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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" >>> >>> 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 >>> Cc: Mathieu Desnoyers >>> Cc: Sedat Dilek >>> Cc: Davidlohr Bueso >>> Cc: Rik van Riel >>> Cc: Thomas Gleixner >>> Cc: Linus Torvalds >>> Tested-by: Sedat Dilek >>> [ 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(); > > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >