[patch 0/6] rcu head debugobjects

[patch 0/6] rcu head debugobjects

[Mathieu Desnoyers]

Hi,

Here is an updated version of the rcu head debugobjects, following the comments
I received in the last rounds.

It applies on top of -tip, based on 2.6.34-rc2, commit
2e958f219d2b8d192d44e2472a214b3a93c44673

Unless people have any objection, it should be ready to be merged. I think the
appropriate maintainer to perform this merge would be Paul E. McKenney, because
this patchset is RCU-related.

Thanks,

Mathieu

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

[patch 1/6] commit 501fdb3aeeb2444f86d289a4a044cf7c8fbc17df Author: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> Date: Sat Mar 27 10:52:11 2010 -0400

[Mathieu Desnoyers]

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

    Revert "net: remove INIT_RCU_HEAD() usage"
    
    This reverts commit dc4c2c31053ba5bf685d273cd62ecca406dddb2d.
    
    If we ever want to be able to properly detect double rcu_head activation, we
    need to separate the initialization from call_rcu().
    
    Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
    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>

diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index f8c8749..b8e9d3a 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -296,6 +296,7 @@ static int dropmon_net_event(struct notifier_block *ev_block,
 
 		new_stat->dev = dev;
 		new_stat->last_rx = jiffies;
+		INIT_RCU_HEAD(&new_stat->rcu);
 		spin_lock(&trace_state_lock);
 		list_add_rcu(&new_stat->list, &hw_stats_list);
 		spin_unlock(&trace_state_lock);
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index b1eea81..51eb389 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -363,6 +363,7 @@ ipip6_tunnel_add_prl(struct ip_tunnel *t, struct ip_tunnel_prl *a, int chg)
 		goto out;
 	}
 
+	INIT_RCU_HEAD(&p->rcu_head);
 	p->next = t->prl;
 	p->addr = a->addr;
 	p->flags = a->flags;
diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c
index fa85a7d..ddce21e 100644
--- a/net/ipv6/xfrm6_tunnel.c
+++ b/net/ipv6/xfrm6_tunnel.c
@@ -161,6 +161,7 @@ alloc_spi:
 	if (!x6spi)
 		goto out;
 
+	INIT_RCU_HEAD(&x6spi->rcu_head);
 	memcpy(&x6spi->addr, saddr, sizeof(x6spi->addr));
 	x6spi->spi = spi;
 	atomic_set(&x6spi->refcnt, 1);
diff --git a/net/netlabel/netlabel_domainhash.c b/net/netlabel/netlabel_domainhash.c
index 0bfeaab..c5d9f97 100644
--- a/net/netlabel/netlabel_domainhash.c
+++ b/net/netlabel/netlabel_domainhash.c
@@ -315,6 +315,7 @@ int netlbl_domhsh_add(struct netlbl_dom_map *entry,
 		entry_old = netlbl_domhsh_search_def(entry->domain);
 	if (entry_old == NULL) {
 		entry->valid = 1;
+		INIT_RCU_HEAD(&entry->rcu);
 
 		if (entry->domain != NULL) {
 			u32 bkt = netlbl_domhsh_hash(entry->domain);
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 852d9d7..98ed22e 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -327,6 +327,7 @@ static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
 	entry->list.addr = addr->s_addr & mask->s_addr;
 	entry->list.mask = mask->s_addr;
 	entry->list.valid = 1;
+	INIT_RCU_HEAD(&entry->rcu);
 	entry->secid = secid;
 
 	spin_lock(&netlbl_unlhsh_lock);
@@ -372,6 +373,7 @@ static int netlbl_unlhsh_add_addr6(struct netlbl_unlhsh_iface *iface,
 	entry->list.addr.s6_addr32[3] &= mask->s6_addr32[3];
 	ipv6_addr_copy(&entry->list.mask, mask);
 	entry->list.valid = 1;
+	INIT_RCU_HEAD(&entry->rcu);
 	entry->secid = secid;
 
 	spin_lock(&netlbl_unlhsh_lock);
@@ -408,6 +410,7 @@ static struct netlbl_unlhsh_iface *netlbl_unlhsh_add_iface(int ifindex)
 	INIT_LIST_HEAD(&iface->addr4_list);
 	INIT_LIST_HEAD(&iface->addr6_list);
 	iface->valid = 1;
+	INIT_RCU_HEAD(&iface->rcu);
 
 	spin_lock(&netlbl_unlhsh_lock);
 	if (ifindex > 0) {
diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
index bef1337..13a6fba 100644
--- a/net/sctp/bind_addr.c
+++ b/net/sctp/bind_addr.c
@@ -186,6 +186,7 @@ int sctp_add_bind_addr(struct sctp_bind_addr *bp, union sctp_addr *new,
 	addr->valid = 1;
 
 	INIT_LIST_HEAD(&addr->list);
+	INIT_RCU_HEAD(&addr->rcu);
 
 	/* We always hold a socket lock when calling this function,
 	 * and that acts as a writer synchronizing lock.
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 1d7ac70..cc50fbe 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -381,6 +381,7 @@ static void sctp_v6_copy_addrlist(struct list_head *addrlist,
 			addr->a.v6.sin6_scope_id = dev->ifindex;
 			addr->valid = 1;
 			INIT_LIST_HEAD(&addr->list);
+			INIT_RCU_HEAD(&addr->rcu);
 			list_add_tail(&addr->list, addrlist);
 		}
 	}
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index e771690..9687177 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -188,6 +188,7 @@ static void sctp_v4_copy_addrlist(struct list_head *addrlist,
 			addr->a.v4.sin_addr.s_addr = ifa->ifa_local;
 			addr->valid = 1;
 			INIT_LIST_HEAD(&addr->list);
+			INIT_RCU_HEAD(&addr->rcu);
 			list_add_tail(&addr->list, addrlist);
 		}
 	}

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

[patch 2/6] commit afd066d60b77e28651bb8323fc8cfcedacc5cbf8 Author: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> Date: Sat Mar 27 10:53:30 2010 -0400

[Mathieu Desnoyers]

[-- Attachment #1: revert-netfilter-dont-use-init-rcu-head.patch --]
[-- Type: text/plain, Size: 2180 bytes --]

    Revert "netfilter: don't use INIT_RCU_HEAD()"
    
    This reverts commit ca1c2e2da9637c131436bf6d6ae41b58f5353afe.
    
    If we ever want to be able to properly detect double rcu_head activation, we
    need to separate the initialization from call_rcu().
    
    Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
    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>

diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index acb29cc..d9f8a22 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -239,6 +239,7 @@ struct nf_conntrack_expect *nf_ct_expect_alloc(struct nf_conn *me)
 
 	new->master = me;
 	atomic_set(&new->use, 1);
+	INIT_RCU_HEAD(&new->rcu);
 	return new;
 }
 EXPORT_SYMBOL_GPL(nf_ct_expect_alloc);
diff --git a/net/netfilter/nf_conntrack_extend.c b/net/netfilter/nf_conntrack_extend.c
index fdc8fb4..fef95be 100644
--- a/net/netfilter/nf_conntrack_extend.c
+++ b/net/netfilter/nf_conntrack_extend.c
@@ -59,6 +59,7 @@ nf_ct_ext_create(struct nf_ct_ext **ext, enum nf_ct_ext_id id, gfp_t gfp)
 	if (!*ext)
 		return NULL;
 
+	INIT_RCU_HEAD(&(*ext)->rcu);
 	(*ext)->offset[id] = off;
 	(*ext)->len = len;
 
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 7ba4abc..90cf36d 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -112,6 +112,7 @@ instance_create(u_int16_t queue_num, int pid)
 	inst->copy_mode = NFQNL_COPY_NONE;
 	spin_lock_init(&inst->lock);
 	INIT_LIST_HEAD(&inst->queue_list);
+	INIT_RCU_HEAD(&inst->rcu);
 
 	if (!try_module_get(THIS_MODULE)) {
 		err = -EAGAIN;

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

[patch 3/6] commit 418b6f2c2ddba7c91d1186b68618092910260c32 Author: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> Date: Sat Mar 27 11:05:38 2010 -0400

[Mathieu Desnoyers]

[-- Attachment #1: revert-net-dont-use-init-rcu-head.patch --]
[-- Type: text/plain, Size: 3922 bytes --]

    Revert "net: don't use INIT_RCU_HEAD"
    
    This reverts commit 93adcc80f3288f1827baf6f821af818f6eeef7f9.
    
    Conflicts (resolved):
    
    	net/core/net_namespace.c
    
    If we ever want to be able to properly detect double rcu_head activation, we
    need to separate the initialization from call_rcu().
    
    Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
    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>

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 6cee643..99d6ac9 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1346,6 +1346,7 @@ struct neigh_parms *neigh_parms_alloc(struct net_device *dev,
 	if (p) {
 		p->tbl		  = tbl;
 		atomic_set(&p->refcnt, 1);
+		INIT_RCU_HEAD(&p->rcu_head);
 		p->reachable_time =
 				neigh_rand_reach_time(p->base_reachable_time);
 
@@ -1413,6 +1414,7 @@ void neigh_table_init_no_netlink(struct neigh_table *tbl)
 
 	write_pnet(&tbl->parms.net, &init_net);
 	atomic_set(&tbl->parms.refcnt, 1);
+	INIT_RCU_HEAD(&tbl->parms.rcu_head);
 	tbl->parms.reachable_time =
 			  neigh_rand_reach_time(tbl->parms.base_reachable_time);
 
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index bd8c471..cad70c6 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -124,8 +124,10 @@ static struct net_generic *net_alloc_generic(void)
 		INITIAL_NET_GEN_PTRS * sizeof(void *);
 
 	ng = kzalloc(generic_size, GFP_KERNEL);
-	if (ng)
+	if (ng) {
 		ng->len = INITIAL_NET_GEN_PTRS;
+		INIT_RCU_HEAD(&ng->rcu);
+	}
 
 	return ng;
 }
@@ -563,6 +565,7 @@ int net_assign_generic(struct net *net, int id, void *data)
 	 */
 
 	ng->len = id;
+	INIT_RCU_HEAD(&ng->rcu);
 	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len * sizeof(void*));
 
 	rcu_assign_pointer(net->gen, ng);
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 1e029dc..231972b 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -502,6 +502,7 @@ int cipso_v4_doi_add(struct cipso_v4_doi *doi_def,
 	}
 
 	atomic_set(&doi_def->refcount, 1);
+	INIT_RCU_HEAD(&doi_def->rcu);
 
 	spin_lock(&cipso_v4_doi_list_lock);
 	if (cipso_v4_doi_search(doi_def->doi) != NULL) {
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 51ca946..3088271 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -112,7 +112,13 @@ static inline void devinet_sysctl_unregister(struct in_device *idev)
 
 static struct in_ifaddr *inet_alloc_ifa(void)
 {
-	return kzalloc(sizeof(struct in_ifaddr), GFP_KERNEL);
+	struct in_ifaddr *ifa = kzalloc(sizeof(*ifa), GFP_KERNEL);
+
+	if (ifa) {
+		INIT_RCU_HEAD(&ifa->rcu_head);
+	}
+
+	return ifa;
 }
 
 static void inet_rcu_free_ifa(struct rcu_head *head)
@@ -155,6 +161,7 @@ static struct in_device *inetdev_init(struct net_device *dev)
 	in_dev = kzalloc(sizeof(*in_dev), GFP_KERNEL);
 	if (!in_dev)
 		goto out;
+	INIT_RCU_HEAD(&in_dev->rcu_head);
 	memcpy(&in_dev->cnf, dev_net(dev)->ipv4.devconf_dflt,
 			sizeof(in_dev->cnf));
 	in_dev->cnf.sysctl = NULL;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 54fd68c..ba42180 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1434,6 +1434,7 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 
 				/* Copy all the information. */
 				*rt = *rth;
+				INIT_RCU_HEAD(&rt->u.dst.rcu_head);
 				rt->u.dst.__use		= 1;
 				atomic_set(&rt->u.dst.__refcnt, 1);
 				rt->u.dst.child		= NULL;

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

[patch 4/6] Debugobjects transition check

[Mathieu Desnoyers]

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

Implement a basic state machine checker in the debugobjects.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
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-2.6-lttng/include/linux/debugobjects.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/debugobjects.h	2010-03-19 15:26:02.000000000 -0400
+++ linux-2.6-lttng/include/linux/debugobjects.h	2010-03-22 10:06:23.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-2.6-lttng/lib/debugobjects.c
===================================================================
--- linux-2.6-lttng.orig/lib/debugobjects.c	2010-03-19 15:26:02.000000000 -0400
+++ linux-2.6-lttng/lib/debugobjects.c	2010-03-19 15:26:06.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

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

[Mathieu Desnoyers]

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

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

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.

[bissectability warning]
This patch and "kernel call_rcu usage: initialize rcu_head structures" should be
applied together.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
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 |   61 +++++++++++++++--
 kernel/rcupdate.c        |  168 +++++++++++++++++++++++++++++++++++++++++++++++
 kernel/rcutiny.c         |    2 
 kernel/rcutree.c         |    2 
 lib/Kconfig.debug        |    6 +
 5 files changed, 235 insertions(+), 4 deletions(-)

Index: linux.trees.git/include/linux/rcupdate.h
===================================================================
--- linux.trees.git.orig/include/linux/rcupdate.h	2010-03-27 11:16:29.000000000 -0400
+++ linux.trees.git/include/linux/rcupdate.h	2010-03-27 11:16:37.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,11 +78,26 @@ extern void rcu_scheduler_starting(void)
 #error "Unknown RCU implementation specified to kernel configuration"
 #endif
 
+/* For dynamic initialization of rcu_head allocated in memory */
+extern void rcu_head_init(struct rcu_head *head);
+
+/* For static initialization of rcu_head */
 #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)
+
+/* For dynamic initialization and destruction of rcu_head on the stack */
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+extern void rcu_head_init_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 rcu_head_init_on_stack(struct rcu_head *head)
+{
+	rcu_head_init(head);
+}
+
+static inline void destroy_rcu_head_on_stack(struct rcu_head *head)
+{
+}
+#endif	/* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
 
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 
@@ -454,4 +470,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-27 11:16:29.000000000 -0400
+++ linux.trees.git/kernel/rcutree.c	2010-03-27 11:16:44.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-27 11:16:29.000000000 -0400
+++ linux.trees.git/lib/Kconfig.debug	2010-03-27 11:16:37.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-27 11:16:29.000000000 -0400
+++ linux.trees.git/kernel/rcutiny.c	2010-03-27 11:16:37.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-27 11:16:29.000000000 -0400
+++ linux.trees.git/kernel/rcupdate.c	2010-03-27 11:16:37.000000000 -0400
@@ -115,3 +115,171 @@ 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_rcu_head_init(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().
+ * Let's make it valid to activate a static object.
+ */
+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. The work struct was
+		 * statically initialized. 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 rcu_head_init_on_stack(struct rcu_head *head)
+{
+	debug_object_init_on_stack(head, &rcuhead_debug_descr);
+	rcu_head_init(head);
+}
+EXPORT_SYMBOL_GPL(rcu_head_init_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);
+#else	/* !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+static inline void debug_rcu_head_init(struct rcu_head *head)
+{
+}
+#endif /* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
+
+/**
+ * rcu_head_init - initialize a RCU head
+ * @head:     the rcu head to be initialized
+ */
+void rcu_head_init(struct rcu_head *head)
+{
+	debug_rcu_head_init(head);
+	head->next = NULL;
+	head->func = NULL;
+}
+EXPORT_SYMBOL_GPL(rcu_head_init);

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

[patch 6/6] kernel call_rcu usage: initialize rcu_head structures (v2)

[Mathieu Desnoyers]

[-- Attachment #1: rcu-init-null.patch --]
[-- Type: text/plain, Size: 24121 bytes --]

Initialize rcu_head structures with rcu_head_init() before passing them to
call_rcu().

Currently, the rcu_head debug object is not very strict: it will not complain if
a non-initialized rcu_head is passed to call_rcu(), because we currently have no
way to tag the statically initialized objects. However, we should keep the API
in place so we can activate this strict check in a near future.

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

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
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                   |    2 +-
 block/cfq-iosched.c                         |    2 +-
 block/genhd.c                               |    2 +-
 drivers/staging/batman-adv/hard-interface.c |    2 +-
 fs/file.c                                   |    4 ++--
 fs/fs-writeback.c                           |   13 +++++++++----
 fs/partitions/check.c                       |    2 +-
 kernel/fork.c                               |    1 +
 kernel/pid.c                                |    1 +
 kernel/rcutiny.c                            |    6 ++++++
 kernel/rcutorture.c                         |    2 ++
 kernel/rcutree.c                            |    4 ++++
 mm/backing-dev.c                            |    2 +-
 mm/slob.c                                   |    2 +-
 net/core/drop_monitor.c                     |    2 +-
 net/core/neighbour.c                        |    4 ++--
 net/core/net_namespace.c                    |    4 ++--
 net/ipv4/cipso_ipv4.c                       |    2 +-
 net/ipv4/devinet.c                          |    4 ++--
 net/ipv4/route.c                            |    2 +-
 net/ipv6/sit.c                              |    2 +-
 net/ipv6/xfrm6_tunnel.c                     |    2 +-
 net/netfilter/nf_conntrack_expect.c         |    2 +-
 net/netfilter/nf_conntrack_extend.c         |    2 +-
 net/netfilter/nfnetlink_queue.c             |    2 +-
 net/netlabel/netlabel_domainhash.c          |    2 +-
 net/netlabel/netlabel_unlabeled.c           |    6 +++---
 net/sctp/bind_addr.c                        |    2 +-
 net/sctp/ipv6.c                             |    2 +-
 net/sctp/protocol.c                         |    2 +-
 security/selinux/avc.c                      |    2 +-
 security/selinux/netnode.c                  |    2 +-
 32 files changed, 55 insertions(+), 36 deletions(-)

Index: linux.trees.git/kernel/pid.c
===================================================================
--- linux.trees.git.orig/kernel/pid.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/kernel/pid.c	2010-03-27 11:05:54.000000000 -0400
@@ -265,6 +265,7 @@ struct pid *alloc_pid(struct pid_namespa
 
 	get_pid_ns(ns);
 	pid->level = ns->level;
+	rcu_head_init(&pid->rcu);
 	atomic_set(&pid->count, 1);
 	for (type = 0; type < PIDTYPE_MAX; ++type)
 		INIT_HLIST_HEAD(&pid->tasks[type]);
Index: linux.trees.git/kernel/fork.c
===================================================================
--- linux.trees.git.orig/kernel/fork.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/kernel/fork.c	2010-03-27 11:05:54.000000000 -0400
@@ -1038,6 +1038,7 @@ static struct task_struct *copy_process(
 	INIT_LIST_HEAD(&p->children);
 	INIT_LIST_HEAD(&p->sibling);
 	rcu_copy_process(p);
+	rcu_head_init(&p->rcu);
 	p->vfork_done = NULL;
 	spin_lock_init(&p->alloc_lock);
 
Index: linux.trees.git/kernel/rcutiny.c
===================================================================
--- linux.trees.git.orig/kernel/rcutiny.c	2010-03-27 11:05:53.000000000 -0400
+++ linux.trees.git/kernel/rcutiny.c	2010-03-27 11:05:54.000000000 -0400
@@ -246,11 +246,13 @@ void rcu_barrier(void)
 {
 	struct rcu_synchronize rcu;
 
+	rcu_head_init_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);
 
@@ -258,11 +260,13 @@ void rcu_barrier_bh(void)
 {
 	struct rcu_synchronize rcu;
 
+	rcu_head_init_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);
 
@@ -270,11 +274,13 @@ void rcu_barrier_sched(void)
 {
 	struct rcu_synchronize rcu;
 
+	rcu_head_init_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-27 11:02:22.000000000 -0400
+++ linux.trees.git/kernel/rcutorture.c	2010-03-27 11:05:54.000000000 -0400
@@ -464,9 +464,11 @@ static void rcu_bh_torture_synchronize(v
 {
 	struct rcu_bh_torture_synchronize rcu;
 
+	rcu_head_init_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-27 11:05:53.000000000 -0400
+++ linux.trees.git/kernel/rcutree.c	2010-03-27 11:05:54.000000000 -0400
@@ -1451,11 +1451,13 @@ void synchronize_sched(void)
 	if (rcu_blocking_is_gp())
 		return;
 
+	rcu_head_init_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);
 
@@ -1475,11 +1477,13 @@ void synchronize_rcu_bh(void)
 	if (rcu_blocking_is_gp())
 		return;
 
+	rcu_head_init_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-27 11:02:22.000000000 -0400
+++ linux.trees.git/mm/backing-dev.c	2010-03-27 11:05:54.000000000 -0400
@@ -653,7 +653,7 @@ 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);
+	rcu_head_init(&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-27 11:02:22.000000000 -0400
+++ linux.trees.git/mm/slob.c	2010-03-27 11:05:54.000000000 -0400
@@ -647,7 +647,7 @@ 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);
+		rcu_head_init(&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-27 11:02:22.000000000 -0400
+++ linux.trees.git/fs/file.c	2010-03-27 11:05:54.000000000 -0400
@@ -178,7 +178,7 @@ 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);
+	rcu_head_init(&fdt->rcu);
 	fdt->next = NULL;
 
 	return fdt;
@@ -312,7 +312,7 @@ 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);
+	rcu_head_init(&new_fdt->rcu);
 	new_fdt->next = NULL;
 
 	spin_lock(&oldf->file_lock);
Index: linux.trees.git/fs/fs-writeback.c
===================================================================
--- linux.trees.git.orig/fs/fs-writeback.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/fs/fs-writeback.c	2010-03-27 11:05:54.000000000 -0400
@@ -75,9 +75,13 @@ static inline bool bdi_work_on_stack(str
 }
 
 static inline void bdi_work_init(struct bdi_work *work,
-				 struct wb_writeback_args *args)
+				 struct wb_writeback_args *args,
+				 int on_stack)
 {
-	INIT_RCU_HEAD(&work->rcu_head);
+	if (on_stack)
+		rcu_head_init_on_stack(&work->rcu_head);
+	else
+		rcu_head_init(&work->rcu_head);
 	work->args = *args;
 	work->state = WS_USED;
 }
@@ -201,7 +205,7 @@ static void bdi_alloc_queue_work(struct 
 	 */
 	work = kmalloc(sizeof(*work), GFP_ATOMIC);
 	if (work) {
-		bdi_work_init(work, args);
+		bdi_work_init(work, args, 0);
 		bdi_queue_work(bdi, work);
 	} else {
 		struct bdi_writeback *wb = &bdi->wb;
@@ -232,11 +236,12 @@ static void bdi_sync_writeback(struct ba
 	};
 	struct bdi_work work;
 
-	bdi_work_init(&work, &args);
+	bdi_work_init(&work, &args, 1);
 	work.state |= WS_ONSTACK;
 
 	bdi_queue_work(bdi, &work);
 	bdi_wait_on_work_clear(&work);
+	destroy_rcu_head_on_stack(&work.rcu_head);
 }
 
 /**
Index: linux.trees.git/fs/partitions/check.c
===================================================================
--- linux.trees.git.orig/fs/partitions/check.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/fs/partitions/check.c	2010-03-27 11:05:54.000000000 -0400
@@ -455,7 +455,7 @@ struct hd_struct *add_partition(struct g
 	}
 
 	/* everything is up and running, commence */
-	INIT_RCU_HEAD(&p->rcu_head);
+	rcu_head_init(&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-27 11:02:22.000000000 -0400
+++ linux.trees.git/block/cfq-iosched.c	2010-03-27 11:05:54.000000000 -0400
@@ -3719,7 +3719,7 @@ 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);
+	rcu_head_init(&cfqd->rcu);
 	return cfqd;
 }
 
Index: linux.trees.git/block/genhd.c
===================================================================
--- linux.trees.git.orig/block/genhd.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/block/genhd.c	2010-03-27 11:05:54.000000000 -0400
@@ -987,7 +987,7 @@ int disk_expand_part_tbl(struct gendisk 
 	if (!new_ptbl)
 		return -ENOMEM;
 
-	INIT_RCU_HEAD(&new_ptbl->rcu_head);
+	rcu_head_init(&new_ptbl->rcu_head);
 	new_ptbl->len = target;
 
 	for (i = 0; i < len; i++)
Index: linux.trees.git/net/netfilter/nfnetlink_queue.c
===================================================================
--- linux.trees.git.orig/net/netfilter/nfnetlink_queue.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/netfilter/nfnetlink_queue.c	2010-03-27 11:05:54.000000000 -0400
@@ -112,7 +112,7 @@ instance_create(u_int16_t queue_num, int
 	inst->copy_mode = NFQNL_COPY_NONE;
 	spin_lock_init(&inst->lock);
 	INIT_LIST_HEAD(&inst->queue_list);
-	INIT_RCU_HEAD(&inst->rcu);
+	rcu_head_init(&inst->rcu);
 
 	if (!try_module_get(THIS_MODULE)) {
 		err = -EAGAIN;
Index: linux.trees.git/net/core/drop_monitor.c
===================================================================
--- linux.trees.git.orig/net/core/drop_monitor.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/core/drop_monitor.c	2010-03-27 11:05:54.000000000 -0400
@@ -296,7 +296,7 @@ static int dropmon_net_event(struct noti
 
 		new_stat->dev = dev;
 		new_stat->last_rx = jiffies;
-		INIT_RCU_HEAD(&new_stat->rcu);
+		rcu_head_init(&new_stat->rcu);
 		spin_lock(&trace_state_lock);
 		list_add_rcu(&new_stat->list, &hw_stats_list);
 		spin_unlock(&trace_state_lock);
Index: linux.trees.git/net/ipv6/sit.c
===================================================================
--- linux.trees.git.orig/net/ipv6/sit.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/ipv6/sit.c	2010-03-27 11:05:54.000000000 -0400
@@ -363,7 +363,7 @@ ipip6_tunnel_add_prl(struct ip_tunnel *t
 		goto out;
 	}
 
-	INIT_RCU_HEAD(&p->rcu_head);
+	rcu_head_init(&p->rcu_head);
 	p->next = t->prl;
 	p->addr = a->addr;
 	p->flags = a->flags;
Index: linux.trees.git/net/ipv6/xfrm6_tunnel.c
===================================================================
--- linux.trees.git.orig/net/ipv6/xfrm6_tunnel.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/ipv6/xfrm6_tunnel.c	2010-03-27 11:05:54.000000000 -0400
@@ -161,7 +161,7 @@ alloc_spi:
 	if (!x6spi)
 		goto out;
 
-	INIT_RCU_HEAD(&x6spi->rcu_head);
+	rcu_head_init(&x6spi->rcu_head);
 	memcpy(&x6spi->addr, saddr, sizeof(x6spi->addr));
 	x6spi->spi = spi;
 	atomic_set(&x6spi->refcnt, 1);
Index: linux.trees.git/net/netfilter/nf_conntrack_expect.c
===================================================================
--- linux.trees.git.orig/net/netfilter/nf_conntrack_expect.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/netfilter/nf_conntrack_expect.c	2010-03-27 11:05:54.000000000 -0400
@@ -239,7 +239,7 @@ struct nf_conntrack_expect *nf_ct_expect
 
 	new->master = me;
 	atomic_set(&new->use, 1);
-	INIT_RCU_HEAD(&new->rcu);
+	rcu_head_init(&new->rcu);
 	return new;
 }
 EXPORT_SYMBOL_GPL(nf_ct_expect_alloc);
Index: linux.trees.git/net/netfilter/nf_conntrack_extend.c
===================================================================
--- linux.trees.git.orig/net/netfilter/nf_conntrack_extend.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/netfilter/nf_conntrack_extend.c	2010-03-27 11:05:54.000000000 -0400
@@ -59,7 +59,7 @@ nf_ct_ext_create(struct nf_ct_ext **ext,
 	if (!*ext)
 		return NULL;
 
-	INIT_RCU_HEAD(&(*ext)->rcu);
+	rcu_head_init(&(*ext)->rcu);
 	(*ext)->offset[id] = off;
 	(*ext)->len = len;
 
Index: linux.trees.git/net/netlabel/netlabel_domainhash.c
===================================================================
--- linux.trees.git.orig/net/netlabel/netlabel_domainhash.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/netlabel/netlabel_domainhash.c	2010-03-27 11:05:54.000000000 -0400
@@ -315,7 +315,7 @@ int netlbl_domhsh_add(struct netlbl_dom_
 		entry_old = netlbl_domhsh_search_def(entry->domain);
 	if (entry_old == NULL) {
 		entry->valid = 1;
-		INIT_RCU_HEAD(&entry->rcu);
+		rcu_head_init(&entry->rcu);
 
 		if (entry->domain != NULL) {
 			u32 bkt = netlbl_domhsh_hash(entry->domain);
Index: linux.trees.git/net/netlabel/netlabel_unlabeled.c
===================================================================
--- linux.trees.git.orig/net/netlabel/netlabel_unlabeled.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/netlabel/netlabel_unlabeled.c	2010-03-27 11:05:54.000000000 -0400
@@ -327,7 +327,7 @@ static int netlbl_unlhsh_add_addr4(struc
 	entry->list.addr = addr->s_addr & mask->s_addr;
 	entry->list.mask = mask->s_addr;
 	entry->list.valid = 1;
-	INIT_RCU_HEAD(&entry->rcu);
+	rcu_head_init(&entry->rcu);
 	entry->secid = secid;
 
 	spin_lock(&netlbl_unlhsh_lock);
@@ -373,7 +373,7 @@ static int netlbl_unlhsh_add_addr6(struc
 	entry->list.addr.s6_addr32[3] &= mask->s6_addr32[3];
 	ipv6_addr_copy(&entry->list.mask, mask);
 	entry->list.valid = 1;
-	INIT_RCU_HEAD(&entry->rcu);
+	rcu_head_init(&entry->rcu);
 	entry->secid = secid;
 
 	spin_lock(&netlbl_unlhsh_lock);
@@ -410,7 +410,7 @@ static struct netlbl_unlhsh_iface *netlb
 	INIT_LIST_HEAD(&iface->addr4_list);
 	INIT_LIST_HEAD(&iface->addr6_list);
 	iface->valid = 1;
-	INIT_RCU_HEAD(&iface->rcu);
+	rcu_head_init(&iface->rcu);
 
 	spin_lock(&netlbl_unlhsh_lock);
 	if (ifindex > 0) {
Index: linux.trees.git/net/sctp/bind_addr.c
===================================================================
--- linux.trees.git.orig/net/sctp/bind_addr.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/sctp/bind_addr.c	2010-03-27 11:05:54.000000000 -0400
@@ -186,7 +186,7 @@ int sctp_add_bind_addr(struct sctp_bind_
 	addr->valid = 1;
 
 	INIT_LIST_HEAD(&addr->list);
-	INIT_RCU_HEAD(&addr->rcu);
+	rcu_head_init(&addr->rcu);
 
 	/* We always hold a socket lock when calling this function,
 	 * and that acts as a writer synchronizing lock.
Index: linux.trees.git/net/sctp/ipv6.c
===================================================================
--- linux.trees.git.orig/net/sctp/ipv6.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/sctp/ipv6.c	2010-03-27 11:05:54.000000000 -0400
@@ -381,7 +381,7 @@ static void sctp_v6_copy_addrlist(struct
 			addr->a.v6.sin6_scope_id = dev->ifindex;
 			addr->valid = 1;
 			INIT_LIST_HEAD(&addr->list);
-			INIT_RCU_HEAD(&addr->rcu);
+			rcu_head_init(&addr->rcu);
 			list_add_tail(&addr->list, addrlist);
 		}
 	}
Index: linux.trees.git/net/sctp/protocol.c
===================================================================
--- linux.trees.git.orig/net/sctp/protocol.c	2010-03-27 11:02:22.000000000 -0400
+++ linux.trees.git/net/sctp/protocol.c	2010-03-27 11:05:54.000000000 -0400
@@ -188,7 +188,7 @@ static void sctp_v4_copy_addrlist(struct
 			addr->a.v4.sin_addr.s_addr = ifa->ifa_local;
 			addr->valid = 1;
 			INIT_LIST_HEAD(&addr->list);
-			INIT_RCU_HEAD(&addr->rcu);
+			rcu_head_init(&addr->rcu);
 			list_add_tail(&addr->list, addrlist);
 		}
 	}
Index: linux.trees.git/net/core/net_namespace.c
===================================================================
--- linux.trees.git.orig/net/core/net_namespace.c	2010-03-27 11:06:17.000000000 -0400
+++ linux.trees.git/net/core/net_namespace.c	2010-03-27 11:07:00.000000000 -0400
@@ -126,7 +126,7 @@ static struct net_generic *net_alloc_gen
 	ng = kzalloc(generic_size, GFP_KERNEL);
 	if (ng) {
 		ng->len = INITIAL_NET_GEN_PTRS;
-		INIT_RCU_HEAD(&ng->rcu);
+		rcu_head_init(&ng->rcu);
 	}
 
 	return ng;
@@ -565,7 +565,7 @@ int net_assign_generic(struct net *net, 
 	 */
 
 	ng->len = id;
-	INIT_RCU_HEAD(&ng->rcu);
+	rcu_head_init(&ng->rcu);
 	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len * sizeof(void*));
 
 	rcu_assign_pointer(net->gen, ng);
Index: linux.trees.git/net/core/neighbour.c
===================================================================
--- linux.trees.git.orig/net/core/neighbour.c	2010-03-27 11:07:06.000000000 -0400
+++ linux.trees.git/net/core/neighbour.c	2010-03-27 11:07:18.000000000 -0400
@@ -1346,7 +1346,7 @@ struct neigh_parms *neigh_parms_alloc(st
 	if (p) {
 		p->tbl		  = tbl;
 		atomic_set(&p->refcnt, 1);
-		INIT_RCU_HEAD(&p->rcu_head);
+		rcu_head_init(&p->rcu_head);
 		p->reachable_time =
 				neigh_rand_reach_time(p->base_reachable_time);
 
@@ -1414,7 +1414,7 @@ void neigh_table_init_no_netlink(struct 
 
 	write_pnet(&tbl->parms.net, &init_net);
 	atomic_set(&tbl->parms.refcnt, 1);
-	INIT_RCU_HEAD(&tbl->parms.rcu_head);
+	rcu_head_init(&tbl->parms.rcu_head);
 	tbl->parms.reachable_time =
 			  neigh_rand_reach_time(tbl->parms.base_reachable_time);
 
Index: linux.trees.git/net/ipv4/cipso_ipv4.c
===================================================================
--- linux.trees.git.orig/net/ipv4/cipso_ipv4.c	2010-03-27 11:07:33.000000000 -0400
+++ linux.trees.git/net/ipv4/cipso_ipv4.c	2010-03-27 11:07:46.000000000 -0400
@@ -502,7 +502,7 @@ int cipso_v4_doi_add(struct cipso_v4_doi
 	}
 
 	atomic_set(&doi_def->refcount, 1);
-	INIT_RCU_HEAD(&doi_def->rcu);
+	rcu_head_init(&doi_def->rcu);
 
 	spin_lock(&cipso_v4_doi_list_lock);
 	if (cipso_v4_doi_search(doi_def->doi) != NULL) {
Index: linux.trees.git/net/ipv4/devinet.c
===================================================================
--- linux.trees.git.orig/net/ipv4/devinet.c	2010-03-27 11:07:54.000000000 -0400
+++ linux.trees.git/net/ipv4/devinet.c	2010-03-27 11:08:11.000000000 -0400
@@ -115,7 +115,7 @@ static struct in_ifaddr *inet_alloc_ifa(
 	struct in_ifaddr *ifa = kzalloc(sizeof(*ifa), GFP_KERNEL);
 
 	if (ifa) {
-		INIT_RCU_HEAD(&ifa->rcu_head);
+		rcu_head_init(&ifa->rcu_head);
 	}
 
 	return ifa;
@@ -161,7 +161,7 @@ static struct in_device *inetdev_init(st
 	in_dev = kzalloc(sizeof(*in_dev), GFP_KERNEL);
 	if (!in_dev)
 		goto out;
-	INIT_RCU_HEAD(&in_dev->rcu_head);
+	rcu_head_init(&in_dev->rcu_head);
 	memcpy(&in_dev->cnf, dev_net(dev)->ipv4.devconf_dflt,
 			sizeof(in_dev->cnf));
 	in_dev->cnf.sysctl = NULL;
Index: linux.trees.git/net/ipv4/route.c
===================================================================
--- linux.trees.git.orig/net/ipv4/route.c	2010-03-27 11:08:22.000000000 -0400
+++ linux.trees.git/net/ipv4/route.c	2010-03-27 11:08:32.000000000 -0400
@@ -1434,7 +1434,7 @@ void ip_rt_redirect(__be32 old_gw, __be3
 
 				/* Copy all the information. */
 				*rt = *rth;
-				INIT_RCU_HEAD(&rt->u.dst.rcu_head);
+				rcu_head_init(&rt->u.dst.rcu_head);
 				rt->u.dst.__use		= 1;
 				atomic_set(&rt->u.dst.__refcnt, 1);
 				rt->u.dst.child		= NULL;
Index: linux.trees.git/arch/powerpc/mm/pgtable.c
===================================================================
--- linux.trees.git.orig/arch/powerpc/mm/pgtable.c	2010-03-27 11:08:59.000000000 -0400
+++ linux.trees.git/arch/powerpc/mm/pgtable.c	2010-03-27 11:09:09.000000000 -0400
@@ -91,7 +91,7 @@ static void pte_free_rcu_callback(struct
 
 static void pte_free_submit(struct pte_freelist_batch *batch)
 {
-	INIT_RCU_HEAD(&batch->rcu);
+	rcu_head_init(&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-27 11:10:30.000000000 -0400
+++ linux.trees.git/security/selinux/avc.c	2010-03-27 11:10:52.000000000 -0400
@@ -288,7 +288,7 @@ static struct avc_node *avc_alloc_node(v
 	if (!node)
 		goto out;
 
-	INIT_RCU_HEAD(&node->rhead);
+	rcu_head_init(&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-27 11:10:58.000000000 -0400
+++ linux.trees.git/security/selinux/netnode.c	2010-03-27 11:11:06.000000000 -0400
@@ -182,7 +182,7 @@ static void sel_netnode_insert(struct se
 		BUG();
 	}
 
-	INIT_RCU_HEAD(&node->rcu);
+	rcu_head_init(&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 */
Index: linux.trees.git/drivers/staging/batman-adv/hard-interface.c
===================================================================
--- linux.trees.git.orig/drivers/staging/batman-adv/hard-interface.c	2010-03-27 11:13:10.000000000 -0400
+++ linux.trees.git/drivers/staging/batman-adv/hard-interface.c	2010-03-27 11:13:20.000000000 -0400
@@ -301,7 +301,7 @@ 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);
+	rcu_head_init(&batman_if->rcu);
 
 	printk(KERN_INFO "batman-adv:Adding interface: %s\n", dev);
 	avail_ifs++;

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

Re: [patch 0/6] rcu head debugobjects

[David Miller]

From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Sat, 27 Mar 2010 11:32:33 -0400

> Unless people have any objection, it should be ready to be merged. I
> think the appropriate maintainer to perform this merge would be Paul
> E. McKenney, because this patchset is RCU-related.

Agreed, and I'm fine with the networking parts, so:

Acked-by: David S. Miller <davem@davemloft.net>

Re: [patch 0/6] rcu head debugobjects

[Paul E. McKenney]

On Sat, Mar 27, 2010 at 11:32:33AM -0400, Mathieu Desnoyers wrote:
> Hi,
> 
> Here is an updated version of the rcu head debugobjects, following the comments
> I received in the last rounds.
> 
> It applies on top of -tip, based on 2.6.34-rc2, commit
> 2e958f219d2b8d192d44e2472a214b3a93c44673
> 
> Unless people have any objection, it should be ready to be merged. I think the
> appropriate maintainer to perform this merge would be Paul E. McKenney, because
> this patchset is RCU-related.

This should be very helpful in tracking down otherwise-painful bugs!!!
Thank you, Mathieu!!!  I am happy to apply this, especially given Dave
Miller's Acked-by.

A few questions and comments:

o	Patches 1/6, 2/6, 3/6: Was the intent for the three Subject
	lines to read as follows?

	[patch 1/6] Revert "net: remove INIT_RCU_HEAD() usage"
	[patch 2/6] Revert "netfilter: don't use INIT_RCU_HEAD()"
	[patch 3/6] Revert "net: don't use INIT_RCU_HEAD"

o	Patch 4/6 looks good to me, and given that Thomas hasn't
	complained, I am guessing that he is OK with it.

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(),
		rcu_head_init_on_stack(), and rcu_head_init() with
		their !CONFIG_DEBUG_OBJECTS_RCU_HEAD definitions.

	b.	Move current patch 6/6 to this position.

	c.	Move current patch 5/6 to this position, updating
		to reflect the new patch added in (a) above.

o	Patch 6/6: Would it be possible to use the object_is_on_stack()
	function defined in include/linux/sched.h instead of passing
	in the flag on_stack to bdi_work_init()?  It looks like
	fs/fs-writeback.c already includes include/linux/sched.h, so
	shouldn't be a problem from a #include-hell viewpoint.

Please let me know if I am missing something on any of the above.  If
these changes seem reasonable to you, please either submit a new patch
set or let me know that you are OK with me making these changes.

							Thanx, Paul

Re: [patch 0/6] rcu head debugobjects

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> On Sat, Mar 27, 2010 at 11:32:33AM -0400, Mathieu Desnoyers wrote:
> > Hi,
> > 
> > Here is an updated version of the rcu head debugobjects, following the comments
> > I received in the last rounds.
> > 
> > It applies on top of -tip, based on 2.6.34-rc2, commit
> > 2e958f219d2b8d192d44e2472a214b3a93c44673
> > 
> > Unless people have any objection, it should be ready to be merged. I think the
> > appropriate maintainer to perform this merge would be Paul E. McKenney, because
> > this patchset is RCU-related.
> 
> This should be very helpful in tracking down otherwise-painful bugs!!!
> Thank you, Mathieu!!!  I am happy to apply this, especially given Dave
> Miller's Acked-by.
> 
> A few questions and comments:
> 
> o	Patches 1/6, 2/6, 3/6: Was the intent for the three Subject
> 	lines to read as follows?
> 
> 	[patch 1/6] Revert "net: remove INIT_RCU_HEAD() usage"
> 	[patch 2/6] Revert "netfilter: don't use INIT_RCU_HEAD()"
> 	[patch 3/6] Revert "net: don't use INIT_RCU_HEAD"

Yep. Oops, got burnt by git show > patches/patchname.patch. Will fix and
re-send.

> 
> o	Patch 4/6 looks good to me, and given that Thomas hasn't
> 	complained, I am guessing that he is OK with it.

OK.

> 
> 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(),
> 		rcu_head_init_on_stack(), and rcu_head_init() with
> 		their !CONFIG_DEBUG_OBJECTS_RCU_HEAD definitions.
> 
> 	b.	Move current patch 6/6 to this position.
> 
> 	c.	Move current patch 5/6 to this position, updating
> 		to reflect the new patch added in (a) above.
> 

Sure. Will do.

> o	Patch 6/6: Would it be possible to use the object_is_on_stack()
> 	function defined in include/linux/sched.h instead of passing
> 	in the flag on_stack to bdi_work_init()?  It looks like
> 	fs/fs-writeback.c already includes include/linux/sched.h, so
> 	shouldn't be a problem from a #include-hell viewpoint.

Wow, that's cool! We learn about exciting internal API functions every day,
isn't life great ? I will definitely change the fs-writeback.c code to make use
of it.

We might event want to go further. A similar scheme could be used for the
rcu_head debugobject activation fixup. Basically, I need a way to distinguish
between:

A) objects on stack and allocated objects
and
B) objects statically initialized

So either we use something resembling:

  if (object_is_on_stack() || object_is_allocated())

or

  if (object_is_static())

I am not aware of the proper API members to do that though.

> 
> Please let me know if I am missing something on any of the above.  If
> these changes seem reasonable to you, please either submit a new patch
> set or let me know that you are OK with me making these changes.

As soon as I get the information about the static vs stack/alloc, I'll complete
the update and re-send the updated patchset.

Thanks,

Mathieu


> 
> 							Thanx, Paul

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

Re: [patch 0/6] rcu head debugobjects

[Mathieu Desnoyers]

* Mathieu Desnoyers (mathieu.desnoyers@efficios.com) wrote:
> * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
[...]
> > o	Patch 6/6: Would it be possible to use the object_is_on_stack()
> > 	function defined in include/linux/sched.h instead of passing
> > 	in the flag on_stack to bdi_work_init()?  It looks like
> > 	fs/fs-writeback.c already includes include/linux/sched.h, so
> > 	shouldn't be a problem from a #include-hell viewpoint.
> 
> Wow, that's cool! We learn about exciting internal API functions every day,
> isn't life great ? I will definitely change the fs-writeback.c code to make use
> of it.
> 
> We might event want to go further. A similar scheme could be used for the
> rcu_head debugobject activation fixup. Basically, I need a way to distinguish
> between:
> 
> A) objects on stack and allocated objects
> and
> B) objects statically initialized
> 
> So either we use something resembling:
> 
>   if (object_is_on_stack() || object_is_allocated())
> 
> or
> 
>   if (object_is_static())
> 
> I am not aware of the proper API members to do that though.
> 

Something close to "object_is_static()" would be "kernel_text_address()", but it
only considers the text section addresses. I'd need something that would be
broader than that, containing all core kernel and module bss and data sections.

Still looking...

Thanks,

Mathieu


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

Re: [patch 0/6] rcu head debugobjects

[Paul E. McKenney]

On Sat, Mar 27, 2010 at 07:20:24PM -0400, Mathieu Desnoyers wrote:
> * Mathieu Desnoyers (mathieu.desnoyers@efficios.com) wrote:
> > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> [...]
> > > o	Patch 6/6: Would it be possible to use the object_is_on_stack()
> > > 	function defined in include/linux/sched.h instead of passing
> > > 	in the flag on_stack to bdi_work_init()?  It looks like
> > > 	fs/fs-writeback.c already includes include/linux/sched.h, so
> > > 	shouldn't be a problem from a #include-hell viewpoint.
> > 
> > Wow, that's cool! We learn about exciting internal API functions every day,
> > isn't life great ? I will definitely change the fs-writeback.c code to make use
> > of it.
> > 
> > We might event want to go further. A similar scheme could be used for the
> > rcu_head debugobject activation fixup. Basically, I need a way to distinguish
> > between:
> > 
> > A) objects on stack and allocated objects
> > and
> > B) objects statically initialized
> > 
> > So either we use something resembling:
> > 
> >   if (object_is_on_stack() || object_is_allocated())
> > 
> > or
> > 
> >   if (object_is_static())
> > 
> > I am not aware of the proper API members to do that though.
> > 
> 
> Something close to "object_is_static()" would be "kernel_text_address()", but it
> only considers the text section addresses. I'd need something that would be
> broader than that, containing all core kernel and module bss and data sections.
> 
> Still looking...

I was actually feeling pretty good about remembering object_is_on_stack()
and finding it again.  ;-)

I am not seeing anything the identifies data or bss, and in any case,
other situations such as per-CPU, __read_mostly, and who knows what all
else would also need to be handled.  So in the short term, my guess would
be that it would be best to provide the three functions (possibly renaming
them as noted above), but to leave the responsibility for figuring out
which to invoke with the caller.  Always happy to be proven wrong, of
course!

							Thanx, Paul

[RFC patch] extable and module add object is static

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> On Sat, Mar 27, 2010 at 07:20:24PM -0400, Mathieu Desnoyers wrote:
[...]
> > Something close to "object_is_static()" would be "kernel_text_address()", but it
> > only considers the text section addresses. I'd need something that would be
> > broader than that, containing all core kernel and module bss and data sections.
> > 
> > Still looking...
> 
> I was actually feeling pretty good about remembering object_is_on_stack()
> and finding it again.  ;-)
> 
> I am not seeing anything the identifies data or bss, and in any case,
> other situations such as per-CPU, __read_mostly, and who knows what all
> else would also need to be handled.  So in the short term, my guess would
> be that it would be best to provide the three functions (possibly renaming
> them as noted above), but to leave the responsibility for figuring out
> which to invoke with the caller.  Always happy to be proven wrong, of
> course!
> 
> 							Thanx, Paul

I think I found a neat way to do object_is_static(), which is all we need here.
Can you have a look at the following patch ? If you think it's right, I'll add
it to the patchset for the next submission.


extable and module add object is static

Adds an API to extable.c to check if an object is statically defined. This
permits, along with object_is_on_stack(), to figure out is an object is either
statically defined (object_is_static()) or allocated on the stack
(object_is_on_stack()).

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/kernel.h |    2 +
 kernel/extable.c       |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

Index: linux.trees.git/include/linux/kernel.h
===================================================================
--- linux.trees.git.orig/include/linux/kernel.h	2010-03-27 19:27:02.000000000 -0400
+++ linux.trees.git/include/linux/kernel.h	2010-03-27 19:28:02.000000000 -0400
@@ -218,6 +218,8 @@ extern int __kernel_text_address(unsigne
 extern int kernel_text_address(unsigned long addr);
 extern int func_ptr_is_kernel_text(void *ptr);
 
+extern int object_is_static(void *obj);
+
 struct pid;
 extern struct pid *session_of_pgrp(struct pid *pgrp);
 
Index: linux.trees.git/kernel/extable.c
===================================================================
--- linux.trees.git.orig/kernel/extable.c	2010-03-27 19:28:06.000000000 -0400
+++ linux.trees.git/kernel/extable.c	2010-03-27 19:54:40.000000000 -0400
@@ -60,6 +60,14 @@ static inline int init_kernel_text(unsig
 	return 0;
 }
 
+static inline int init_kernel_text_or_data(unsigned long addr)
+{
+	if (addr >= (unsigned long)__init_begin &&
+	    addr <= (unsigned long)__init_end)
+		return 1;
+	return 0;
+}
+
 int core_kernel_text(unsigned long addr)
 {
 	if (addr >= (unsigned long)_stext &&
@@ -113,3 +121,47 @@ int func_ptr_is_kernel_text(void *ptr)
 		return 1;
 	return is_module_text_address(addr);
 }
+
+/*
+ * Taking the safe approach to refer to each section individually rather than
+ * just taking a range between _stext and _end, just in case an architecture
+ * would decide to override the standard linker scripts and put a section
+ * outside of this range.
+ *
+ * Should be kept in sync with linux/section.h and asm-generic/vmlinux.lds.h.
+ */
+static int core_object_is_static(void *obj)
+{
+	unsigned long addr = (unsigned long) obj;
+
+	if (addr >= (unsigned long)_sdata &&
+	    addr <= (unsigned long)_edata)
+		return 1;
+	if (addr >= (unsigned long)__bss_start &&
+	    addr <= (unsigned long)__bss_stop)
+		return 1;
+	if (addr >= (unsigned long)__per_cpu_start &&
+	    addr <= (unsigned long)__per_cpu_end)
+		return 1;
+	if (addr >= (unsigned long)__start_rodata &&
+	    addr <= (unsigned long)__end_rodata)
+		return 1;
+	if (system_state == SYSTEM_BOOTING &&
+	    init_kernel_text_or_data(addr))
+		return 1;
+	if (core_kernel_text(addr))
+		return 1;
+	return 0;
+}
+
+/*
+ * object_is_static - returns true is an object is statically defined
+ */
+int object_is_static(void *obj)
+{
+	if (core_object_is_static(obj))
+		return 1;
+	if (is_module_address((unsigned long) obj))
+		return 1;
+	return 0;
+}


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

Re: [RFC patch] extable and module add object is static

[Paul E. McKenney]

On Sat, Mar 27, 2010 at 08:02:34PM -0400, Mathieu Desnoyers wrote:
> * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > On Sat, Mar 27, 2010 at 07:20:24PM -0400, Mathieu Desnoyers wrote:
> [...]
> > > Something close to "object_is_static()" would be "kernel_text_address()", but it
> > > only considers the text section addresses. I'd need something that would be
> > > broader than that, containing all core kernel and module bss and data sections.
> > > 
> > > Still looking...
> > 
> > I was actually feeling pretty good about remembering object_is_on_stack()
> > and finding it again.  ;-)
> > 
> > I am not seeing anything the identifies data or bss, and in any case,
> > other situations such as per-CPU, __read_mostly, and who knows what all
> > else would also need to be handled.  So in the short term, my guess would
> > be that it would be best to provide the three functions (possibly renaming
> > them as noted above), but to leave the responsibility for figuring out
> > which to invoke with the caller.  Always happy to be proven wrong, of
> > course!
> > 
> > 							Thanx, Paul
> 
> I think I found a neat way to do object_is_static(), which is all we need here.
> Can you have a look at the following patch ? If you think it's right, I'll add
> it to the patchset for the next submission.

Looks plausible to me!

Rusty, any thoughts?

							Thanx, Paul

> extable and module add object is static
> 
> Adds an API to extable.c to check if an object is statically defined. This
> permits, along with object_is_on_stack(), to figure out is an object is either
> statically defined (object_is_static()) or allocated on the stack
> (object_is_on_stack()).
> 
> 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/kernel.h |    2 +
>  kernel/extable.c       |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+)
> 
> Index: linux.trees.git/include/linux/kernel.h
> ===================================================================
> --- linux.trees.git.orig/include/linux/kernel.h	2010-03-27 19:27:02.000000000 -0400
> +++ linux.trees.git/include/linux/kernel.h	2010-03-27 19:28:02.000000000 -0400
> @@ -218,6 +218,8 @@ extern int __kernel_text_address(unsigne
>  extern int kernel_text_address(unsigned long addr);
>  extern int func_ptr_is_kernel_text(void *ptr);
> 
> +extern int object_is_static(void *obj);
> +
>  struct pid;
>  extern struct pid *session_of_pgrp(struct pid *pgrp);
> 
> Index: linux.trees.git/kernel/extable.c
> ===================================================================
> --- linux.trees.git.orig/kernel/extable.c	2010-03-27 19:28:06.000000000 -0400
> +++ linux.trees.git/kernel/extable.c	2010-03-27 19:54:40.000000000 -0400
> @@ -60,6 +60,14 @@ static inline int init_kernel_text(unsig
>  	return 0;
>  }
> 
> +static inline int init_kernel_text_or_data(unsigned long addr)
> +{
> +	if (addr >= (unsigned long)__init_begin &&
> +	    addr <= (unsigned long)__init_end)
> +		return 1;
> +	return 0;
> +}
> +
>  int core_kernel_text(unsigned long addr)
>  {
>  	if (addr >= (unsigned long)_stext &&
> @@ -113,3 +121,47 @@ int func_ptr_is_kernel_text(void *ptr)
>  		return 1;
>  	return is_module_text_address(addr);
>  }
> +
> +/*
> + * Taking the safe approach to refer to each section individually rather than
> + * just taking a range between _stext and _end, just in case an architecture
> + * would decide to override the standard linker scripts and put a section
> + * outside of this range.
> + *
> + * Should be kept in sync with linux/section.h and asm-generic/vmlinux.lds.h.
> + */
> +static int core_object_is_static(void *obj)
> +{
> +	unsigned long addr = (unsigned long) obj;
> +
> +	if (addr >= (unsigned long)_sdata &&
> +	    addr <= (unsigned long)_edata)
> +		return 1;
> +	if (addr >= (unsigned long)__bss_start &&
> +	    addr <= (unsigned long)__bss_stop)
> +		return 1;
> +	if (addr >= (unsigned long)__per_cpu_start &&
> +	    addr <= (unsigned long)__per_cpu_end)
> +		return 1;
> +	if (addr >= (unsigned long)__start_rodata &&
> +	    addr <= (unsigned long)__end_rodata)
> +		return 1;
> +	if (system_state == SYSTEM_BOOTING &&
> +	    init_kernel_text_or_data(addr))
> +		return 1;
> +	if (core_kernel_text(addr))
> +		return 1;
> +	return 0;
> +}
> +
> +/*
> + * object_is_static - returns true is an object is statically defined
> + */
> +int object_is_static(void *obj)
> +{
> +	if (core_object_is_static(obj))
> +		return 1;
> +	if (is_module_address((unsigned long) obj))
> +		return 1;
> +	return 0;
> +}
> 
> 
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com

Re: [RFC patch] extable and module add object is static

[Lai Jiangshan]

Mathieu Desnoyers wrote:
> +static int core_object_is_static(void *obj)
> +{
[...]
> +	if (addr >= (unsigned long)__per_cpu_start &&
> +	    addr <= (unsigned long)__per_cpu_end)
> +		return 1;

This may only correct for UP.
You may need arch-special codes for SMP.

Re: [RFC patch] extable and module add object is static

[Mathieu Desnoyers]

* Lai Jiangshan (laijs@cn.fujitsu.com) wrote:
> Mathieu Desnoyers wrote:
> > +static int core_object_is_static(void *obj)
> > +{
> [...]
> > +	if (addr >= (unsigned long)__per_cpu_start &&
> > +	    addr <= (unsigned long)__per_cpu_end)
> > +		return 1;
> 
> This may only correct for UP.
> You may need arch-special codes for SMP.
> 

looking at include/linux/percpu.h:

#ifndef PERCPU_ENOUGH_ROOM
#define PERCPU_ENOUGH_ROOM                                              \
        (ALIGN(__per_cpu_end - __per_cpu_start, SMP_CACHE_BYTES) +      \
         PERCPU_MODULE_RESERVE)
#endif

I was under the impression that most architectures were keeping their per-cpu
data within the __per_cpu_start .. __per_cpu_end range. But I see that ia64
is the only one to redefine PERCPU_ENOUGH_ROOM. I'm not sure if it can be a
problem. Is that what you had in mind ?

(adding Tony and Fenghua in CC so they can confirm)

Thanks,

Mathieu


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

Re: [RFC patch] extable and module add object is static

[Peter Zijlstra]

On Mon, 2010-03-29 at 09:39 +0800, Lai Jiangshan wrote:
> Mathieu Desnoyers wrote:
> > +static int core_object_is_static(void *obj)
> > +{
> [...]
> > +	if (addr >= (unsigned long)__per_cpu_start &&
> > +	    addr <= (unsigned long)__per_cpu_end)
> > +		return 1;
> 
> This may only correct for UP.
> You may need arch-special codes for SMP.
> 

Tejun has patches for this.

Re: [RFC patch] extable and module add object is static

[Mathieu Desnoyers]

* Peter Zijlstra (peterz@infradead.org) wrote:
> On Mon, 2010-03-29 at 09:39 +0800, Lai Jiangshan wrote:
> > Mathieu Desnoyers wrote:
> > > +static int core_object_is_static(void *obj)
> > > +{
> > [...]
> > > +	if (addr >= (unsigned long)__per_cpu_start &&
> > > +	    addr <= (unsigned long)__per_cpu_end)
> > > +		return 1;
> > 
> > This may only correct for UP.
> > You may need arch-special codes for SMP.
> > 
> 
> Tejun has patches for this.
> 

That's great! Tejun, can you point me out to an update version of these
patches ? I am particularly interested in being able to know the range of
statically defined per-cpu data.

Thanks,

Mathieu

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

Re: [RFC patch] extable and module add object is static

[Tejun Heo]

Hello,

On 03/29/2010 10:16 PM, Mathieu Desnoyers wrote:
> That's great! Tejun, can you point me out to an update version of these
> patches ? I am particularly interested in being able to know the range of
> statically defined per-cpu data.

  http://thread.gmane.org/gmane.linux.kernel/958794/focus=959493

These were waiting for Rusty's ACK.  They got ACKed today and will be
pushed to mainline through percpu tree soonish.

Thanks.

-- 
tejun

Re: [RFC patch] extable and module add object is static

[Mathieu Desnoyers]

* Tejun Heo (tj@kernel.org) wrote:
> Hello,
> 
> On 03/29/2010 10:16 PM, Mathieu Desnoyers wrote:
> > That's great! Tejun, can you point me out to an update version of these
> > patches ? I am particularly interested in being able to know the range of
> > statically defined per-cpu data.
> 
>   http://thread.gmane.org/gmane.linux.kernel/958794/focus=959493
> 
> These were waiting for Rusty's ACK.  They got ACKed today and will be
> pushed to mainline through percpu tree soonish.

OK. I just figured that I could initialize the rcu_heads in all cases in the
debugobject fixup anyway, so I guess I won't need "object_is_static()" after
all. But I can keep the patch around so it can eventually be re-sent to
standardize the debugobjects activation fixups. They currently need to keep a
flag around to identify statically defined objects.

Thanks,

Mathieu

> 
> Thanks.
> 
> -- 
> tejun

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

Re: [patch 0/6] rcu head debugobjects

[Mathieu Desnoyers]

Hi Paul,

Thinking about the rcu head init topic, we might be able to drop the
init_rcu_head() initializer. The idea is the following:

- We need init_rcu_head_on_stack()/destroy_rcu_head_on_stack().
- call_rcu() populates the rcu_head and normally does not care about it being
  pre-initialized.
- The activation fixup can detect if a non-initialized rcu head is being
  activated and just perform the fixup without complaining.
- If we have two call_rcu() in a row in the same GP on the same rcu_head, the
  activation check will detect it.

So either we remove all the init_rcu_head(), as was originally proposed, or we
use one that is a no-op on !DEBUG configs and initialize the object with DEBUG
configs.

That removes the dependency on object_is_static().

Thoughts ?

Thanks,

Mathieu


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

Re: [patch 0/6] rcu head debugobjects

[Paul E. McKenney]

On Mon, Mar 29, 2010 at 09:39:33AM -0400, Mathieu Desnoyers wrote:
> Hi Paul,
> 
> Thinking about the rcu head init topic, we might be able to drop the
> init_rcu_head() initializer. The idea is the following:
> 
> - We need init_rcu_head_on_stack()/destroy_rcu_head_on_stack().
> - call_rcu() populates the rcu_head and normally does not care about it being
>   pre-initialized.
> - The activation fixup can detect if a non-initialized rcu head is being
>   activated and just perform the fixup without complaining.
> - If we have two call_rcu() in a row in the same GP on the same rcu_head, the
>   activation check will detect it.
> 
> So either we remove all the init_rcu_head(), as was originally proposed, or we
> use one that is a no-op on !DEBUG configs and initialize the object with DEBUG
> configs.
> 
> That removes the dependency on object_is_static().

If I understand correctly, this does sound good.  Here is what I think
you are proposing:

o	call_rcu() and friends only complain if handed an rcu_head
	structure that is still queued awaiting a grace period.
	They don't care otherwise.

o	rcu_do_batch() complains unless the rcu_head structure has
	most recently been enqueued by call_rcu() or one if its friends.

Did I get it right?

							Thanx, Paul

Re: [patch 0/6] rcu head debugobjects

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> On Mon, Mar 29, 2010 at 09:39:33AM -0400, Mathieu Desnoyers wrote:
> > Hi Paul,
> > 
> > Thinking about the rcu head init topic, we might be able to drop the
> > init_rcu_head() initializer. The idea is the following:
> > 
> > - We need init_rcu_head_on_stack()/destroy_rcu_head_on_stack().
> > - call_rcu() populates the rcu_head and normally does not care about it being
> >   pre-initialized.
> > - The activation fixup can detect if a non-initialized rcu head is being
> >   activated and just perform the fixup without complaining.
> > - If we have two call_rcu() in a row in the same GP on the same rcu_head, the
> >   activation check will detect it.
> > 
> > So either we remove all the init_rcu_head(), as was originally proposed, or we
> > use one that is a no-op on !DEBUG configs and initialize the object with DEBUG
> > configs.
> > 
> > That removes the dependency on object_is_static().
> 
> If I understand correctly, this does sound good.  Here is what I think
> you are proposing:
> 
> o	call_rcu() and friends only complain if handed an rcu_head
> 	structure that is still queued awaiting a grace period.
> 	They don't care otherwise.
> 
> o	rcu_do_batch() complains unless the rcu_head structure has
> 	most recently been enqueued by call_rcu() or one if its friends.
> 
> Did I get it right?

Exactly.

Thanks,

Mathieu

> 
> 							Thanx, Paul

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

Re: [patch 0/6] rcu head debugobjects

[Paul E. McKenney]

On Mon, Mar 29, 2010 at 11:04:25AM -0400, Mathieu Desnoyers wrote:
> * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > On Mon, Mar 29, 2010 at 09:39:33AM -0400, Mathieu Desnoyers wrote:
> > > Hi Paul,
> > > 
> > > Thinking about the rcu head init topic, we might be able to drop the
> > > init_rcu_head() initializer. The idea is the following:
> > > 
> > > - We need init_rcu_head_on_stack()/destroy_rcu_head_on_stack().
> > > - call_rcu() populates the rcu_head and normally does not care about it being
> > >   pre-initialized.
> > > - The activation fixup can detect if a non-initialized rcu head is being
> > >   activated and just perform the fixup without complaining.
> > > - If we have two call_rcu() in a row in the same GP on the same rcu_head, the
> > >   activation check will detect it.
> > > 
> > > So either we remove all the init_rcu_head(), as was originally proposed, or we
> > > use one that is a no-op on !DEBUG configs and initialize the object with DEBUG
> > > configs.
> > > 
> > > That removes the dependency on object_is_static().
> > 
> > If I understand correctly, this does sound good.  Here is what I think
> > you are proposing:
> > 
> > o	call_rcu() and friends only complain if handed an rcu_head
> > 	structure that is still queued awaiting a grace period.
> > 	They don't care otherwise.
> > 
> > o	rcu_do_batch() complains unless the rcu_head structure has
> > 	most recently been enqueued by call_rcu() or one if its friends.
> > 
> > Did I get it right?
> 
> Exactly.

Very good!!!

							Thanx, Paul

Re: [patch 0/6] rcu head debugobjects

[Thomas Gleixner]

B1;2005;0cOn Sat, 27 Mar 2010, Paul E. McKenney wrote:
> 
> o	Patch 4/6 looks good to me, and given that Thomas hasn't
> 	complained, I am guessing that he is OK with it.

Looks sane at the first glance. Will go over it in detail tomorrow.
 
> o	Patch 6/6: Would it be possible to use the object_is_on_stack()
> 	function defined in include/linux/sched.h instead of passing
> 	in the flag on_stack to bdi_work_init()?  It looks like
> 	fs/fs-writeback.c already includes include/linux/sched.h, so
> 	shouldn't be a problem from a #include-hell viewpoint.

Well, I'm a bit wary about that. The reason is that we really want
the annotation of:

       init_on_stack();
       ....
       destroy_on_stack();

instead of the confusing:

       init();
       ....
       destroy_on_stack();

So having an automatism in the bdi_work_init() function will people
make forget to put the destroy_on_stack() annotation into it.

The flag is horrible as well. How about this:

/* helper function, do not use in code ! */
__bdi_work_init(....., onstack)
{
	....
	if (on_stack) {
	   work.state |= WS_ONSTACK;
	   init_rcu_head_on_stack(&work.rcu_head);
	} else {
	....
}

See, how this moves also the "work.state |= WS_ONSTACK;" line out of
the calling code.

bdi_work_init(...)
{
	 __bdi_work_init(...., false);
}

bdi_work_init_on_stack(...)
{
	 __bdi_work_init(...., true);
}


out of the code.

To make it complete, please do not use the asymmetric:

   destroy_rcu_head_on_stack(&work.rcu_head);

Create a helper function:

bdi_destroy_work_on_stack(...)
{
   destroy_rcu_head_on_stack(work->rcu_head);
}    

That makes it way more readable and we did that with the other on
stack initializers as well.

Thanks,

	tglx

Re: [patch 0/6] rcu head debugobjects

[Paul E. McKenney]

On Sun, Mar 28, 2010 at 04:07:10AM +0200, Thomas Gleixner wrote:
> B1;2005;0cOn Sat, 27 Mar 2010, Paul E. McKenney wrote:
> > 
> > o	Patch 4/6 looks good to me, and given that Thomas hasn't
> > 	complained, I am guessing that he is OK with it.
> 
> Looks sane at the first glance. Will go over it in detail tomorrow.
> 
> > o	Patch 6/6: Would it be possible to use the object_is_on_stack()
> > 	function defined in include/linux/sched.h instead of passing
> > 	in the flag on_stack to bdi_work_init()?  It looks like
> > 	fs/fs-writeback.c already includes include/linux/sched.h, so
> > 	shouldn't be a problem from a #include-hell viewpoint.
> 
> Well, I'm a bit wary about that. The reason is that we really want
> the annotation of:
> 
>        init_on_stack();
>        ....
>        destroy_on_stack();
> 
> instead of the confusing:
> 
>        init();
>        ....
>        destroy_on_stack();
> 
> So having an automatism in the bdi_work_init() function will people
> make forget to put the destroy_on_stack() annotation into it.
> 
> The flag is horrible as well. How about this:
> 
> /* helper function, do not use in code ! */
> __bdi_work_init(....., onstack)
> {
> 	....
> 	if (on_stack) {
> 	   work.state |= WS_ONSTACK;
> 	   init_rcu_head_on_stack(&work.rcu_head);
> 	} else {
> 	....
> }
> 
> See, how this moves also the "work.state |= WS_ONSTACK;" line out of
> the calling code.
> 
> bdi_work_init(...)
> {
> 	 __bdi_work_init(...., false);
> }
> 
> bdi_work_init_on_stack(...)
> {
> 	 __bdi_work_init(...., true);
> }
> 
> 
> out of the code.
> 
> To make it complete, please do not use the asymmetric:
> 
>    destroy_rcu_head_on_stack(&work.rcu_head);
> 
> Create a helper function:
> 
> bdi_destroy_work_on_stack(...)
> {
>    destroy_rcu_head_on_stack(work->rcu_head);
> }    
> 
> That makes it way more readable and we did that with the other on
> stack initializers as well.

Hello, Thomas,

I must defer to you on this one.  ;-)

							Thanx, Paul

Re: [patch 0/6] rcu head debugobjects

[Mathieu Desnoyers]

* Thomas Gleixner (tglx@linutronix.de) wrote:
> B1;2005;0cOn Sat, 27 Mar 2010, Paul E. McKenney wrote:
> > 
> > o	Patch 4/6 looks good to me, and given that Thomas hasn't
> > 	complained, I am guessing that he is OK with it.
> 
> Looks sane at the first glance. Will go over it in detail tomorrow.
>  
> > o	Patch 6/6: Would it be possible to use the object_is_on_stack()
> > 	function defined in include/linux/sched.h instead of passing
> > 	in the flag on_stack to bdi_work_init()?  It looks like
> > 	fs/fs-writeback.c already includes include/linux/sched.h, so
> > 	shouldn't be a problem from a #include-hell viewpoint.
> 
> Well, I'm a bit wary about that. The reason is that we really want
> the annotation of:
> 
>        init_on_stack();
>        ....
>        destroy_on_stack();
> 
> instead of the confusing:
> 
>        init();
>        ....
>        destroy_on_stack();
> 
> So having an automatism in the bdi_work_init() function will people
> make forget to put the destroy_on_stack() annotation into it.
> 
> The flag is horrible as well. How about this:
> 
> /* helper function, do not use in code ! */
> __bdi_work_init(....., onstack)
> {
> 	....
> 	if (on_stack) {
> 	   work.state |= WS_ONSTACK;
> 	   init_rcu_head_on_stack(&work.rcu_head);
> 	} else {
> 	....
> }
> 
> See, how this moves also the "work.state |= WS_ONSTACK;" line out of
> the calling code.
> 
> bdi_work_init(...)
> {
> 	 __bdi_work_init(...., false);
> }
> 
> bdi_work_init_on_stack(...)
> {
> 	 __bdi_work_init(...., true);
> }
> 
> 
> out of the code.
> 
> To make it complete, please do not use the asymmetric:
> 
>    destroy_rcu_head_on_stack(&work.rcu_head);
> 
> Create a helper function:
> 
> bdi_destroy_work_on_stack(...)
> {
>    destroy_rcu_head_on_stack(work->rcu_head);
> }    
> 
> That makes it way more readable and we did that with the other on
> stack initializers as well.

These changes are queued for v3. Thanks Thomas!

Mathieu

> 
> Thanks,
> 
> 	tglx

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