* [PATCH tip/core/rcu 2/4] debugobjects: Make debug_object_activate() return status
2013-04-24 0:33 ` [PATCH tip/core/rcu 1/4] rcu: Simplify debug-objects fixups Paul E. McKenney
@ 2013-04-24 0:33 ` Paul E. McKenney
2013-04-24 0:33 ` [PATCH tip/core/rcu 3/4] rcu: Make call_rcu() leak callbacks for debug-object errors Paul E. McKenney
2013-04-24 0:33 ` [PATCH tip/core/rcu 4/4] rcu: Add duplicate-callback tests to rcutorture Paul E. McKenney
2 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2013-04-24 0:33 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, Valdis.Kletnieks, dhowells, edumazet, darren,
fweisbec, sbw, Paul E. McKenney, Mathieu Desnoyers, Sedat Dilek,
Davidlohr Bueso, Rik van Riel, Linus Torvalds
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
In order to better respond to things like duplicate invocations
of call_rcu(), RCU needs to see the status of a call to
debug_object_activate(). This would allow RCU to leak the callback in
order to avoid adding freelist-reuse mischief to the duplicate invoations.
This commit therefore makes debug_object_activate() return status,
zero for success and -EINVAL for failure.
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
include/linux/debugobjects.h | 6 +++---
lib/debugobjects.c | 20 ++++++++++++++------
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/include/linux/debugobjects.h b/include/linux/debugobjects.h
index 0e5f578..98ffcbd 100644
--- a/include/linux/debugobjects.h
+++ b/include/linux/debugobjects.h
@@ -63,7 +63,7 @@ struct debug_obj_descr {
extern void debug_object_init (void *addr, struct debug_obj_descr *descr);
extern void
debug_object_init_on_stack(void *addr, struct debug_obj_descr *descr);
-extern void debug_object_activate (void *addr, struct debug_obj_descr *descr);
+extern int debug_object_activate (void *addr, struct debug_obj_descr *descr);
extern void debug_object_deactivate(void *addr, struct debug_obj_descr *descr);
extern void debug_object_destroy (void *addr, struct debug_obj_descr *descr);
extern void debug_object_free (void *addr, struct debug_obj_descr *descr);
@@ -85,8 +85,8 @@ static inline void
debug_object_init (void *addr, struct debug_obj_descr *descr) { }
static inline void
debug_object_init_on_stack(void *addr, struct debug_obj_descr *descr) { }
-static inline void
-debug_object_activate (void *addr, struct debug_obj_descr *descr) { }
+static inline int
+debug_object_activate (void *addr, struct debug_obj_descr *descr) { return 0; }
static inline void
debug_object_deactivate(void *addr, struct debug_obj_descr *descr) { }
static inline void
diff --git a/lib/debugobjects.c b/lib/debugobjects.c
index 37061ed..bf2c8b1 100644
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -381,19 +381,21 @@ void debug_object_init_on_stack(void *addr, struct debug_obj_descr *descr)
* debug_object_activate - debug checks when an object is activated
* @addr: address of the object
* @descr: pointer to an object specific debug description structure
+ * Returns 0 for success, -EINVAL for check failed.
*/
-void debug_object_activate(void *addr, struct debug_obj_descr *descr)
+int debug_object_activate(void *addr, struct debug_obj_descr *descr)
{
enum debug_obj_state state;
struct debug_bucket *db;
struct debug_obj *obj;
unsigned long flags;
+ int ret;
struct debug_obj o = { .object = addr,
.state = ODEBUG_STATE_NOTAVAILABLE,
.descr = descr };
if (!debug_objects_enabled)
- return;
+ return 0;
db = get_bucket((unsigned long) addr);
@@ -405,23 +407,26 @@ void debug_object_activate(void *addr, struct debug_obj_descr *descr)
case ODEBUG_STATE_INIT:
case ODEBUG_STATE_INACTIVE:
obj->state = ODEBUG_STATE_ACTIVE;
+ ret = 0;
break;
case ODEBUG_STATE_ACTIVE:
debug_print_object(obj, "activate");
state = obj->state;
raw_spin_unlock_irqrestore(&db->lock, flags);
- debug_object_fixup(descr->fixup_activate, addr, state);
- return;
+ ret = debug_object_fixup(descr->fixup_activate, addr, state);
+ return ret ? -EINVAL : 0;
case ODEBUG_STATE_DESTROYED:
debug_print_object(obj, "activate");
+ ret = -EINVAL;
break;
default:
+ ret = 0;
break;
}
raw_spin_unlock_irqrestore(&db->lock, flags);
- return;
+ return ret;
}
raw_spin_unlock_irqrestore(&db->lock, flags);
@@ -431,8 +436,11 @@ void debug_object_activate(void *addr, struct debug_obj_descr *descr)
* true or not.
*/
if (debug_object_fixup(descr->fixup_activate, addr,
- ODEBUG_STATE_NOTAVAILABLE))
+ ODEBUG_STATE_NOTAVAILABLE)) {
debug_print_object(&o, "activate");
+ return -EINVAL;
+ }
+ return 0;
}
/**
--
1.8.1.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH tip/core/rcu 3/4] rcu: Make call_rcu() leak callbacks for debug-object errors
2013-04-24 0:33 ` [PATCH tip/core/rcu 1/4] rcu: Simplify debug-objects fixups Paul E. McKenney
2013-04-24 0:33 ` [PATCH tip/core/rcu 2/4] debugobjects: Make debug_object_activate() return status Paul E. McKenney
@ 2013-04-24 0:33 ` Paul E. McKenney
2013-04-24 0:33 ` [PATCH tip/core/rcu 4/4] rcu: Add duplicate-callback tests to rcutorture Paul E. McKenney
2 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2013-04-24 0:33 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, Valdis.Kletnieks, dhowells, edumazet, darren,
fweisbec, sbw, Paul E. McKenney, Mathieu Desnoyers, Sedat Dilek,
Davidlohr Bueso, Rik van Riel, Linus Torvalds
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
If someone does a duplicate call_rcu(), the worst thing the second
call_rcu() could do would be to actually queue the callback the second
time because doing so corrupts whatever list the callback was already
queued on. This commit therefore makes __call_rcu() check the new
return value from debug-objects and leak the callback upon error.
This commit also substitutes rcu_leak_callback() for whatever callback
function was previously in place in order to avoid freeing the callback
out from under any readers that might still be referencing it.
These changes increase the probability that the debug-objects error
messages will actually make it somewhere visible.
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
kernel/rcu.h | 10 +++++++---
kernel/rcutree.c | 14 +++++++++++++-
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/kernel/rcu.h b/kernel/rcu.h
index 7f8e759..ad6c3f5 100644
--- a/kernel/rcu.h
+++ b/kernel/rcu.h
@@ -67,12 +67,15 @@
extern struct debug_obj_descr rcuhead_debug_descr;
-static inline void debug_rcu_head_queue(struct rcu_head *head)
+static inline int debug_rcu_head_queue(struct rcu_head *head)
{
- debug_object_activate(head, &rcuhead_debug_descr);
+ int r1;
+
+ r1 = debug_object_activate(head, &rcuhead_debug_descr);
debug_object_active_state(head, &rcuhead_debug_descr,
STATE_RCU_HEAD_READY,
STATE_RCU_HEAD_QUEUED);
+ return r1;
}
static inline void debug_rcu_head_unqueue(struct rcu_head *head)
@@ -83,8 +86,9 @@ static inline void debug_rcu_head_unqueue(struct rcu_head *head)
debug_object_deactivate(head, &rcuhead_debug_descr);
}
#else /* !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
-static inline void debug_rcu_head_queue(struct rcu_head *head)
+static inline int debug_rcu_head_queue(struct rcu_head *head)
{
+ return 0;
}
static inline void debug_rcu_head_unqueue(struct rcu_head *head)
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index d226e55..df3094c 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -2276,6 +2276,13 @@ static void __call_rcu_core(struct rcu_state *rsp, struct rcu_data *rdp,
}
/*
+ * RCU callback function to leak a callback.
+ */
+static void rcu_leak_callback(struct rcu_head *rhp)
+{
+}
+
+/*
* Helper function for call_rcu() and friends. The cpu argument will
* normally be -1, indicating "currently running CPU". It may specify
* a CPU only if that CPU is a no-CBs CPU. Currently, only _rcu_barrier()
@@ -2289,7 +2296,12 @@ __call_rcu(struct rcu_head *head, void (*func)(struct rcu_head *rcu),
struct rcu_data *rdp;
WARN_ON_ONCE((unsigned long)head & 0x3); /* Misaligned rcu_head! */
- debug_rcu_head_queue(head);
+ if (debug_rcu_head_queue(head)) {
+ /* Probable double call_rcu(), so leak the callback. */
+ ACCESS_ONCE(head->func) = rcu_leak_callback;
+ WARN_ONCE(1, "__call_rcu(): Leaked duplicate callback\n");
+ return;
+ }
head->func = func;
head->next = NULL;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH tip/core/rcu 4/4] rcu: Add duplicate-callback tests to rcutorture
2013-04-24 0:33 ` [PATCH tip/core/rcu 1/4] rcu: Simplify debug-objects fixups Paul E. McKenney
2013-04-24 0:33 ` [PATCH tip/core/rcu 2/4] debugobjects: Make debug_object_activate() return status Paul E. McKenney
2013-04-24 0:33 ` [PATCH tip/core/rcu 3/4] rcu: Make call_rcu() leak callbacks for debug-object errors Paul E. McKenney
@ 2013-04-24 0:33 ` Paul E. McKenney
2 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2013-04-24 0:33 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, Valdis.Kletnieks, dhowells, edumazet, darren,
fweisbec, sbw, Paul E. McKenney, Mathieu Desnoyers, Sedat Dilek,
Davidlohr Bueso, Rik van Riel, Linus Torvalds
From: "Paul E. McKenney" <paulmck@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 <paulmck@linux.vnet.ibm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
kernel/rcutorture.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index b1fa551..2f34ebf 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: 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: 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
^ permalink raw reply related [flat|nested] 7+ messages in thread