rcu head debugobjects

PATCH In review Mathieu Desnoyers 3 participants 13 messages Apr 17, 2010 – Apr 22, 2010

Subsystems (10)

Memory management VFS PowerPC Block layer +6 more subsystems

lore

Share & tools

lore

Mathieu Desnoyers

author reviewer

2010-04-17 12:51 UTC #2 lore raw

PATCH 2/5 rcu head introduce rcu head init on stack

In reply to Mathieu Desnoyers

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 <[email protected]>

CC "Paul E. McKenney" <[email protected]>

CC David S. Miller <[email protected]>

CC [email protected]

CC Alexey Dobriyan <[email protected]>

---
 include/linux/rcupdate.h |    8 ++++++++
 1 file changed, 8 insertions(+)

include/linux/rcupdate.h +8 -0

Index: linux.trees.git/include/linux/rcupdate.h
===================================================================
@@ -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;

Signature · 2 lines

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

Mathieu Desnoyers

author reviewer

2010-04-17 12:51 UTC #1 lore raw

PATCH 1/5 Debugobjects transition check

In reply to 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 <[email protected]>

Reviewed-by Thomas Gleixner <[email protected]>

Acked-by David S. Miller <[email protected]>

CC "Paul E. McKenney" <[email protected]>

CC [email protected]

CC Alexey Dobriyan <[email protected]>

---
 include/linux/debugobjects.h |   11 ++++++++
 lib/debugobjects.c           |   59 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 67 insertions(+), 3 deletions(-)

include/linux/debugobjects.h +11 -0

Index: linux.trees.git/include/linux/debugobjects.h
===================================================================
@@ -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

lib/debugobjects.c +56 -3

83 lines not on this page · raw message

Signature · 2 lines

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

Mathieu Desnoyers

author reviewer

2010-04-17 12:51 UTC #4 lore raw

PATCH 4/5 rcu head remove init

In reply to Mathieu Desnoyers

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 <[email protected]>

CC David S. Miller <[email protected]>

CC "Paul E. McKenney" <[email protected]>

CC [email protected]

CC Alexey Dobriyan <[email protected]>

---
 include/linux/rcupdate.h |    6 ------
 1 file changed, 6 deletions(-)

include/linux/rcupdate.h +0 -6

Index: linux.trees.git/include/linux/rcupdate.h
===================================================================
@@ -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)
 {
 }

Signature · 2 lines

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

Mathieu Desnoyers

author reviewer

2010-04-17 12:52 UTC #0 lore raw

cover rcu head debugobjects

Here is a repost of the rcu head debugobjects patchset, with updated changelogs.

Paul, this would be ready to be integrated with the RCU patches.

Thanks,

Mathieu

Signature · 2 lines

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

Mathieu Desnoyers

author reviewer

2010-04-17 12:52 UTC #5 lore raw

PATCH 5/5 tree/tiny rcu: Add debug RCU head objects (v5)

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 <[email protected]>

CC David S. Miller <[email protected]>

CC "Paul E. McKenney" <[email protected]>

CC [email protected]

CC Alexey Dobriyan <[email protected]>

---
 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(+)

include/linux/rcupdate.h +49 -0

Index: linux.trees.git/include/linux/rcupdate.h
===================================================================
@@ -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 */

kernel/rcutree.c +2 -0

Index: linux.trees.git/kernel/rcutree.c
===================================================================
@@ -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;

lib/Kconfig.debug +6 -0

Index: linux.trees.git/lib/Kconfig.debug
===================================================================
@@ -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

kernel/rcutiny.c +2 -0

Index: linux.trees.git/kernel/rcutiny.c
===================================================================
@@ -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;

kernel/rcupdate.c +149 -0

152 lines not on this page · raw message

Signature · 2 lines

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

Mathieu Desnoyers

author reviewer

2010-04-17 12:52 UTC #3 lore raw

PATCH 3/5 remove all rcu head initializations, except on_stack initializations

In reply to Mathieu Desnoyers

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 <[email protected]>

CC David S. Miller <[email protected]>

CC "Paul E. McKenney" <[email protected]>

CC [email protected]

CC Alexey Dobriyan <[email protected]>

---
 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(-)

kernel/rcutiny.c +6 -0

Index: linux.trees.git/kernel/rcutiny.c
===================================================================
@@ -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);

kernel/rcutorture.c +2 -0

Index: linux.trees.git/kernel/rcutorture.c
===================================================================
@@ -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 = {

kernel/rcutree.c +4 -0

Index: linux.trees.git/kernel/rcutree.c
===================================================================
@@ -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);

mm/backing-dev.c +0 -1

Index: linux.trees.git/mm/backing-dev.c
===================================================================
@@ -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);

mm/slob.c +0 -1

Index: linux.trees.git/mm/slob.c
===================================================================
@@ -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 {

fs/file.c +0 -3

Index: linux.trees.git/fs/file.c
===================================================================
@@ -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),
 };

fs/fs-writeback.c +26 -5

Index: linux.trees.git/fs/fs-writeback.c
===================================================================
@@ -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);
 }
 
 /**

fs/partitions/check.c +0 -1

Index: linux.trees.git/fs/partitions/check.c
===================================================================
@@ -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 */

block/cfq-iosched.c +0 -1

Index: linux.trees.git/block/cfq-iosched.c
===================================================================
@@ -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;
 }

block/genhd.c +0 -1

Index: linux.trees.git/block/genhd.c
===================================================================
@@ -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++)

arch/powerpc/mm/pgtable.c +0 -1

Index: linux.trees.git/arch/powerpc/mm/pgtable.c
===================================================================
@@ -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);
 }

security/selinux/avc.c +0 -1

Index: linux.trees.git/security/selinux/avc.c
===================================================================
@@ -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);

security/selinux/netnode.c +0 -2

Index: linux.trees.git/security/selinux/netnode.c
===================================================================
@@ -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);

drivers/staging/batman-adv/hard-interface.c +0 -1

Index: linux.trees.git/drivers/staging/batman-adv/hard-interface.c
===================================================================
@@ -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++;

include/linux/init_task.h +0 -1

Index: linux.trees.git/include/linux/init_task.h
===================================================================
@@ -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,					\

Signature · 2 lines

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

Paul E. McKenney

2010-04-18 00:48 UTC #8 lore raw

In reply to Mathieu Desnoyers

Quoted 3 lines · Mathieu Desnoyers

Here is a repost of the rcu head debugobjects patchset, with updated changelogs.
Paul, this would be ready to be integrated with the RCU patches.

Thank you, Mathieu, queued up for 2.6.35!

Thanx, Paul

Lai Jiangshan

Reviewed-by on v1 5/5

2010-04-19 01:17 UTC #6 lore raw

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

In reply to Mathieu Desnoyers

Reviewed-by Lai Jiangshan <[email protected]>

Quoted 14 lines · Mathieu Desnoyers

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

Paul E. McKenney

2010-04-19 13:34 UTC #7 lore raw

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

Quoted 2 lines · Lai Jiangshan

Reviewed-by Lai Jiangshan <[email protected]>

Thank you, Lai!!! I have added your Reviewed-by.

Thanx, Paul

Quoted 15 lines

Quoted 14 lines · Mathieu Desnoyers
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

Paul E. McKenney

2010-04-21 17:31 UTC #9 lore raw

In reply to Paul E. McKenney

Quoted 6 lines · Paul E. McKenney

Quoted 3 lines · Mathieu Desnoyers
Here is a repost of the rcu head debugobjects patchset, with updated changelogs.
Paul, this would be ready to be integrated with the RCU patches.
Thank you, Mathieu, queued up for 2.6.35!

And testing got me the following debugobjects splat, which baffles me. My first thought was that one of the synchronize_rcu() variants was missing the init_rcu_head_on_stack(), but not so. Then I started looking through the debugobjects code, and found the following:

	static void debug_object_is_on_stack(void *addr, int onstack)
	{
		int is_on_stack;
		static int limit;

		if (limit > 4)
			return;

This really confuses me. We are using a static variable, but as near as I can tell, it is being guarded by a per-bucket lock:

	raw_spin_lock_irqsave(&db->lock, flags);

If I understand correctly, this means that multiple CPUs might be concurrently updating the static variable "limit", which might in turn be causing the splat below.

Or am I missing something?

Thanx, Paul

------------------------------------------------------------------------

ODEBUG: object is on stack, but not annotated

------------[ cut here ]------------
Badness at lib/debugobjects.c:294
NIP: c0000000002c76f0 LR: c0000000002c76ec CTR: c00000000041ecd8
REGS: c0000001de71b280 TRAP: 0700   Tainted: G        W   (2.6.34-rc3-autokern1)
MSR: 8000000000029032 <EE,ME,CE,IR,DR>  CR: 24000424  XER: 0000000f

TASK = c0000001de7dca00[3695] 'arping' THREAD: c0000001de718000 CPU: 1
GPR00: c0000000002c76ec c0000001de71b500 c00000000096c048 0000000000000034
GPR04: 0000000000000001 c000000000063918 0000000000000000 0000000000000002
GPR08: 0000000000000003 0000000000000000 c000000000086f68 c0000001de7dca00
GPR12: 000000000000256d c0000000074e4200 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 00000000201b8f60
GPR20: 00000000201b8f70 00000000201b8f48 0000000000000000 c0000000008766b8
GPR24: c0000001de71b800 0000000000000001 c0000000008ad400 c000000001247478
GPR28: c0000000e6abb8c0 c0000000e6abb8c0 c000000000904570 c000000001247470 NIP [c0000000002c76f0] .__debug_object_init+0x314/0x40c
LR [c0000000002c76ec] .__debug_object_init+0x310/0x40c

Call Trace:
[c0000001de71b500] [c0000000002c76ec] .__debug_object_init+0x310/0x40c (unreliable)
[c0000001de71b5d0] [c00000000007d990] .rcuhead_fixup_activate+0x40/0xdc
[c0000001de71b660] [c0000000002c6a7c] .debug_object_fixup+0x4c/0x74
[c0000001de71b6f0] [c0000000000c5e54] .__call_rcu+0x3c/0x1d4
[c0000001de71b790] [c0000000000c6050] .synchronize_rcu+0x4c/0x6c
[c0000001de71b870] [c0000000004be218] .synchronize_net+0x10/0x24
[c0000001de71b8e0] [c0000000005498c8] .packet_release+0x1d4/0x274
[c0000001de71b990] [c0000000004ac1f0] .sock_release+0x54/0x124
[c0000001de71ba20] [c0000000004ac9e4] .sock_close+0x34/0x4c
[c0000001de71baa0] [c00000000012469c] .__fput+0x174/0x264
[c0000001de71bb40] [c000000000120c54] .filp_close+0xb0/0xd8
[c0000001de71bbd0] [c000000000065e70] .put_files_struct+0x1a8/0x314
[c0000001de71bc70] [c000000000067e04] .do_exit+0x234/0x6f0
[c0000001de71bd30] [c000000000068354] .do_group_exit+0x94/0xc8
[c0000001de71bdc0] [c00000000006839c] .SyS_exit_group+0x14/0x28
[c0000001de71be30] [c000000000008554] syscall_exit+0x0/0x40

Instruction dump:
7f80b000 419e0030 2fa00000 e93e8140 380b0001 90090000 419e000c e87e8148 48000008 e87e8150 4bd9cb89 60000000 <0fe00000> 801c0010 2f800003 419e0024

Mathieu Desnoyers

author reviewer

2010-04-22 01:20 UTC #10 lore raw

Quoted 24 lines · * Paul E. McKenney ([email protected]) wrote:

Quoted 6 lines · Paul E. McKenney
Quoted 3 lines · Mathieu Desnoyers
Here is a repost of the rcu head debugobjects patchset, with updated changelogs.
Paul, this would be ready to be integrated with the RCU patches.
Thank you, Mathieu, queued up for 2.6.35!
And testing got me the following debugobjects splat, which baffles me. My first thought was that one of the synchronize_rcu() variants was missing the init_rcu_head_on_stack(), but not so. Then I started looking through the debugobjects code, and found the following:
	static void debug_object_is_on_stack(void *addr, int onstack)
	{
		int is_on_stack;
		static int limit;

		if (limit > 4)
			return;
This really confuses me. We are using a static variable, but as near as I can tell, it is being guarded by a per-bucket lock:

Quoted 7 lines

	raw_spin_lock_irqsave(&db->lock, flags);
If I understand correctly, this means that multiple CPUs might be concurrently updating the static variable "limit", which might in turn be causing the splat below.
Or am I missing something?

This "limit" static variable is really only a printk suppressor: it stops the printk warning output after approximately 5 occurences (modulo racy increments). But normally, it should not _cause_ a splat if there ain't any in the first place.

Will send the fix in a following email.

Thanks,

Mathieu

Signature · 4 lines

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

Mathieu Desnoyers

author reviewer

2010-04-22 01:23 UTC #11 lore raw

PATCH 1/1 rcu fix rcu head init on stack in rcutree_plugin.h

In reply to Paul E. McKenney

[Paul]
[...]
And testing got me the following debugobjects splat [...]
ODEBUG: object is on stack, but not annotated

------------[ cut here ]------------
Badness at lib/debugobjects.c:294
NIP: c0000000002c76f0 LR: c0000000002c76ec CTR: c00000000041ecd8
REGS: c0000001de71b280 TRAP: 0700   Tainted: G        W   (2.6.34-rc3-autokern1)
MSR: 8000000000029032 <EE,ME,CE,IR,DR>  CR: 24000424  XER: 0000000f

Call Trace:
[c0000001de71b500] [c0000000002c76ec] .__debug_object_init+0x310/0x40c (unreliable)
[c0000001de71b5d0] [c00000000007d990] .rcuhead_fixup_activate+0x40/0xdc
[c0000001de71b660] [c0000000002c6a7c] .debug_object_fixup+0x4c/0x74
[c0000001de71b6f0] [c0000000000c5e54] .__call_rcu+0x3c/0x1d4
[c0000001de71b790] [c0000000000c6050] .synchronize_rcu+0x4c/0x6c
[c0000001de71b870] [c0000000004be218] .synchronize_net+0x10/0x24
[c0000001de71b8e0] [c0000000005498c8] .packet_release+0x1d4/0x274
[c0000001de71b990] [c0000000004ac1f0] .sock_release+0x54/0x124
[c0000001de71ba20] [c0000000004ac9e4] .sock_close+0x34/0x4c
[c0000001de71baa0] [c00000000012469c] .__fput+0x174/0x264
[c0000001de71bb40] [c000000000120c54] .filp_close+0xb0/0xd8
[c0000001de71bbd0] [c000000000065e70] .put_files_struct+0x1a8/0x314
[c0000001de71bc70] [c000000000067e04] .do_exit+0x234/0x6f0
[c0000001de71bd30] [c000000000068354] .do_group_exit+0x94/0xc8
[c0000001de71bdc0] [c00000000006839c] .SyS_exit_group+0x14/0x28
[c0000001de71be30] [c000000000008554] syscall_exit+0x0/0x40

Instruction dump:
7f80b000 419e0030 2fa00000 e93e8140 380b0001 90090000 419e000c e87e8148 48000008 e87e8150 4bd9cb89 60000000 <0fe00000> 801c0010 2f800003 419e0024

[Mathieu]

Here is the fix.

Signed-off-by Mathieu Desnoyers <[email protected]>

CC "Paul E. McKenney" <[email protected]>

---
 kernel/rcutree_plugin.h |    2 ++
 1 file changed, 2 insertions(+)

kernel/rcutree_plugin.h +2 -0

Index: linux.trees.git/kernel/rcutree_plugin.h
===================================================================
@@ -515,11 +515,13 @@ void synchronize_rcu(void)
 	if (!rcu_scheduler_active)
 		return;
 
+	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(synchronize_rcu);

Signature · 4 lines

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

Paul E. McKenney

2010-04-22 22:52 UTC #12 lore raw

Re: [PATCH] rcu fix rcu head init on stack in rcutree_plugin.h

Quoted 38 lines · Mathieu Desnoyers

[Paul]
[...]
And testing got me the following debugobjects splat [...]
ODEBUG: object is on stack, but not annotated
------------[ cut here ]------------
Badness at lib/debugobjects.c:294
NIP: c0000000002c76f0 LR: c0000000002c76ec CTR: c00000000041ecd8
REGS: c0000001de71b280 TRAP: 0700   Tainted: G        W   (2.6.34-rc3-autokern1)
MSR: 8000000000029032 <EE,ME,CE,IR,DR>  CR: 24000424  XER: 0000000f
TASK = c0000001de7dca00[3695] 'arping' THREAD: c0000001de718000 CPU: 1
GPR00: c0000000002c76ec c0000001de71b500 c00000000096c048 0000000000000034
GPR04: 0000000000000001 c000000000063918 0000000000000000 0000000000000002
GPR08: 0000000000000003 0000000000000000 c000000000086f68 c0000001de7dca00
GPR12: 000000000000256d c0000000074e4200 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 00000000201b8f60
GPR20: 00000000201b8f70 00000000201b8f48 0000000000000000 c0000000008766b8
GPR24: c0000001de71b800 0000000000000001 c0000000008ad400 c000000001247478
GPR28: c0000000e6abb8c0 c0000000e6abb8c0 c000000000904570 c000000001247470 NIP [c0000000002c76f0] .__debug_object_init+0x314/0x40c
LR [c0000000002c76ec] .__debug_object_init+0x310/0x40c
Call Trace:
[c0000001de71b500] [c0000000002c76ec] .__debug_object_init+0x310/0x40c (unreliable)
[c0000001de71b5d0] [c00000000007d990] .rcuhead_fixup_activate+0x40/0xdc
[c0000001de71b660] [c0000000002c6a7c] .debug_object_fixup+0x4c/0x74
[c0000001de71b6f0] [c0000000000c5e54] .__call_rcu+0x3c/0x1d4
[c0000001de71b790] [c0000000000c6050] .synchronize_rcu+0x4c/0x6c
[c0000001de71b870] [c0000000004be218] .synchronize_net+0x10/0x24
[c0000001de71b8e0] [c0000000005498c8] .packet_release+0x1d4/0x274
[c0000001de71b990] [c0000000004ac1f0] .sock_release+0x54/0x124
[c0000001de71ba20] [c0000000004ac9e4] .sock_close+0x34/0x4c
[c0000001de71baa0] [c00000000012469c] .__fput+0x174/0x264
[c0000001de71bb40] [c000000000120c54] .filp_close+0xb0/0xd8
[c0000001de71bbd0] [c000000000065e70] .put_files_struct+0x1a8/0x314
[c0000001de71bc70] [c000000000067e04] .do_exit+0x234/0x6f0
[c0000001de71bd30] [c000000000068354] .do_group_exit+0x94/0xc8
[c0000001de71bdc0] [c00000000006839c] .SyS_exit_group+0x14/0x28
[c0000001de71be30] [c000000000008554] syscall_exit+0x0/0x40

Quoted 7 lines

Instruction dump:
7f80b000 419e0030 2fa00000 e93e8140 380b0001 90090000 419e000c e87e8148 48000008 e87e8150 4bd9cb89 60000000 <0fe00000> 801c0010 2f800003 419e0024
[Mathieu]
Here is the fix.

Thank you very much, Mathieu!!! Queued for 2.6.35.

Quoted 34 lines

Signed-off-by Mathieu Desnoyers <[email protected]>

CC "Paul E. McKenney" <[email protected]>

---
 kernel/rcutree_plugin.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux.trees.git/kernel/rcutree_plugin.h
===================================================================

kernel/rcutree_plugin.h +2 -0

@@ -515,11 +515,13 @@ void synchronize_rcu(void)
 	if (!rcu_scheduler_active)
 		return;
 
+	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(synchronize_rcu);

Signature · 9 lines

Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/