[patch 0/5] RCU head debug (v5)

[patch 0/5] RCU head debug (v5)

[Mathieu Desnoyers]

Hi,

Here is an updated version of the RCU head debug. It removes all rcu head
initializations except for "on_stack" initializations. The state of rcu_head
before call_rcu() really does not matter.

The debug objects use a fixup to keep track of rcu_heads from activation
performed from call_rcu(), except for "on_stack" initialization, which needs to
be explicit.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[patch 1/5] Debugobjects transition check

[Mathieu Desnoyers]

[-- Attachment #1: debugobjects-transition-check.patch --]
[-- Type: text/plain, Size: 4834 bytes --]

Implement a basic state machine checker in the debugobjects.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Acked-by: David S. Miller <davem@davemloft.net>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: akpm@linux-foundation.org
CC: mingo@elte.hu
CC: laijs@cn.fujitsu.com
CC: dipankar@in.ibm.com
CC: josh@joshtriplett.org
CC: dvhltc@us.ibm.com
CC: niv@us.ibm.com
CC: tglx@linutronix.de
CC: peterz@infradead.org
CC: rostedt@goodmis.org
CC: Valdis.Kletnieks@vt.edu
CC: dhowells@redhat.com
CC: eric.dumazet@gmail.com
CC: Alexey Dobriyan <adobriyan@gmail.com>
---
 include/linux/debugobjects.h |   11 ++++++++
 lib/debugobjects.c           |   59 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 67 insertions(+), 3 deletions(-)

Index: linux.trees.git/include/linux/debugobjects.h
===================================================================
--- linux.trees.git.orig/include/linux/debugobjects.h	2010-03-27 18:48:23.000000000 -0400
+++ linux.trees.git/include/linux/debugobjects.h	2010-03-27 18:50:09.000000000 -0400
@@ -20,12 +20,14 @@ struct debug_obj_descr;
  * struct debug_obj - representaion of an tracked object
  * @node:	hlist node to link the object into the tracker list
  * @state:	tracked object state
+ * @astate:	current active state
  * @object:	pointer to the real object
  * @descr:	pointer to an object type specific debug description structure
  */
 struct debug_obj {
 	struct hlist_node	node;
 	enum debug_obj_state	state;
+	unsigned int		astate;
 	void			*object;
 	struct debug_obj_descr	*descr;
 };
@@ -60,6 +62,15 @@ extern void debug_object_deactivate(void
 extern void debug_object_destroy   (void *addr, struct debug_obj_descr *descr);
 extern void debug_object_free      (void *addr, struct debug_obj_descr *descr);
 
+/*
+ * Active state:
+ * - Set at 0 upon initialization.
+ * - Must return to 0 before deactivation.
+ */
+extern void
+debug_object_active_state(void *addr, struct debug_obj_descr *descr,
+			  unsigned int expect, unsigned int next);
+
 extern void debug_objects_early_init(void);
 extern void debug_objects_mem_init(void);
 #else
Index: linux.trees.git/lib/debugobjects.c
===================================================================
--- linux.trees.git.orig/lib/debugobjects.c	2010-03-27 18:48:23.000000000 -0400
+++ linux.trees.git/lib/debugobjects.c	2010-03-27 18:50:09.000000000 -0400
@@ -140,6 +140,7 @@ alloc_object(void *addr, struct debug_bu
 		obj->object = addr;
 		obj->descr  = descr;
 		obj->state  = ODEBUG_STATE_NONE;
+		obj->astate = 0;
 		hlist_del(&obj->node);
 
 		hlist_add_head(&obj->node, &b->list);
@@ -251,8 +252,10 @@ static void debug_print_object(struct de
 
 	if (limit < 5 && obj->descr != descr_test) {
 		limit++;
-		WARN(1, KERN_ERR "ODEBUG: %s %s object type: %s\n", msg,
-		       obj_states[obj->state], obj->descr->name);
+		WARN(1, KERN_ERR "ODEBUG: %s %s (active state %u) "
+				 "object type: %s\n",
+			msg, obj_states[obj->state], obj->astate,
+			obj->descr->name);
 	}
 	debug_objects_warnings++;
 }
@@ -446,7 +449,10 @@ void debug_object_deactivate(void *addr,
 		case ODEBUG_STATE_INIT:
 		case ODEBUG_STATE_INACTIVE:
 		case ODEBUG_STATE_ACTIVE:
-			obj->state = ODEBUG_STATE_INACTIVE;
+			if (!obj->astate)
+				obj->state = ODEBUG_STATE_INACTIVE;
+			else
+				debug_print_object(obj, "deactivate");
 			break;
 
 		case ODEBUG_STATE_DESTROYED:
@@ -552,6 +558,53 @@ out_unlock:
 	raw_spin_unlock_irqrestore(&db->lock, flags);
 }
 
+/**
+ * debug_object_active_state - debug checks object usage state machine
+ * @addr:	address of the object
+ * @descr:	pointer to an object specific debug description structure
+ * @expect:	expected state
+ * @next:	state to move to if expected state is found
+ */
+void
+debug_object_active_state(void *addr, struct debug_obj_descr *descr,
+			  unsigned int expect, unsigned int next)
+{
+	struct debug_bucket *db;
+	struct debug_obj *obj;
+	unsigned long flags;
+
+	if (!debug_objects_enabled)
+		return;
+
+	db = get_bucket((unsigned long) addr);
+
+	raw_spin_lock_irqsave(&db->lock, flags);
+
+	obj = lookup_object(addr, db);
+	if (obj) {
+		switch (obj->state) {
+		case ODEBUG_STATE_ACTIVE:
+			if (obj->astate == expect)
+				obj->astate = next;
+			else
+				debug_print_object(obj, "active_state");
+			break;
+
+		default:
+			debug_print_object(obj, "active_state");
+			break;
+		}
+	} else {
+		struct debug_obj o = { .object = addr,
+				       .state = ODEBUG_STATE_NOTAVAILABLE,
+				       .descr = descr };
+
+		debug_print_object(&o, "active_state");
+	}
+
+	raw_spin_unlock_irqrestore(&db->lock, flags);
+}
+
 #ifdef CONFIG_DEBUG_OBJECTS_FREE
 static void __debug_check_no_obj_freed(const void *address, unsigned long size)
 {

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [patch 1/5] Debugobjects transition check

[Thomas Gleixner]

On Mon, 29 Mar 2010, Mathieu Desnoyers wrote:

> Implement a basic state machine checker in the debugobjects.

Can you please add some real explanation how that checker works and
why we want to have it ?

Thanks,

	tglx

Re: [patch 1/5] Debugobjects transition check

[Mathieu Desnoyers]

* Thomas Gleixner (tglx@linutronix.de) wrote:
> On Mon, 29 Mar 2010, Mathieu Desnoyers wrote:
> 
> > Implement a basic state machine checker in the debugobjects.
> 
> Can you please add some real explanation how that checker works and
> why we want to have it ?

We can add this to the changelog. Is it worth it to create a Documentation file
for it ?


This state machine checker detects races and inconsistencies within the "active"
life of a debugobject. The checker only keeps track of the current state; all
the state machine logic is kept at the object instance level.

The checker works by adding a supplementary "unsigned int astate" field to the
debug_obj structure. It keeps track of the current "active state" of the object.

The only constraints that are imposed on the states by the debugobjects system
is that:

- activation of an object sets the current active state to 0,
- deactivation of an object expects the current active state to be 0.

For the rest of the states, the state mapping is determined by the specific
object instance. Therefore, the logic keeping track of the state machine is
within the specialized instance, without any need to know about it at the
debugobject level.

The current object active state is changed by calling:

debug_object_active_state(addr, descr, expect, next)

where "expect" is the expected state and "next" is the next state to move to if
the expected state is found. A warning is generated if the expected is not
found.


Thanks,

Mathieu

> 
> Thanks,
> 
> 	tglx
> 
> 
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [patch 1/5] Debugobjects transition check

[Thomas Gleixner]

On Wed, 31 Mar 2010, Mathieu Desnoyers wrote:

> * Thomas Gleixner (tglx@linutronix.de) wrote:
> > On Mon, 29 Mar 2010, Mathieu Desnoyers wrote:
> > 
> > > Implement a basic state machine checker in the debugobjects.
> > 
> > Can you please add some real explanation how that checker works and
> > why we want to have it ?
> 
> We can add this to the changelog. Is it worth it to create a Documentation file
> for it ?

I meant the changelog. The "Implement ...." line was not really helpful :)
 
> 
> This state machine checker detects races and inconsistencies within the "active"
> life of a debugobject. The checker only keeps track of the current state; all
> the state machine logic is kept at the object instance level.
> 
> The checker works by adding a supplementary "unsigned int astate" field to the
> debug_obj structure. It keeps track of the current "active state" of the object.
> 
> The only constraints that are imposed on the states by the debugobjects system
> is that:
> 
> - activation of an object sets the current active state to 0,
> - deactivation of an object expects the current active state to be 0.
> 
> For the rest of the states, the state mapping is determined by the specific
> object instance. Therefore, the logic keeping track of the state machine is
> within the specialized instance, without any need to know about it at the
> debugobject level.
> 
> The current object active state is changed by calling:
> 
> debug_object_active_state(addr, descr, expect, next)
> 
> where "expect" is the expected state and "next" is the next state to move to if
> the expected state is found. A warning is generated if the expected is not
> found.

Does it only warn or is there a callback to fixup things as well ?

Thanks,

	tglx

Re: [patch 1/5] Debugobjects transition check

[Mathieu Desnoyers]

* Thomas Gleixner (tglx@linutronix.de) wrote:
> On Wed, 31 Mar 2010, Mathieu Desnoyers wrote:
> 
> > * Thomas Gleixner (tglx@linutronix.de) wrote:
> > > On Mon, 29 Mar 2010, Mathieu Desnoyers wrote:
> > > 
> > > > Implement a basic state machine checker in the debugobjects.
> > > 
> > > Can you please add some real explanation how that checker works and
> > > why we want to have it ?
> > 
> > We can add this to the changelog. Is it worth it to create a Documentation file
> > for it ?
> 
> I meant the changelog. The "Implement ...." line was not really helpful :)
>  
> > 
> > This state machine checker detects races and inconsistencies within the "active"
> > life of a debugobject. The checker only keeps track of the current state; all
> > the state machine logic is kept at the object instance level.
> > 
> > The checker works by adding a supplementary "unsigned int astate" field to the
> > debug_obj structure. It keeps track of the current "active state" of the object.
> > 
> > The only constraints that are imposed on the states by the debugobjects system
> > is that:
> > 
> > - activation of an object sets the current active state to 0,
> > - deactivation of an object expects the current active state to be 0.
> > 
> > For the rest of the states, the state mapping is determined by the specific
> > object instance. Therefore, the logic keeping track of the state machine is
> > within the specialized instance, without any need to know about it at the
> > debugobject level.
> > 
> > The current object active state is changed by calling:
> > 
> > debug_object_active_state(addr, descr, expect, next)
> > 
> > where "expect" is the expected state and "next" is the next state to move to if
> > the expected state is found. A warning is generated if the expected is not
> > found.
> 
> Does it only warn or is there a callback to fixup things as well ?

For the moment, it only warns. I have not seen the need for a fixup callback
yet. It might become useful at some point, but I prefer to proceed
incrementally. This kind of callback could become quite big too, because it
would have to deal with transitions "from each to each" states of the system,
with, in the worse case scenario, different fixups for each situation.

Just for the specific case of "do RCU batch", when detecting that a non-queued
rcu head is there for execution, there are a few cases to consider:

- List corruption
  - Appears in two lists.
  - Appears in the same list twice.
- Race (two threads reading the list at the same time).
- ...

I am probably forgetting about others. So one way to fixup this would be not to
execute the callback, but even then, the lists might be corrupted. So it's not
at all clear to me if we can do much better than reporting the inconsistency
without increasing intrusiveness. But maybe I just need more imagination. ;)

Thanks,

Mathieu

> 
> Thanks,
> 
> 	tglx

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [patch 1/5] Debugobjects transition check

[Thomas Gleixner]

On Wed, 31 Mar 2010, Mathieu Desnoyers wrote:
> > > where "expect" is the expected state and "next" is the next state to move to if
> > > the expected state is found. A warning is generated if the expected is not
> > > found.
> > 
> > Does it only warn or is there a callback to fixup things as well ?
> 
> For the moment, it only warns. I have not seen the need for a fixup callback
> yet. It might become useful at some point, but I prefer to proceed
> incrementally. This kind of callback could become quite big too, because it
> would have to deal with transitions "from each to each" states of the system,
> with, in the worse case scenario, different fixups for each situation.
> 
> Just for the specific case of "do RCU batch", when detecting that a non-queued
> rcu head is there for execution, there are a few cases to consider:
> 
> - List corruption
>   - Appears in two lists.
>   - Appears in the same list twice.
> - Race (two threads reading the list at the same time).
> - ...
> 
> I am probably forgetting about others. So one way to fixup this would be not to
> execute the callback, but even then, the lists might be corrupted. So it's not
> at all clear to me if we can do much better than reporting the inconsistency
> without increasing intrusiveness. But maybe I just need more imagination. ;)

Yes, that sounds tricky to implement and maybe not worth the
effort. Can you resend the debugobjects patch with a full changelog
please ?

Thanks,

	tglx

[patch 1/5] Debugobjects transition check (updated changelog)

[Mathieu Desnoyers]

Implement a basic state machine checker in the debugobjects.

This state machine checker detects races and inconsistencies within the "active"
life of a debugobject. The checker only keeps track of the current state; all
the state machine logic is kept at the object instance level.

The checker works by adding a supplementary "unsigned int astate" field to the
debug_obj structure. It keeps track of the current "active state" of the object.

The only constraints that are imposed on the states by the debugobjects system
is that:

- activation of an object sets the current active state to 0,
- deactivation of an object expects the current active state to be 0.

For the rest of the states, the state mapping is determined by the specific
object instance. Therefore, the logic keeping track of the state machine is
within the specialized instance, without any need to know about it at the
debugobject level.

The current object active state is changed by calling:

debug_object_active_state(addr, descr, expect, next)

where "expect" is the expected state and "next" is the next state to move to if
the expected state is found. A warning is generated if the expected is not
found.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Acked-by: David S. Miller <davem@davemloft.net>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: akpm@linux-foundation.org
CC: mingo@elte.hu
CC: laijs@cn.fujitsu.com
CC: dipankar@in.ibm.com
CC: josh@joshtriplett.org
CC: dvhltc@us.ibm.com
CC: niv@us.ibm.com
CC: tglx@linutronix.de
CC: peterz@infradead.org
CC: rostedt@goodmis.org
CC: Valdis.Kletnieks@vt.edu
CC: dhowells@redhat.com
CC: eric.dumazet@gmail.com
CC: Alexey Dobriyan <adobriyan@gmail.com>
---
 include/linux/debugobjects.h |   11 ++++++++
 lib/debugobjects.c           |   59 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 67 insertions(+), 3 deletions(-)

Index: linux.trees.git/include/linux/debugobjects.h
===================================================================
--- linux.trees.git.orig/include/linux/debugobjects.h	2010-03-27 18:48:23.000000000 -0400
+++ linux.trees.git/include/linux/debugobjects.h	2010-03-27 18:50:09.000000000 -0400
@@ -20,12 +20,14 @@ struct debug_obj_descr;
  * struct debug_obj - representaion of an tracked object
  * @node:	hlist node to link the object into the tracker list
  * @state:	tracked object state
+ * @astate:	current active state
  * @object:	pointer to the real object
  * @descr:	pointer to an object type specific debug description structure
  */
 struct debug_obj {
 	struct hlist_node	node;
 	enum debug_obj_state	state;
+	unsigned int		astate;
 	void			*object;
 	struct debug_obj_descr	*descr;
 };
@@ -60,6 +62,15 @@ extern void debug_object_deactivate(void
 extern void debug_object_destroy   (void *addr, struct debug_obj_descr *descr);
 extern void debug_object_free      (void *addr, struct debug_obj_descr *descr);
 
+/*
+ * Active state:
+ * - Set at 0 upon initialization.
+ * - Must return to 0 before deactivation.
+ */
+extern void
+debug_object_active_state(void *addr, struct debug_obj_descr *descr,
+			  unsigned int expect, unsigned int next);
+
 extern void debug_objects_early_init(void);
 extern void debug_objects_mem_init(void);
 #else
Index: linux.trees.git/lib/debugobjects.c
===================================================================
--- linux.trees.git.orig/lib/debugobjects.c	2010-03-27 18:48:23.000000000 -0400
+++ linux.trees.git/lib/debugobjects.c	2010-03-27 18:50:09.000000000 -0400
@@ -140,6 +140,7 @@ alloc_object(void *addr, struct debug_bu
 		obj->object = addr;
 		obj->descr  = descr;
 		obj->state  = ODEBUG_STATE_NONE;
+		obj->astate = 0;
 		hlist_del(&obj->node);
 
 		hlist_add_head(&obj->node, &b->list);
@@ -251,8 +252,10 @@ static void debug_print_object(struct de
 
 	if (limit < 5 && obj->descr != descr_test) {
 		limit++;
-		WARN(1, KERN_ERR "ODEBUG: %s %s object type: %s\n", msg,
-		       obj_states[obj->state], obj->descr->name);
+		WARN(1, KERN_ERR "ODEBUG: %s %s (active state %u) "
+				 "object type: %s\n",
+			msg, obj_states[obj->state], obj->astate,
+			obj->descr->name);
 	}
 	debug_objects_warnings++;
 }
@@ -446,7 +449,10 @@ void debug_object_deactivate(void *addr,
 		case ODEBUG_STATE_INIT:
 		case ODEBUG_STATE_INACTIVE:
 		case ODEBUG_STATE_ACTIVE:
-			obj->state = ODEBUG_STATE_INACTIVE;
+			if (!obj->astate)
+				obj->state = ODEBUG_STATE_INACTIVE;
+			else
+				debug_print_object(obj, "deactivate");
 			break;
 
 		case ODEBUG_STATE_DESTROYED:
@@ -552,6 +558,53 @@ out_unlock:
 	raw_spin_unlock_irqrestore(&db->lock, flags);
 }
 
+/**
+ * debug_object_active_state - debug checks object usage state machine
+ * @addr:	address of the object
+ * @descr:	pointer to an object specific debug description structure
+ * @expect:	expected state
+ * @next:	state to move to if expected state is found
+ */
+void
+debug_object_active_state(void *addr, struct debug_obj_descr *descr,
+			  unsigned int expect, unsigned int next)
+{
+	struct debug_bucket *db;
+	struct debug_obj *obj;
+	unsigned long flags;
+
+	if (!debug_objects_enabled)
+		return;
+
+	db = get_bucket((unsigned long) addr);
+
+	raw_spin_lock_irqsave(&db->lock, flags);
+
+	obj = lookup_object(addr, db);
+	if (obj) {
+		switch (obj->state) {
+		case ODEBUG_STATE_ACTIVE:
+			if (obj->astate == expect)
+				obj->astate = next;
+			else
+				debug_print_object(obj, "active_state");
+			break;
+
+		default:
+			debug_print_object(obj, "active_state");
+			break;
+		}
+	} else {
+		struct debug_obj o = { .object = addr,
+				       .state = ODEBUG_STATE_NOTAVAILABLE,
+				       .descr = descr };
+
+		debug_print_object(&o, "active_state");
+	}
+
+	raw_spin_unlock_irqrestore(&db->lock, flags);
+}
+
 #ifdef CONFIG_DEBUG_OBJECTS_FREE
 static void __debug_check_no_obj_freed(const void *address, unsigned long size)
 {
-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [patch 1/5] Debugobjects transition check (updated changelog)

[Thomas Gleixner]

B1;2005;0cOn Tue, 6 Apr 2010, Mathieu Desnoyers wrote:
> Implement a basic state machine checker in the debugobjects.
> 
> This state machine checker detects races and inconsistencies within the "active"
> life of a debugobject. The checker only keeps track of the current state; all
> the state machine logic is kept at the object instance level.
> 
> The checker works by adding a supplementary "unsigned int astate" field to the
> debug_obj structure. It keeps track of the current "active state" of the object.
> 
> The only constraints that are imposed on the states by the debugobjects system
> is that:
> 
> - activation of an object sets the current active state to 0,
> - deactivation of an object expects the current active state to be 0.
> 
> For the rest of the states, the state mapping is determined by the specific
> object instance. Therefore, the logic keeping track of the state machine is
> within the specialized instance, without any need to know about it at the
> debugobject level.
> 
> The current object active state is changed by calling:
> 
> debug_object_active_state(addr, descr, expect, next)
> 
> where "expect" is the expected state and "next" is the next state to move to if
> the expected state is found. A warning is generated if the expected is not
> found.

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

Please feel free to pull that into the rcu branch where the users are
going to be.

Thanks,

	tglx

Re: [patch 1/5] Debugobjects transition check (updated changelog)

[Paul E. McKenney]

On Thu, Apr 15, 2010 at 08:32:05PM +0200, Thomas Gleixner wrote:
> B1;2005;0cOn Tue, 6 Apr 2010, Mathieu Desnoyers wrote:
> > Implement a basic state machine checker in the debugobjects.
> > 
> > This state machine checker detects races and inconsistencies within the "active"
> > life of a debugobject. The checker only keeps track of the current state; all
> > the state machine logic is kept at the object instance level.
> > 
> > The checker works by adding a supplementary "unsigned int astate" field to the
> > debug_obj structure. It keeps track of the current "active state" of the object.
> > 
> > The only constraints that are imposed on the states by the debugobjects system
> > is that:
> > 
> > - activation of an object sets the current active state to 0,
> > - deactivation of an object expects the current active state to be 0.
> > 
> > For the rest of the states, the state mapping is determined by the specific
> > object instance. Therefore, the logic keeping track of the state machine is
> > within the specialized instance, without any need to know about it at the
> > debugobject level.
> > 
> > The current object active state is changed by calling:
> > 
> > debug_object_active_state(addr, descr, expect, next)
> > 
> > where "expect" is the expected state and "next" is the next state to move to if
> > the expected state is found. A warning is generated if the expected is not
> > found.
> 
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> 
> Please feel free to pull that into the rcu branch where the users are
> going to be.

Thank you, Thomas!!!

							Thanx, Paul

[patch 2/5] rcu head introduce rcu head init on stack

[Mathieu Desnoyers]

[-- Attachment #1: rcu-head-introduce-rcu-head-init.patch --]
[-- Type: text/plain, Size: 1680 bytes --]

PEM:
o     Would it be possible to make this bisectable as follows?

      a.      Insert a new patch after current patch 4/6 that
              defines destroy_rcu_head_on_stack(),
              init_rcu_head_on_stack(), and init_rcu_head() with
              their !CONFIG_DEBUG_OBJECTS_RCU_HEAD definitions.

This patch performs this transition.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: David S. Miller <davem@davemloft.net>
CC: akpm@linux-foundation.org
CC: mingo@elte.hu
CC: laijs@cn.fujitsu.com
CC: dipankar@in.ibm.com
CC: josh@joshtriplett.org
CC: dvhltc@us.ibm.com
CC: niv@us.ibm.com
CC: tglx@linutronix.de
CC: peterz@infradead.org
CC: rostedt@goodmis.org
CC: Valdis.Kletnieks@vt.edu
CC: dhowells@redhat.com
CC: eric.dumazet@gmail.com
CC: Alexey Dobriyan <adobriyan@gmail.com>
---
 include/linux/rcupdate.h |    8 ++++++++
 1 file changed, 8 insertions(+)

Index: linux.trees.git/include/linux/rcupdate.h
===================================================================
--- linux.trees.git.orig/include/linux/rcupdate.h	2010-03-29 09:43:35.000000000 -0400
+++ linux.trees.git/include/linux/rcupdate.h	2010-03-29 09:57:09.000000000 -0400
@@ -83,6 +83,14 @@ extern void rcu_scheduler_starting(void)
        (ptr)->next = NULL; (ptr)->func = NULL; \
 } while (0)
 
+static inline void init_rcu_head_on_stack(struct rcu_head *head)
+{
+}
+
+static inline void destroy_rcu_head_on_stack(struct rcu_head *head)
+{
+}
+
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 
 extern struct lockdep_map rcu_lock_map;

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[patch 3/5] remove all rcu head initializations, except on_stack initializations

[Mathieu Desnoyers]

[-- Attachment #1: remove-all-rcu-head-initializations.patch --]
[-- Type: text/plain, Size: 12710 bytes --]

Remove all rcu head inits. We don't care about the RCU head state before passing
it to call_rcu() anyway. Only leave the "on_stack" variants so debugobjects can
keep track of objects on stack.

This patch applies to current -tip based on 2.6.34-rc2.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: David S. Miller <davem@davemloft.net>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: akpm@linux-foundation.org
CC: mingo@elte.hu
CC: laijs@cn.fujitsu.com
CC: dipankar@in.ibm.com
CC: josh@joshtriplett.org
CC: dvhltc@us.ibm.com
CC: niv@us.ibm.com
CC: tglx@linutronix.de
CC: peterz@infradead.org
CC: rostedt@goodmis.org
CC: Valdis.Kletnieks@vt.edu
CC: dhowells@redhat.com
CC: eric.dumazet@gmail.com
CC: Alexey Dobriyan <adobriyan@gmail.com>
---
 arch/powerpc/mm/pgtable.c                   |    1 
 block/cfq-iosched.c                         |    1 
 block/genhd.c                               |    1 
 drivers/staging/batman-adv/hard-interface.c |    1 
 fs/file.c                                   |    3 --
 fs/fs-writeback.c                           |   31 +++++++++++++++++++++++-----
 fs/partitions/check.c                       |    1 
 include/linux/init_task.h                   |    1 
 kernel/rcutiny.c                            |    6 +++++
 kernel/rcutorture.c                         |    2 +
 kernel/rcutree.c                            |    4 +++
 mm/backing-dev.c                            |    1 
 mm/slob.c                                   |    1 
 security/selinux/avc.c                      |    1 
 security/selinux/netnode.c                  |    2 -
 15 files changed, 38 insertions(+), 19 deletions(-)

Index: linux.trees.git/kernel/rcutiny.c
===================================================================
--- linux.trees.git.orig/kernel/rcutiny.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/kernel/rcutiny.c	2010-03-29 10:18:05.000000000 -0400
@@ -244,11 +244,13 @@ void rcu_barrier(void)
 {
 	struct rcu_synchronize rcu;
 
+	init_rcu_head_on_stack(&rcu.head);
 	init_completion(&rcu.completion);
 	/* Will wake me after RCU finished. */
 	call_rcu(&rcu.head, wakeme_after_rcu);
 	/* Wait for it. */
 	wait_for_completion(&rcu.completion);
+	destroy_rcu_head_on_stack(&rcu.head);
 }
 EXPORT_SYMBOL_GPL(rcu_barrier);
 
@@ -256,11 +258,13 @@ void rcu_barrier_bh(void)
 {
 	struct rcu_synchronize rcu;
 
+	init_rcu_head_on_stack(&rcu.head);
 	init_completion(&rcu.completion);
 	/* Will wake me after RCU finished. */
 	call_rcu_bh(&rcu.head, wakeme_after_rcu);
 	/* Wait for it. */
 	wait_for_completion(&rcu.completion);
+	destroy_rcu_head_on_stack(&rcu.head);
 }
 EXPORT_SYMBOL_GPL(rcu_barrier_bh);
 
@@ -268,11 +272,13 @@ void rcu_barrier_sched(void)
 {
 	struct rcu_synchronize rcu;
 
+	init_rcu_head_on_stack(&rcu.head);
 	init_completion(&rcu.completion);
 	/* Will wake me after RCU finished. */
 	call_rcu_sched(&rcu.head, wakeme_after_rcu);
 	/* Wait for it. */
 	wait_for_completion(&rcu.completion);
+	destroy_rcu_head_on_stack(&rcu.head);
 }
 EXPORT_SYMBOL_GPL(rcu_barrier_sched);
 
Index: linux.trees.git/kernel/rcutorture.c
===================================================================
--- linux.trees.git.orig/kernel/rcutorture.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/kernel/rcutorture.c	2010-03-29 10:18:05.000000000 -0400
@@ -464,9 +464,11 @@ static void rcu_bh_torture_synchronize(v
 {
 	struct rcu_bh_torture_synchronize rcu;
 
+	init_rcu_head_on_stack(&rcu.head);
 	init_completion(&rcu.completion);
 	call_rcu_bh(&rcu.head, rcu_bh_torture_wakeme_after_cb);
 	wait_for_completion(&rcu.completion);
+	destroy_rcu_head_on_stack(&rcu.head);
 }
 
 static struct rcu_torture_ops rcu_bh_ops = {
Index: linux.trees.git/kernel/rcutree.c
===================================================================
--- linux.trees.git.orig/kernel/rcutree.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/kernel/rcutree.c	2010-03-29 10:18:05.000000000 -0400
@@ -1449,11 +1449,13 @@ void synchronize_sched(void)
 	if (rcu_blocking_is_gp())
 		return;
 
+	init_rcu_head_on_stack(&rcu.head);
 	init_completion(&rcu.completion);
 	/* Will wake me after RCU finished. */
 	call_rcu_sched(&rcu.head, wakeme_after_rcu);
 	/* Wait for it. */
 	wait_for_completion(&rcu.completion);
+	destroy_rcu_head_on_stack(&rcu.head);
 }
 EXPORT_SYMBOL_GPL(synchronize_sched);
 
@@ -1473,11 +1475,13 @@ void synchronize_rcu_bh(void)
 	if (rcu_blocking_is_gp())
 		return;
 
+	init_rcu_head_on_stack(&rcu.head);
 	init_completion(&rcu.completion);
 	/* Will wake me after RCU finished. */
 	call_rcu_bh(&rcu.head, wakeme_after_rcu);
 	/* Wait for it. */
 	wait_for_completion(&rcu.completion);
+	destroy_rcu_head_on_stack(&rcu.head);
 }
 EXPORT_SYMBOL_GPL(synchronize_rcu_bh);
 
Index: linux.trees.git/mm/backing-dev.c
===================================================================
--- linux.trees.git.orig/mm/backing-dev.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/mm/backing-dev.c	2010-03-29 10:18:05.000000000 -0400
@@ -653,7 +653,6 @@ int bdi_init(struct backing_dev_info *bd
 	bdi->max_ratio = 100;
 	bdi->max_prop_frac = PROP_FRAC_BASE;
 	spin_lock_init(&bdi->wb_lock);
-	INIT_RCU_HEAD(&bdi->rcu_head);
 	INIT_LIST_HEAD(&bdi->bdi_list);
 	INIT_LIST_HEAD(&bdi->wb_list);
 	INIT_LIST_HEAD(&bdi->work_list);
Index: linux.trees.git/mm/slob.c
===================================================================
--- linux.trees.git.orig/mm/slob.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/mm/slob.c	2010-03-29 10:18:05.000000000 -0400
@@ -647,7 +647,6 @@ void kmem_cache_free(struct kmem_cache *
 	if (unlikely(c->flags & SLAB_DESTROY_BY_RCU)) {
 		struct slob_rcu *slob_rcu;
 		slob_rcu = b + (c->size - sizeof(struct slob_rcu));
-		INIT_RCU_HEAD(&slob_rcu->head);
 		slob_rcu->size = c->size;
 		call_rcu(&slob_rcu->head, kmem_rcu_free);
 	} else {
Index: linux.trees.git/fs/file.c
===================================================================
--- linux.trees.git.orig/fs/file.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/fs/file.c	2010-03-29 10:18:05.000000000 -0400
@@ -178,7 +178,6 @@ static struct fdtable * alloc_fdtable(un
 	fdt->open_fds = (fd_set *)data;
 	data += nr / BITS_PER_BYTE;
 	fdt->close_on_exec = (fd_set *)data;
-	INIT_RCU_HEAD(&fdt->rcu);
 	fdt->next = NULL;
 
 	return fdt;
@@ -312,7 +311,6 @@ struct files_struct *dup_fd(struct files
 	new_fdt->close_on_exec = (fd_set *)&newf->close_on_exec_init;
 	new_fdt->open_fds = (fd_set *)&newf->open_fds_init;
 	new_fdt->fd = &newf->fd_array[0];
-	INIT_RCU_HEAD(&new_fdt->rcu);
 	new_fdt->next = NULL;
 
 	spin_lock(&oldf->file_lock);
@@ -430,7 +428,6 @@ struct files_struct init_files = {
 		.fd		= &init_files.fd_array[0],
 		.close_on_exec	= (fd_set *)&init_files.close_on_exec_init,
 		.open_fds	= (fd_set *)&init_files.open_fds_init,
-		.rcu		= RCU_HEAD_INIT,
 	},
 	.file_lock	= __SPIN_LOCK_UNLOCKED(init_task.file_lock),
 };
Index: linux.trees.git/fs/fs-writeback.c
===================================================================
--- linux.trees.git.orig/fs/fs-writeback.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/fs/fs-writeback.c	2010-03-29 10:18:05.000000000 -0400
@@ -74,12 +74,33 @@ static inline bool bdi_work_on_stack(str
 	return test_bit(WS_ONSTACK_B, &work->state);
 }
 
-static inline void bdi_work_init(struct bdi_work *work,
-				 struct wb_writeback_args *args)
+static inline void __bdi_work_init(struct bdi_work *work,
+				   struct wb_writeback_args *args,
+				   int on_stack)
 {
-	INIT_RCU_HEAD(&work->rcu_head);
 	work->args = *args;
 	work->state = WS_USED;
+	if (on_stack) {
+		work->state |= WS_ONSTACK;
+		init_rcu_head_on_stack(&work->rcu_head);
+	}
+}
+
+static inline void bdi_work_init(struct bdi_work *work,
+				 struct wb_writeback_args *args)
+{
+	__bdi_work_init(work, args, false);
+}
+
+static inline void bdi_work_init_on_stack(struct bdi_work *work,
+					  struct wb_writeback_args *args)
+{
+	__bdi_work_init(work, args, true);
+}
+
+static inline void bdi_destroy_work_on_stack(struct bdi_work *work)
+{
+	destroy_rcu_head_on_stack(&work->rcu_head);
 }
 
 /**
@@ -232,11 +253,11 @@ static void bdi_sync_writeback(struct ba
 	};
 	struct bdi_work work;
 
-	bdi_work_init(&work, &args);
-	work.state |= WS_ONSTACK;
+	bdi_work_init_on_stack(&work, &args);
 
 	bdi_queue_work(bdi, &work);
 	bdi_wait_on_work_clear(&work);
+	bdi_destroy_work_on_stack(&work);
 }
 
 /**
Index: linux.trees.git/fs/partitions/check.c
===================================================================
--- linux.trees.git.orig/fs/partitions/check.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/fs/partitions/check.c	2010-03-29 10:18:05.000000000 -0400
@@ -455,7 +455,6 @@ struct hd_struct *add_partition(struct g
 	}
 
 	/* everything is up and running, commence */
-	INIT_RCU_HEAD(&p->rcu_head);
 	rcu_assign_pointer(ptbl->part[partno], p);
 
 	/* suppress uevent if the disk supresses it */
Index: linux.trees.git/block/cfq-iosched.c
===================================================================
--- linux.trees.git.orig/block/cfq-iosched.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/block/cfq-iosched.c	2010-03-29 10:18:05.000000000 -0400
@@ -3719,7 +3719,6 @@ static void *cfq_init_queue(struct reque
 	 * second, in order to have larger depth for async operations.
 	 */
 	cfqd->last_delayed_sync = jiffies - HZ;
-	INIT_RCU_HEAD(&cfqd->rcu);
 	return cfqd;
 }
 
Index: linux.trees.git/block/genhd.c
===================================================================
--- linux.trees.git.orig/block/genhd.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/block/genhd.c	2010-03-29 10:18:05.000000000 -0400
@@ -987,7 +987,6 @@ int disk_expand_part_tbl(struct gendisk 
 	if (!new_ptbl)
 		return -ENOMEM;
 
-	INIT_RCU_HEAD(&new_ptbl->rcu_head);
 	new_ptbl->len = target;
 
 	for (i = 0; i < len; i++)
Index: linux.trees.git/arch/powerpc/mm/pgtable.c
===================================================================
--- linux.trees.git.orig/arch/powerpc/mm/pgtable.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/arch/powerpc/mm/pgtable.c	2010-03-29 10:18:05.000000000 -0400
@@ -91,7 +91,6 @@ static void pte_free_rcu_callback(struct
 
 static void pte_free_submit(struct pte_freelist_batch *batch)
 {
-	INIT_RCU_HEAD(&batch->rcu);
 	call_rcu(&batch->rcu, pte_free_rcu_callback);
 }
 
Index: linux.trees.git/security/selinux/avc.c
===================================================================
--- linux.trees.git.orig/security/selinux/avc.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/security/selinux/avc.c	2010-03-29 10:18:05.000000000 -0400
@@ -288,7 +288,6 @@ static struct avc_node *avc_alloc_node(v
 	if (!node)
 		goto out;
 
-	INIT_RCU_HEAD(&node->rhead);
 	INIT_HLIST_NODE(&node->list);
 	avc_cache_stats_incr(allocations);
 
Index: linux.trees.git/security/selinux/netnode.c
===================================================================
--- linux.trees.git.orig/security/selinux/netnode.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/security/selinux/netnode.c	2010-03-29 10:18:05.000000000 -0400
@@ -182,8 +182,6 @@ static void sel_netnode_insert(struct se
 		BUG();
 	}
 
-	INIT_RCU_HEAD(&node->rcu);
-
 	/* we need to impose a limit on the growth of the hash table so check
 	 * this bucket to make sure it is within the specified bounds */
 	list_add_rcu(&node->list, &sel_netnode_hash[idx].list);
Index: linux.trees.git/drivers/staging/batman-adv/hard-interface.c
===================================================================
--- linux.trees.git.orig/drivers/staging/batman-adv/hard-interface.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/drivers/staging/batman-adv/hard-interface.c	2010-03-29 10:18:05.000000000 -0400
@@ -301,7 +301,6 @@ int hardif_add_interface(char *dev, int 
 	batman_if->if_num = if_num;
 	batman_if->dev = dev;
 	batman_if->if_active = IF_INACTIVE;
-	INIT_RCU_HEAD(&batman_if->rcu);
 
 	printk(KERN_INFO "batman-adv:Adding interface: %s\n", dev);
 	avail_ifs++;
Index: linux.trees.git/include/linux/init_task.h
===================================================================
--- linux.trees.git.orig/include/linux/init_task.h	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/include/linux/init_task.h	2010-03-29 10:18:05.000000000 -0400
@@ -49,7 +49,6 @@ extern struct group_info init_groups;
 		{ .first = &init_task.pids[PIDTYPE_PGID].node },	\
 		{ .first = &init_task.pids[PIDTYPE_SID].node },		\
 	},								\
-	.rcu		= RCU_HEAD_INIT,				\
 	.level		= 0,						\
 	.numbers	= { {						\
 		.nr		= 0,					\

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[patch 4/5] rcu head remove init

[Mathieu Desnoyers]

[-- Attachment #1: rcu-head-remove-init.patch --]
[-- Type: text/plain, Size: 1634 bytes --]

RCU heads really don't need to be initialized. Their state before call_rcu()
really does not matter.

We need to keep init/destroy_rcu_head_on_stack() though, since we want
debugobjects to be able to keep track of these objects.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: David S. Miller <davem@davemloft.net>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: akpm@linux-foundation.org
CC: mingo@elte.hu
CC: laijs@cn.fujitsu.com
CC: dipankar@in.ibm.com
CC: josh@joshtriplett.org
CC: dvhltc@us.ibm.com
CC: niv@us.ibm.com
CC: tglx@linutronix.de
CC: peterz@infradead.org
CC: rostedt@goodmis.org
CC: Valdis.Kletnieks@vt.edu
CC: dhowells@redhat.com
CC: eric.dumazet@gmail.com
CC: Alexey Dobriyan <adobriyan@gmail.com>
---
 include/linux/rcupdate.h |    6 ------
 1 file changed, 6 deletions(-)

Index: linux.trees.git/include/linux/rcupdate.h
===================================================================
--- linux.trees.git.orig/include/linux/rcupdate.h	2010-03-29 10:11:32.000000000 -0400
+++ linux.trees.git/include/linux/rcupdate.h	2010-03-29 10:11:42.000000000 -0400
@@ -77,12 +77,6 @@ extern void rcu_scheduler_starting(void)
 #error "Unknown RCU implementation specified to kernel configuration"
 #endif
 
-#define RCU_HEAD_INIT	{ .next = NULL, .func = NULL }
-#define RCU_HEAD(head) struct rcu_head head = RCU_HEAD_INIT
-#define INIT_RCU_HEAD(ptr) do { \
-       (ptr)->next = NULL; (ptr)->func = NULL; \
-} while (0)
-
 static inline void init_rcu_head_on_stack(struct rcu_head *head)
 {
 }

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[patch 5/5] tree/tiny rcu: Add debug RCU head objects (v5)

[Mathieu Desnoyers]

[-- Attachment #1: rcu-head-debug.patch --]
[-- Type: text/plain, Size: 10937 bytes --]

Helps finding racy users of call_rcu(), which results in hangs because list
entries are overwritten and/or skipped.

Changelog since v4:
- Bissectability is now OK
- Now generate a WARN_ON_ONCE() for non-initialized rcu_head passed to
  call_rcu(). Statically initialized objects are detected with
  object_is_static().
- Rename rcu_head_init_on_stack to init_rcu_head_on_stack.
- Remove init_rcu_head() completely.

Changelog since v3:
- Include comments from Lai Jiangshan

This new patch version is based on the debugobjects with the newly introduced
"active state" tracker.

Non-initialized entries are all considered as "statically initialized". An
activation fixup (triggered by call_rcu()) takes care of performing the debug
object initialization without issuing any warning. Since we cannot increase the
size of struct rcu_head, I don't see much room to put an identifier for
statically initialized rcu_head structures. So for now, we have to live without
"activation without explicit init" detection. But the main purpose of this debug
option is to detect double-activations (double call_rcu() use of a rcu_head
before the callback is executed), which is correctly addressed here.

This also detects potential internal RCU callback corruption, which would cause
the callbacks to be executed twice.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: David S. Miller <davem@davemloft.net>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: akpm@linux-foundation.org
CC: mingo@elte.hu
CC: laijs@cn.fujitsu.com
CC: dipankar@in.ibm.com
CC: josh@joshtriplett.org
CC: dvhltc@us.ibm.com
CC: niv@us.ibm.com
CC: tglx@linutronix.de
CC: peterz@infradead.org
CC: rostedt@goodmis.org
CC: Valdis.Kletnieks@vt.edu
CC: dhowells@redhat.com
CC: eric.dumazet@gmail.com
CC: Alexey Dobriyan <adobriyan@gmail.com>
---
 include/linux/rcupdate.h |   49 +++++++++++++++
 kernel/rcupdate.c        |  149 +++++++++++++++++++++++++++++++++++++++++++++++
 kernel/rcutiny.c         |    2 
 kernel/rcutree.c         |    2 
 lib/Kconfig.debug        |    6 +
 5 files changed, 208 insertions(+)

Index: linux.trees.git/include/linux/rcupdate.h
===================================================================
--- linux.trees.git.orig/include/linux/rcupdate.h	2010-03-29 10:19:01.000000000 -0400
+++ linux.trees.git/include/linux/rcupdate.h	2010-03-29 10:19:18.000000000 -0400
@@ -40,6 +40,7 @@
 #include <linux/seqlock.h>
 #include <linux/lockdep.h>
 #include <linux/completion.h>
+#include <linux/debugobjects.h>
 
 #ifdef CONFIG_RCU_TORTURE_TEST
 extern int rcutorture_runnable; /* for sysctl */
@@ -77,6 +78,16 @@ extern void rcu_scheduler_starting(void)
 #error "Unknown RCU implementation specified to kernel configuration"
 #endif
 
+/*
+ * init_rcu_head_on_stack()/destroy_rcu_head_on_stack() are needed for dynamic
+ * initialization and destruction of rcu_head on the stack. rcu_head structures
+ * allocated dynamically in the heap or defined statically don't need any
+ * initialization.
+ */
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+extern void init_rcu_head_on_stack(struct rcu_head *head);
+extern void destroy_rcu_head_on_stack(struct rcu_head *head);
+#else /* !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
 static inline void init_rcu_head_on_stack(struct rcu_head *head)
 {
 }
@@ -84,6 +95,7 @@ static inline void init_rcu_head_on_stac
 static inline void destroy_rcu_head_on_stack(struct rcu_head *head)
 {
 }
+#endif	/* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
 
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 
@@ -456,4 +468,41 @@ extern void call_rcu(struct rcu_head *he
 extern void call_rcu_bh(struct rcu_head *head,
 			void (*func)(struct rcu_head *head));
 
+/*
+ * debug_rcu_head_queue()/debug_rcu_head_unqueue() are used internally
+ * by call_rcu() and rcu callback execution, and are therefore not part of the
+ * RCU API. Leaving in rcupdate.h because they are used by all RCU flavors.
+ */
+
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+# define STATE_RCU_HEAD_READY	0
+# define STATE_RCU_HEAD_QUEUED	1
+
+extern struct debug_obj_descr rcuhead_debug_descr;
+
+static inline void debug_rcu_head_queue(struct rcu_head *head)
+{
+	debug_object_activate(head, &rcuhead_debug_descr);
+	debug_object_active_state(head, &rcuhead_debug_descr,
+				  STATE_RCU_HEAD_READY,
+				  STATE_RCU_HEAD_QUEUED);
+}
+
+static inline void debug_rcu_head_unqueue(struct rcu_head *head)
+{
+	debug_object_active_state(head, &rcuhead_debug_descr,
+				  STATE_RCU_HEAD_QUEUED,
+				  STATE_RCU_HEAD_READY);
+	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 void debug_rcu_head_unqueue(struct rcu_head *head)
+{
+}
+#endif	/* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+
 #endif /* __LINUX_RCUPDATE_H */
Index: linux.trees.git/kernel/rcutree.c
===================================================================
--- linux.trees.git.orig/kernel/rcutree.c	2010-03-29 10:18:05.000000000 -0400
+++ linux.trees.git/kernel/rcutree.c	2010-03-29 10:19:18.000000000 -0400
@@ -1076,6 +1076,7 @@ static void rcu_do_batch(struct rcu_stat
 	while (list) {
 		next = list->next;
 		prefetch(next);
+		debug_rcu_head_unqueue(list);
 		list->func(list);
 		list = next;
 		if (++count >= rdp->blimit)
@@ -1353,6 +1354,7 @@ __call_rcu(struct rcu_head *head, void (
 	unsigned long flags;
 	struct rcu_data *rdp;
 
+	debug_rcu_head_queue(head);
 	head->func = func;
 	head->next = NULL;
 
Index: linux.trees.git/lib/Kconfig.debug
===================================================================
--- linux.trees.git.orig/lib/Kconfig.debug	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/lib/Kconfig.debug	2010-03-29 10:19:18.000000000 -0400
@@ -319,6 +319,12 @@ config DEBUG_OBJECTS_WORK
 	  work queue routines to track the life time of work objects and
 	  validate the work operations.
 
+config DEBUG_OBJECTS_RCU_HEAD
+	bool "Debug RCU callbacks objects"
+	depends on DEBUG_OBJECTS
+	help
+	  Enable this to turn on debugging of RCU list heads (call_rcu() usage).
+
 config DEBUG_OBJECTS_ENABLE_DEFAULT
 	int "debug_objects bootup default value (0-1)"
         range 0 1
Index: linux.trees.git/kernel/rcutiny.c
===================================================================
--- linux.trees.git.orig/kernel/rcutiny.c	2010-03-29 10:18:05.000000000 -0400
+++ linux.trees.git/kernel/rcutiny.c	2010-03-29 10:19:18.000000000 -0400
@@ -163,6 +163,7 @@ static void __rcu_process_callbacks(stru
 	while (list) {
 		next = list->next;
 		prefetch(next);
+		debug_rcu_head_unqueue(list);
 		list->func(list);
 		list = next;
 	}
@@ -210,6 +211,7 @@ static void __call_rcu(struct rcu_head *
 {
 	unsigned long flags;
 
+	debug_rcu_head_queue(head);
 	head->func = func;
 	head->next = NULL;
 
Index: linux.trees.git/kernel/rcupdate.c
===================================================================
--- linux.trees.git.orig/kernel/rcupdate.c	2010-03-29 10:17:50.000000000 -0400
+++ linux.trees.git/kernel/rcupdate.c	2010-03-29 10:21:26.000000000 -0400
@@ -115,3 +115,152 @@ void wakeme_after_rcu(struct rcu_head  *
 	rcu = container_of(head, struct rcu_synchronize, head);
 	complete(&rcu->completion);
 }
+
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+static inline void debug_init_rcu_head(struct rcu_head *head)
+{
+	debug_object_init(head, &rcuhead_debug_descr);
+}
+
+static inline void debug_rcu_head_free(struct rcu_head *head)
+{
+	debug_object_free(head, &rcuhead_debug_descr);
+}
+
+/*
+ * fixup_init is called when:
+ * - an active object is initialized
+ */
+static int rcuhead_fixup_init(void *addr, enum debug_obj_state state)
+{
+	struct rcu_head *head = addr;
+
+	switch (state) {
+	case ODEBUG_STATE_ACTIVE:
+		/*
+		 * Ensure that queued callbacks are all executed.
+		 * If we detect that we are nested in a RCU read-side critical
+		 * section, we should simply fail, otherwise we would deadlock.
+		 */
+#ifndef CONFIG_PREEMPT
+		WARN_ON(1);
+		return 0;
+#else
+		if (rcu_preempt_depth() != 0 || preempt_count() != 0 ||
+		    irqs_disabled()) {
+			WARN_ON(1);
+			return 0;
+		}
+		rcu_barrier();
+		rcu_barrier_sched();
+		rcu_barrier_bh();
+		debug_object_init(head, &rcuhead_debug_descr);
+		return 1;
+#endif
+	default:
+		return 0;
+	}
+}
+
+/*
+ * fixup_activate is called when:
+ * - an active object is activated
+ * - an unknown object is activated (might be a statically initialized object)
+ * Activation is performed internally by call_rcu().
+ */
+static int rcuhead_fixup_activate(void *addr, enum debug_obj_state state)
+{
+	struct rcu_head *head = addr;
+
+	switch (state) {
+
+	case ODEBUG_STATE_NOTAVAILABLE:
+		/*
+		 * This is not really a fixup. We just make sure that it is
+		 * tracked in the object tracker.
+		 */
+		debug_object_init(head, &rcuhead_debug_descr);
+		debug_object_activate(head, &rcuhead_debug_descr);
+		return 0;
+
+	case ODEBUG_STATE_ACTIVE:
+		/*
+		 * Ensure that queued callbacks are all executed.
+		 * If we detect that we are nested in a RCU read-side critical
+		 * section, we should simply fail, otherwise we would deadlock.
+		 */
+#ifndef CONFIG_PREEMPT
+		WARN_ON(1);
+		return 0;
+#else
+		if (rcu_preempt_depth() != 0 || preempt_count() != 0 ||
+		    irqs_disabled()) {
+			WARN_ON(1);
+			return 0;
+		}
+		rcu_barrier();
+		rcu_barrier_sched();
+		rcu_barrier_bh();
+		debug_object_activate(head, &rcuhead_debug_descr);
+		return 1;
+#endif
+	default:
+		return 0;
+	}
+}
+
+/*
+ * fixup_free is called when:
+ * - an active object is freed
+ */
+static int rcuhead_fixup_free(void *addr, enum debug_obj_state state)
+{
+	struct rcu_head *head = addr;
+
+	switch (state) {
+	case ODEBUG_STATE_ACTIVE:
+		/*
+		 * Ensure that queued callbacks are all executed.
+		 * If we detect that we are nested in a RCU read-side critical
+		 * section, we should simply fail, otherwise we would deadlock.
+		 */
+#ifndef CONFIG_PREEMPT
+		WARN_ON(1);
+		return 0;
+#else
+		if (rcu_preempt_depth() != 0 || preempt_count() != 0 ||
+		    irqs_disabled()) {
+			WARN_ON(1);
+			return 0;
+		}
+		rcu_barrier();
+		rcu_barrier_sched();
+		rcu_barrier_bh();
+		debug_object_free(head, &rcuhead_debug_descr);
+		return 1;
+#endif
+	default:
+		return 0;
+	}
+}
+
+void init_rcu_head_on_stack(struct rcu_head *head)
+{
+	debug_object_init_on_stack(head, &rcuhead_debug_descr);
+}
+EXPORT_SYMBOL_GPL(init_rcu_head_on_stack);
+
+void destroy_rcu_head_on_stack(struct rcu_head *head)
+{
+	debug_object_free(head, &rcuhead_debug_descr);
+}
+EXPORT_SYMBOL_GPL(destroy_rcu_head_on_stack);
+
+struct debug_obj_descr rcuhead_debug_descr = {
+	.name = "rcu_head",
+	.fixup_init = rcuhead_fixup_init,
+	.fixup_activate = rcuhead_fixup_activate,
+	.fixup_free = rcuhead_fixup_free,
+};
+EXPORT_SYMBOL_GPL(rcuhead_debug_descr);
+#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[PATCH] rcu head debug consider on stack fixup

[Mathieu Desnoyers]

Complain if activation receives a non-initialized object on the stack.

[This patch applies after "tree/tiny rcu: Add debug RCU head objects (v5)"]

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: David S. Miller <davem@davemloft.net>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: akpm@linux-foundation.org
CC: mingo@elte.hu
CC: laijs@cn.fujitsu.com
CC: dipankar@in.ibm.com
CC: josh@joshtriplett.org
CC: dvhltc@us.ibm.com
CC: niv@us.ibm.com
CC: tglx@linutronix.de
CC: peterz@infradead.org
CC: rostedt@goodmis.org
CC: Valdis.Kletnieks@vt.edu
CC: dhowells@redhat.com
CC: eric.dumazet@gmail.com
CC: Alexey Dobriyan <adobriyan@gmail.com>
---
 kernel/rcupdate.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Index: linux.trees.git/kernel/rcupdate.c
===================================================================
--- linux.trees.git.orig/kernel/rcupdate.c	2010-03-29 11:05:35.000000000 -0400
+++ linux.trees.git/kernel/rcupdate.c	2010-03-29 11:06:23.000000000 -0400
@@ -179,8 +179,12 @@ static int rcuhead_fixup_activate(void *
 		 * This is not really a fixup. We just make sure that it is
 		 * tracked in the object tracker.
 		 */
-		debug_object_init(head, &rcuhead_debug_descr);
-		debug_object_activate(head, &rcuhead_debug_descr);
+		if (!object_is_on_stack(head)) {
+			debug_object_init(head, &rcuhead_debug_descr);
+			debug_object_activate(head, &rcuhead_debug_descr);
+			return 0;
+		}
+		WARN_ON_ONCE(1);
 		return 0;
 
 	case ODEBUG_STATE_ACTIVE:
-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com