Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Sat, Aug 24, 2013 at 03:25:36PM -0400, Mathieu Desnoyers wrote:
> * Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote:
> [...]
> > The result is as follows. Better?
>
> Hi Paul,
>
> Pitching in late in the thread, so that I can get a share of the fun ;-)
>
> > 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 */
> >
>
> 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)
> {
> struct rcu_head rh1;
> struct rcu_head rh2;
>
> init_rcu_head_on_stack();
> init_rcu_head_on_stack();
> 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(, rcu_torture_leak_cb); /* Start grace period. */
> local_irq_disable(); /* Make it harder to start a new grace period. */
> call_rcu(, rcu_torture_leak_cb);
> call_rcu(, 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();
> destroy_rcu_head_on_stack();
> }
> #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 */
The objection to this is that it duplicates the function header, both
copies of which must be updated. See Josh's and my discussion on this
point earlier in the thread. Who knows, Josh might eventually convince
me to individually ifdef the functions in kernel/rcutree_plugin.h,
but I am not quite there yet. ;-)
That said, I do see two benefits of individual ifdef:
1. It is easy to see when a given function is included.
As it is, there are runs of many hundreds of lines of
code where it might not be obvious to the casual reader.
2. Duplicated code is asking for silent rebase and merge errors.
This can happen when a change to the function header collides
with a change that duplicates the function under ifdef.
So perhaps I will eventually convert to the individual-ifdef style.
At the moment, I am in no hurry.
> More comments inlined in the code below,
>
> > /*
> > * 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();
> > init_rcu_head_on_stack();
> > 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(, 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
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Sat, Aug 24, 2013 at 03:25:36PM -0400, Mathieu Desnoyers wrote:
* Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote:
[...]
The result is as follows. Better?
Hi Paul,
Pitching in late in the thread, so that I can get a share of the fun ;-)
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 */
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)
{
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 */
The objection to this is that it duplicates the function header, both
copies of which must be updated. See Josh's and my discussion on this
point earlier in the thread. Who knows, Josh might eventually convince
me to individually ifdef the functions in kernel/rcutree_plugin.h,
but I am not quite there yet. ;-)
That said, I do see two benefits of individual ifdef:
1. It is easy to see when a given function is included.
As it is, there are runs of many hundreds of lines of
code where it might not be obvious to the casual reader.
2. Duplicated code is asking for silent rebase and merge errors.
This can happen when a change to the function header collides
with a change that duplicates the function under ifdef.
So perhaps I will eventually convert to the individual-ifdef style.
At the moment, I am in no hurry.
More comments inlined in the code below,
/*
* 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. */
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.
If this was code that ran normally, I would agree with you.
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
* Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote:
[...]
> The result is as follows. Better?
Hi Paul,
Pitching in late in the thread, so that I can get a share of the fun ;-)
> 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 */
>
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)
{
struct rcu_head rh1;
struct rcu_head rh2;
init_rcu_head_on_stack();
init_rcu_head_on_stack();
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(, rcu_torture_leak_cb); /* Start grace period. */
local_irq_disable(); /* Make it harder to start a new grace period. */
call_rcu(, rcu_torture_leak_cb);
call_rcu(, 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();
destroy_rcu_head_on_stack();
}
#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,
> /*
> * 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();
> init_rcu_head_on_stack();
> 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(, 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
> local_irq_disable(); /* Make it harder to start a new grace period. */
> call_rcu(, rcu_torture_leak_cb);
> call_rcu(, 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();
> destroy_rcu_head_on_stack();
> #else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> pr_alert("rcutorture:
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
* Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote:
[...]
The result is as follows. Better?
Hi Paul,
Pitching in late in the thread, so that I can get a share of the fun ;-)
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 */
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)
{
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,
/*
* 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. */
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
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
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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"
> >>>
> >>> 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();
> >>> + init_rcu_head_on_stack();
> >>> + 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(, 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(, 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(, rcu_torture_leak_cb);
> call_rcu(, 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(, rcu_torture_err_cb); /* duplicate callback. */
> >>> + local_irq_enable();
> >>> + rcu_barrier();
> >>> + pr_alert("rcutorture: WARN: Duplicate
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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();
>>> + init_rcu_head_on_stack();
>>> + 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(, 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(, 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(, rcu_torture_leak_cb);
call_rcu(, 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(, 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();
>>> + destroy_rcu_head_on_stack();
>>> +#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
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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();
> > + init_rcu_head_on_stack();
> > + 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(, 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(, 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!
> > + call_rcu(, 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();
> > + destroy_rcu_head_on_stack();
> > +#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,
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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();
> + init_rcu_head_on_stack();
> + 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.
> + call_rcu(, rcu_torture_leak_cb); /* start grace period. */
> + call_rcu(, rcu_torture_err_cb);
> + call_rcu(, 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();
> + destroy_rcu_head_on_stack();
> +#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(_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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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.
+ 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;
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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...
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.
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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();
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
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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:
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Mon, Aug 19, 2013 at 07:51:10PM -0700, 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. ]
Reviewed-by: 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();
> + init_rcu_head_on_stack();
> + pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> + local_irq_disable(); /* Make it hard to finish grace period. */
> + call_rcu(, rcu_torture_leak_cb); /* start grace period. */
> + call_rcu(, rcu_torture_err_cb);
> + call_rcu(, 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();
> + destroy_rcu_head_on_stack();
> +#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(_mutex);
> return 0;
> --
> 1.8.1.5
>
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Mon, Aug 19, 2013 at 07:05:58PM -0700, Paul E. McKenney wrote:
> On Mon, Aug 19, 2013 at 10:16:52AM -0700, Josh Triplett wrote:
> > On Mon, Aug 19, 2013 at 09:09:45AM -0700, Paul E. McKenney wrote:
> > > On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
> > > > On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
> > > > > On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> > > > > > On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
> > > > > >
> > > > > > Two comments below; with those fixed,
> > > > > > Reviewed-by: Josh Triplett
> > > > > >
> > > > > > > ---
> > > > > > > @@ -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.
> > > > >
> > > > > 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.
> > >
> > > 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();
> > > init_rcu_head_on_stack();
> > > pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> > > local_irq_disable(); /* Make it hard to finish grace period. */
> > > call_rcu(, rcu_torture_leak_cb); /* start grace period. */
> > > call_rcu(, rcu_torture_err_cb);
> > > call_rcu(, 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();
> > > destroy_rcu_head_on_stack();
> > > #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 */
> > > }
> >
> > 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
[PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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();
+ init_rcu_head_on_stack();
+ pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
+ local_irq_disable(); /* Make it hard to finish grace period. */
+ call_rcu(, rcu_torture_leak_cb); /* start grace period. */
+ call_rcu(, rcu_torture_err_cb);
+ call_rcu(, 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();
+ destroy_rcu_head_on_stack();
+#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(_mutex);
return 0;
--
1.8.1.5
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Mon, Aug 19, 2013 at 10:16:52AM -0700, Josh Triplett wrote:
> On Mon, Aug 19, 2013 at 09:09:45AM -0700, Paul E. McKenney wrote:
> > On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
> > > On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
> > > > On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> > > > > On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
> > > > >
> > > > > Two comments below; with those fixed,
> > > > > Reviewed-by: Josh Triplett
> > > > >
> > > > > > ---
> > > > > > @@ -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.
> > > >
> > > > 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.
> >
> > 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();
> > init_rcu_head_on_stack();
> > pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> > local_irq_disable(); /* Make it hard to finish grace period. */
> > call_rcu(, rcu_torture_leak_cb); /* start grace period. */
> > call_rcu(, rcu_torture_err_cb);
> > call_rcu(, 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();
> > destroy_rcu_head_on_stack();
> > #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 */
> > }
>
> 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!
Thanx, Paul
--
To unsubscribe from this
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Mon, Aug 19, 2013 at 09:09:45AM -0700, Paul E. McKenney wrote:
> On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
> > On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
> > > On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> > > > On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
> > > >
> > > > Two comments below; with those fixed,
> > > > Reviewed-by: Josh Triplett
> > > >
> > > > > ---
> > > > > @@ -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.
> > >
> > > 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.
>
> 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();
> init_rcu_head_on_stack();
> pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
> local_irq_disable(); /* Make it hard to finish grace period. */
> call_rcu(, rcu_torture_leak_cb); /* start grace period. */
> call_rcu(, rcu_torture_err_cb);
> call_rcu(, 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();
> destroy_rcu_head_on_stack();
> #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 */
> }
>
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.
- Josh Triplett
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
> On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
> > On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> > > On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
> > >
> > > Two comments below; with those fixed,
> > > Reviewed-by: Josh Triplett
> > >
> > > > ---
> > > > @@ -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.
> >
> > 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.
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.
Thanx, Paul
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();
init_rcu_head_on_stack();
pr_alert("rcutorture: WARN: Duplicate call_rcu() test starting.\n");
local_irq_disable(); /* Make it hard to finish grace period. */
call_rcu(, rcu_torture_leak_cb); /* start grace period. */
call_rcu(, rcu_torture_err_cb);
call_rcu(, 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();
destroy_rcu_head_on_stack();
#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 */
}
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
Two comments below; with those fixed,
Reviewed-by: Josh Triplett j...@joshtriplett.org
---
@@ -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.
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.
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.
Thanx, Paul
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 */
}
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Mon, Aug 19, 2013 at 09:09:45AM -0700, Paul E. McKenney wrote:
On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
Two comments below; with those fixed,
Reviewed-by: Josh Triplett j...@joshtriplett.org
---
@@ -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.
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.
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 */
}
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.
- Josh Triplett
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Mon, Aug 19, 2013 at 10:16:52AM -0700, Josh Triplett wrote:
On Mon, Aug 19, 2013 at 09:09:45AM -0700, Paul E. McKenney wrote:
On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
Two comments below; with those fixed,
Reviewed-by: Josh Triplett j...@joshtriplett.org
---
@@ -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.
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.
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 */
}
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!
Thanx, Paul
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo
[PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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. */
+ 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;
--
1.8.1.5
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Mon, Aug 19, 2013 at 07:05:58PM -0700, Paul E. McKenney wrote:
On Mon, Aug 19, 2013 at 10:16:52AM -0700, Josh Triplett wrote:
On Mon, Aug 19, 2013 at 09:09:45AM -0700, Paul E. McKenney wrote:
On Sun, Aug 18, 2013 at 09:19:25PM -0700, Josh Triplett wrote:
On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote:
On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
Two comments below; with those fixed,
Reviewed-by: Josh Triplett j...@joshtriplett.org
---
@@ -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.
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.
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 */
}
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!
OK; I can live with that.
Reviewed-by: Josh Triplett
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Mon, Aug 19, 2013 at 07:51:10PM -0700, 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. ]
Reviewed-by: Josh Triplett j...@joshtriplett.org
---
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. */
+ 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;
--
1.8.1.5
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote: > On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote: > > On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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 > > > > Two comments below; with those fixed, > > Reviewed-by: Josh Triplett > > > > > --- > > > @@ -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. > > 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. - Josh Triplett -- 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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
> On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
>
> Two comments below; with those fixed,
> Reviewed-by: Josh Triplett
>
> > ---
> > @@ -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.
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?
> > @@ -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();
> > + init_rcu_head_on_stack();
> > + pr_alert("rcutorture: WARN: Duplicate call_rcu() test
> > starting.\n");
> > + local_irq_disable(); /* Make it hard to finish grace period. */
> > + call_rcu(, rcu_torture_leak_cb); /* start grace period. */
> > + call_rcu(, rcu_torture_err_cb);
> > + call_rcu(, 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();
> > + destroy_rcu_head_on_stack();
> > +#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.)
Force of habit, fixed. ;-)
Thanx, Paul
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote:
On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
Two comments below; with those fixed,
Reviewed-by: Josh Triplett j...@joshtriplett.org
---
@@ -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.
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?
@@ -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);
+ 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.)
Force of habit, fixed. ;-)
Thanx, Paul
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Sun, Aug 18, 2013 at 08:55:28PM -0700, Paul E. McKenney wrote: On Sat, Aug 17, 2013 at 07:54:20PM -0700, Josh Triplett wrote: On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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 Two comments below; with those fixed, Reviewed-by: Josh Triplett j...@joshtriplett.org --- @@ -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. 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. - Josh Triplett -- 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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
Two comments below; with those fixed,
Reviewed-by: Josh Triplett
> ---
> @@ -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.
> @@ -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();
> + init_rcu_head_on_stack();
> + pr_alert("rcutorture: WARN: Duplicate call_rcu() test
> starting.\n");
> + local_irq_disable(); /* Make it hard to finish grace period. */
> + call_rcu(, rcu_torture_leak_cb); /* start grace period. */
> + call_rcu(, rcu_torture_err_cb);
> + call_rcu(, 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();
> + destroy_rcu_head_on_stack();
> +#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.)
- Josh Triplett
--
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/
[PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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
---
kernel/rcutorture.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 3d936f0f..efb1c74 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,18 @@ 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 */
+
static int __init
rcu_torture_init(void)
{
@@ -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();
+ init_rcu_head_on_stack();
+ pr_alert("rcutorture: WARN: Duplicate call_rcu() test
starting.\n");
+ local_irq_disable(); /* Make it hard to finish grace period. */
+ call_rcu(, rcu_torture_leak_cb); /* start grace period. */
+ call_rcu(, rcu_torture_err_cb);
+ call_rcu(, 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();
+ destroy_rcu_head_on_stack();
+#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+ pr_alert("rcutorture: !%s, not testing duplicate call_rcu()\n",
+"CONFIG_DEBUG_OBJECTS_RCU_HEAD");
+#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+ }
rcutorture_record_test_transition();
mutex_unlock(_mutex);
return 0;
--
1.8.1.5
--
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/
[PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
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
---
kernel/rcutorture.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 3d936f0f..efb1c74 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,18 @@ 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 */
+
static int __init
rcu_torture_init(void)
{
@@ -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);
+ 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);
+#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+ }
rcutorture_record_test_transition();
mutex_unlock(fullstop_mutex);
return 0;
--
1.8.1.5
--
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/
Re: [PATCH tip/core/rcu 1/5] rcu: Add duplicate-callback tests to rcutorture
On Sat, Aug 17, 2013 at 07:25:13PM -0700, 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
Two comments below; with those fixed,
Reviewed-by: Josh Triplett j...@joshtriplett.org
---
@@ -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.
@@ -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);
+ 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.)
- Josh Triplett
--
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/
