[PATCH tip/core/rcu 0/23] infrastructure for sparse checks for RCU usage

[PATCH tip/core/rcu 0/23] infrastructure for sparse checks for RCU usage

[Paul E. McKenney]

Hello!

This patchset pulls Arnd's sparse-checking commits out of the earlier
patchbomb (http://lkml.org/lkml/2010/5/4/298):

1.	Add an rcu_dereference_check() API in order to continue supporting
	the array-index use cases that would otherwise be invalidated
	by this change.
2.	Add an empty __rcu annotation API to reduce inter-commit dependency.
3-5.	Updates to vfs, net, and mce to avoid breakage by later commits.
6.	Introduce sparse __rcu functionality.
7.	Update rculist primitives to avoid false positives.
8-22.	Introduce __rcu annotations to cgroups, credentials, keys, nfs,
	net, perf_event, notifiers, radix-tree, idr, input, net-netfilter,
	kvm, and kernel.
23.	Introduce __rcu annotations and checks to vhost.

These have been reordered and consolidated to reduce inter-commit
dependencies.

							Thanx, Paul

 b/arch/x86/include/asm/kvm_host.h      |    2 
 b/arch/x86/kernel/cpu/mcheck/mce.c     |    2 
 b/drivers/input/evdev.c                |    2 
 b/drivers/net/bnx2.h                   |    2 
 b/drivers/net/bnx2x.h                  |    2 
 b/drivers/net/cnic.h                   |    2 
 b/drivers/net/macvtap.c                |    2 
 b/drivers/vhost/net.c                  |   11 -
 b/drivers/vhost/vhost.c                |   14 -
 b/drivers/vhost/vhost.h                |    4 
 b/include/linux/cgroup.h               |    4 
 b/include/linux/compiler.h             |    2 
 b/include/linux/cred.h                 |    2 
 b/include/linux/fdtable.h              |    1 
 b/include/linux/fs.h                   |    2 
 b/include/linux/genhd.h                |    6 
 b/include/linux/idr.h                  |    4 
 b/include/linux/if_bridge.h            |    3 
 b/include/linux/if_macvlan.h           |    2 
 b/include/linux/igmp.h                 |    4 
 b/include/linux/init_task.h            |    4 
 b/include/linux/input.h                |    2 
 b/include/linux/iocontext.h            |    2 
 b/include/linux/key.h                  |    3 
 b/include/linux/kvm_host.h             |    2 
 b/include/linux/mm_types.h             |    2 
 b/include/linux/netdevice.h            |   12 -
 b/include/linux/nfs_fs.h               |    2 
 b/include/linux/notifier.h             |   10 
 b/include/linux/perf_event.h           |    6 
 b/include/linux/radix-tree.h           |    4 
 b/include/linux/rculist.h              |   53 +++-
 b/include/linux/rculist_nulls.h        |   16 +
 b/include/linux/rcupdate.h             |   33 +++
 b/include/linux/sched.h                |    2 
 b/include/linux/srcu.h                 |   27 ++
 b/include/linux/sunrpc/auth_gss.h      |    4 
 b/include/net/dst.h                    |    2 
 b/include/net/fib_rules.h              |    2 
 b/include/net/garp.h                   |    2 
 b/include/net/inet_sock.h              |    2 
 b/include/net/ip6_tunnel.h             |    2 
 b/include/net/ipip.h                   |    6 
 b/include/net/net_namespace.h          |    2 
 b/include/net/netfilter/nf_conntrack.h |    2 
 b/include/net/netns/xfrm.h             |    2 
 b/include/net/sock.h                   |    4 
 b/kernel/cgroup.c                      |    2 
 b/kernel/pid.c                         |    2 
 b/kernel/rcupdate.c                    |    6 
 b/kernel/sched.c                       |    2 
 b/lib/Kconfig.debug                    |   13 +
 b/lib/radix-tree.c                     |    2 
 b/net/802/stp.c                        |    4 
 b/net/bridge/br_fdb.c                  |    2 
 b/net/bridge/br_private.h              |    8 
 b/net/bridge/netfilter/ebt_redirect.c  |    2 
 b/net/bridge/netfilter/ebt_ulog.c      |    4 
 b/net/bridge/netfilter/ebtables.c      |    4 
 b/net/ipv4/ip_gre.c                    |    2 
 b/net/ipv4/ipip.c                      |   10 
 b/net/ipv4/netfilter/nf_nat_core.c     |    2 
 b/net/ipv4/protocol.c                  |    2 
 b/net/ipv4/route.c                     |    2 
 b/net/ipv4/tcp.c                       |    4 
 b/net/ipv6/ip6_tunnel.c                |    6 
 b/net/ipv6/protocol.c                  |    2 
 b/net/ipv6/sit.c                       |   10 
 b/net/mac80211/ieee80211_i.h           |   15 -
 b/net/mac80211/sta_info.h              |    4 
 b/net/netfilter/core.c                 |    2 
 b/net/netfilter/nf_conntrack_ecache.c  |    4 
 b/net/netfilter/nf_conntrack_extend.c  |    2 
 b/net/netfilter/nf_conntrack_proto.c   |    4 
 b/net/netfilter/nf_log.c               |    2 
 b/net/netfilter/nf_queue.c             |    2 
 b/net/netfilter/nfnetlink_log.c        |    4 
 b/net/netfilter/nfnetlink_queue.c      |    4 
 b/net/netlabel/netlabel_domainhash.c   |    4 
 b/net/netlabel/netlabel_unlabeled.c    |    4 
 b/net/netlink/af_netlink.c             |    2 
 b/net/phonet/af_phonet.c               |    2 
 b/net/phonet/pn_dev.c                  |    2 
 b/net/socket.c                         |    2 
 include/linux/compiler.h               |    4 
 include/linux/fdtable.h                |    6 
 include/linux/kvm_host.h               |    4 
 include/linux/rcupdate.h               |  352 +++++++++++++++++++--------------
 include/linux/sched.h                  |    6 
 89 files changed, 498 insertions(+), 307 deletions(-)

[PATCH RFC tip/core/rcu 01/23] rcu: add an rcu_dereference_index_check()

[Paul E. McKenney]

The sparse RCU-pointer checking relies on type magic that dereferences
the pointer in question.  This does not work if the pointer is in fact
an array index.  This commit therefore supplies a new RCU API that
omits the sparse checking to continue to support rcu_dereference()
on integers.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcupdate.h |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 2f9e56c..3be0ad7 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -560,4 +560,37 @@ static inline void debug_rcu_head_unqueue(struct rcu_head *head)
 }
 #endif	/* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
 
+#ifndef CONFIG_PROVE_RCU
+#define __do_rcu_dereference_check(c) do { } while (0)
+#endif /* #ifdef CONFIG_PROVE_RCU */
+
+#define __rcu_dereference_index_check(p, c) \
+	({ \
+		typeof(p) _________p1 = ACCESS_ONCE(p); \
+		__do_rcu_dereference_check(c); \
+		smp_read_barrier_depends(); \
+		(_________p1); \
+	})
+
+/**
+ * rcu_dereference_index_check() - rcu_dereference for indices with debug checking
+ * @p: The pointer to read, prior to dereferencing
+ * @c: The conditions under which the dereference will take place
+ *
+ * Similar to rcu_dereference_check(), but omits the sparse checking.
+ * This allows rcu_dereference_index_check() to be used on integers,
+ * which can then be used as array indices.  Attempting to use
+ * rcu_dereference_check() on an integer will give compiler warnings
+ * because the sparse address-space mechanism relies on dereferencing
+ * the RCU-protected pointer.  Dereferencing integers is not something
+ * that even gcc will put up with.
+ *
+ * Note that this function does not implicitly check for RCU read-side
+ * critical sections.  If this function gains lots of uses, it might
+ * make sense to provide versions for each flavor of RCU, but it does
+ * not make sense as of early 2010.
+ */
+#define rcu_dereference_index_check(p, c) \
+	__rcu_dereference_index_check((p), (c))
+
 #endif /* __LINUX_RCUPDATE_H */
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 02/23] rcu: add __rcu API for later sparse checking

[Paul E. McKenney]

This commit defines an __rcu API, but provides only vacuous definitions
for it.  This breaks dependencies among most of the subsequent patches,
allowing them to reach mainline asynchronously via whatever trees are
appropriate.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Josh Triplett <josh@joshtriplett.org>
---
 include/linux/compiler.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index a5a472b..c1a62c5 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -16,6 +16,7 @@
 # define __release(x)	__context__(x,-1)
 # define __cond_lock(x,c)	((c) ? ({ __acquire(x); 1; }) : 0)
 # define __percpu	__attribute__((noderef, address_space(3)))
+# define __rcu
 extern void __chk_user_ptr(const volatile void __user *);
 extern void __chk_io_ptr(const volatile void __iomem *);
 #else
@@ -34,6 +35,7 @@ extern void __chk_io_ptr(const volatile void __iomem *);
 # define __release(x) (void)0
 # define __cond_lock(x,c) (c)
 # define __percpu
+# define __rcu
 #endif
 
 #ifdef __KERNEL__
-- 
1.7.0.6

Re: [PATCH RFC tip/core/rcu 02/23] rcu: add __rcu API for later sparse checking

[Matt Helsley]

On Wed, May 12, 2010 at 02:33:21PM -0700, Paul E. McKenney wrote:
> This commit defines an __rcu API, but provides only vacuous definitions
> for it.  This breaks dependencies among most of the subsequent patches,
> allowing them to reach mainline asynchronously via whatever trees are
> appropriate.

Seems like a good plan to me.

I know it's not the right time to push it but I am curious to see what,
approximately, you expect a non-vacuous __rcu definition to look like.
(i.e. when it's being run through sparse)

Cheers,
	-Matt Helsley

> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Christopher Li <sparse@chrisli.org>
> Cc: Josh Triplett <josh@joshtriplett.org>
> ---
>  include/linux/compiler.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index a5a472b..c1a62c5 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -16,6 +16,7 @@
>  # define __release(x)	__context__(x,-1)
>  # define __cond_lock(x,c)	((c) ? ({ __acquire(x); 1; }) : 0)
>  # define __percpu	__attribute__((noderef, address_space(3)))
> +# define __rcu
>  extern void __chk_user_ptr(const volatile void __user *);
>  extern void __chk_io_ptr(const volatile void __iomem *);
>  #else
> @@ -34,6 +35,7 @@ extern void __chk_io_ptr(const volatile void __iomem *);
>  # define __release(x) (void)0
>  # define __cond_lock(x,c) (c)
>  # define __percpu
> +# define __rcu
>  #endif
> 
>  #ifdef __KERNEL__
> -- 
> 1.7.0.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC tip/core/rcu 02/23] rcu: add __rcu API for later sparse checking

[Paul E. McKenney]

On Thu, May 13, 2010 at 01:53:07PM -0700, Matt Helsley wrote:
> On Wed, May 12, 2010 at 02:33:21PM -0700, Paul E. McKenney wrote:
> > This commit defines an __rcu API, but provides only vacuous definitions
> > for it.  This breaks dependencies among most of the subsequent patches,
> > allowing them to reach mainline asynchronously via whatever trees are
> > appropriate.
> 
> Seems like a good plan to me.
> 
> I know it's not the right time to push it but I am curious to see what,
> approximately, you expect a non-vacuous __rcu definition to look like.
> (i.e. when it's being run through sparse)

Patch 6 of this series defines the sparse magic and the rcu_dereference()
checks that do this.  But yes, there are dependencies among these patches.

							Thanx, Paul

> Cheers,
> 	-Matt Helsley
> 
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: Christopher Li <sparse@chrisli.org>
> > Cc: Josh Triplett <josh@joshtriplett.org>
> > ---
> >  include/linux/compiler.h |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > index a5a472b..c1a62c5 100644
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -16,6 +16,7 @@
> >  # define __release(x)	__context__(x,-1)
> >  # define __cond_lock(x,c)	((c) ? ({ __acquire(x); 1; }) : 0)
> >  # define __percpu	__attribute__((noderef, address_space(3)))
> > +# define __rcu
> >  extern void __chk_user_ptr(const volatile void __user *);
> >  extern void __chk_io_ptr(const volatile void __iomem *);
> >  #else
> > @@ -34,6 +35,7 @@ extern void __chk_io_ptr(const volatile void __iomem *);
> >  # define __release(x) (void)0
> >  # define __cond_lock(x,c) (c)
> >  # define __percpu
> > +# define __rcu
> >  #endif
> > 
> >  #ifdef __KERNEL__
> > -- 
> > 1.7.0.6
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

[PATCH RFC tip/core/rcu 03/23] vfs: add fs.h to define struct file

[Paul E. McKenney]

The sparse RCU-pointer annotations require definition of the
underlying type of any pointer passed to rcu_dereference() and friends.
So fcheck_files() needs "struct file" to be defined, so include fs.h.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
---
 include/linux/fdtable.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 013dc52..551671e 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -11,6 +11,7 @@
 #include <linux/rcupdate.h>
 #include <linux/types.h>
 #include <linux/init.h>
+#include <linux/fs.h>
 
 #include <asm/atomic.h>
 
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 04/23] net: Make accesses to ->br_port safe for sparse RCU

[Paul E. McKenney]

The new versions of the rcu_dereference() APIs requires that any pointers
passed to one of these APIs be fully defined.  The ->br_port field
in struct net_device points to a struct net_bridge_port, which is an
incomplete type.  This commit therefore changes ->br_port to be a void*,
and introduces a br_port() helper function to convert the type to struct
net_bridge_port, and applies this new helper function where required.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: David Miller <davem@davemloft.net>
Cc: Stephen Hemminger <shemminger@linux-foundation.org>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/if_bridge.h           |    3 +++
 net/bridge/br_fdb.c                 |    2 +-
 net/bridge/br_private.h             |    8 ++++++++
 net/bridge/netfilter/ebt_redirect.c |    2 +-
 net/bridge/netfilter/ebt_ulog.c     |    4 ++--
 net/bridge/netfilter/ebtables.c     |    4 ++--
 net/netfilter/nfnetlink_log.c       |    4 ++--
 net/netfilter/nfnetlink_queue.c     |    4 ++--
 8 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 938b7e8..d001d78 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -101,6 +101,9 @@ struct __fdb_entry {
 
 #include <linux/netdevice.h>
 
+/* br_handle_frame_hook() needs the following forward declaration. */
+struct net_bridge_port;
+
 extern void brioctl_set(int (*ioctl_hook)(struct net *, unsigned int, void __user *));
 extern struct sk_buff *(*br_handle_frame_hook)(struct net_bridge_port *p,
 					       struct sk_buff *skb);
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 9101a4e..3f66cd1 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -246,7 +246,7 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
 		return 0;
 
 	rcu_read_lock();
-	fdb = __br_fdb_get(dev->br_port->br, addr);
+	fdb = __br_fdb_get(br_port(dev)->br, addr);
 	ret = fdb && fdb->dst->dev != dev &&
 		fdb->dst->state == BR_STATE_FORWARDING;
 	rcu_read_unlock();
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 846d7d1..4fedb60 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -229,6 +229,14 @@ static inline int br_is_root_bridge(const struct net_bridge *br)
 	return !memcmp(&br->bridge_id, &br->designated_root, 8);
 }
 
+static inline struct net_bridge_port *br_port(const struct net_device *dev)
+{
+	if (!dev)
+		return NULL;
+
+	return rcu_dereference(dev->br_port);
+}
+
 /* br_device.c */
 extern void br_dev_setup(struct net_device *dev);
 extern netdev_tx_t br_dev_xmit(struct sk_buff *skb,
diff --git a/net/bridge/netfilter/ebt_redirect.c b/net/bridge/netfilter/ebt_redirect.c
index 9be8fbc..4fa8377 100644
--- a/net/bridge/netfilter/ebt_redirect.c
+++ b/net/bridge/netfilter/ebt_redirect.c
@@ -25,7 +25,7 @@ ebt_redirect_tg(struct sk_buff *skb, const struct xt_target_param *par)
 
 	if (par->hooknum != NF_BR_BROUTING)
 		memcpy(eth_hdr(skb)->h_dest,
-		       par->in->br_port->br->dev->dev_addr, ETH_ALEN);
+		       br_port(par->in)->br->dev->dev_addr, ETH_ALEN);
 	else
 		memcpy(eth_hdr(skb)->h_dest, par->in->dev_addr, ETH_ALEN);
 	skb->pkt_type = PACKET_HOST;
diff --git a/net/bridge/netfilter/ebt_ulog.c b/net/bridge/netfilter/ebt_ulog.c
index f9560f3..32ca502 100644
--- a/net/bridge/netfilter/ebt_ulog.c
+++ b/net/bridge/netfilter/ebt_ulog.c
@@ -183,7 +183,7 @@ static void ebt_ulog_packet(unsigned int hooknr, const struct sk_buff *skb,
 		strcpy(pm->physindev, in->name);
 		/* If in isn't a bridge, then physindev==indev */
 		if (in->br_port)
-			strcpy(pm->indev, in->br_port->br->dev->name);
+			strcpy(pm->indev, br_port(in)->br->dev->name);
 		else
 			strcpy(pm->indev, in->name);
 	} else
@@ -192,7 +192,7 @@ static void ebt_ulog_packet(unsigned int hooknr, const struct sk_buff *skb,
 	if (out) {
 		/* If out exists, then out is a bridge port */
 		strcpy(pm->physoutdev, out->name);
-		strcpy(pm->outdev, out->br_port->br->dev->name);
+		strcpy(pm->outdev, br_port(out)->br->dev->name);
 	} else
 		pm->outdev[0] = pm->physoutdev[0] = '\0';
 
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index f0865fd..ac1361b 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -141,10 +141,10 @@ ebt_basic_match(const struct ebt_entry *e, const struct ethhdr *h,
 	if (FWINV2(ebt_dev_check(e->out, out), EBT_IOUT))
 		return 1;
 	if ((!in || !in->br_port) ? 0 : FWINV2(ebt_dev_check(
-	   e->logical_in, in->br_port->br->dev), EBT_ILOGICALIN))
+	   e->logical_in, br_port(in)->br->dev), EBT_ILOGICALIN))
 		return 1;
 	if ((!out || !out->br_port) ? 0 : FWINV2(ebt_dev_check(
-	   e->logical_out, out->br_port->br->dev), EBT_ILOGICALOUT))
+	   e->logical_out, br_port(out)->br->dev), EBT_ILOGICALOUT))
 		return 1;
 
 	if (e->bitmask & EBT_SOURCEMAC) {
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 203643f..d89c1be 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -404,7 +404,7 @@ __build_packet_message(struct nfulnl_instance *inst,
 				     htonl(indev->ifindex));
 			/* this is the bridge group "brX" */
 			NLA_PUT_BE32(inst->skb, NFULA_IFINDEX_INDEV,
-				     htonl(indev->br_port->br->dev->ifindex));
+				     htonl(br_port(indev)->br->dev->ifindex));
 		} else {
 			/* Case 2: indev is bridge group, we need to look for
 			 * physical device (when called from ipv4) */
@@ -431,7 +431,7 @@ __build_packet_message(struct nfulnl_instance *inst,
 				     htonl(outdev->ifindex));
 			/* this is the bridge group "brX" */
 			NLA_PUT_BE32(inst->skb, NFULA_IFINDEX_OUTDEV,
-				     htonl(outdev->br_port->br->dev->ifindex));
+				     htonl(br_port(outdev)->br->dev->ifindex));
 		} else {
 			/* Case 2: indev is a bridge group, we need to look
 			 * for physical device (when called from ipv4) */
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index e70a6ef..0be9156 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -298,7 +298,7 @@ nfqnl_build_packet_message(struct nfqnl_instance *queue,
 				     htonl(indev->ifindex));
 			/* this is the bridge group "brX" */
 			NLA_PUT_BE32(skb, NFQA_IFINDEX_INDEV,
-				     htonl(indev->br_port->br->dev->ifindex));
+				     htonl(br_port(indev)->br->dev->ifindex));
 		} else {
 			/* Case 2: indev is bridge group, we need to look for
 			 * physical device (when called from ipv4) */
@@ -323,7 +323,7 @@ nfqnl_build_packet_message(struct nfqnl_instance *queue,
 				     htonl(outdev->ifindex));
 			/* this is the bridge group "brX" */
 			NLA_PUT_BE32(skb, NFQA_IFINDEX_OUTDEV,
-				     htonl(outdev->br_port->br->dev->ifindex));
+				     htonl(br_port(outdev)->br->dev->ifindex));
 		} else {
 			/* Case 2: outdev is bridge group, we need to look for
 			 * physical output device (when called from ipv4) */
-- 
1.7.0.6

Re: [PATCH RFC tip/core/rcu 04/23] net: Make accesses to ->br_port safe for sparse RCU

[Stephen Hemminger]

On Wed, 12 May 2010 14:33:23 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 9101a4e..3f66cd1 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -246,7 +246,7 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
>  		return 0;
>  
>  	rcu_read_lock();
> -	fdb = __br_fdb_get(dev->br_port->br, addr);
> +	fdb = __br_fdb_get(br_port(dev)->br, addr);
>  	ret = fdb && fdb->dst->dev != dev &&
>  		fdb->dst->state == BR_STATE_FORWARDING;
>  	rcu_read_unlock();
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 846d7d1..4fedb60 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -229,6 +229,14 @@ static inline int br_is_root_bridge(const struct net_bridge *br)
>  	return !memcmp(&br->bridge_id, &br->designated_root, 8);
>  }
>  
> +static inline struct net_bridge_port *br_port(const struct net_device *dev)
> +{
> +	if (!dev)
> +		return NULL;
> +
> +	return rcu_dereference(dev->br_port);
> +}

Looks like this is wrapping existing problems, and hurting not helping.

Why introduce a wrapper that could return NULL and not check the
result?

I would rather that:
   1. dev should never be null in this cases so the first if() is 
      unnecessary, and confuses the semantics.
   2. don't use wrapper br_port()
   3. have callers check that rcu_dereference(dev->br_port) did not
      return NULL.
      If they derefernce does return NULL, then it means other CPU
      has started tear down and this CPU should just go home quietly.

-- 

Re: [PATCH RFC tip/core/rcu 04/23] net: Make accesses to ->br_port safe for sparse RCU

[Paul E. McKenney]

On Wed, May 12, 2010 at 02:44:53PM -0700, Stephen Hemminger wrote:
> On Wed, 12 May 2010 14:33:23 -0700
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> 
> > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> > index 9101a4e..3f66cd1 100644
> > --- a/net/bridge/br_fdb.c
> > +++ b/net/bridge/br_fdb.c
> > @@ -246,7 +246,7 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
> >  		return 0;
> >  
> >  	rcu_read_lock();
> > -	fdb = __br_fdb_get(dev->br_port->br, addr);
> > +	fdb = __br_fdb_get(br_port(dev)->br, addr);
> >  	ret = fdb && fdb->dst->dev != dev &&
> >  		fdb->dst->state == BR_STATE_FORWARDING;
> >  	rcu_read_unlock();
> > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > index 846d7d1..4fedb60 100644
> > --- a/net/bridge/br_private.h
> > +++ b/net/bridge/br_private.h
> > @@ -229,6 +229,14 @@ static inline int br_is_root_bridge(const struct net_bridge *br)
> >  	return !memcmp(&br->bridge_id, &br->designated_root, 8);
> >  }
> >  
> > +static inline struct net_bridge_port *br_port(const struct net_device *dev)
> > +{
> > +	if (!dev)
> > +		return NULL;
> > +
> > +	return rcu_dereference(dev->br_port);
> > +}
> 
> Looks like this is wrapping existing problems, and hurting not helping.
> 
> Why introduce a wrapper that could return NULL and not check the
> result?

Fair point!

> I would rather that:
>    1. dev should never be null in this cases so the first if() is 
>       unnecessary, and confuses the semantics.
>    2. don't use wrapper br_port()
>    3. have callers check that rcu_dereference(dev->br_port) did not
>       return NULL.
>       If they derefernce does return NULL, then it means other CPU
>       has started tear down and this CPU should just go home quietly.

OK.

The reason for br_port() is to allow ->br_port to be a void*.  If we
eliminate br_port(), then it is necessary to make the definition of the
struct net_bridge_port available everywhere that ->br_port is given to
rcu_dereference().  The reason for this is that Arnd's sparse-based RCU
checking code uses __rcu to tag the data pointed to by an RCU-protected
pointer.  This in turn means that rcu_dereference() and friends must
now have access to the pointed-to type, as is done in patch 6 in this
series.

One way to make struct net_bridge_port available is to put:

	#include "../../net/bridge/br_private.h"

in include/linux/netdevice.h.

However, when I try this, I get lots of build errors, which was what led
us to the path of making ->br_port be a void*, thus requiring the br_port()
helper function in cases where the caller needs the underlying type.

What should we be doing instead?

							Thanx, Paul

Re: [PATCH RFC tip/core/rcu 04/23] net: Make accesses to ->br_port safe for sparse RCU

[Stephen Hemminger]

On Wed, 12 May 2010 15:35:25 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> On Wed, May 12, 2010 at 02:44:53PM -0700, Stephen Hemminger wrote:
> > On Wed, 12 May 2010 14:33:23 -0700
> > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> > > index 9101a4e..3f66cd1 100644
> > > --- a/net/bridge/br_fdb.c
> > > +++ b/net/bridge/br_fdb.c
> > > @@ -246,7 +246,7 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
> > >  		return 0;
> > >  
> > >  	rcu_read_lock();
> > > -	fdb = __br_fdb_get(dev->br_port->br, addr);
> > > +	fdb = __br_fdb_get(br_port(dev)->br, addr);
> > >  	ret = fdb && fdb->dst->dev != dev &&
> > >  		fdb->dst->state == BR_STATE_FORWARDING;
> > >  	rcu_read_unlock();
> > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > > index 846d7d1..4fedb60 100644
> > > --- a/net/bridge/br_private.h
> > > +++ b/net/bridge/br_private.h
> > > @@ -229,6 +229,14 @@ static inline int br_is_root_bridge(const struct net_bridge *br)
> > >  	return !memcmp(&br->bridge_id, &br->designated_root, 8);
> > >  }
> > >  
> > > +static inline struct net_bridge_port *br_port(const struct net_device *dev)
> > > +{
> > > +	if (!dev)
> > > +		return NULL;
> > > +
> > > +	return rcu_dereference(dev->br_port);
> > > +}
> > 
> > Looks like this is wrapping existing problems, and hurting not helping.
> > 
> > Why introduce a wrapper that could return NULL and not check the
> > result?
> 
> Fair point!
> 
> > I would rather that:
> >    1. dev should never be null in this cases so the first if() is 
> >       unnecessary, and confuses the semantics.
> >    2. don't use wrapper br_port()
> >    3. have callers check that rcu_dereference(dev->br_port) did not
> >       return NULL.
> >       If they derefernce does return NULL, then it means other CPU
> >       has started tear down and this CPU should just go home quietly.
> 
> OK.
> 
> The reason for br_port() is to allow ->br_port to be a void*.  If we
> eliminate br_port(), then it is necessary to make the definition of the
> struct net_bridge_port available everywhere that ->br_port is given to
> rcu_dereference().  The reason for this is that Arnd's sparse-based RCU
> checking code uses __rcu to tag the data pointed to by an RCU-protected
> pointer.  This in turn means that rcu_dereference() and friends must
> now have access to the pointed-to type, as is done in patch 6 in this
> series.

Then ok. leave the wrapper, but get rid of the !dev part.

I can do it if you want.

Still don't like changing working code to conform to code checking tools.
Especially when code checking tool is missing bad RCU usage that already
exists (like this case). It is a big problem if code assumes rcu_deref
always returns non NULL.

-- 

Re: [PATCH RFC tip/core/rcu 04/23] net: Make accesses to ->br_port safe for sparse RCU

[Paul E. McKenney]

On Wed, May 12, 2010 at 06:33:59PM -0700, Stephen Hemminger wrote:
> On Wed, 12 May 2010 15:35:25 -0700
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> 
> > On Wed, May 12, 2010 at 02:44:53PM -0700, Stephen Hemminger wrote:
> > > On Wed, 12 May 2010 14:33:23 -0700
> > > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > > 
> > > > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> > > > index 9101a4e..3f66cd1 100644
> > > > --- a/net/bridge/br_fdb.c
> > > > +++ b/net/bridge/br_fdb.c
> > > > @@ -246,7 +246,7 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
> > > >  		return 0;
> > > >  
> > > >  	rcu_read_lock();
> > > > -	fdb = __br_fdb_get(dev->br_port->br, addr);
> > > > +	fdb = __br_fdb_get(br_port(dev)->br, addr);
> > > >  	ret = fdb && fdb->dst->dev != dev &&
> > > >  		fdb->dst->state == BR_STATE_FORWARDING;
> > > >  	rcu_read_unlock();
> > > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > > > index 846d7d1..4fedb60 100644
> > > > --- a/net/bridge/br_private.h
> > > > +++ b/net/bridge/br_private.h
> > > > @@ -229,6 +229,14 @@ static inline int br_is_root_bridge(const struct net_bridge *br)
> > > >  	return !memcmp(&br->bridge_id, &br->designated_root, 8);
> > > >  }
> > > >  
> > > > +static inline struct net_bridge_port *br_port(const struct net_device *dev)
> > > > +{
> > > > +	if (!dev)
> > > > +		return NULL;
> > > > +
> > > > +	return rcu_dereference(dev->br_port);
> > > > +}
> > > 
> > > Looks like this is wrapping existing problems, and hurting not helping.
> > > 
> > > Why introduce a wrapper that could return NULL and not check the
> > > result?
> > 
> > Fair point!
> > 
> > > I would rather that:
> > >    1. dev should never be null in this cases so the first if() is 
> > >       unnecessary, and confuses the semantics.
> > >    2. don't use wrapper br_port()
> > >    3. have callers check that rcu_dereference(dev->br_port) did not
> > >       return NULL.
> > >       If they derefernce does return NULL, then it means other CPU
> > >       has started tear down and this CPU should just go home quietly.
> > 
> > OK.
> > 
> > The reason for br_port() is to allow ->br_port to be a void*.  If we
> > eliminate br_port(), then it is necessary to make the definition of the
> > struct net_bridge_port available everywhere that ->br_port is given to
> > rcu_dereference().  The reason for this is that Arnd's sparse-based RCU
> > checking code uses __rcu to tag the data pointed to by an RCU-protected
> > pointer.  This in turn means that rcu_dereference() and friends must
> > now have access to the pointed-to type, as is done in patch 6 in this
> > series.
> 
> Then ok. leave the wrapper, but get rid of the !dev part.
> 
> I can do it if you want.

Done!

I would normally accept your offer in a heartbeat, but there are
dependencies among the patches.  :-(

> Still don't like changing working code to conform to code checking tools.
> Especially when code checking tool is missing bad RCU usage that already
> exists (like this case). It is a big problem if code assumes rcu_deref
> always returns non NULL.

Indeed, I would have liked some way of making this work without having
to make rcu_dereference() know about the pointed-too type.  I have not
yet been able to find one, though.  :-(

							Thanx, Paul

[PATCH RFC tip/core/rcu 05/23] mce: convert to rcu_dereference_index_check()

[Paul E. McKenney]

The mce processing applies rcu_dereference_check() to integers used as
array indices.  This patch therefore moves mce to the new RCU API
rcu_dereference_index_check() that avoids the sparse processing that
would otherwise result in compiler errors.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 8a6f0af..f00a9f2 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -50,7 +50,7 @@
 static DEFINE_MUTEX(mce_read_mutex);
 
 #define rcu_dereference_check_mce(p) \
-	rcu_dereference_check((p), \
+	rcu_dereference_index_check((p), \
 			      rcu_read_lock_sched_held() || \
 			      lockdep_is_held(&mce_read_mutex))
 
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 06/23] rcu: define __rcu address space modifier for sparse

[Paul E. McKenney]

This commit provides definitions for the __rcu annotation defined earlier.
This annotation permits sparse to check for correct use of RCU-protected
pointers.  If a pointer that is annotated with __rcu is accessed
directly (as opposed to via rcu_dereference(), rcu_assign_pointer(),
or one of their variants), sparse can be made to complain.  To enable
such complaints, use the new default-disabled CONFIG_SPARSE_RCU_POINTER
kernel configuration option.  Please note that these sparse complaints are
intended to be a debugging aid, -not- a code-style-enforcement mechanism.

There are special rcu_dereference_protected() and rcu_access_pointer()
accessors for use when RCU read-side protection is not required, for
example, when no other CPU has access to the data structure in question
or while the current CPU hold the update-side lock.

This patch also updates a number of docbook comments that were showing
their age.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Josh Triplett <josh@joshtriplett.org>
---
 include/linux/compiler.h |    4 +
 include/linux/rcupdate.h |  352 ++++++++++++++++++++++++++++------------------
 include/linux/srcu.h     |   27 +++-
 kernel/rcupdate.c        |    6 +-
 lib/Kconfig.debug        |   13 ++
 5 files changed, 257 insertions(+), 145 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index c1a62c5..320d6c9 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -16,7 +16,11 @@
 # define __release(x)	__context__(x,-1)
 # define __cond_lock(x,c)	((c) ? ({ __acquire(x); 1; }) : 0)
 # define __percpu	__attribute__((noderef, address_space(3)))
+#ifdef CONFIG_SPARSE_RCU_POINTER
+# define __rcu		__attribute__((noderef, address_space(4)))
+#else
 # define __rcu
+#endif
 extern void __chk_user_ptr(const volatile void __user *);
 extern void __chk_io_ptr(const volatile void __iomem *);
 #else
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 3be0ad7..0ce0f05 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -41,6 +41,7 @@
 #include <linux/lockdep.h>
 #include <linux/completion.h>
 #include <linux/debugobjects.h>
+#include <linux/compiler.h>
 
 #ifdef CONFIG_RCU_TORTURE_TEST
 extern int rcutorture_runnable; /* for sysctl */
@@ -114,14 +115,15 @@ extern struct lockdep_map rcu_sched_lock_map;
 extern int debug_lockdep_rcu_enabled(void);
 
 /**
- * rcu_read_lock_held - might we be in RCU read-side critical section?
+ * rcu_read_lock_held() - might we be in RCU read-side critical section?
  *
  * If CONFIG_DEBUG_LOCK_ALLOC is selected, returns nonzero iff in an RCU
  * read-side critical section.  In absence of CONFIG_DEBUG_LOCK_ALLOC,
  * this assumes we are in an RCU read-side critical section unless it can
- * prove otherwise.
+ * prove otherwise.  This is useful for debug checks in functions that
+ * require that they be called within an RCU read-side critical section.
  *
- * Check debug_lockdep_rcu_enabled() to prevent false positives during boot
+ * Checks debug_lockdep_rcu_enabled() to prevent false positives during boot
  * and while lockdep is disabled.
  */
 static inline int rcu_read_lock_held(void)
@@ -138,14 +140,16 @@ static inline int rcu_read_lock_held(void)
 extern int rcu_read_lock_bh_held(void);
 
 /**
- * rcu_read_lock_sched_held - might we be in RCU-sched read-side critical section?
+ * rcu_read_lock_sched_held() - might we be in RCU-sched read-side critical section?
  *
  * If CONFIG_DEBUG_LOCK_ALLOC is selected, returns nonzero iff in an
  * RCU-sched read-side critical section.  In absence of
  * CONFIG_DEBUG_LOCK_ALLOC, this assumes we are in an RCU-sched read-side
  * critical section unless it can prove otherwise.  Note that disabling
  * of preemption (including disabling irqs) counts as an RCU-sched
- * read-side critical section.
+ * read-side critical section.  This is useful for debug checks in functions
+ * that required that they be called within an RCU-sched read-side
+ * critical section.
  *
  * Check debug_lockdep_rcu_enabled() to prevent false positives during boot
  * and while lockdep is disabled.
@@ -214,41 +218,155 @@ extern int rcu_my_thread_group_empty(void);
 		}							\
 	} while (0)
 
+#else /* #ifdef CONFIG_PROVE_RCU */
+
+#define __do_rcu_dereference_check(c) do { } while (0)
+
+#endif /* #else #ifdef CONFIG_PROVE_RCU */
+
+/*
+ * Helper functions for rcu_dereference_check(), rcu_dereference_protected()
+ * and rcu_assign_pointer().  Some of these could be folded into their
+ * callers, but they are left separate in order to ease introduction of
+ * multiple flavors of pointers to match the multiple flavors of RCU
+ * (e.g., __rcu_bh, * __rcu_sched, and __srcu), should this make sense in
+ * the future.
+ */
+#define __rcu_access_pointer(p, space) \
+	({ \
+		typeof(*p) *_________p1 = (typeof(*p)*__force )ACCESS_ONCE(p); \
+		(void) (((typeof (*p) space *)p) == p); \
+		((typeof(*p) __force __kernel *)(_________p1)); \
+	})
+#define __rcu_dereference_check(p, c, space) \
+	({ \
+		typeof(*p) *_________p1 = (typeof(*p)*__force )ACCESS_ONCE(p); \
+		__do_rcu_dereference_check(c); \
+		(void) (((typeof (*p) space *)p) == p); \
+		smp_read_barrier_depends(); \
+		((typeof(*p) __force __kernel *)(_________p1)); \
+	})
+#define __rcu_dereference_protected(p, c, space) \
+	({ \
+		__do_rcu_dereference_check(c); \
+		(void) (((typeof (*p) space *)p) == p); \
+		((typeof(*p) __force __kernel *)(p)); \
+	})
+
+#define __rcu_dereference_index_check(p, c) \
+	({ \
+		typeof(p) _________p1 = ACCESS_ONCE(p); \
+		__do_rcu_dereference_check(c); \
+		smp_read_barrier_depends(); \
+		(_________p1); \
+	})
+#define __rcu_assign_pointer(p, v, space) \
+	({ \
+		if (!__builtin_constant_p(v) || \
+		    ((v) != NULL)) \
+			smp_wmb(); \
+		(p) = (typeof(*v) __force space *)(v); \
+	})
+
+
+/**
+ * rcu_access_pointer() - fetch RCU pointer with no dereferencing
+ * @p: The pointer to read
+ *
+ * Return the value of the specified RCU-protected pointer, but omit the
+ * smp_read_barrier_depends() and keep the ACCESS_ONCE().  This is useful
+ * when the value of this pointer is accessed, but the pointer is not
+ * dereferenced, for example, when testing an RCU-protected pointer against
+ * NULL.  Although rcu_access_pointer() may also be used in cases where
+ * update-side locks prevent the value of the pointer from changing, you
+ * should instead use rcu_dereference_protected() for this use case.
+ */
+#define rcu_access_pointer(p) __rcu_access_pointer((p), __rcu)
+
 /**
- * rcu_dereference_check - rcu_dereference with debug checking
+ * rcu_dereference_check() - rcu_dereference with debug checking
  * @p: The pointer to read, prior to dereferencing
  * @c: The conditions under which the dereference will take place
  *
  * Do an rcu_dereference(), but check that the conditions under which the
- * dereference will take place are correct.  Typically the conditions indicate
- * the various locking conditions that should be held at that point.  The check
- * should return true if the conditions are satisfied.
+ * dereference will take place are correct.  Typically the conditions
+ * indicate the various locking conditions that should be held at that
+ * point.  The check should return true if the conditions are satisfied.
+ * An implicit check for being in an RCU read-side critical section
+ * (rcu_read_lock()) is included.
  *
  * For example:
  *
- *	bar = rcu_dereference_check(foo->bar, rcu_read_lock_held() ||
- *					      lockdep_is_held(&foo->lock));
+ *	bar = rcu_dereference_check(foo->bar, lockdep_is_held(&foo->lock));
  *
  * could be used to indicate to lockdep that foo->bar may only be dereferenced
- * if either the RCU read lock is held, or that the lock required to replace
+ * if either rcu_read_lock() is held, or that the lock required to replace
  * the bar struct at foo->bar is held.
  *
  * Note that the list of conditions may also include indications of when a lock
  * need not be held, for example during initialisation or destruction of the
  * target struct:
  *
- *	bar = rcu_dereference_check(foo->bar, rcu_read_lock_held() ||
- *					      lockdep_is_held(&foo->lock) ||
+ *	bar = rcu_dereference_check(foo->bar, lockdep_is_held(&foo->lock) ||
  *					      atomic_read(&foo->usage) == 0);
+ *
+ * Inserts memory barriers on architectures that require them
+ * (currently only the Alpha), prevents the compiler from refetching
+ * (and from merging fetches), and, more importantly, documents exactly
+ * which pointers are protected by RCU and checks that the pointer is
+ * annotated as __rcu.
  */
 #define rcu_dereference_check(p, c) \
-	({ \
-		__do_rcu_dereference_check(c); \
-		rcu_dereference_raw(p); \
-	})
+	__rcu_dereference_check((p), rcu_read_lock_held() || (c), __rcu)
+
+/**
+ * rcu_dereference_bh_check() - rcu_dereference_bh with debug checking
+ * @p: The pointer to read, prior to dereferencing
+ * @c: The conditions under which the dereference will take place
+ *
+ * This is the RCU-bh counterpart to rcu_dereference_check().
+ */
+#define rcu_dereference_bh_check(p, c) \
+	__rcu_dereference_check((p), rcu_read_lock_bh_held() || (c), __rcu)
 
 /**
- * rcu_dereference_protected - fetch RCU pointer when updates prevented
+ * rcu_dereference_sched_check() - rcu_dereference_sched with debug checking
+ * @p: The pointer to read, prior to dereferencing
+ * @c: The conditions under which the dereference will take place
+ *
+ * This is the RCU-sched counterpart to rcu_dereference_check().
+ */
+#define rcu_dereference_sched_check(p, c) \
+	__rcu_dereference_check((p), rcu_read_lock_sched_held() || (c), \
+				__rcu)
+
+#define rcu_dereference_raw(p) rcu_dereference_check(p, 1) /*@@@ needed? @@@*/
+
+/**
+ * rcu_dereference_index_check() - rcu_dereference for indices with debug checking
+ * @p: The pointer to read, prior to dereferencing
+ * @c: The conditions under which the dereference will take place
+ *
+ * Similar to rcu_dereference_check(), but omits the sparse checking.
+ * This allows rcu_dereference_index_check() to be used on integers,
+ * which can then be used as array indices.  Attempting to use
+ * rcu_dereference_check() on an integer will give compiler warnings
+ * because the sparse address-space mechanism relies on dereferencing
+ * the RCU-protected pointer.  Dereferencing integers is not something
+ * that even gcc will put up with.
+ *
+ * Note that this function does not implicitly check for RCU read-side
+ * critical sections.  If this function gains lots of uses, it might
+ * make sense to provide versions for each flavor of RCU, but it does
+ * not make sense as of early 2010.
+ */
+#define rcu_dereference_index_check(p, c) \
+	__rcu_dereference_index_check((p), (c))
+
+/**
+ * rcu_dereference_protected() - fetch RCU pointer when updates prevented
+ * @p: The pointer to read, prior to dereferencing
+ * @c: The conditions under which the dereference will take place
  *
  * Return the value of the specified RCU-protected pointer, but omit
  * both the smp_read_barrier_depends() and the ACCESS_ONCE().  This
@@ -257,35 +375,61 @@ extern int rcu_my_thread_group_empty(void);
  * prevent the compiler from repeating this reference or combining it
  * with other references, so it should not be used without protection
  * of appropriate locks.
+ *
+ * This function is only for update-side use.  Using this function
+ * when protected only by rcu_read_lock() will result in infrequent
+ * but very ugly failures.
  */
 #define rcu_dereference_protected(p, c) \
-	({ \
-		__do_rcu_dereference_check(c); \
-		(p); \
-	})
+	__rcu_dereference_protected((p), (c), __rcu)
 
-#else /* #ifdef CONFIG_PROVE_RCU */
+/**
+ * rcu_dereference_bh_protected() - fetch RCU-bh pointer when updates prevented
+ * @p: The pointer to read, prior to dereferencing
+ * @c: The conditions under which the dereference will take place
+ *
+ * This is the RCU-bh counterpart to rcu_dereference_protected().
+ */
+#define rcu_dereference_bh_protected(p, c) \
+	__rcu_dereference_protected((p), (c), __rcu)
 
-#define rcu_dereference_check(p, c)	rcu_dereference_raw(p)
-#define rcu_dereference_protected(p, c) (p)
+/**
+ * rcu_dereference_sched_protected() - fetch RCU-sched pointer when updates prevented
+ * @p: The pointer to read, prior to dereferencing
+ * @c: The conditions under which the dereference will take place
+ *
+ * This is the RCU-sched counterpart to rcu_dereference_protected().
+ */
+#define rcu_dereference_sched_protected(p, c) \
+	__rcu_dereference_protected((p), (c), __rcu)
 
-#endif /* #else #ifdef CONFIG_PROVE_RCU */
 
 /**
- * rcu_access_pointer - fetch RCU pointer with no dereferencing
+ * rcu_dereference() - fetch RCU-protected pointer for dereferencing
+ * @p: The pointer to read, prior to dereferencing
  *
- * Return the value of the specified RCU-protected pointer, but omit the
- * smp_read_barrier_depends() and keep the ACCESS_ONCE().  This is useful
- * when the value of this pointer is accessed, but the pointer is not
- * dereferenced, for example, when testing an RCU-protected pointer against
- * NULL.  This may also be used in cases where update-side locks prevent
- * the value of the pointer from changing, but rcu_dereference_protected()
- * is a lighter-weight primitive for this use case.
+ * This is a simple wrapper around rcu_dereference_check().
  */
-#define rcu_access_pointer(p)	ACCESS_ONCE(p)
+#define rcu_dereference(p) rcu_dereference_check(p, 0)
 
 /**
- * rcu_read_lock - mark the beginning of an RCU read-side critical section.
+ * rcu_dereference_bh() - fetch an RCU-bh-protected pointer for dereferencing
+ * @p: The pointer to read, prior to dereferencing
+ *
+ * Makes rcu_dereference_check() do the dirty work.
+ */
+#define rcu_dereference_bh(p) rcu_dereference_bh_check(p, 0)
+
+/**
+ * rcu_dereference_sched() - fetch RCU-sched-protected pointer for dereferencing
+ * @p: The pointer to read, prior to dereferencing
+ *
+ * Makes rcu_dereference_check() do the dirty work.
+ */
+#define rcu_dereference_sched(p) rcu_dereference_sched_check(p, 0)
+
+/**
+ * rcu_read_lock() - mark the beginning of an RCU read-side critical section
  *
  * When synchronize_rcu() is invoked on one CPU while other CPUs
  * are within RCU read-side critical sections, then the
@@ -331,7 +475,7 @@ static inline void rcu_read_lock(void)
  */
 
 /**
- * rcu_read_unlock - marks the end of an RCU read-side critical section.
+ * rcu_read_unlock() - marks the end of an RCU read-side critical section.
  *
  * See rcu_read_lock() for more information.
  */
@@ -343,15 +487,16 @@ static inline void rcu_read_unlock(void)
 }
 
 /**
- * rcu_read_lock_bh - mark the beginning of a softirq-only RCU critical section
+ * rcu_read_lock_bh() - mark the beginning of an RCU-bh critical section
  *
  * This is equivalent of rcu_read_lock(), but to be used when updates
- * are being done using call_rcu_bh(). Since call_rcu_bh() callbacks
- * consider completion of a softirq handler to be a quiescent state,
- * a process in RCU read-side critical section must be protected by
- * disabling softirqs. Read-side critical sections in interrupt context
- * can use just rcu_read_lock().
- *
+ * are being done using call_rcu_bh() or synchronize_rcu_bh(). Since
+ * both call_rcu_bh() and synchronize_rcu_bh() consider completion of a
+ * softirq handler to be a quiescent state, a process in RCU read-side
+ * critical section must be protected by disabling softirqs. Read-side
+ * critical sections in interrupt context can use just rcu_read_lock(),
+ * though this should at least be commented to avoid confusing people
+ * reading the code.
  */
 static inline void rcu_read_lock_bh(void)
 {
@@ -373,13 +518,12 @@ static inline void rcu_read_unlock_bh(void)
 }
 
 /**
- * rcu_read_lock_sched - mark the beginning of a RCU-classic critical section
+ * rcu_read_lock_sched() - mark the beginning of a RCU-sched critical section
  *
- * Should be used with either
- * - synchronize_sched()
- * or
- * - call_rcu_sched() and rcu_barrier_sched()
- * on the write-side to insure proper synchronization.
+ * This is equivalent of rcu_read_lock(), but to be used when updates
+ * are being done using call_rcu_sched() or synchronize_rcu_sched().
+ * Read-side critical sections can also be introduced by anything that
+ * disables preemption, including local_irq_disable() and friends.
  */
 static inline void rcu_read_lock_sched(void)
 {
@@ -414,54 +558,14 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
 	preempt_enable_notrace();
 }
 
-
 /**
- * rcu_dereference_raw - fetch an RCU-protected pointer
+ * rcu_assign_pointer() - assign to RCU-protected pointer
+ * @p: pointer to assign to
+ * @v: value to assign (publish)
  *
- * The caller must be within some flavor of RCU read-side critical
- * section, or must be otherwise preventing the pointer from changing,
- * for example, by holding an appropriate lock.  This pointer may later
- * be safely dereferenced.  It is the caller's responsibility to have
- * done the right thing, as this primitive does no checking of any kind.
- *
- * Inserts memory barriers on architectures that require them
- * (currently only the Alpha), and, more importantly, documents
- * exactly which pointers are protected by RCU.
- */
-#define rcu_dereference_raw(p)	({ \
-				typeof(p) _________p1 = ACCESS_ONCE(p); \
-				smp_read_barrier_depends(); \
-				(_________p1); \
-				})
-
-/**
- * rcu_dereference - fetch an RCU-protected pointer, checking for RCU
- *
- * Makes rcu_dereference_check() do the dirty work.
- */
-#define rcu_dereference(p) \
-	rcu_dereference_check(p, rcu_read_lock_held())
-
-/**
- * rcu_dereference_bh - fetch an RCU-protected pointer, checking for RCU-bh
- *
- * Makes rcu_dereference_check() do the dirty work.
- */
-#define rcu_dereference_bh(p) \
-		rcu_dereference_check(p, rcu_read_lock_bh_held())
-
-/**
- * rcu_dereference_sched - fetch RCU-protected pointer, checking for RCU-sched
- *
- * Makes rcu_dereference_check() do the dirty work.
- */
-#define rcu_dereference_sched(p) \
-		rcu_dereference_check(p, rcu_read_lock_sched_held())
-
-/**
- * rcu_assign_pointer - assign (publicize) a pointer to a newly
- * initialized structure that will be dereferenced by RCU read-side
- * critical sections.  Returns the value assigned.
+ * Assigns the specified value to the specified RCU-protected
+ * pointer, ensuring that any concurrent RCU readers will see
+ * any prior initialization.  Returns the value assigned.
  *
  * Inserts memory barriers on architectures that require them
  * (pretty much all of them other than x86), and also prevents
@@ -470,14 +574,17 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
  * call documents which pointers will be dereferenced by RCU read-side
  * code.
  */
-
 #define rcu_assign_pointer(p, v) \
-	({ \
-		if (!__builtin_constant_p(v) || \
-		    ((v) != NULL)) \
-			smp_wmb(); \
-		(p) = (v); \
-	})
+	__rcu_assign_pointer((p), (v), __rcu)
+
+/**
+ * RCU_INIT_POINTER() - initialize an RCU protected pointer
+ *
+ * Initialize an RCU-protected pointer in such a way to avoid RCU-lockdep
+ * splats.
+ */
+#define RCU_INIT_POINTER(p, v) \
+		p = (typeof(*v) __force __rcu *)(v)
 
 /* Infrastructure to implement the synchronize_() primitives. */
 
@@ -489,7 +596,7 @@ struct rcu_synchronize {
 extern void wakeme_after_rcu(struct rcu_head  *head);
 
 /**
- * call_rcu - Queue an RCU callback for invocation after a grace period.
+ * call_rcu() - Queue an RCU callback for invocation after a grace period.
  * @head: structure to be used for queueing the RCU updates.
  * @func: actual update function to be invoked after the grace period
  *
@@ -503,7 +610,7 @@ extern void call_rcu(struct rcu_head *head,
 			      void (*func)(struct rcu_head *head));
 
 /**
- * call_rcu_bh - Queue an RCU for invocation after a quicker grace period.
+ * call_rcu_bh() - Queue an RCU for invocation after a quicker grace period.
  * @head: structure to be used for queueing the RCU updates.
  * @func: actual update function to be invoked after the grace period
  *
@@ -560,37 +667,4 @@ static inline void debug_rcu_head_unqueue(struct rcu_head *head)
 }
 #endif	/* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
 
-#ifndef CONFIG_PROVE_RCU
-#define __do_rcu_dereference_check(c) do { } while (0)
-#endif /* #ifdef CONFIG_PROVE_RCU */
-
-#define __rcu_dereference_index_check(p, c) \
-	({ \
-		typeof(p) _________p1 = ACCESS_ONCE(p); \
-		__do_rcu_dereference_check(c); \
-		smp_read_barrier_depends(); \
-		(_________p1); \
-	})
-
-/**
- * rcu_dereference_index_check() - rcu_dereference for indices with debug checking
- * @p: The pointer to read, prior to dereferencing
- * @c: The conditions under which the dereference will take place
- *
- * Similar to rcu_dereference_check(), but omits the sparse checking.
- * This allows rcu_dereference_index_check() to be used on integers,
- * which can then be used as array indices.  Attempting to use
- * rcu_dereference_check() on an integer will give compiler warnings
- * because the sparse address-space mechanism relies on dereferencing
- * the RCU-protected pointer.  Dereferencing integers is not something
- * that even gcc will put up with.
- *
- * Note that this function does not implicitly check for RCU read-side
- * critical sections.  If this function gains lots of uses, it might
- * make sense to provide versions for each flavor of RCU, but it does
- * not make sense as of early 2010.
- */
-#define rcu_dereference_index_check(p, c) \
-	__rcu_dereference_index_check((p), (c))
-
 #endif /* __LINUX_RCUPDATE_H */
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 4d5d2f5..6f456a7 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -108,12 +108,31 @@ static inline int srcu_read_lock_held(struct srcu_struct *sp)
 #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
 
 /**
- * srcu_dereference - fetch SRCU-protected pointer with checking
+ * srcu_dereference_check - fetch SRCU-protected pointer for later dereferencing
+ * @p: the pointer to fetch and protect for later dereferencing
+ * @sp: pointer to the srcu_struct, which is used to check that we
+ *	really are in an SRCU read-side critical section.
+ * @c: condition to check for update-side use
  *
- * Makes rcu_dereference_check() do the dirty work.
+ * If PROVE_RCU is enabled, invoking this outside of an RCU read-side
+ * critical section will result in an RCU-lockdep splat, unless @c evaluates
+ * to 1.  The @c argument will normally be a logical expression containing
+ * lockdep_is_held() calls.
  */
-#define srcu_dereference(p, sp) \
-		rcu_dereference_check(p, srcu_read_lock_held(sp))
+#define srcu_dereference_check(p, sp, c) \
+	__rcu_dereference_check((p), srcu_read_lock_held(sp) || (c), __rcu)
+
+/**
+ * srcu_dereference - fetch SRCU-protected pointer for later dereferencing
+ * @p: the pointer to fetch and protect for later dereferencing
+ * @sp: pointer to the srcu_struct, which is used to check that we
+ *	really are in an SRCU read-side critical section.
+ *
+ * Makes rcu_dereference_check() do the dirty work.  If PROVE_RCU
+ * is enabled, invoking this outside of an RCU read-side critical
+ * section will result in an RCU-lockdep splat.
+ */
+#define srcu_dereference(p, sp) srcu_dereference_check((p), (sp), 0)
 
 /**
  * srcu_read_lock - register a new reader for an SRCU-protected structure.
diff --git a/kernel/rcupdate.c b/kernel/rcupdate.c
index 5ab5a91..2d33c7e 100644
--- a/kernel/rcupdate.c
+++ b/kernel/rcupdate.c
@@ -73,12 +73,14 @@ int debug_lockdep_rcu_enabled(void)
 EXPORT_SYMBOL_GPL(debug_lockdep_rcu_enabled);
 
 /**
- * rcu_read_lock_bh_held - might we be in RCU-bh read-side critical section?
+ * rcu_read_lock_bh_held() - might we be in RCU-bh read-side critical section?
  *
  * Check for bottom half being disabled, which covers both the
  * CONFIG_PROVE_RCU and not cases.  Note that if someone uses
  * rcu_read_lock_bh(), but then later enables BH, lockdep (if enabled)
- * will show the situation.
+ * will show the situation.  This is useful for debug checks in functions
+ * that require that they be called within an RCU read-side critical
+ * section.
  *
  * Check debug_lockdep_rcu_enabled() to prevent false positives during boot.
  */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8689646..7698752 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -528,6 +528,19 @@ config PROVE_RCU_REPEATEDLY
 	 disabling, allowing multiple RCU-lockdep warnings to be printed
 	 on a single reboot.
 
+config SPARSE_RCU_POINTER
+	bool "RCU debugging: sparse-based checks for pointer usage"
+	default n
+	help
+	 This feature enables the __rcu sparse annotation for
+	 RCU-protected pointers.  This annotation will cause sparse
+	 to flag any non-RCU used of annotated pointers.  This can be
+	 helpful when debugging RCU usage.  Please note that this feature
+	 is not intended to enforce code cleanliness; it is instead merely
+	 a debugging aid.
+
+	 Say Y to make sparse flag questionable use of RCU-protected pointers
+
 	 Say N if you are unsure.
 
 config LOCKDEP
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 07/23] rculist: avoid __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

This avoids warnings from missing __rcu annotations
in the rculist implementation, making it possible to
use the same lists in both RCU and non-RCU cases.

We can add rculist annotations later, together with
lockdep support for rculist, which is missing as well,
but that may involve changing all the users.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Pavel Emelyanov <xemul@openvz.org>
Cc: Sukadev Bhattiprolu <sukadev@us.ibm.com>
---
 include/linux/rculist.h       |   53 ++++++++++++++++++++++++++--------------
 include/linux/rculist_nulls.h |   16 ++++++++----
 kernel/pid.c                  |    2 +-
 3 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index 2c9b46c..891c52a 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -10,6 +10,12 @@
 #include <linux/rcupdate.h>
 
 /*
+ * return the ->next pointer of a list_head in an rcu safe
+ * way, we must not access it directly
+ */
+#define list_next_rcu(list)	(*((struct list_head __rcu **)(&(list)->next)))
+
+/*
  * Insert a new entry between two known consecutive entries.
  *
  * This is only for internal list manipulation where we know
@@ -20,7 +26,7 @@ static inline void __list_add_rcu(struct list_head *new,
 {
 	new->next = next;
 	new->prev = prev;
-	rcu_assign_pointer(prev->next, new);
+	rcu_assign_pointer(list_next_rcu(prev), new);
 	next->prev = new;
 }
 
@@ -138,7 +144,7 @@ static inline void list_replace_rcu(struct list_head *old,
 {
 	new->next = old->next;
 	new->prev = old->prev;
-	rcu_assign_pointer(new->prev->next, new);
+	rcu_assign_pointer(list_next_rcu(new->prev), new);
 	new->next->prev = new;
 	old->prev = LIST_POISON2;
 }
@@ -193,7 +199,7 @@ static inline void list_splice_init_rcu(struct list_head *list,
 	 */
 
 	last->next = at;
-	rcu_assign_pointer(head->next, first);
+	rcu_assign_pointer(list_next_rcu(head), first);
 	first->prev = head;
 	at->prev = last;
 }
@@ -208,7 +214,9 @@ static inline void list_splice_init_rcu(struct list_head *list,
  * primitives such as list_add_rcu() as long as it's guarded by rcu_read_lock().
  */
 #define list_entry_rcu(ptr, type, member) \
-	container_of(rcu_dereference_raw(ptr), type, member)
+	({typeof (*ptr) __rcu *__ptr = (typeof (*ptr) __rcu __force *)ptr; \
+	 container_of((typeof(ptr))rcu_dereference_raw(__ptr), type, member); \
+	})
 
 /**
  * list_first_entry_rcu - get the first element from a list
@@ -225,9 +233,9 @@ static inline void list_splice_init_rcu(struct list_head *list,
 	list_entry_rcu((ptr)->next, type, member)
 
 #define __list_for_each_rcu(pos, head) \
-	for (pos = rcu_dereference_raw((head)->next); \
+	for (pos = rcu_dereference_raw(list_next_rcu(head)); \
 		pos != (head); \
-		pos = rcu_dereference_raw(pos->next))
+		pos = rcu_dereference_raw(list_next_rcu((pos)))
 
 /**
  * list_for_each_entry_rcu	-	iterate over rcu list of given type
@@ -257,9 +265,9 @@ static inline void list_splice_init_rcu(struct list_head *list,
  * as long as the traversal is guarded by rcu_read_lock().
  */
 #define list_for_each_continue_rcu(pos, head) \
-	for ((pos) = rcu_dereference_raw((pos)->next); \
+	for ((pos) = rcu_dereference_raw(list_next_rcu(pos)); \
 		prefetch((pos)->next), (pos) != (head); \
-		(pos) = rcu_dereference_raw((pos)->next))
+		(pos) = rcu_dereference_raw(list_next_rcu(pos)))
 
 /**
  * list_for_each_entry_continue_rcu - continue iteration over list of given type
@@ -314,12 +322,19 @@ static inline void hlist_replace_rcu(struct hlist_node *old,
 
 	new->next = next;
 	new->pprev = old->pprev;
-	rcu_assign_pointer(*new->pprev, new);
+	rcu_assign_pointer(*(struct hlist_node __rcu **)new->pprev, new);
 	if (next)
 		new->next->pprev = &new->next;
 	old->pprev = LIST_POISON2;
 }
 
+/*
+ * return the first or the next element in an RCU protected hlist
+ */
+#define hlist_first_rcu(head)	(*((struct hlist_node __rcu **)(&(head)->first)))
+#define hlist_next_rcu(node)	(*((struct hlist_node __rcu **)(&(node)->next)))
+#define hlist_pprev_rcu(node)	(*((struct hlist_node __rcu **)((node)->pprev)))
+
 /**
  * hlist_add_head_rcu
  * @n: the element to add to the hash list.
@@ -346,7 +361,7 @@ static inline void hlist_add_head_rcu(struct hlist_node *n,
 
 	n->next = first;
 	n->pprev = &h->first;
-	rcu_assign_pointer(h->first, n);
+	rcu_assign_pointer(hlist_first_rcu(h), n);
 	if (first)
 		first->pprev = &n->next;
 }
@@ -374,7 +389,7 @@ static inline void hlist_add_before_rcu(struct hlist_node *n,
 {
 	n->pprev = next->pprev;
 	n->next = next;
-	rcu_assign_pointer(*(n->pprev), n);
+	rcu_assign_pointer(hlist_pprev_rcu(n), n);
 	next->pprev = &n->next;
 }
 
@@ -401,15 +416,15 @@ static inline void hlist_add_after_rcu(struct hlist_node *prev,
 {
 	n->next = prev->next;
 	n->pprev = &prev->next;
-	rcu_assign_pointer(prev->next, n);
+	rcu_assign_pointer(hlist_next_rcu(prev), n);
 	if (n->next)
 		n->next->pprev = &n->next;
 }
 
-#define __hlist_for_each_rcu(pos, head)			\
-	for (pos = rcu_dereference((head)->first);	\
-	     pos && ({ prefetch(pos->next); 1; });	\
-	     pos = rcu_dereference(pos->next))
+#define __hlist_for_each_rcu(pos, head)				\
+	for (pos = rcu_dereference(hlist_first_rcu(head));	\
+	     pos && ({ prefetch(pos->next); 1; });		\
+	     pos = rcu_dereference(hlist_next_rcu(pos)))
 
 /**
  * hlist_for_each_entry_rcu - iterate over rcu list of given type
@@ -422,11 +437,11 @@ static inline void hlist_add_after_rcu(struct hlist_node *prev,
  * the _rcu list-mutation primitives such as hlist_add_head_rcu()
  * as long as the traversal is guarded by rcu_read_lock().
  */
-#define hlist_for_each_entry_rcu(tpos, pos, head, member)		 \
-	for (pos = rcu_dereference_raw((head)->first);			 \
+#define hlist_for_each_entry_rcu(tpos, pos, head, member)		\
+	for (pos = rcu_dereference_raw(hlist_first_rcu(head));		\
 		pos && ({ prefetch(pos->next); 1; }) &&			 \
 		({ tpos = hlist_entry(pos, typeof(*tpos), member); 1; }); \
-		pos = rcu_dereference_raw(pos->next))
+		pos = rcu_dereference_raw(hlist_next_rcu(pos)))
 
 #endif	/* __KERNEL__ */
 #endif
diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
index b70ffe5..2ae1371 100644
--- a/include/linux/rculist_nulls.h
+++ b/include/linux/rculist_nulls.h
@@ -37,6 +37,12 @@ static inline void hlist_nulls_del_init_rcu(struct hlist_nulls_node *n)
 	}
 }
 
+#define hlist_nulls_first_rcu(head) \
+	(*((struct hlist_nulls_node __rcu __force **)&(head)->first))
+
+#define hlist_nulls_next_rcu(node) \
+	(*((struct hlist_nulls_node __rcu __force **)&(node)->next))
+
 /**
  * hlist_nulls_del_rcu - deletes entry from hash list without re-initialization
  * @n: the element to delete from the hash list.
@@ -88,7 +94,7 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
 
 	n->next = first;
 	n->pprev = &h->first;
-	rcu_assign_pointer(h->first, n);
+	rcu_assign_pointer(hlist_nulls_first_rcu(h), n);
 	if (!is_a_nulls(first))
 		first->pprev = &n->next;
 }
@@ -100,11 +106,11 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
  * @member:	the name of the hlist_nulls_node within the struct.
  *
  */
-#define hlist_nulls_for_each_entry_rcu(tpos, pos, head, member) \
-	for (pos = rcu_dereference_raw((head)->first);			 \
-		(!is_a_nulls(pos)) &&			\
+#define hlist_nulls_for_each_entry_rcu(tpos, pos, head, member)			\
+	for (pos = rcu_dereference_raw(hlist_nulls_first_rcu(head));		\
+		(!is_a_nulls(pos)) &&						\
 		({ tpos = hlist_nulls_entry(pos, typeof(*tpos), member); 1; }); \
-		pos = rcu_dereference_raw(pos->next))
+		pos = rcu_dereference_raw(hlist_nulls_next_rcu(pos)))
 
 #endif
 #endif
diff --git a/kernel/pid.c b/kernel/pid.c
index aebb30d..8bb38ee 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -367,7 +367,7 @@ struct task_struct *pid_task(struct pid *pid, enum pid_type type)
 	struct task_struct *result = NULL;
 	if (pid) {
 		struct hlist_node *first;
-		first = rcu_dereference_check(pid->tasks[type].first,
+		first = rcu_dereference_check(hlist_first_rcu(&pid->tasks[type]),
 					      rcu_read_lock_held() ||
 					      lockdep_tasklist_lock_is_held());
 		if (first)
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 08/23] cgroups: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Paul Menage <menage@google.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
---
 include/linux/cgroup.h |    4 ++--
 include/linux/sched.h  |    2 +-
 kernel/cgroup.c        |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 8f78073..b147fd5 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -75,7 +75,7 @@ struct cgroup_subsys_state {
 
 	unsigned long flags;
 	/* ID for this css, if possible */
-	struct css_id *id;
+	struct css_id __rcu *id;
 };
 
 /* bits in struct cgroup_subsys_state flags field */
@@ -205,7 +205,7 @@ struct cgroup {
 	struct list_head children;	/* my children */
 
 	struct cgroup *parent;		/* my parent */
-	struct dentry *dentry;	  	/* cgroup fs entry, RCU protected */
+	struct dentry __rcu *dentry;	/* cgroup fs entry, RCU protected */
 
 	/* Private pointers for each registered subsystem */
 	struct cgroup_subsys_state *subsys[CGROUP_SUBSYS_COUNT];
diff --git a/include/linux/sched.h b/include/linux/sched.h
index dad7f66..7307c74 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1429,7 +1429,7 @@ struct task_struct {
 #endif
 #ifdef CONFIG_CGROUPS
 	/* Control Group info protected by css_set_lock */
-	struct css_set *cgroups;
+	struct css_set __rcu *cgroups;
 	/* cg_list protected by css_set_lock and tsk->alloc_lock */
 	struct list_head cg_list;
 #endif
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 3a53c77..5cfbc93 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -138,7 +138,7 @@ struct css_id {
 	 * is called after synchronize_rcu(). But for safe use, css_is_removed()
 	 * css_tryget() should be used for avoiding race.
 	 */
-	struct cgroup_subsys_state *css;
+	struct cgroup_subsys_state __rcu *css;
 	/*
 	 * ID of this css.
 	 */
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 09/23] credentials: rcu annotation

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
---
 include/linux/sched.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 7307c74..74b3125 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1301,9 +1301,9 @@ struct task_struct {
 	struct list_head cpu_timers[3];
 
 /* process credentials */
-	const struct cred *real_cred;	/* objective and real subjective task
+	const struct cred __rcu *real_cred; /* objective and real subjective task
 					 * credentials (COW) */
-	const struct cred *cred;	/* effective (overridable) subjective task
+	const struct cred __rcu *cred;	/* effective (overridable) subjective task
 					 * credentials (COW) */
 	struct mutex cred_guard_mutex;	/* guard against foreign influences on
 					 * credential calculations
-- 
1.7.0.6

Re: [PATCH RFC tip/core/rcu 09/23] credentials: rcu annotation

[David Howells]

Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> Subject: [PATCH RFC tip/core/rcu 09/23] credentials: rcu annotation
> 
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>

Acked-by: David Howells <dhowells@redhat.com>

[PATCH RFC tip/core/rcu 10/23] keys: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: David Howells <dhowells@redhat.com>
---
 include/linux/cred.h |    2 +-
 include/linux/key.h  |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/cred.h b/include/linux/cred.h
index 52507c3..413f98a 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -84,7 +84,7 @@ struct thread_group_cred {
 	atomic_t	usage;
 	pid_t		tgid;			/* thread group process ID */
 	spinlock_t	lock;
-	struct key	*session_keyring;	/* keyring inherited over fork */
+	struct key __rcu *session_keyring;	/* keyring inherited over fork */
 	struct key	*process_keyring;	/* keyring private to this process */
 	struct rcu_head	rcu;			/* RCU deletion hook */
 };
diff --git a/include/linux/key.h b/include/linux/key.h
index cd50dfa..3db0adc 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -178,8 +178,9 @@ struct key {
 	 */
 	union {
 		unsigned long		value;
+		void __rcu		*rcudata;
 		void			*data;
-		struct keyring_list	*subscriptions;
+		struct keyring_list __rcu *subscriptions;
 	} payload;
 };
 
-- 
1.7.0.6

Re: [PATCH RFC tip/core/rcu 10/23] keys: __rcu annotations

[David Howells]

Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> Subject: [PATCH RFC tip/core/rcu 10/23] keys: __rcu annotations
> 
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Acked-by: David Howells <dhowells@redhat.com>

[PATCH RFC tip/core/rcu 11/23] nfs: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 include/linux/nfs_fs.h          |    2 +-
 include/linux/sunrpc/auth_gss.h |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 07ce460..491da02 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -178,7 +178,7 @@ struct nfs_inode {
 	struct nfs4_cached_acl	*nfs4_acl;
         /* NFSv4 state */
 	struct list_head	open_states;
-	struct nfs_delegation	*delegation;
+	struct nfs_delegation __rcu *delegation;
 	fmode_t			 delegation_state;
 	struct rw_semaphore	rwsem;
 #endif /* CONFIG_NFS_V4*/
diff --git a/include/linux/sunrpc/auth_gss.h b/include/linux/sunrpc/auth_gss.h
index d48d4e6..994db5a 100644
--- a/include/linux/sunrpc/auth_gss.h
+++ b/include/linux/sunrpc/auth_gss.h
@@ -69,7 +69,7 @@ struct gss_cl_ctx {
 	enum rpc_gss_proc	gc_proc;
 	u32			gc_seq;
 	spinlock_t		gc_seq_lock;
-	struct gss_ctx		*gc_gss_ctx;
+	struct gss_ctx __rcu	*gc_gss_ctx;
 	struct xdr_netobj	gc_wire_ctx;
 	u32			gc_win;
 	unsigned long		gc_expiry;
@@ -80,7 +80,7 @@ struct gss_upcall_msg;
 struct gss_cred {
 	struct rpc_cred		gc_base;
 	enum rpc_gss_svc	gc_service;
-	struct gss_cl_ctx	*gc_ctx;
+	struct gss_cl_ctx __rcu	*gc_ctx;
 	struct gss_upcall_msg	*gc_upcall;
 	unsigned char		gc_machine_cred : 1;
 };
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 12/23] net: __rcu annotations for drivers

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/bnx2.h         |    2 +-
 drivers/net/bnx2x.h        |    2 +-
 drivers/net/cnic.h         |    2 +-
 drivers/net/macvtap.c      |    2 +-
 include/linux/if_macvlan.h |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h
index cd4b0e4..7bdb1cb 100644
--- a/drivers/net/bnx2.h
+++ b/drivers/net/bnx2.h
@@ -6746,7 +6746,7 @@ struct bnx2 {
 	u32		tx_wake_thresh;
 
 #ifdef BCM_CNIC
-	struct cnic_ops		*cnic_ops;
+	struct cnic_ops	__rcu	*cnic_ops;
 	void			*cnic_data;
 #endif
 
diff --git a/drivers/net/bnx2x.h b/drivers/net/bnx2x.h
index 3c48a7a..9dfb57b 100644
--- a/drivers/net/bnx2x.h
+++ b/drivers/net/bnx2x.h
@@ -1007,7 +1007,7 @@ struct bnx2x {
 	dma_addr_t		timers_mapping;
 	void			*qm;
 	dma_addr_t		qm_mapping;
-	struct cnic_ops		*cnic_ops;
+	struct cnic_ops __rcu	*cnic_ops;
 	void			*cnic_data;
 	u32			cnic_tag;
 	struct cnic_eth_dev	cnic_eth_dev;
diff --git a/drivers/net/cnic.h b/drivers/net/cnic.h
index a0d853d..9852375 100644
--- a/drivers/net/cnic.h
+++ b/drivers/net/cnic.h
@@ -177,7 +177,7 @@ struct cnic_local {
 #define ULP_F_INIT	0
 #define ULP_F_START	1
 #define ULP_F_CALL_PENDING	2
-	struct cnic_ulp_ops *ulp_ops[MAX_CNIC_ULP_TYPE];
+	struct cnic_ulp_ops __rcu *ulp_ops[MAX_CNIC_ULP_TYPE];
 
 	/* protected by ulp_lock */
 	u32 cnic_local_flags;
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index abba3cc..adf0145 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -37,7 +37,7 @@
 struct macvtap_queue {
 	struct sock sk;
 	struct socket sock;
-	struct macvlan_dev *vlan;
+	struct macvlan_dev __rcu *vlan;
 	struct file *file;
 	unsigned int flags;
 };
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index b78a712..c15ed77 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -47,7 +47,7 @@ struct macvlan_dev {
 	enum macvlan_mode	mode;
 	int (*receive)(struct sk_buff *skb);
 	int (*forward)(struct net_device *dev, struct sk_buff *skb);
-	struct macvtap_queue	*tap;
+	struct macvtap_queue __rcu *tap;
 };
 
 static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 13/23] perf_event: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 include/linux/perf_event.h |    6 +++---
 include/linux/sched.h      |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index c8e3754..48132eb 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -582,7 +582,7 @@ struct perf_event {
 	int				nr_siblings;
 	int				group_flags;
 	struct perf_event		*group_leader;
-	struct perf_event		*output;
+	struct perf_event __rcu		*output;
 	const struct pmu		*pmu;
 
 	enum perf_event_active_state	state;
@@ -643,7 +643,7 @@ struct perf_event {
 	/* mmap bits */
 	struct mutex			mmap_mutex;
 	atomic_t			mmap_count;
-	struct perf_mmap_data		*data;
+	struct perf_mmap_data __rcu	*data;
 
 	/* poll related */
 	wait_queue_head_t		waitq;
@@ -710,7 +710,7 @@ struct perf_event_context {
 	 * These fields let us detect when two contexts have both
 	 * been cloned (inherited) from a common ancestor.
 	 */
-	struct perf_event_context	*parent_ctx;
+	struct perf_event_context __rcu *parent_ctx;
 	u64				parent_gen;
 	u64				generation;
 	int				pin_count;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 74b3125..34d28f7 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1442,7 +1442,7 @@ struct task_struct {
 	struct futex_pi_state *pi_state_cache;
 #endif
 #ifdef CONFIG_PERF_EVENTS
-	struct perf_event_context *perf_event_ctxp;
+	struct perf_event_context __rcu *perf_event_ctxp;
 	struct mutex perf_event_mutex;
 	struct list_head perf_event_list;
 #endif
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 14/23] notifiers: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 include/linux/notifier.h |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index fee6c2f..f05f5e4 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -49,28 +49,28 @@
 
 struct notifier_block {
 	int (*notifier_call)(struct notifier_block *, unsigned long, void *);
-	struct notifier_block *next;
+	struct notifier_block __rcu *next;
 	int priority;
 };
 
 struct atomic_notifier_head {
 	spinlock_t lock;
-	struct notifier_block *head;
+	struct notifier_block __rcu *head;
 };
 
 struct blocking_notifier_head {
 	struct rw_semaphore rwsem;
-	struct notifier_block *head;
+	struct notifier_block __rcu *head;
 };
 
 struct raw_notifier_head {
-	struct notifier_block *head;
+	struct notifier_block __rcu *head;
 };
 
 struct srcu_notifier_head {
 	struct mutex mutex;
 	struct srcu_struct srcu;
-	struct notifier_block *head;
+	struct notifier_block __rcu *head;
 };
 
 #define ATOMIC_INIT_NOTIFIER_HEAD(name) do {	\
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 15/23] radix-tree: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Nick Piggin <npiggin@suse.de>
---
 include/linux/radix-tree.h |    4 +++-
 lib/radix-tree.c           |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
index 55ca73c..d801044 100644
--- a/include/linux/radix-tree.h
+++ b/include/linux/radix-tree.h
@@ -47,6 +47,8 @@ static inline void *radix_tree_indirect_to_ptr(void *ptr)
 {
 	return (void *)((unsigned long)ptr & ~RADIX_TREE_INDIRECT_PTR);
 }
+#define radix_tree_indirect_to_ptr(ptr) \
+	radix_tree_indirect_to_ptr((void __force *)(ptr))
 
 static inline int radix_tree_is_indirect_ptr(void *ptr)
 {
@@ -61,7 +63,7 @@ static inline int radix_tree_is_indirect_ptr(void *ptr)
 struct radix_tree_root {
 	unsigned int		height;
 	gfp_t			gfp_mask;
-	struct radix_tree_node	*rnode;
+	struct radix_tree_node	__rcu *rnode;
 };
 
 #define RADIX_TREE_INIT(mask)	{					\
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 2a087e0..08f86cc 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -49,7 +49,7 @@ struct radix_tree_node {
 	unsigned int	height;		/* Height from the bottom */
 	unsigned int	count;
 	struct rcu_head	rcu_head;
-	void		*slots[RADIX_TREE_MAP_SIZE];
+	void __rcu	*slots[RADIX_TREE_MAP_SIZE];
 	unsigned long	tags[RADIX_TREE_MAX_TAGS][RADIX_TREE_TAG_LONGS];
 };
 
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 16/23] idr: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Manfred Spraul <manfred@colorfullife.com>
---
 include/linux/idr.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/idr.h b/include/linux/idr.h
index e968db7..cdb715e 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -50,14 +50,14 @@
 
 struct idr_layer {
 	unsigned long		 bitmap; /* A zero bit means "space here" */
-	struct idr_layer	*ary[1<<IDR_BITS];
+	struct idr_layer __rcu	*ary[1<<IDR_BITS];
 	int			 count;	 /* When zero, we can release it */
 	int			 layer;	 /* distance from leaf */
 	struct rcu_head		 rcu_head;
 };
 
 struct idr {
-	struct idr_layer *top;
+	struct idr_layer __rcu *top;
 	struct idr_layer *id_free;
 	int		  layers; /* only valid without concurrent changes */
 	int		  id_free_cnt;
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 17/23] input: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/evdev.c |    2 +-
 include/linux/input.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 2ee6c7a..73b1208 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -28,7 +28,7 @@ struct evdev {
 	int minor;
 	struct input_handle handle;
 	wait_queue_head_t wait;
-	struct evdev_client *grab;
+	struct evdev_client __rcu *grab;
 	struct list_head client_list;
 	spinlock_t client_lock; /* protects client_list */
 	struct mutex mutex;
diff --git a/include/linux/input.h b/include/linux/input.h
index 7ed2251..850b6b7 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -1173,7 +1173,7 @@ struct input_dev {
 	int (*flush)(struct input_dev *dev, struct file *file);
 	int (*event)(struct input_dev *dev, unsigned int type, unsigned int code, int value);
 
-	struct input_handle *grab;
+	struct input_handle __rcu *grab;
 
 	spinlock_t event_lock;
 	struct mutex mutex;
-- 
1.7.0.6

Re: [PATCH RFC tip/core/rcu 17/23] input: __rcu annotations

[Dmitry Torokhov]

On Wed, May 12, 2010 at 02:33:36PM -0700, Paul E. McKenney wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Acked-by: Dmitry Torokhov <dtor@mail.ru>

Please merge with the rest of RCU annotations.

-- 
Dmitry

[PATCH RFC tip/core/rcu 18/23] net/netfilter: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@relay.de.ibm.com>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/netfilter/nf_conntrack.h |    2 +-
 net/ipv4/netfilter/nf_nat_core.c     |    2 +-
 net/netfilter/core.c                 |    2 +-
 net/netfilter/nf_conntrack_ecache.c  |    4 ++--
 net/netfilter/nf_conntrack_extend.c  |    2 +-
 net/netfilter/nf_conntrack_proto.c   |    4 ++--
 net/netfilter/nf_log.c               |    2 +-
 net/netfilter/nf_queue.c             |    2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
index bde095f..92229d1 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -75,7 +75,7 @@ struct nf_conntrack_helper;
 /* nf_conn feature for connections that have a helper */
 struct nf_conn_help {
 	/* Helper. if any */
-	struct nf_conntrack_helper *helper;
+	struct nf_conntrack_helper __rcu *helper;
 
 	union nf_conntrack_help help;
 
diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index 4f8bddb..1263f2a 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -38,7 +38,7 @@ static DEFINE_SPINLOCK(nf_nat_lock);
 static struct nf_conntrack_l3proto *l3proto __read_mostly;
 
 #define MAX_IP_NAT_PROTO 256
-static const struct nf_nat_protocol *nf_nat_protos[MAX_IP_NAT_PROTO]
+static const struct nf_nat_protocol __rcu *nf_nat_protos[MAX_IP_NAT_PROTO]
 						__read_mostly;
 
 static inline const struct nf_nat_protocol *
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 78b505d..fdaec7d 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -27,7 +27,7 @@
 
 static DEFINE_MUTEX(afinfo_mutex);
 
-const struct nf_afinfo *nf_afinfo[NFPROTO_NUMPROTO] __read_mostly;
+const struct nf_afinfo __rcu *nf_afinfo[NFPROTO_NUMPROTO] __read_mostly;
 EXPORT_SYMBOL(nf_afinfo);
 
 int nf_register_afinfo(const struct nf_afinfo *afinfo)
diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
index f516961..97619fc 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -26,10 +26,10 @@
 
 static DEFINE_MUTEX(nf_ct_ecache_mutex);
 
-struct nf_ct_event_notifier *nf_conntrack_event_cb __read_mostly;
+struct nf_ct_event_notifier __rcu *nf_conntrack_event_cb __read_mostly;
 EXPORT_SYMBOL_GPL(nf_conntrack_event_cb);
 
-struct nf_exp_event_notifier *nf_expect_event_cb __read_mostly;
+struct nf_exp_event_notifier __rcu *nf_expect_event_cb __read_mostly;
 EXPORT_SYMBOL_GPL(nf_expect_event_cb);
 
 /* deliver cached events and clear cache entry - must be called with locally
diff --git a/net/netfilter/nf_conntrack_extend.c b/net/netfilter/nf_conntrack_extend.c
index fdc8fb4..9a0f75f 100644
--- a/net/netfilter/nf_conntrack_extend.c
+++ b/net/netfilter/nf_conntrack_extend.c
@@ -16,7 +16,7 @@
 #include <linux/skbuff.h>
 #include <net/netfilter/nf_conntrack_extend.h>
 
-static struct nf_ct_ext_type *nf_ct_ext_types[NF_CT_EXT_NUM];
+static struct nf_ct_ext_type __rcu *nf_ct_ext_types[NF_CT_EXT_NUM];
 static DEFINE_MUTEX(nf_ct_ext_type_mutex);
 
 void __nf_ct_ext_destroy(struct nf_conn *ct)
diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index a44fa75..45b8091 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -30,8 +30,8 @@
 #include <net/netfilter/nf_conntrack_l4proto.h>
 #include <net/netfilter/nf_conntrack_core.h>
 
-static struct nf_conntrack_l4proto **nf_ct_protos[PF_MAX] __read_mostly;
-struct nf_conntrack_l3proto *nf_ct_l3protos[AF_MAX] __read_mostly;
+static struct nf_conntrack_l4proto __rcu **nf_ct_protos[PF_MAX] __read_mostly;
+struct nf_conntrack_l3proto __rcu *nf_ct_l3protos[AF_MAX] __read_mostly;
 EXPORT_SYMBOL_GPL(nf_ct_l3protos);
 
 static DEFINE_MUTEX(nf_ct_proto_mutex);
diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index 015725a..64ba4d6 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -16,7 +16,7 @@
 #define NF_LOG_PREFIXLEN		128
 #define NFLOGGER_NAME_LEN		64
 
-static const struct nf_logger *nf_loggers[NFPROTO_NUMPROTO] __read_mostly;
+static const struct nf_logger __rcu *nf_loggers[NFPROTO_NUMPROTO] __read_mostly;
 static struct list_head nf_loggers_l[NFPROTO_NUMPROTO] __read_mostly;
 static DEFINE_MUTEX(nf_log_mutex);
 
diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index c49ef21..e137c6b 100644
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -17,7 +17,7 @@
  * long term mutex.  The handler must provide an an outfn() to accept packets
  * for queueing and must reinject all packets it receives, no matter what.
  */
-static const struct nf_queue_handler *queue_handler[NFPROTO_NUMPROTO] __read_mostly;
+static const struct nf_queue_handler __rcu *queue_handler[NFPROTO_NUMPROTO] __read_mostly;
 
 static DEFINE_MUTEX(queue_handler_mutex);
 
-- 
1.7.0.6

Re: [PATCH RFC tip/core/rcu 18/23] net/netfilter: __rcu annotations

[Patrick McHardy]

Paul E. McKenney wrote:
> From: Arnd Bergmann <arnd@relay.de.ibm.com>
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  include/net/netfilter/nf_conntrack.h |    2 +-
>  net/ipv4/netfilter/nf_nat_core.c     |    2 +-
>  net/netfilter/core.c                 |    2 +-
>  net/netfilter/nf_conntrack_ecache.c  |    4 ++--
>  net/netfilter/nf_conntrack_extend.c  |    2 +-
>  net/netfilter/nf_conntrack_proto.c   |    4 ++--
>  net/netfilter/nf_log.c               |    2 +-
>  net/netfilter/nf_queue.c             |    2 +-
>  8 files changed, 10 insertions(+), 10 deletions(-)

Acked-by: Patrick McHardy <kaber@trash.net>

[PATCH RFC tip/core/rcu 19/23] kvm: add __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Avi Kivity <avi@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
---
 include/linux/kvm_host.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 169d077..08fe794 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -197,7 +197,7 @@ struct kvm {
 
 	struct mutex irq_lock;
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
-	struct kvm_irq_routing_table *irq_routing;
+	struct kvm_irq_routing_table __rcu *irq_routing;
 	struct hlist_head mask_notifier_list;
 	struct hlist_head irq_ack_notifier_list;
 #endif
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 20/23] kernel: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@arndb.de>

This adds annotations for RCU operations in core kernel components

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/fdtable.h   |    6 +++---
 include/linux/fs.h        |    2 +-
 include/linux/genhd.h     |    6 +++---
 include/linux/init_task.h |    4 ++--
 include/linux/iocontext.h |    2 +-
 include/linux/mm_types.h  |    2 +-
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 551671e..3e4c4f4 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -31,7 +31,7 @@ struct embedded_fd_set {
 
 struct fdtable {
 	unsigned int max_fds;
-	struct file ** fd;      /* current fd array */
+	struct file __rcu **fd;      /* current fd array */
 	fd_set *close_on_exec;
 	fd_set *open_fds;
 	struct rcu_head rcu;
@@ -46,7 +46,7 @@ struct files_struct {
    * read mostly part
    */
 	atomic_t count;
-	struct fdtable *fdt;
+	struct fdtable __rcu *fdt;
 	struct fdtable fdtab;
   /*
    * written part on a separate cache line in SMP
@@ -55,7 +55,7 @@ struct files_struct {
 	int next_fd;
 	struct embedded_fd_set close_on_exec_init;
 	struct embedded_fd_set open_fds_init;
-	struct file * fd_array[NR_OPEN_DEFAULT];
+	struct file __rcu * fd_array[NR_OPEN_DEFAULT];
 };
 
 #define rcu_dereference_check_fdtable(files, fdtfd) \
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 44f35ae..752bc76 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1382,7 +1382,7 @@ struct super_block {
 	 * Saved mount options for lazy filesystems using
 	 * generic_show_options()
 	 */
-	char *s_options;
+	char __rcu *s_options;
 };
 
 extern struct timespec current_fs_time(struct super_block *sb);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 5f2f4c4..af3f06b 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -129,8 +129,8 @@ struct blk_scsi_cmd_filter {
 struct disk_part_tbl {
 	struct rcu_head rcu_head;
 	int len;
-	struct hd_struct *last_lookup;
-	struct hd_struct *part[];
+	struct hd_struct __rcu *last_lookup;
+	struct hd_struct __rcu *part[];
 };
 
 struct gendisk {
@@ -149,7 +149,7 @@ struct gendisk {
 	 * non-critical accesses use RCU.  Always access through
 	 * helpers.
 	 */
-	struct disk_part_tbl *part_tbl;
+	struct disk_part_tbl __rcu *part_tbl;
 	struct hd_struct part0;
 
 	const struct block_device_operations *fops;
diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index 7996fc2..f05af8c 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -137,8 +137,8 @@ extern struct cred init_cred;
 	.children	= LIST_HEAD_INIT(tsk.children),			\
 	.sibling	= LIST_HEAD_INIT(tsk.sibling),			\
 	.group_leader	= &tsk,						\
-	.real_cred	= &init_cred,					\
-	.cred		= &init_cred,					\
+	RCU_INIT_POINTER(.real_cred, &init_cred),			\
+	RCU_INIT_POINTER(.cred, &init_cred),				\
 	.cred_guard_mutex =						\
 		 __MUTEX_INITIALIZER(tsk.cred_guard_mutex),		\
 	.comm		= "swapper",					\
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index a0bb301..6d4cd79 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -54,7 +54,7 @@ struct io_context {
 
 	struct radix_tree_root radix_root;
 	struct hlist_head cic_list;
-	void *ioc_data;
+	void __rcu *ioc_data;
 };
 
 static inline struct io_context *ioc_task_link(struct io_context *ioc)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index b8bb9a6..05537a5 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -299,7 +299,7 @@ struct mm_struct {
 	 * new_owner->mm == mm
 	 * new_owner->alloc_lock is held
 	 */
-	struct task_struct *owner;
+	struct task_struct __rcu *owner;
 #endif
 
 #ifdef CONFIG_PROC_FS
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 21/23] net: __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@relay.de.ibm.com>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/igmp.h               |    4 ++--
 include/linux/netdevice.h          |   12 ++++++------
 include/net/dst.h                  |    2 +-
 include/net/fib_rules.h            |    2 +-
 include/net/garp.h                 |    2 +-
 include/net/inet_sock.h            |    2 +-
 include/net/ip6_tunnel.h           |    2 +-
 include/net/ipip.h                 |    6 +++---
 include/net/net_namespace.h        |    2 +-
 include/net/netns/xfrm.h           |    2 +-
 include/net/sock.h                 |    4 ++--
 kernel/sched.c                     |    2 +-
 net/802/stp.c                      |    4 ++--
 net/ipv4/ip_gre.c                  |    2 +-
 net/ipv4/ipip.c                    |   10 +++++-----
 net/ipv4/protocol.c                |    2 +-
 net/ipv4/route.c                   |    2 +-
 net/ipv4/tcp.c                     |    4 ++--
 net/ipv6/ip6_tunnel.c              |    6 +++---
 net/ipv6/protocol.c                |    2 +-
 net/ipv6/sit.c                     |   10 +++++-----
 net/mac80211/ieee80211_i.h         |   15 ++++++++-------
 net/mac80211/sta_info.h            |    4 ++--
 net/netlabel/netlabel_domainhash.c |    4 ++--
 net/netlabel/netlabel_unlabeled.c  |    4 ++--
 net/netlink/af_netlink.c           |    2 +-
 net/phonet/af_phonet.c             |    2 +-
 net/phonet/pn_dev.c                |    2 +-
 net/socket.c                       |    2 +-
 29 files changed, 60 insertions(+), 59 deletions(-)

diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index 93fc244..39dd315 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -167,10 +167,10 @@ struct ip_sf_socklist {
  */
 
 struct ip_mc_socklist {
-	struct ip_mc_socklist	*next;
+	struct ip_mc_socklist __rcu *next;
 	struct ip_mreqn		multi;
 	unsigned int		sfmode;		/* MCAST_{INCLUDE,EXCLUDE} */
-	struct ip_sf_socklist	*sflist;
+	struct ip_sf_socklist __rcu *sflist;
 	struct rcu_head		rcu;
 };
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index fa8b476..425be97 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -855,10 +855,10 @@ struct net_device {
 #ifdef CONFIG_NET_DSA
 	void			*dsa_ptr;	/* dsa specific data */
 #endif
-	void 			*atalk_ptr;	/* AppleTalk link 	*/
-	void			*ip_ptr;	/* IPv4 specific data	*/
+	void			*atalk_ptr;	/* AppleTalk link	*/
+	void __rcu		*ip_ptr;	/* IPv4 specific data	*/
 	void                    *dn_ptr;        /* DECnet specific data */
-	void                    *ip6_ptr;       /* IPv6 specific data */
+	void __rcu              *ip6_ptr;       /* IPv6 specific data */
 	void			*ec_ptr;	/* Econet specific data	*/
 	void			*ax25_ptr;	/* AX.25 specific data */
 	struct wireless_dev	*ieee80211_ptr;	/* IEEE 802.11 specific data,
@@ -947,11 +947,11 @@ struct net_device {
 	void			*ml_priv;
 
 	/* bridge stuff */
-	struct net_bridge_port	*br_port;
+	void __rcu		*br_port;
 	/* macvlan */
-	struct macvlan_port	*macvlan_port;
+	struct macvlan_port __rcu *macvlan_port;
 	/* GARP */
-	struct garp_port	*garp_port;
+	struct garp_port __rcu	*garp_port;
 
 	/* class/net/name entry */
 	struct device		dev;
diff --git a/include/net/dst.h b/include/net/dst.h
index ce078cd..5f839aa 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -94,7 +94,7 @@ struct dst_entry {
 	unsigned long		lastuse;
 	union {
 		struct dst_entry *next;
-		struct rtable    *rt_next;
+		struct rtable   __rcu *rt_next;
 		struct rt6_info   *rt6_next;
 		struct dn_route  *dn_next;
 	};
diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index c49086d..e88dc69 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -20,7 +20,7 @@ struct fib_rule {
 	u32			table;
 	u8			action;
 	u32			target;
-	struct fib_rule *	ctarget;
+	struct fib_rule __rcu  *ctarget;
 	char			iifname[IFNAMSIZ];
 	char			oifname[IFNAMSIZ];
 	struct rcu_head		rcu;
diff --git a/include/net/garp.h b/include/net/garp.h
index 825f172..15b30ba 100644
--- a/include/net/garp.h
+++ b/include/net/garp.h
@@ -107,7 +107,7 @@ struct garp_applicant {
 };
 
 struct garp_port {
-	struct garp_applicant	*applicants[GARP_APPLICATION_MAX + 1];
+	struct garp_applicant __rcu *applicants[GARP_APPLICATION_MAX + 1];
 };
 
 extern int	garp_register_application(struct garp_application *app);
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 83fd344..b16192f 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -139,7 +139,7 @@ struct inet_sock {
 				mc_all:1;
 	int			mc_index;
 	__be32			mc_addr;
-	struct ip_mc_socklist	*mc_list;
+	struct ip_mc_socklist __rcu *mc_list;
 	struct {
 		unsigned int		flags;
 		unsigned int		fragsize;
diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index fbf9d1c..94fa2cc 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -13,7 +13,7 @@
 /* IPv6 tunnel */
 
 struct ip6_tnl {
-	struct ip6_tnl *next;	/* next tunnel in list */
+	struct ip6_tnl __rcu *next;	/* next tunnel in list */
 	struct net_device *dev;	/* virtual device associated with tunnel */
 	struct ip6_tnl_parm parms;	/* tunnel configuration parameters */
 	struct flowi fl;	/* flowi template for xmit */
diff --git a/include/net/ipip.h b/include/net/ipip.h
index 11e8513..ea186ab 100644
--- a/include/net/ipip.h
+++ b/include/net/ipip.h
@@ -16,7 +16,7 @@ struct ip_tunnel_6rd_parm {
 };
 
 struct ip_tunnel {
-	struct ip_tunnel	*next;
+	struct ip_tunnel __rcu	*next;
 	struct net_device	*dev;
 
 	int			err_count;	/* Number of arrived ICMP errors */
@@ -34,12 +34,12 @@ struct ip_tunnel {
 #ifdef CONFIG_IPV6_SIT_6RD
 	struct ip_tunnel_6rd_parm	ip6rd;
 #endif
-	struct ip_tunnel_prl_entry	*prl;		/* potential router list */
+	struct ip_tunnel_prl_entry __rcu *prl;		/* potential router list */
 	unsigned int			prl_count;	/* # of entries in PRL */
 };
 
 struct ip_tunnel_prl_entry {
-	struct ip_tunnel_prl_entry	*next;
+	struct ip_tunnel_prl_entry __rcu *next;
 	__be32				addr;
 	u16				flags;
 	struct rcu_head			rcu_head;
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index bd10a79..573d100 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -90,7 +90,7 @@ struct net {
 #ifdef CONFIG_WEXT_CORE
 	struct sk_buff_head	wext_nlevents;
 #endif
-	struct net_generic	*gen;
+	struct net_generic __rcu *gen;
 };
 
 
diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index 74f119a..9e0915d 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -48,7 +48,7 @@ struct netns_xfrm {
 	struct dst_ops		xfrm6_dst_ops;
 #endif
 
-	struct sock		*nlsk;
+	struct sock __rcu	*nlsk;
 	struct sock		*nlsk_stash;
 
 	u32			sysctl_aevent_etime;
diff --git a/include/net/sock.h b/include/net/sock.h
index 1ad6435..a1d5a76 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -148,7 +148,7 @@ struct sock_common {
 	};
 	struct proto		*skc_prot;
 #ifdef CONFIG_NET_NS
-	struct net	 	*skc_net;
+	struct net __rcu	*skc_net;
 #endif
 };
 
@@ -293,7 +293,7 @@ struct sock {
 	struct ucred		sk_peercred;
 	long			sk_rcvtimeo;
 	long			sk_sndtimeo;
-	struct sk_filter      	*sk_filter;
+	struct sk_filter __rcu	*sk_filter;
 	void			*sk_protinfo;
 	struct timer_list	sk_timer;
 	ktime_t			sk_stamp;
diff --git a/kernel/sched.c b/kernel/sched.c
index d8a213c..53e1670 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -539,7 +539,7 @@ struct rq {
 
 #ifdef CONFIG_SMP
 	struct root_domain *rd;
-	struct sched_domain *sd;
+	struct sched_domain __rcu *sd;
 
 	unsigned char idle_at_tick;
 	/* For active balancing */
diff --git a/net/802/stp.c b/net/802/stp.c
index 53c8f77..978c30b 100644
--- a/net/802/stp.c
+++ b/net/802/stp.c
@@ -21,8 +21,8 @@
 #define GARP_ADDR_MAX	0x2F
 #define GARP_ADDR_RANGE	(GARP_ADDR_MAX - GARP_ADDR_MIN)
 
-static const struct stp_proto *garp_protos[GARP_ADDR_RANGE + 1] __read_mostly;
-static const struct stp_proto *stp_proto __read_mostly;
+static const struct stp_proto __rcu *garp_protos[GARP_ADDR_RANGE + 1] __read_mostly;
+static const struct stp_proto __rcu *stp_proto __read_mostly;
 
 static struct llc_sap *sap __read_mostly;
 static unsigned int sap_registered;
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index fe381d1..7d065cb 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -128,7 +128,7 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev);
 
 static int ipgre_net_id __read_mostly;
 struct ipgre_net {
-	struct ip_tunnel *tunnels[4][HASH_SIZE];
+	struct ip_tunnel __rcu *tunnels[4][HASH_SIZE];
 
 	struct net_device *fb_tunnel_dev;
 };
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 0b27b14..7b5bc05 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -122,11 +122,11 @@
 
 static int ipip_net_id __read_mostly;
 struct ipip_net {
-	struct ip_tunnel *tunnels_r_l[HASH_SIZE];
-	struct ip_tunnel *tunnels_r[HASH_SIZE];
-	struct ip_tunnel *tunnels_l[HASH_SIZE];
-	struct ip_tunnel *tunnels_wc[1];
-	struct ip_tunnel **tunnels[4];
+	struct ip_tunnel __rcu *tunnels_r_l[HASH_SIZE];
+	struct ip_tunnel __rcu *tunnels_r[HASH_SIZE];
+	struct ip_tunnel __rcu *tunnels_l[HASH_SIZE];
+	struct ip_tunnel __rcu *tunnels_wc[1];
+	struct ip_tunnel __rcu **tunnels[4];
 
 	struct net_device *fb_tunnel_dev;
 };
diff --git a/net/ipv4/protocol.c b/net/ipv4/protocol.c
index 542f22f..ac2cf39 100644
--- a/net/ipv4/protocol.c
+++ b/net/ipv4/protocol.c
@@ -28,7 +28,7 @@
 #include <linux/spinlock.h>
 #include <net/protocol.h>
 
-const struct net_protocol *inet_protos[MAX_INET_PROTOS] ____cacheline_aligned_in_smp;
+const struct net_protocol __rcu *inet_protos[MAX_INET_PROTOS] ____cacheline_aligned_in_smp;
 static DEFINE_SPINLOCK(inet_proto_lock);
 
 /*
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index cb562fd..c204b6e 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -200,7 +200,7 @@ const __u8 ip_tos2prio[16] = {
  */
 
 struct rt_hash_bucket {
-	struct rtable	*chain;
+	struct rtable __rcu *chain;
 };
 
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK) || \
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 0f8caf6..2cb54fa 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3057,9 +3057,9 @@ static struct tcp_cookie_secret tcp_secret_one;
 static struct tcp_cookie_secret tcp_secret_two;
 
 /* Essentially a circular list, without dynamic allocation. */
-static struct tcp_cookie_secret *tcp_secret_generating;
+static struct tcp_cookie_secret __rcu *tcp_secret_generating;
 static struct tcp_cookie_secret *tcp_secret_primary;
-static struct tcp_cookie_secret *tcp_secret_retiring;
+static struct tcp_cookie_secret __rcu *tcp_secret_retiring;
 static struct tcp_cookie_secret *tcp_secret_secondary;
 
 static DEFINE_SPINLOCK(tcp_secret_locker);
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 2599870..981ed03 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -83,9 +83,9 @@ struct ip6_tnl_net {
 	/* the IPv6 tunnel fallback device */
 	struct net_device *fb_tnl_dev;
 	/* lists for storing tunnels in use */
-	struct ip6_tnl *tnls_r_l[HASH_SIZE];
-	struct ip6_tnl *tnls_wc[1];
-	struct ip6_tnl **tnls[2];
+	struct ip6_tnl __rcu *tnls_r_l[HASH_SIZE];
+	struct ip6_tnl __rcu *tnls_wc[1];
+	struct ip6_tnl __rcu **tnls[2];
 };
 
 /*
diff --git a/net/ipv6/protocol.c b/net/ipv6/protocol.c
index 1fa3468..dee7e9d 100644
--- a/net/ipv6/protocol.c
+++ b/net/ipv6/protocol.c
@@ -25,7 +25,7 @@
 #include <linux/spinlock.h>
 #include <net/protocol.h>
 
-const struct inet6_protocol *inet6_protos[MAX_INET_PROTOS];
+const struct inet6_protocol __rcu *inet6_protos[MAX_INET_PROTOS];
 static DEFINE_SPINLOCK(inet6_proto_lock);
 
 
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 5abae10..dc77b54 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -68,11 +68,11 @@ static void ipip6_tunnel_setup(struct net_device *dev);
 
 static int sit_net_id __read_mostly;
 struct sit_net {
-	struct ip_tunnel *tunnels_r_l[HASH_SIZE];
-	struct ip_tunnel *tunnels_r[HASH_SIZE];
-	struct ip_tunnel *tunnels_l[HASH_SIZE];
-	struct ip_tunnel *tunnels_wc[1];
-	struct ip_tunnel **tunnels[4];
+	struct ip_tunnel __rcu *tunnels_r_l[HASH_SIZE];
+	struct ip_tunnel __rcu *tunnels_r[HASH_SIZE];
+	struct ip_tunnel __rcu *tunnels_l[HASH_SIZE];
+	struct ip_tunnel __rcu *tunnels_wc[1];
+	struct ip_tunnel __rcu **tunnels[4];
 
 	struct net_device *fb_tunnel_dev;
 };
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 241533e..08476d2 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -192,7 +192,7 @@ struct beacon_data {
 };
 
 struct ieee80211_if_ap {
-	struct beacon_data *beacon;
+	struct beacon_data __rcu *beacon;
 
 	struct list_head vlans;
 
@@ -214,7 +214,7 @@ struct ieee80211_if_vlan {
 	struct list_head list;
 
 	/* used for all tx if the VLAN is configured to 4-addr mode */
-	struct sta_info *sta;
+	struct sta_info __rcu *sta;
 };
 
 struct mesh_stats {
@@ -388,7 +388,8 @@ struct ieee80211_if_ibss {
 
 	unsigned long ibss_join_req;
 	/* probe response/beacon for IBSS */
-	struct sk_buff *presp, *skb;
+	struct sk_buff __rcu *presp;
+	struct sk_buff *skb;
 
 	enum {
 		IEEE80211_IBSS_MLME_SEARCH,
@@ -492,9 +493,9 @@ struct ieee80211_sub_if_data {
 
 #define NUM_DEFAULT_KEYS 4
 #define NUM_DEFAULT_MGMT_KEYS 2
-	struct ieee80211_key *keys[NUM_DEFAULT_KEYS + NUM_DEFAULT_MGMT_KEYS];
-	struct ieee80211_key *default_key;
-	struct ieee80211_key *default_mgmt_key;
+	struct ieee80211_key __rcu *keys[NUM_DEFAULT_KEYS + NUM_DEFAULT_MGMT_KEYS];
+	struct ieee80211_key __rcu *default_key;
+	struct ieee80211_key __rcu *default_mgmt_key;
 
 	u16 sequence_number;
 
@@ -698,7 +699,7 @@ struct ieee80211_local {
 	spinlock_t sta_lock;
 	unsigned long num_sta;
 	struct list_head sta_list, sta_pending_list;
-	struct sta_info *sta_hash[STA_HASH_SIZE];
+	struct sta_info __rcu *sta_hash[STA_HASH_SIZE];
 	struct timer_list sta_cleanup;
 	struct work_struct sta_finish_work;
 	int sta_generation;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 822d845..8034e8d 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -229,10 +229,10 @@ struct sta_ampdu_mlme {
 struct sta_info {
 	/* General information, mostly static */
 	struct list_head list;
-	struct sta_info *hnext;
+	struct sta_info __rcu *hnext;
 	struct ieee80211_local *local;
 	struct ieee80211_sub_if_data *sdata;
-	struct ieee80211_key *key;
+	struct ieee80211_key __rcu *key;
 	struct rate_control_ref *rate_ctrl;
 	void *rate_ctrl_priv;
 	spinlock_t lock;
diff --git a/net/netlabel/netlabel_domainhash.c b/net/netlabel/netlabel_domainhash.c
index d37b7f8..f6803cf 100644
--- a/net/netlabel/netlabel_domainhash.c
+++ b/net/netlabel/netlabel_domainhash.c
@@ -57,8 +57,8 @@ static DEFINE_SPINLOCK(netlbl_domhsh_lock);
 #define netlbl_domhsh_rcu_deref(p) \
 	rcu_dereference_check(p, rcu_read_lock_held() || \
 				 lockdep_is_held(&netlbl_domhsh_lock))
-static struct netlbl_domhsh_tbl *netlbl_domhsh = NULL;
-static struct netlbl_dom_map *netlbl_domhsh_def = NULL;
+static struct netlbl_domhsh_tbl __rcu *netlbl_domhsh = NULL;
+static struct netlbl_dom_map __rcu *netlbl_domhsh_def = NULL;
 
 /*
  * Domain Hash Table Helper Functions
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index a3d64aa..9c5bc42 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -118,8 +118,8 @@ static DEFINE_SPINLOCK(netlbl_unlhsh_lock);
 #define netlbl_unlhsh_rcu_deref(p) \
 	rcu_dereference_check(p, rcu_read_lock_held() || \
 				 lockdep_is_held(&netlbl_unlhsh_lock))
-static struct netlbl_unlhsh_tbl *netlbl_unlhsh = NULL;
-static struct netlbl_unlhsh_iface *netlbl_unlhsh_def = NULL;
+static struct netlbl_unlhsh_tbl __rcu *netlbl_unlhsh = NULL;
+static struct netlbl_unlhsh_iface __rcu *netlbl_unlhsh_def = NULL;
 
 /* Accept unlabeled packets flag */
 static u8 netlabel_unlabel_acceptflg = 0;
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7954243..b917d4a 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -119,7 +119,7 @@ struct nl_pid_hash {
 struct netlink_table {
 	struct nl_pid_hash hash;
 	struct hlist_head mc_list;
-	unsigned long *listeners;
+	unsigned long __rcu *listeners;
 	unsigned int nl_nonroot;
 	unsigned int groups;
 	struct mutex *cb_mutex;
diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c
index 73aee7f..e3baaa1 100644
--- a/net/phonet/af_phonet.c
+++ b/net/phonet/af_phonet.c
@@ -35,7 +35,7 @@
 #include <net/phonet/pn_dev.h>
 
 /* Transport protocol registration */
-static struct phonet_protocol *proto_tab[PHONET_NPROTO] __read_mostly;
+static struct phonet_protocol __rcu *proto_tab[PHONET_NPROTO] __read_mostly;
 
 static struct phonet_protocol *phonet_proto_get(int protocol)
 {
diff --git a/net/phonet/pn_dev.c b/net/phonet/pn_dev.c
index 9b4ced6..d15b97e 100644
--- a/net/phonet/pn_dev.c
+++ b/net/phonet/pn_dev.c
@@ -36,7 +36,7 @@
 
 struct phonet_routes {
 	struct mutex		lock;
-	struct net_device	*table[64];
+	struct net_device __rcu	*table[64];
 };
 
 struct phonet_net {
diff --git a/net/socket.c b/net/socket.c
index 5e8d0af..18b3427 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -155,7 +155,7 @@ static const struct file_operations socket_file_ops = {
  */
 
 static DEFINE_SPINLOCK(net_family_lock);
-static const struct net_proto_family *net_families[NPROTO] __read_mostly;
+static const struct net_proto_family __rcu *net_families[NPROTO] __read_mostly;
 
 /*
  *	Statistics counters of the socket lists
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 22/23] kvm: more __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@relay.de.ibm.com>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Avi Kivity <avi@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |    2 +-
 include/linux/kvm_host.h        |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 06d9e79..9c65faf 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -384,7 +384,7 @@ struct kvm_mem_aliases {
 };
 
 struct kvm_arch {
-	struct kvm_mem_aliases *aliases;
+	struct kvm_mem_aliases __rcu *aliases;
 
 	unsigned int n_free_mmu_pages;
 	unsigned int n_requested_mmu_pages;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 08fe794..282b041 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -169,7 +169,7 @@ struct kvm {
 	raw_spinlock_t requests_lock;
 	struct mutex slots_lock;
 	struct mm_struct *mm; /* userspace tied to this vm */
-	struct kvm_memslots *memslots;
+	struct kvm_memslots __rcu *memslots;
 	struct srcu_struct srcu;
 #ifdef CONFIG_KVM_APIC_ARCHITECTURE
 	u32 bsp_vcpu_id;
@@ -179,7 +179,7 @@ struct kvm {
 	atomic_t online_vcpus;
 	struct list_head vm_list;
 	struct mutex lock;
-	struct kvm_io_bus *buses[KVM_NR_BUSES];
+	struct kvm_io_bus __rcu *buses[KVM_NR_BUSES];
 #ifdef CONFIG_HAVE_KVM_EVENTFD
 	struct {
 		spinlock_t        lock;
-- 
1.7.0.6

[PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

From: Arnd Bergmann <arnd@relay.de.ibm.com>

Also add rcu_dreference_protected() for code paths where locks are held.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
---
 drivers/vhost/net.c   |   11 ++++++++---
 drivers/vhost/vhost.c |   14 ++++++++------
 drivers/vhost/vhost.h |    4 ++--
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 9777583..945c5cb 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -364,7 +364,10 @@ static void vhost_net_disable_vq(struct vhost_net *n,
 static void vhost_net_enable_vq(struct vhost_net *n,
 				struct vhost_virtqueue *vq)
 {
-	struct socket *sock = vq->private_data;
+	struct socket *sock;
+
+	sock = rcu_dereference_protected(vq->private_data,
+					 lockdep_is_held(&vq->mutex));
 	if (!sock)
 		return;
 	if (vq == n->vqs + VHOST_NET_VQ_TX) {
@@ -380,7 +383,8 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n,
 	struct socket *sock;
 
 	mutex_lock(&vq->mutex);
-	sock = vq->private_data;
+	sock = rcu_dereference_protected(vq->private_data,
+					 lockdep_is_held(&vq->mutex));
 	vhost_net_disable_vq(n, vq);
 	rcu_assign_pointer(vq->private_data, NULL);
 	mutex_unlock(&vq->mutex);
@@ -518,7 +522,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
 	}
 
 	/* start polling new socket */
-	oldsock = vq->private_data;
+	oldsock = rcu_dereference_protected(vq->private_data,
+					    lockdep_is_held(&vq->mutex));
 	if (sock == oldsock)
 		goto done;
 
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index e69d238..fc0c077 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -180,7 +180,7 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
 	vhost_dev_cleanup(dev);
 
 	memory->nregions = 0;
-	dev->memory = memory;
+	RCU_INIT_POINTER(dev->memory, memory);
 	return 0;
 }
 
@@ -212,8 +212,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
 		fput(dev->log_file);
 	dev->log_file = NULL;
 	/* No one will access memory at this point */
-	kfree(dev->memory);
-	dev->memory = NULL;
+	kfree(rcu_dereference_protected(dev->memory,
+					lockdep_is_held(&dev->mutex)));
+	RCU_INIT_POINTER(dev->memory, NULL);
 	if (dev->mm)
 		mmput(dev->mm);
 	dev->mm = NULL;
@@ -294,14 +295,14 @@ static int vq_access_ok(unsigned int num,
 /* Caller should have device mutex but not vq mutex */
 int vhost_log_access_ok(struct vhost_dev *dev)
 {
-	return memory_access_ok(dev, dev->memory, 1);
+	return memory_access_ok(dev, rcu_dereference_protected(dev->memory, lockdep_is_held(&dev->mutex)), 1);
 }
 
 /* Verify access for write logging. */
 /* Caller should have vq mutex and device mutex */
 static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
 {
-	return vq_memory_access_ok(log_base, vq->dev->memory,
+	return vq_memory_access_ok(log_base, rcu_dereference_protected(vq->dev->memory, lockdep_is_held(&dev->mutex)),
 			    vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
 		(!vq->log_used || log_access_ok(log_base, vq->log_addr,
 					sizeof *vq->used +
@@ -342,7 +343,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
 
 	if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL)))
 		return -EFAULT;
-	oldmem = d->memory;
+	oldmem = rcu_dereference_protected(d->memory,
+					   lockdep_is_held(&d->mutex));
 	rcu_assign_pointer(d->memory, newmem);
 	synchronize_rcu();
 	kfree(oldmem);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 44591ba..240396c 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -92,7 +92,7 @@ struct vhost_virtqueue {
 	 * work item execution acts instead of rcu_read_lock() and the end of
 	 * work item execution acts instead of rcu_read_lock().
 	 * Writers use virtqueue mutex. */
-	void *private_data;
+	void __rcu *private_data;
 	/* Log write descriptors */
 	void __user *log_base;
 	struct vhost_log log[VHOST_NET_MAX_SG];
@@ -102,7 +102,7 @@ struct vhost_dev {
 	/* Readers use RCU to access memory table pointer
 	 * log base pointer and features.
 	 * Writers use mutex below.*/
-	struct vhost_memory *memory;
+	struct vhost_memory __rcu *memory;
 	struct mm_struct *mm;
 	struct mutex mutex;
 	unsigned acked_features;
-- 
1.7.0.6

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Michael S. Tsirkin]

On Wed, May 12, 2010 at 02:33:42PM -0700, Paul E. McKenney wrote:
> From: Arnd Bergmann <arnd@relay.de.ibm.com>
> 
> Also add rcu_dreference_protected() for code paths where locks are held.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>

Maybe long lines can be fixed? Otherwise looks ok.

> ---
>  drivers/vhost/net.c   |   11 ++++++++---
>  drivers/vhost/vhost.c |   14 ++++++++------
>  drivers/vhost/vhost.h |    4 ++--
>  3 files changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9777583..945c5cb 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -364,7 +364,10 @@ static void vhost_net_disable_vq(struct vhost_net *n,
>  static void vhost_net_enable_vq(struct vhost_net *n,
>  				struct vhost_virtqueue *vq)
>  {
> -	struct socket *sock = vq->private_data;
> +	struct socket *sock;
> +
> +	sock = rcu_dereference_protected(vq->private_data,
> +					 lockdep_is_held(&vq->mutex));
>  	if (!sock)
>  		return;
>  	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> @@ -380,7 +383,8 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n,
>  	struct socket *sock;
>  
>  	mutex_lock(&vq->mutex);
> -	sock = vq->private_data;
> +	sock = rcu_dereference_protected(vq->private_data,
> +					 lockdep_is_held(&vq->mutex));
>  	vhost_net_disable_vq(n, vq);
>  	rcu_assign_pointer(vq->private_data, NULL);
>  	mutex_unlock(&vq->mutex);
> @@ -518,7 +522,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
>  	}
>  
>  	/* start polling new socket */
> -	oldsock = vq->private_data;
> +	oldsock = rcu_dereference_protected(vq->private_data,
> +					    lockdep_is_held(&vq->mutex));
>  	if (sock == oldsock)
>  		goto done;
>  
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index e69d238..fc0c077 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -180,7 +180,7 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
>  	vhost_dev_cleanup(dev);
>  
>  	memory->nregions = 0;
> -	dev->memory = memory;
> +	RCU_INIT_POINTER(dev->memory, memory);
>  	return 0;
>  }
>  
> @@ -212,8 +212,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
>  		fput(dev->log_file);
>  	dev->log_file = NULL;
>  	/* No one will access memory at this point */
> -	kfree(dev->memory);
> -	dev->memory = NULL;
> +	kfree(rcu_dereference_protected(dev->memory,
> +					lockdep_is_held(&dev->mutex)));
> +	RCU_INIT_POINTER(dev->memory, NULL);
>  	if (dev->mm)
>  		mmput(dev->mm);
>  	dev->mm = NULL;
> @@ -294,14 +295,14 @@ static int vq_access_ok(unsigned int num,
>  /* Caller should have device mutex but not vq mutex */
>  int vhost_log_access_ok(struct vhost_dev *dev)
>  {
> -	return memory_access_ok(dev, dev->memory, 1);
> +	return memory_access_ok(dev, rcu_dereference_protected(dev->memory, lockdep_is_held(&dev->mutex)), 1);
>  }
>  
>  /* Verify access for write logging. */
>  /* Caller should have vq mutex and device mutex */
>  static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
>  {
> -	return vq_memory_access_ok(log_base, vq->dev->memory,
> +	return vq_memory_access_ok(log_base, rcu_dereference_protected(vq->dev->memory, lockdep_is_held(&dev->mutex)),
>  			    vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
>  		(!vq->log_used || log_access_ok(log_base, vq->log_addr,
>  					sizeof *vq->used +
> @@ -342,7 +343,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
>  
>  	if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL)))
>  		return -EFAULT;
> -	oldmem = d->memory;
> +	oldmem = rcu_dereference_protected(d->memory,
> +					   lockdep_is_held(&d->mutex));
>  	rcu_assign_pointer(d->memory, newmem);
>  	synchronize_rcu();
>  	kfree(oldmem);
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 44591ba..240396c 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -92,7 +92,7 @@ struct vhost_virtqueue {
>  	 * work item execution acts instead of rcu_read_lock() and the end of
>  	 * work item execution acts instead of rcu_read_lock().
>  	 * Writers use virtqueue mutex. */
> -	void *private_data;
> +	void __rcu *private_data;
>  	/* Log write descriptors */
>  	void __user *log_base;
>  	struct vhost_log log[VHOST_NET_MAX_SG];
> @@ -102,7 +102,7 @@ struct vhost_dev {
>  	/* Readers use RCU to access memory table pointer
>  	 * log base pointer and features.
>  	 * Writers use mutex below.*/
> -	struct vhost_memory *memory;
> +	struct vhost_memory __rcu *memory;
>  	struct mm_struct *mm;
>  	struct mutex mutex;
>  	unsigned acked_features;
> -- 
> 1.7.0.6

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

On Thu, May 13, 2010 at 12:48:47AM +0300, Michael S. Tsirkin wrote:
> On Wed, May 12, 2010 at 02:33:42PM -0700, Paul E. McKenney wrote:
> > From: Arnd Bergmann <arnd@relay.de.ibm.com>
> > 
> > Also add rcu_dreference_protected() for code paths where locks are held.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> 
> Maybe long lines can be fixed? Otherwise looks ok.

Done.  I introduced locals to make it fit.

One other thing...  We need some API that says "we are running in the
context of a work queue."  Otherwise, we will get false positives when
called in a work queue without locks held.

Any thoughts?  One approach would be to create a separate lockdep class
for vhost workqueue state, similar to the approach used in instrument
rcu_read_lock() and friends.

							Thanx, Paul

> > ---
> >  drivers/vhost/net.c   |   11 ++++++++---
> >  drivers/vhost/vhost.c |   14 ++++++++------
> >  drivers/vhost/vhost.h |    4 ++--
> >  3 files changed, 18 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 9777583..945c5cb 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -364,7 +364,10 @@ static void vhost_net_disable_vq(struct vhost_net *n,
> >  static void vhost_net_enable_vq(struct vhost_net *n,
> >  				struct vhost_virtqueue *vq)
> >  {
> > -	struct socket *sock = vq->private_data;
> > +	struct socket *sock;
> > +
> > +	sock = rcu_dereference_protected(vq->private_data,
> > +					 lockdep_is_held(&vq->mutex));
> >  	if (!sock)
> >  		return;
> >  	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> > @@ -380,7 +383,8 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> >  	struct socket *sock;
> >  
> >  	mutex_lock(&vq->mutex);
> > -	sock = vq->private_data;
> > +	sock = rcu_dereference_protected(vq->private_data,
> > +					 lockdep_is_held(&vq->mutex));
> >  	vhost_net_disable_vq(n, vq);
> >  	rcu_assign_pointer(vq->private_data, NULL);
> >  	mutex_unlock(&vq->mutex);
> > @@ -518,7 +522,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> >  	}
> >  
> >  	/* start polling new socket */
> > -	oldsock = vq->private_data;
> > +	oldsock = rcu_dereference_protected(vq->private_data,
> > +					    lockdep_is_held(&vq->mutex));
> >  	if (sock == oldsock)
> >  		goto done;
> >  
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index e69d238..fc0c077 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -180,7 +180,7 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
> >  	vhost_dev_cleanup(dev);
> >  
> >  	memory->nregions = 0;
> > -	dev->memory = memory;
> > +	RCU_INIT_POINTER(dev->memory, memory);
> >  	return 0;
> >  }
> >  
> > @@ -212,8 +212,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
> >  		fput(dev->log_file);
> >  	dev->log_file = NULL;
> >  	/* No one will access memory at this point */
> > -	kfree(dev->memory);
> > -	dev->memory = NULL;
> > +	kfree(rcu_dereference_protected(dev->memory,
> > +					lockdep_is_held(&dev->mutex)));
> > +	RCU_INIT_POINTER(dev->memory, NULL);
> >  	if (dev->mm)
> >  		mmput(dev->mm);
> >  	dev->mm = NULL;
> > @@ -294,14 +295,14 @@ static int vq_access_ok(unsigned int num,
> >  /* Caller should have device mutex but not vq mutex */
> >  int vhost_log_access_ok(struct vhost_dev *dev)
> >  {
> > -	return memory_access_ok(dev, dev->memory, 1);
> > +	return memory_access_ok(dev, rcu_dereference_protected(dev->memory, lockdep_is_held(&dev->mutex)), 1);
> >  }
> >  
> >  /* Verify access for write logging. */
> >  /* Caller should have vq mutex and device mutex */
> >  static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
> >  {
> > -	return vq_memory_access_ok(log_base, vq->dev->memory,
> > +	return vq_memory_access_ok(log_base, rcu_dereference_protected(vq->dev->memory, lockdep_is_held(&dev->mutex)),
> >  			    vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
> >  		(!vq->log_used || log_access_ok(log_base, vq->log_addr,
> >  					sizeof *vq->used +
> > @@ -342,7 +343,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
> >  
> >  	if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL)))
> >  		return -EFAULT;
> > -	oldmem = d->memory;
> > +	oldmem = rcu_dereference_protected(d->memory,
> > +					   lockdep_is_held(&d->mutex));
> >  	rcu_assign_pointer(d->memory, newmem);
> >  	synchronize_rcu();
> >  	kfree(oldmem);
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > index 44591ba..240396c 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -92,7 +92,7 @@ struct vhost_virtqueue {
> >  	 * work item execution acts instead of rcu_read_lock() and the end of
> >  	 * work item execution acts instead of rcu_read_lock().
> >  	 * Writers use virtqueue mutex. */
> > -	void *private_data;
> > +	void __rcu *private_data;
> >  	/* Log write descriptors */
> >  	void __user *log_base;
> >  	struct vhost_log log[VHOST_NET_MAX_SG];
> > @@ -102,7 +102,7 @@ struct vhost_dev {
> >  	/* Readers use RCU to access memory table pointer
> >  	 * log base pointer and features.
> >  	 * Writers use mutex below.*/
> > -	struct vhost_memory *memory;
> > +	struct vhost_memory __rcu *memory;
> >  	struct mm_struct *mm;
> >  	struct mutex mutex;
> >  	unsigned acked_features;
> > -- 
> > 1.7.0.6

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Michael S. Tsirkin]

On Wed, May 12, 2010 at 04:00:57PM -0700, Paul E. McKenney wrote:
> On Thu, May 13, 2010 at 12:48:47AM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 12, 2010 at 02:33:42PM -0700, Paul E. McKenney wrote:
> > > From: Arnd Bergmann <arnd@relay.de.ibm.com>
> > > 
> > > Also add rcu_dreference_protected() for code paths where locks are held.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > 
> > Maybe long lines can be fixed? Otherwise looks ok.
> 
> Done.  I introduced locals to make it fit.
> 
> One other thing...  We need some API that says "we are running in the
> context of a work queue."  Otherwise, we will get false positives when
> called in a work queue without locks held.
> 
> Any thoughts?  One approach would be to create a separate lockdep class
> for vhost workqueue state, similar to the approach used in instrument
> rcu_read_lock() and friends.
> 
> 							Thanx, Paul

And we would want some way to tag the flushes ... not easy.
Let's start with switching to raw to avoid the errors.

> > > ---
> > >  drivers/vhost/net.c   |   11 ++++++++---
> > >  drivers/vhost/vhost.c |   14 ++++++++------
> > >  drivers/vhost/vhost.h |    4 ++--
> > >  3 files changed, 18 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > index 9777583..945c5cb 100644
> > > --- a/drivers/vhost/net.c
> > > +++ b/drivers/vhost/net.c
> > > @@ -364,7 +364,10 @@ static void vhost_net_disable_vq(struct vhost_net *n,
> > >  static void vhost_net_enable_vq(struct vhost_net *n,
> > >  				struct vhost_virtqueue *vq)
> > >  {
> > > -	struct socket *sock = vq->private_data;
> > > +	struct socket *sock;
> > > +
> > > +	sock = rcu_dereference_protected(vq->private_data,
> > > +					 lockdep_is_held(&vq->mutex));
> > >  	if (!sock)
> > >  		return;
> > >  	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> > > @@ -380,7 +383,8 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> > >  	struct socket *sock;
> > >  
> > >  	mutex_lock(&vq->mutex);
> > > -	sock = vq->private_data;
> > > +	sock = rcu_dereference_protected(vq->private_data,
> > > +					 lockdep_is_held(&vq->mutex));
> > >  	vhost_net_disable_vq(n, vq);
> > >  	rcu_assign_pointer(vq->private_data, NULL);
> > >  	mutex_unlock(&vq->mutex);
> > > @@ -518,7 +522,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> > >  	}
> > >  
> > >  	/* start polling new socket */
> > > -	oldsock = vq->private_data;
> > > +	oldsock = rcu_dereference_protected(vq->private_data,
> > > +					    lockdep_is_held(&vq->mutex));
> > >  	if (sock == oldsock)
> > >  		goto done;
> > >  
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index e69d238..fc0c077 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -180,7 +180,7 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
> > >  	vhost_dev_cleanup(dev);
> > >  
> > >  	memory->nregions = 0;
> > > -	dev->memory = memory;
> > > +	RCU_INIT_POINTER(dev->memory, memory);
> > >  	return 0;
> > >  }
> > >  
> > > @@ -212,8 +212,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
> > >  		fput(dev->log_file);
> > >  	dev->log_file = NULL;
> > >  	/* No one will access memory at this point */
> > > -	kfree(dev->memory);
> > > -	dev->memory = NULL;
> > > +	kfree(rcu_dereference_protected(dev->memory,
> > > +					lockdep_is_held(&dev->mutex)));
> > > +	RCU_INIT_POINTER(dev->memory, NULL);
> > >  	if (dev->mm)
> > >  		mmput(dev->mm);
> > >  	dev->mm = NULL;
> > > @@ -294,14 +295,14 @@ static int vq_access_ok(unsigned int num,
> > >  /* Caller should have device mutex but not vq mutex */
> > >  int vhost_log_access_ok(struct vhost_dev *dev)
> > >  {
> > > -	return memory_access_ok(dev, dev->memory, 1);
> > > +	return memory_access_ok(dev, rcu_dereference_protected(dev->memory, lockdep_is_held(&dev->mutex)), 1);
> > >  }
> > >  
> > >  /* Verify access for write logging. */
> > >  /* Caller should have vq mutex and device mutex */
> > >  static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
> > >  {
> > > -	return vq_memory_access_ok(log_base, vq->dev->memory,
> > > +	return vq_memory_access_ok(log_base, rcu_dereference_protected(vq->dev->memory, lockdep_is_held(&dev->mutex)),
> > >  			    vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
> > >  		(!vq->log_used || log_access_ok(log_base, vq->log_addr,
> > >  					sizeof *vq->used +
> > > @@ -342,7 +343,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
> > >  
> > >  	if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL)))
> > >  		return -EFAULT;
> > > -	oldmem = d->memory;
> > > +	oldmem = rcu_dereference_protected(d->memory,
> > > +					   lockdep_is_held(&d->mutex));
> > >  	rcu_assign_pointer(d->memory, newmem);
> > >  	synchronize_rcu();
> > >  	kfree(oldmem);
> > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > > index 44591ba..240396c 100644
> > > --- a/drivers/vhost/vhost.h
> > > +++ b/drivers/vhost/vhost.h
> > > @@ -92,7 +92,7 @@ struct vhost_virtqueue {
> > >  	 * work item execution acts instead of rcu_read_lock() and the end of
> > >  	 * work item execution acts instead of rcu_read_lock().
> > >  	 * Writers use virtqueue mutex. */
> > > -	void *private_data;
> > > +	void __rcu *private_data;
> > >  	/* Log write descriptors */
> > >  	void __user *log_base;
> > >  	struct vhost_log log[VHOST_NET_MAX_SG];
> > > @@ -102,7 +102,7 @@ struct vhost_dev {
> > >  	/* Readers use RCU to access memory table pointer
> > >  	 * log base pointer and features.
> > >  	 * Writers use mutex below.*/
> > > -	struct vhost_memory *memory;
> > > +	struct vhost_memory __rcu *memory;
> > >  	struct mm_struct *mm;
> > >  	struct mutex mutex;
> > >  	unsigned acked_features;
> > > -- 
> > > 1.7.0.6

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

On Thu, May 13, 2010 at 06:53:45AM +0300, Michael S. Tsirkin wrote:
> On Wed, May 12, 2010 at 04:00:57PM -0700, Paul E. McKenney wrote:
> > On Thu, May 13, 2010 at 12:48:47AM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 12, 2010 at 02:33:42PM -0700, Paul E. McKenney wrote:
> > > > From: Arnd Bergmann <arnd@relay.de.ibm.com>
> > > > 
> > > > Also add rcu_dreference_protected() for code paths where locks are held.
> > > > 
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > > 
> > > Maybe long lines can be fixed? Otherwise looks ok.
> > 
> > Done.  I introduced locals to make it fit.
> > 
> > One other thing...  We need some API that says "we are running in the
> > context of a work queue."  Otherwise, we will get false positives when
> > called in a work queue without locks held.
> > 
> > Any thoughts?  One approach would be to create a separate lockdep class
> > for vhost workqueue state, similar to the approach used in instrument
> > rcu_read_lock() and friends.
> > 
> > 							Thanx, Paul
> 
> And we would want some way to tag the flushes ... not easy.
> Let's start with switching to raw to avoid the errors.

The flushes turn out to be a non-problem for the moment due to the fact
that there is no syntactic way to tie them to the read-side critical
sections.

On the read-side critical sections, I would not be above looking at the
->comm field and checking for the possible command names of the workqueue
kernel threads.  Might get some false negatives due to choices of user
command names, but that is OK.  We just need a high probability of
catching errors, not a certainty.  ;-)

							Thanx, Paul

> > > > ---
> > > >  drivers/vhost/net.c   |   11 ++++++++---
> > > >  drivers/vhost/vhost.c |   14 ++++++++------
> > > >  drivers/vhost/vhost.h |    4 ++--
> > > >  3 files changed, 18 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > > index 9777583..945c5cb 100644
> > > > --- a/drivers/vhost/net.c
> > > > +++ b/drivers/vhost/net.c
> > > > @@ -364,7 +364,10 @@ static void vhost_net_disable_vq(struct vhost_net *n,
> > > >  static void vhost_net_enable_vq(struct vhost_net *n,
> > > >  				struct vhost_virtqueue *vq)
> > > >  {
> > > > -	struct socket *sock = vq->private_data;
> > > > +	struct socket *sock;
> > > > +
> > > > +	sock = rcu_dereference_protected(vq->private_data,
> > > > +					 lockdep_is_held(&vq->mutex));
> > > >  	if (!sock)
> > > >  		return;
> > > >  	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> > > > @@ -380,7 +383,8 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> > > >  	struct socket *sock;
> > > >  
> > > >  	mutex_lock(&vq->mutex);
> > > > -	sock = vq->private_data;
> > > > +	sock = rcu_dereference_protected(vq->private_data,
> > > > +					 lockdep_is_held(&vq->mutex));
> > > >  	vhost_net_disable_vq(n, vq);
> > > >  	rcu_assign_pointer(vq->private_data, NULL);
> > > >  	mutex_unlock(&vq->mutex);
> > > > @@ -518,7 +522,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> > > >  	}
> > > >  
> > > >  	/* start polling new socket */
> > > > -	oldsock = vq->private_data;
> > > > +	oldsock = rcu_dereference_protected(vq->private_data,
> > > > +					    lockdep_is_held(&vq->mutex));
> > > >  	if (sock == oldsock)
> > > >  		goto done;
> > > >  
> > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > index e69d238..fc0c077 100644
> > > > --- a/drivers/vhost/vhost.c
> > > > +++ b/drivers/vhost/vhost.c
> > > > @@ -180,7 +180,7 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
> > > >  	vhost_dev_cleanup(dev);
> > > >  
> > > >  	memory->nregions = 0;
> > > > -	dev->memory = memory;
> > > > +	RCU_INIT_POINTER(dev->memory, memory);
> > > >  	return 0;
> > > >  }
> > > >  
> > > > @@ -212,8 +212,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
> > > >  		fput(dev->log_file);
> > > >  	dev->log_file = NULL;
> > > >  	/* No one will access memory at this point */
> > > > -	kfree(dev->memory);
> > > > -	dev->memory = NULL;
> > > > +	kfree(rcu_dereference_protected(dev->memory,
> > > > +					lockdep_is_held(&dev->mutex)));
> > > > +	RCU_INIT_POINTER(dev->memory, NULL);
> > > >  	if (dev->mm)
> > > >  		mmput(dev->mm);
> > > >  	dev->mm = NULL;
> > > > @@ -294,14 +295,14 @@ static int vq_access_ok(unsigned int num,
> > > >  /* Caller should have device mutex but not vq mutex */
> > > >  int vhost_log_access_ok(struct vhost_dev *dev)
> > > >  {
> > > > -	return memory_access_ok(dev, dev->memory, 1);
> > > > +	return memory_access_ok(dev, rcu_dereference_protected(dev->memory, lockdep_is_held(&dev->mutex)), 1);
> > > >  }
> > > >  
> > > >  /* Verify access for write logging. */
> > > >  /* Caller should have vq mutex and device mutex */
> > > >  static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
> > > >  {
> > > > -	return vq_memory_access_ok(log_base, vq->dev->memory,
> > > > +	return vq_memory_access_ok(log_base, rcu_dereference_protected(vq->dev->memory, lockdep_is_held(&dev->mutex)),
> > > >  			    vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
> > > >  		(!vq->log_used || log_access_ok(log_base, vq->log_addr,
> > > >  					sizeof *vq->used +
> > > > @@ -342,7 +343,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
> > > >  
> > > >  	if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL)))
> > > >  		return -EFAULT;
> > > > -	oldmem = d->memory;
> > > > +	oldmem = rcu_dereference_protected(d->memory,
> > > > +					   lockdep_is_held(&d->mutex));
> > > >  	rcu_assign_pointer(d->memory, newmem);
> > > >  	synchronize_rcu();
> > > >  	kfree(oldmem);
> > > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > > > index 44591ba..240396c 100644
> > > > --- a/drivers/vhost/vhost.h
> > > > +++ b/drivers/vhost/vhost.h
> > > > @@ -92,7 +92,7 @@ struct vhost_virtqueue {
> > > >  	 * work item execution acts instead of rcu_read_lock() and the end of
> > > >  	 * work item execution acts instead of rcu_read_lock().
> > > >  	 * Writers use virtqueue mutex. */
> > > > -	void *private_data;
> > > > +	void __rcu *private_data;
> > > >  	/* Log write descriptors */
> > > >  	void __user *log_base;
> > > >  	struct vhost_log log[VHOST_NET_MAX_SG];
> > > > @@ -102,7 +102,7 @@ struct vhost_dev {
> > > >  	/* Readers use RCU to access memory table pointer
> > > >  	 * log base pointer and features.
> > > >  	 * Writers use mutex below.*/
> > > > -	struct vhost_memory *memory;
> > > > +	struct vhost_memory __rcu *memory;
> > > >  	struct mm_struct *mm;
> > > >  	struct mutex mutex;
> > > >  	unsigned acked_features;
> > > > -- 
> > > > 1.7.0.6

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Michael S. Tsirkin]

On Wed, May 12, 2010 at 04:00:57PM -0700, Paul E. McKenney wrote:
> On Thu, May 13, 2010 at 12:48:47AM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 12, 2010 at 02:33:42PM -0700, Paul E. McKenney wrote:
> > > From: Arnd Bergmann <arnd@relay.de.ibm.com>
> > > 
> > > Also add rcu_dreference_protected() for code paths where locks are held.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > 
> > Maybe long lines can be fixed? Otherwise looks ok.
> 
> Done.  I introduced locals to make it fit.

Just pls check this does not lead to unused variable
warnings when the whole lockdep is off.

> One other thing...  We need some API that says "we are running in the
> context of a work queue."  Otherwise, we will get false positives when
> called in a work queue without locks held.
> 
> Any thoughts?  One approach would be to create a separate lockdep class
> for vhost workqueue state, similar to the approach used in instrument
> rcu_read_lock() and friends.
> 
> 							Thanx, Paul
> 
> > > ---
> > >  drivers/vhost/net.c   |   11 ++++++++---
> > >  drivers/vhost/vhost.c |   14 ++++++++------
> > >  drivers/vhost/vhost.h |    4 ++--
> > >  3 files changed, 18 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > index 9777583..945c5cb 100644
> > > --- a/drivers/vhost/net.c
> > > +++ b/drivers/vhost/net.c
> > > @@ -364,7 +364,10 @@ static void vhost_net_disable_vq(struct vhost_net *n,
> > >  static void vhost_net_enable_vq(struct vhost_net *n,
> > >  				struct vhost_virtqueue *vq)
> > >  {
> > > -	struct socket *sock = vq->private_data;
> > > +	struct socket *sock;
> > > +
> > > +	sock = rcu_dereference_protected(vq->private_data,
> > > +					 lockdep_is_held(&vq->mutex));
> > >  	if (!sock)
> > >  		return;
> > >  	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> > > @@ -380,7 +383,8 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> > >  	struct socket *sock;
> > >  
> > >  	mutex_lock(&vq->mutex);
> > > -	sock = vq->private_data;
> > > +	sock = rcu_dereference_protected(vq->private_data,
> > > +					 lockdep_is_held(&vq->mutex));
> > >  	vhost_net_disable_vq(n, vq);
> > >  	rcu_assign_pointer(vq->private_data, NULL);
> > >  	mutex_unlock(&vq->mutex);
> > > @@ -518,7 +522,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> > >  	}
> > >  
> > >  	/* start polling new socket */
> > > -	oldsock = vq->private_data;
> > > +	oldsock = rcu_dereference_protected(vq->private_data,
> > > +					    lockdep_is_held(&vq->mutex));
> > >  	if (sock == oldsock)
> > >  		goto done;
> > >  
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index e69d238..fc0c077 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -180,7 +180,7 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
> > >  	vhost_dev_cleanup(dev);
> > >  
> > >  	memory->nregions = 0;
> > > -	dev->memory = memory;
> > > +	RCU_INIT_POINTER(dev->memory, memory);
> > >  	return 0;
> > >  }
> > >  
> > > @@ -212,8 +212,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
> > >  		fput(dev->log_file);
> > >  	dev->log_file = NULL;
> > >  	/* No one will access memory at this point */
> > > -	kfree(dev->memory);
> > > -	dev->memory = NULL;
> > > +	kfree(rcu_dereference_protected(dev->memory,
> > > +					lockdep_is_held(&dev->mutex)));
> > > +	RCU_INIT_POINTER(dev->memory, NULL);
> > >  	if (dev->mm)
> > >  		mmput(dev->mm);
> > >  	dev->mm = NULL;
> > > @@ -294,14 +295,14 @@ static int vq_access_ok(unsigned int num,
> > >  /* Caller should have device mutex but not vq mutex */
> > >  int vhost_log_access_ok(struct vhost_dev *dev)
> > >  {
> > > -	return memory_access_ok(dev, dev->memory, 1);
> > > +	return memory_access_ok(dev, rcu_dereference_protected(dev->memory, lockdep_is_held(&dev->mutex)), 1);
> > >  }
> > >  
> > >  /* Verify access for write logging. */
> > >  /* Caller should have vq mutex and device mutex */
> > >  static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
> > >  {
> > > -	return vq_memory_access_ok(log_base, vq->dev->memory,
> > > +	return vq_memory_access_ok(log_base, rcu_dereference_protected(vq->dev->memory, lockdep_is_held(&dev->mutex)),
> > >  			    vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
> > >  		(!vq->log_used || log_access_ok(log_base, vq->log_addr,
> > >  					sizeof *vq->used +
> > > @@ -342,7 +343,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
> > >  
> > >  	if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL)))
> > >  		return -EFAULT;
> > > -	oldmem = d->memory;
> > > +	oldmem = rcu_dereference_protected(d->memory,
> > > +					   lockdep_is_held(&d->mutex));
> > >  	rcu_assign_pointer(d->memory, newmem);
> > >  	synchronize_rcu();
> > >  	kfree(oldmem);
> > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > > index 44591ba..240396c 100644
> > > --- a/drivers/vhost/vhost.h
> > > +++ b/drivers/vhost/vhost.h
> > > @@ -92,7 +92,7 @@ struct vhost_virtqueue {
> > >  	 * work item execution acts instead of rcu_read_lock() and the end of
> > >  	 * work item execution acts instead of rcu_read_lock().
> > >  	 * Writers use virtqueue mutex. */
> > > -	void *private_data;
> > > +	void __rcu *private_data;
> > >  	/* Log write descriptors */
> > >  	void __user *log_base;
> > >  	struct vhost_log log[VHOST_NET_MAX_SG];
> > > @@ -102,7 +102,7 @@ struct vhost_dev {
> > >  	/* Readers use RCU to access memory table pointer
> > >  	 * log base pointer and features.
> > >  	 * Writers use mutex below.*/
> > > -	struct vhost_memory *memory;
> > > +	struct vhost_memory __rcu *memory;
> > >  	struct mm_struct *mm;
> > >  	struct mutex mutex;
> > >  	unsigned acked_features;
> > > -- 
> > > 1.7.0.6

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

On Thu, May 13, 2010 at 07:50:50AM +0300, Michael S. Tsirkin wrote:
> On Wed, May 12, 2010 at 04:00:57PM -0700, Paul E. McKenney wrote:
> > On Thu, May 13, 2010 at 12:48:47AM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 12, 2010 at 02:33:42PM -0700, Paul E. McKenney wrote:
> > > > From: Arnd Bergmann <arnd@relay.de.ibm.com>
> > > > 
> > > > Also add rcu_dreference_protected() for code paths where locks are held.
> > > > 
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > > 
> > > Maybe long lines can be fixed? Otherwise looks ok.
> > 
> > Done.  I introduced locals to make it fit.
> 
> Just pls check this does not lead to unused variable
> warnings when the whole lockdep is off.

Will do!  It should not, because the assignment happens independently
of lockdep, but I will nevertheless build both ways.

I am also merging in Peter Zijlstra's suggestion of leveraging the
existing lockdep on workqueues.

							Thanx, Paul

> > One other thing...  We need some API that says "we are running in the
> > context of a work queue."  Otherwise, we will get false positives when
> > called in a work queue without locks held.
> > 
> > Any thoughts?  One approach would be to create a separate lockdep class
> > for vhost workqueue state, similar to the approach used in instrument
> > rcu_read_lock() and friends.
> > 
> > 							Thanx, Paul
> > 
> > > > ---
> > > >  drivers/vhost/net.c   |   11 ++++++++---
> > > >  drivers/vhost/vhost.c |   14 ++++++++------
> > > >  drivers/vhost/vhost.h |    4 ++--
> > > >  3 files changed, 18 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > > index 9777583..945c5cb 100644
> > > > --- a/drivers/vhost/net.c
> > > > +++ b/drivers/vhost/net.c
> > > > @@ -364,7 +364,10 @@ static void vhost_net_disable_vq(struct vhost_net *n,
> > > >  static void vhost_net_enable_vq(struct vhost_net *n,
> > > >  				struct vhost_virtqueue *vq)
> > > >  {
> > > > -	struct socket *sock = vq->private_data;
> > > > +	struct socket *sock;
> > > > +
> > > > +	sock = rcu_dereference_protected(vq->private_data,
> > > > +					 lockdep_is_held(&vq->mutex));
> > > >  	if (!sock)
> > > >  		return;
> > > >  	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> > > > @@ -380,7 +383,8 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> > > >  	struct socket *sock;
> > > >  
> > > >  	mutex_lock(&vq->mutex);
> > > > -	sock = vq->private_data;
> > > > +	sock = rcu_dereference_protected(vq->private_data,
> > > > +					 lockdep_is_held(&vq->mutex));
> > > >  	vhost_net_disable_vq(n, vq);
> > > >  	rcu_assign_pointer(vq->private_data, NULL);
> > > >  	mutex_unlock(&vq->mutex);
> > > > @@ -518,7 +522,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> > > >  	}
> > > >  
> > > >  	/* start polling new socket */
> > > > -	oldsock = vq->private_data;
> > > > +	oldsock = rcu_dereference_protected(vq->private_data,
> > > > +					    lockdep_is_held(&vq->mutex));
> > > >  	if (sock == oldsock)
> > > >  		goto done;
> > > >  
> > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > index e69d238..fc0c077 100644
> > > > --- a/drivers/vhost/vhost.c
> > > > +++ b/drivers/vhost/vhost.c
> > > > @@ -180,7 +180,7 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
> > > >  	vhost_dev_cleanup(dev);
> > > >  
> > > >  	memory->nregions = 0;
> > > > -	dev->memory = memory;
> > > > +	RCU_INIT_POINTER(dev->memory, memory);
> > > >  	return 0;
> > > >  }
> > > >  
> > > > @@ -212,8 +212,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
> > > >  		fput(dev->log_file);
> > > >  	dev->log_file = NULL;
> > > >  	/* No one will access memory at this point */
> > > > -	kfree(dev->memory);
> > > > -	dev->memory = NULL;
> > > > +	kfree(rcu_dereference_protected(dev->memory,
> > > > +					lockdep_is_held(&dev->mutex)));
> > > > +	RCU_INIT_POINTER(dev->memory, NULL);
> > > >  	if (dev->mm)
> > > >  		mmput(dev->mm);
> > > >  	dev->mm = NULL;
> > > > @@ -294,14 +295,14 @@ static int vq_access_ok(unsigned int num,
> > > >  /* Caller should have device mutex but not vq mutex */
> > > >  int vhost_log_access_ok(struct vhost_dev *dev)
> > > >  {
> > > > -	return memory_access_ok(dev, dev->memory, 1);
> > > > +	return memory_access_ok(dev, rcu_dereference_protected(dev->memory, lockdep_is_held(&dev->mutex)), 1);
> > > >  }
> > > >  
> > > >  /* Verify access for write logging. */
> > > >  /* Caller should have vq mutex and device mutex */
> > > >  static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
> > > >  {
> > > > -	return vq_memory_access_ok(log_base, vq->dev->memory,
> > > > +	return vq_memory_access_ok(log_base, rcu_dereference_protected(vq->dev->memory, lockdep_is_held(&dev->mutex)),
> > > >  			    vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
> > > >  		(!vq->log_used || log_access_ok(log_base, vq->log_addr,
> > > >  					sizeof *vq->used +
> > > > @@ -342,7 +343,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
> > > >  
> > > >  	if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL)))
> > > >  		return -EFAULT;
> > > > -	oldmem = d->memory;
> > > > +	oldmem = rcu_dereference_protected(d->memory,
> > > > +					   lockdep_is_held(&d->mutex));
> > > >  	rcu_assign_pointer(d->memory, newmem);
> > > >  	synchronize_rcu();
> > > >  	kfree(oldmem);
> > > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > > > index 44591ba..240396c 100644
> > > > --- a/drivers/vhost/vhost.h
> > > > +++ b/drivers/vhost/vhost.h
> > > > @@ -92,7 +92,7 @@ struct vhost_virtqueue {
> > > >  	 * work item execution acts instead of rcu_read_lock() and the end of
> > > >  	 * work item execution acts instead of rcu_read_lock().
> > > >  	 * Writers use virtqueue mutex. */
> > > > -	void *private_data;
> > > > +	void __rcu *private_data;
> > > >  	/* Log write descriptors */
> > > >  	void __user *log_base;
> > > >  	struct vhost_log log[VHOST_NET_MAX_SG];
> > > > @@ -102,7 +102,7 @@ struct vhost_dev {
> > > >  	/* Readers use RCU to access memory table pointer
> > > >  	 * log base pointer and features.
> > > >  	 * Writers use mutex below.*/
> > > > -	struct vhost_memory *memory;
> > > > +	struct vhost_memory __rcu *memory;
> > > >  	struct mm_struct *mm;
> > > >  	struct mutex mutex;
> > > >  	unsigned acked_features;
> > > > -- 
> > > > 1.7.0.6

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Peter Zijlstra]

On Wed, 2010-05-12 at 16:00 -0700, Paul E. McKenney wrote:
> Any thoughts?  One approach would be to create a separate lockdep class
> for vhost workqueue state, similar to the approach used in instrument
> rcu_read_lock() and friends. 

workqueue_struct::lockdep_map, its held while executing worklets.

lock_is_held(&vhost_workqueue->lockdep_map), should do as you want.

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

On Thu, May 13, 2010 at 03:07:23PM +0200, Peter Zijlstra wrote:
> On Wed, 2010-05-12 at 16:00 -0700, Paul E. McKenney wrote:
> > Any thoughts?  One approach would be to create a separate lockdep class
> > for vhost workqueue state, similar to the approach used in instrument
> > rcu_read_lock() and friends. 
> 
> workqueue_struct::lockdep_map, its held while executing worklets.
> 
> lock_is_held(&vhost_workqueue->lockdep_map), should do as you want.

Thank you, Peter!!!

							Thanx, Paul

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Michael S. Tsirkin]

On Thu, May 13, 2010 at 08:23:40AM -0700, Paul E. McKenney wrote:
> On Thu, May 13, 2010 at 03:07:23PM +0200, Peter Zijlstra wrote:
> > On Wed, 2010-05-12 at 16:00 -0700, Paul E. McKenney wrote:
> > > Any thoughts?  One approach would be to create a separate lockdep class
> > > for vhost workqueue state, similar to the approach used in instrument
> > > rcu_read_lock() and friends. 
> > 
> > workqueue_struct::lockdep_map, its held while executing worklets.
> > 
> > lock_is_held(&vhost_workqueue->lockdep_map), should do as you want.
> 
> Thank you, Peter!!!
> 
> 							Thanx, Paul

vhost in fact does flush_work rather than
flush_workqueue, so while for now everything runs
from vhost_workqueue in theory nothing would break
if we use some other workqueue or even a combination
thereof.

I guess when/if this happens, we could start by converting
to _raw and then devise a solution.

By the way what would be really nice is if we had a way
to trap when rcu protected pointer is freed without a flush
while some reader is running. Current annotation does not
allow this, does it?

-- 
MST

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

On Mon, May 17, 2010 at 11:33:49PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 13, 2010 at 08:23:40AM -0700, Paul E. McKenney wrote:
> > On Thu, May 13, 2010 at 03:07:23PM +0200, Peter Zijlstra wrote:
> > > On Wed, 2010-05-12 at 16:00 -0700, Paul E. McKenney wrote:
> > > > Any thoughts?  One approach would be to create a separate lockdep class
> > > > for vhost workqueue state, similar to the approach used in instrument
> > > > rcu_read_lock() and friends. 
> > > 
> > > workqueue_struct::lockdep_map, its held while executing worklets.
> > > 
> > > lock_is_held(&vhost_workqueue->lockdep_map), should do as you want.
> > 
> > Thank you, Peter!!!
> > 
> > 							Thanx, Paul
> 
> vhost in fact does flush_work rather than
> flush_workqueue, so while for now everything runs
> from vhost_workqueue in theory nothing would break
> if we use some other workqueue or even a combination
> thereof.
> 
> I guess when/if this happens, we could start by converting
> to _raw and then devise a solution.

If there are a small finite number of work queues involved, we can
easily do something like:

	#ifdef CONFIG_PROVE_RCU
	int in_vhost_workqueue(void)
	{
		return in_workqueue_context(vhost_workqueue) ||
		       in_workqueue_context(vhost_other_workqueue) ||
		       in_workqueue_context(yet_another_vhost_workqueue);
	}
	#endif /* #ifdef CONFIG_PROVE_RCU */

Seem reasonable?

> By the way what would be really nice is if we had a way
> to trap when rcu protected pointer is freed without a flush
> while some reader is running. Current annotation does not
> allow this, does it?

Right now, it does not, but I wonder if something like Thomas's and
Mathieu's debugobjects work could be brought to bear on this problem?
This would need to be implemented in vhost, as synchronize_rcu() has
no way to know what memory it is flushing, nor does flush_work().

						Thanx, Paul

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> On Mon, May 17, 2010 at 11:33:49PM +0300, Michael S. Tsirkin wrote:
> > On Thu, May 13, 2010 at 08:23:40AM -0700, Paul E. McKenney wrote:
> > > On Thu, May 13, 2010 at 03:07:23PM +0200, Peter Zijlstra wrote:
> > > > On Wed, 2010-05-12 at 16:00 -0700, Paul E. McKenney wrote:
> > > > > Any thoughts?  One approach would be to create a separate lockdep class
> > > > > for vhost workqueue state, similar to the approach used in instrument
> > > > > rcu_read_lock() and friends. 
> > > > 
> > > > workqueue_struct::lockdep_map, its held while executing worklets.
> > > > 
> > > > lock_is_held(&vhost_workqueue->lockdep_map), should do as you want.
> > > 
> > > Thank you, Peter!!!
> > > 
> > > 							Thanx, Paul
> > 
> > vhost in fact does flush_work rather than
> > flush_workqueue, so while for now everything runs
> > from vhost_workqueue in theory nothing would break
> > if we use some other workqueue or even a combination
> > thereof.
> > 
> > I guess when/if this happens, we could start by converting
> > to _raw and then devise a solution.
> 
> If there are a small finite number of work queues involved, we can
> easily do something like:
> 
> 	#ifdef CONFIG_PROVE_RCU
> 	int in_vhost_workqueue(void)
> 	{
> 		return in_workqueue_context(vhost_workqueue) ||
> 		       in_workqueue_context(vhost_other_workqueue) ||
> 		       in_workqueue_context(yet_another_vhost_workqueue);
> 	}
> 	#endif /* #ifdef CONFIG_PROVE_RCU */
> 
> Seem reasonable?
> 
> > By the way what would be really nice is if we had a way
> > to trap when rcu protected pointer is freed without a flush
> > while some reader is running. Current annotation does not
> > allow this, does it?
> 
> Right now, it does not, but I wonder if something like Thomas's and
> Mathieu's debugobjects work could be brought to bear on this problem?
> This would need to be implemented in vhost, as synchronize_rcu() has
> no way to know what memory it is flushing, nor does flush_work().

We can think of my recent debugobjects addition as a small state machine
that is described by the code that owns the objects. At each state
transition, the code passes the expected state as well as the next
state.

The current implementation can only keep track of a single "state" per
object at once. This should be extended to be able to count the number
RCU read side C.S. in flight that are accessing to an object.

We could use a hook in rcu_dereference (which knows about the object)
and a hook in rcu_read_unlock (which determines the end of valid object
use).

We should hook into rcu_assign_pointer() to detect RCU structure
privatization. It should put these objects in a "privatized" hash table.

We should also hook into synchronize_rcu/sched() to remove the
privatized structures from the privatized hash.

A hook in "kfree" (maybe a new rcu_free(void (fctptr*)(void *)) wrapper ?)
would call a debugobject hook that would lookup the "privatized" hash.
If it contains the object to free, we check if there are RCU read-side
C.S. in flight using this object at the same time, and show an error if
both are true.

Thoughts ?

Mathieu


> 
> 						Thanx, Paul

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

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

On Mon, May 17, 2010 at 06:00:25PM -0400, Mathieu Desnoyers wrote:
> * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > On Mon, May 17, 2010 at 11:33:49PM +0300, Michael S. Tsirkin wrote:
> > > On Thu, May 13, 2010 at 08:23:40AM -0700, Paul E. McKenney wrote:
> > > > On Thu, May 13, 2010 at 03:07:23PM +0200, Peter Zijlstra wrote:
> > > > > On Wed, 2010-05-12 at 16:00 -0700, Paul E. McKenney wrote:
> > > > > > Any thoughts?  One approach would be to create a separate lockdep class
> > > > > > for vhost workqueue state, similar to the approach used in instrument
> > > > > > rcu_read_lock() and friends. 
> > > > > 
> > > > > workqueue_struct::lockdep_map, its held while executing worklets.
> > > > > 
> > > > > lock_is_held(&vhost_workqueue->lockdep_map), should do as you want.
> > > > 
> > > > Thank you, Peter!!!
> > > > 
> > > > 							Thanx, Paul
> > > 
> > > vhost in fact does flush_work rather than
> > > flush_workqueue, so while for now everything runs
> > > from vhost_workqueue in theory nothing would break
> > > if we use some other workqueue or even a combination
> > > thereof.
> > > 
> > > I guess when/if this happens, we could start by converting
> > > to _raw and then devise a solution.
> > 
> > If there are a small finite number of work queues involved, we can
> > easily do something like:
> > 
> > 	#ifdef CONFIG_PROVE_RCU
> > 	int in_vhost_workqueue(void)
> > 	{
> > 		return in_workqueue_context(vhost_workqueue) ||
> > 		       in_workqueue_context(vhost_other_workqueue) ||
> > 		       in_workqueue_context(yet_another_vhost_workqueue);
> > 	}
> > 	#endif /* #ifdef CONFIG_PROVE_RCU */
> > 
> > Seem reasonable?
> > 
> > > By the way what would be really nice is if we had a way
> > > to trap when rcu protected pointer is freed without a flush
> > > while some reader is running. Current annotation does not
> > > allow this, does it?
> > 
> > Right now, it does not, but I wonder if something like Thomas's and
> > Mathieu's debugobjects work could be brought to bear on this problem?
> > This would need to be implemented in vhost, as synchronize_rcu() has
> > no way to know what memory it is flushing, nor does flush_work().
> 
> We can think of my recent debugobjects addition as a small state machine
> that is described by the code that owns the objects. At each state
> transition, the code passes the expected state as well as the next
> state.
> 
> The current implementation can only keep track of a single "state" per
> object at once. This should be extended to be able to count the number
> RCU read side C.S. in flight that are accessing to an object.

Not a problem, as vhost doesn't use call_rcu().  So there won't be a
conflict between different debugobjects views of the same memory.

> We could use a hook in rcu_dereference (which knows about the object)
> and a hook in rcu_read_unlock (which determines the end of valid object
> use).
> 
> We should hook into rcu_assign_pointer() to detect RCU structure
> privatization. It should put these objects in a "privatized" hash table.
> 
> We should also hook into synchronize_rcu/sched() to remove the
> privatized structures from the privatized hash.
> 
> A hook in "kfree" (maybe a new rcu_free(void (fctptr*)(void *)) wrapper ?)
> would call a debugobject hook that would lookup the "privatized" hash.
> If it contains the object to free, we check if there are RCU read-side
> C.S. in flight using this object at the same time, and show an error if
> both are true.

I believe that we can't bury this into the RCU primitives, because
rcu_read_unlock() doesn't know what objects were referenced in the
RCU read-side critical section.

But perhaps we should be simply treating this as a use-after-free
problem, so that RCU is not directly involved. Isn't that the standard
use of debugobjects anyway?

							Thanx, Paul

> Thoughts ?
> 
> Mathieu
> 
> 
> > 
> > 						Thanx, Paul
> 
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Michael S. Tsirkin]

On Mon, May 17, 2010 at 04:05:33PM -0700, Paul E. McKenney wrote:
> On Mon, May 17, 2010 at 06:00:25PM -0400, Mathieu Desnoyers wrote:
> > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > On Mon, May 17, 2010 at 11:33:49PM +0300, Michael S. Tsirkin wrote:
> > > > On Thu, May 13, 2010 at 08:23:40AM -0700, Paul E. McKenney wrote:
> > > > > On Thu, May 13, 2010 at 03:07:23PM +0200, Peter Zijlstra wrote:
> > > > > > On Wed, 2010-05-12 at 16:00 -0700, Paul E. McKenney wrote:
> > > > > > > Any thoughts?  One approach would be to create a separate lockdep class
> > > > > > > for vhost workqueue state, similar to the approach used in instrument
> > > > > > > rcu_read_lock() and friends. 
> > > > > > 
> > > > > > workqueue_struct::lockdep_map, its held while executing worklets.
> > > > > > 
> > > > > > lock_is_held(&vhost_workqueue->lockdep_map), should do as you want.
> > > > > 
> > > > > Thank you, Peter!!!
> > > > > 
> > > > > 							Thanx, Paul
> > > > 
> > > > vhost in fact does flush_work rather than
> > > > flush_workqueue, so while for now everything runs
> > > > from vhost_workqueue in theory nothing would break
> > > > if we use some other workqueue or even a combination
> > > > thereof.
> > > > 
> > > > I guess when/if this happens, we could start by converting
> > > > to _raw and then devise a solution.
> > > 
> > > If there are a small finite number of work queues involved, we can
> > > easily do something like:
> > > 
> > > 	#ifdef CONFIG_PROVE_RCU
> > > 	int in_vhost_workqueue(void)
> > > 	{
> > > 		return in_workqueue_context(vhost_workqueue) ||
> > > 		       in_workqueue_context(vhost_other_workqueue) ||
> > > 		       in_workqueue_context(yet_another_vhost_workqueue);
> > > 	}
> > > 	#endif /* #ifdef CONFIG_PROVE_RCU */
> > > 
> > > Seem reasonable?
> > > 
> > > > By the way what would be really nice is if we had a way
> > > > to trap when rcu protected pointer is freed without a flush
> > > > while some reader is running. Current annotation does not
> > > > allow this, does it?
> > > 
> > > Right now, it does not, but I wonder if something like Thomas's and
> > > Mathieu's debugobjects work could be brought to bear on this problem?
> > > This would need to be implemented in vhost, as synchronize_rcu() has
> > > no way to know what memory it is flushing, nor does flush_work().
> > 
> > We can think of my recent debugobjects addition as a small state machine
> > that is described by the code that owns the objects. At each state
> > transition, the code passes the expected state as well as the next
> > state.
> > 
> > The current implementation can only keep track of a single "state" per
> > object at once. This should be extended to be able to count the number
> > RCU read side C.S. in flight that are accessing to an object.
> 
> Not a problem, as vhost doesn't use call_rcu().  So there won't be a
> conflict between different debugobjects views of the same memory.
> 
> > We could use a hook in rcu_dereference (which knows about the object)
> > and a hook in rcu_read_unlock (which determines the end of valid object
> > use).
> > 
> > We should hook into rcu_assign_pointer() to detect RCU structure
> > privatization. It should put these objects in a "privatized" hash table.
> > 
> > We should also hook into synchronize_rcu/sched() to remove the
> > privatized structures from the privatized hash.
> > 
> > A hook in "kfree" (maybe a new rcu_free(void (fctptr*)(void *)) wrapper ?)
> > would call a debugobject hook that would lookup the "privatized" hash.
> > If it contains the object to free, we check if there are RCU read-side
> > C.S. in flight using this object at the same time, and show an error if
> > both are true.
> 
> I believe that we can't bury this into the RCU primitives, because
> rcu_read_unlock() doesn't know what objects were referenced in the
> RCU read-side critical section.
> 
> But perhaps we should be simply treating this as a use-after-free
> problem, so that RCU is not directly involved. Isn't that the standard
> use of debugobjects anyway?
> 
> 							Thanx, Paul

Well, it does not have to be freed, memory could be reused
for something else. I was just saying that it would be nice
to catch the class of errors which includes a missing sync.


> > Thoughts ?
> > 
> > Mathieu
> > 
> > 
> > > 
> > > 						Thanx, Paul
> > 
> > -- 
> > Mathieu Desnoyers
> > Operating System Efficiency R&D Consultant
> > EfficiOS Inc.
> > http://www.efficios.com

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> On Mon, May 17, 2010 at 06:00:25PM -0400, Mathieu Desnoyers wrote:
> > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > On Mon, May 17, 2010 at 11:33:49PM +0300, Michael S. Tsirkin wrote:
> > > > On Thu, May 13, 2010 at 08:23:40AM -0700, Paul E. McKenney wrote:
> > > > > On Thu, May 13, 2010 at 03:07:23PM +0200, Peter Zijlstra wrote:
> > > > > > On Wed, 2010-05-12 at 16:00 -0700, Paul E. McKenney wrote:
> > > > > > > Any thoughts?  One approach would be to create a separate lockdep class
> > > > > > > for vhost workqueue state, similar to the approach used in instrument
> > > > > > > rcu_read_lock() and friends. 
> > > > > > 
> > > > > > workqueue_struct::lockdep_map, its held while executing worklets.
> > > > > > 
> > > > > > lock_is_held(&vhost_workqueue->lockdep_map), should do as you want.
> > > > > 
> > > > > Thank you, Peter!!!
> > > > > 
> > > > > 							Thanx, Paul
> > > > 
> > > > vhost in fact does flush_work rather than
> > > > flush_workqueue, so while for now everything runs
> > > > from vhost_workqueue in theory nothing would break
> > > > if we use some other workqueue or even a combination
> > > > thereof.
> > > > 
> > > > I guess when/if this happens, we could start by converting
> > > > to _raw and then devise a solution.
> > > 
> > > If there are a small finite number of work queues involved, we can
> > > easily do something like:
> > > 
> > > 	#ifdef CONFIG_PROVE_RCU
> > > 	int in_vhost_workqueue(void)
> > > 	{
> > > 		return in_workqueue_context(vhost_workqueue) ||
> > > 		       in_workqueue_context(vhost_other_workqueue) ||
> > > 		       in_workqueue_context(yet_another_vhost_workqueue);
> > > 	}
> > > 	#endif /* #ifdef CONFIG_PROVE_RCU */
> > > 
> > > Seem reasonable?
> > > 
> > > > By the way what would be really nice is if we had a way
> > > > to trap when rcu protected pointer is freed without a flush
> > > > while some reader is running. Current annotation does not
> > > > allow this, does it?
> > > 
> > > Right now, it does not, but I wonder if something like Thomas's and
> > > Mathieu's debugobjects work could be brought to bear on this problem?
> > > This would need to be implemented in vhost, as synchronize_rcu() has
> > > no way to know what memory it is flushing, nor does flush_work().
> > 
> > We can think of my recent debugobjects addition as a small state machine
> > that is described by the code that owns the objects. At each state
> > transition, the code passes the expected state as well as the next
> > state.
> > 
> > The current implementation can only keep track of a single "state" per
> > object at once. This should be extended to be able to count the number
> > RCU read side C.S. in flight that are accessing to an object.
> 
> Not a problem, as vhost doesn't use call_rcu().  So there won't be a
> conflict between different debugobjects views of the same memory.

Not quite sure I follow you here.

> 
> > We could use a hook in rcu_dereference (which knows about the object)
> > and a hook in rcu_read_unlock (which determines the end of valid object
> > use).
> > 
> > We should hook into rcu_assign_pointer() to detect RCU structure
> > privatization. It should put these objects in a "privatized" hash table.
> > 
> > We should also hook into synchronize_rcu/sched() to remove the
> > privatized structures from the privatized hash.
> > 
> > A hook in "kfree" (maybe a new rcu_free(void (fctptr*)(void *)) wrapper ?)
> > would call a debugobject hook that would lookup the "privatized" hash.
> > If it contains the object to free, we check if there are RCU read-side
> > C.S. in flight using this object at the same time, and show an error if
> > both are true.
> 
> I believe that we can't bury this into the RCU primitives, because
> rcu_read_unlock() doesn't know what objects were referenced in the
> RCU read-side critical section.

Well, if we can find a way to match a sequence of rcu_dereference
performed from a thread with the following rcu_read_unlock(), then we
might have the information we need. But we would have to somehow tie the
debugobject context to the thread context. That sounds too complex for
what we are trying to achieve here.

> 
> But perhaps we should be simply treating this as a use-after-free
> problem, so that RCU is not directly involved. Isn't that the standard
> use of debugobjects anyway?

OK so we could tie "rcu_dereference" do debugobjects, and free would be
a standard free. Yes, I think it could be done. It looks a bit like the
memory allocation debugging code. If we know that a certain
rcu_dereference always access dynamically allocated memory, we could
probably add some checks there based on the memory allocator debug
objects.

Thanks,

Mathieu


> 
> 							Thanx, Paul
> 
> > Thoughts ?
> > 
> > Mathieu
> > 
> > 
> > > 
> > > 						Thanx, Paul
> > 
> > -- 
> > Mathieu Desnoyers
> > Operating System Efficiency R&D Consultant
> > EfficiOS Inc.
> > http://www.efficios.com

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

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

On Mon, May 17, 2010 at 07:40:25PM -0400, Mathieu Desnoyers wrote:
> * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > On Mon, May 17, 2010 at 06:00:25PM -0400, Mathieu Desnoyers wrote:
> > > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > > On Mon, May 17, 2010 at 11:33:49PM +0300, Michael S. Tsirkin wrote:
> > > > > On Thu, May 13, 2010 at 08:23:40AM -0700, Paul E. McKenney wrote:
> > > > > > On Thu, May 13, 2010 at 03:07:23PM +0200, Peter Zijlstra wrote:
> > > > > > > On Wed, 2010-05-12 at 16:00 -0700, Paul E. McKenney wrote:
> > > > > > > > Any thoughts?  One approach would be to create a separate lockdep class
> > > > > > > > for vhost workqueue state, similar to the approach used in instrument
> > > > > > > > rcu_read_lock() and friends. 
> > > > > > > 
> > > > > > > workqueue_struct::lockdep_map, its held while executing worklets.
> > > > > > > 
> > > > > > > lock_is_held(&vhost_workqueue->lockdep_map), should do as you want.
> > > > > > 
> > > > > > Thank you, Peter!!!
> > > > > > 
> > > > > > 							Thanx, Paul
> > > > > 
> > > > > vhost in fact does flush_work rather than
> > > > > flush_workqueue, so while for now everything runs
> > > > > from vhost_workqueue in theory nothing would break
> > > > > if we use some other workqueue or even a combination
> > > > > thereof.
> > > > > 
> > > > > I guess when/if this happens, we could start by converting
> > > > > to _raw and then devise a solution.
> > > > 
> > > > If there are a small finite number of work queues involved, we can
> > > > easily do something like:
> > > > 
> > > > 	#ifdef CONFIG_PROVE_RCU
> > > > 	int in_vhost_workqueue(void)
> > > > 	{
> > > > 		return in_workqueue_context(vhost_workqueue) ||
> > > > 		       in_workqueue_context(vhost_other_workqueue) ||
> > > > 		       in_workqueue_context(yet_another_vhost_workqueue);
> > > > 	}
> > > > 	#endif /* #ifdef CONFIG_PROVE_RCU */
> > > > 
> > > > Seem reasonable?
> > > > 
> > > > > By the way what would be really nice is if we had a way
> > > > > to trap when rcu protected pointer is freed without a flush
> > > > > while some reader is running. Current annotation does not
> > > > > allow this, does it?
> > > > 
> > > > Right now, it does not, but I wonder if something like Thomas's and
> > > > Mathieu's debugobjects work could be brought to bear on this problem?
> > > > This would need to be implemented in vhost, as synchronize_rcu() has
> > > > no way to know what memory it is flushing, nor does flush_work().
> > > 
> > > We can think of my recent debugobjects addition as a small state machine
> > > that is described by the code that owns the objects. At each state
> > > transition, the code passes the expected state as well as the next
> > > state.
> > > 
> > > The current implementation can only keep track of a single "state" per
> > > object at once. This should be extended to be able to count the number
> > > RCU read side C.S. in flight that are accessing to an object.
> > 
> > Not a problem, as vhost doesn't use call_rcu().  So there won't be a
> > conflict between different debugobjects views of the same memory.
> 
> Not quite sure I follow you here.

vhost uses only synchronize_rcu() and flush_work().  The existing
debugobjects tagging would therefore be unaware of the actual object,
instead tagging the rcu_head that synchronize_rcu() allocated on
the stack, and being out of the picture completely in the case of
flush_work().

Either way, RCU is completely unaware of exactly which structure is
being pushed through a grace period, so RCU's debugobjects tagging cannot
possibly conflict with any tagging that vhost does.

> > > We could use a hook in rcu_dereference (which knows about the object)
> > > and a hook in rcu_read_unlock (which determines the end of valid object
> > > use).
> > > 
> > > We should hook into rcu_assign_pointer() to detect RCU structure
> > > privatization. It should put these objects in a "privatized" hash table.
> > > 
> > > We should also hook into synchronize_rcu/sched() to remove the
> > > privatized structures from the privatized hash.
> > > 
> > > A hook in "kfree" (maybe a new rcu_free(void (fctptr*)(void *)) wrapper ?)
> > > would call a debugobject hook that would lookup the "privatized" hash.
> > > If it contains the object to free, we check if there are RCU read-side
> > > C.S. in flight using this object at the same time, and show an error if
> > > both are true.
> > 
> > I believe that we can't bury this into the RCU primitives, because
> > rcu_read_unlock() doesn't know what objects were referenced in the
> > RCU read-side critical section.
> 
> Well, if we can find a way to match a sequence of rcu_dereference
> performed from a thread with the following rcu_read_unlock(), then we
> might have the information we need. But we would have to somehow tie the
> debugobject context to the thread context. That sounds too complex for
> what we are trying to achieve here.

Indeed!  Especially given the fact that RCU read-side critical sections
can be nested.  Which rcu_dereference() calls go with which RCU read-side
critical section?

> > But perhaps we should be simply treating this as a use-after-free
> > problem, so that RCU is not directly involved. Isn't that the standard
> > use of debugobjects anyway?
> 
> OK so we could tie "rcu_dereference" do debugobjects, and free would be
> a standard free. Yes, I think it could be done. It looks a bit like the
> memory allocation debugging code. If we know that a certain
> rcu_dereference always access dynamically allocated memory, we could
> probably add some checks there based on the memory allocator debug
> objects.

We probably need vhost to add code at the end of the relevant RCU
read-side critical section checking that the pointers returned by
any rcu_dereference() calls still point to valid memory.  Don't get
me wrong, your approach could find bugs in which someone forgot to
remove the RCU-protected structure from a public list, but it could
not detect failure to wait a grace period between the time of removal
and the time of freeing.

							Thanx, Paul

> Thanks,
> 
> Mathieu
> 
> 
> > 
> > 							Thanx, Paul
> > 
> > > Thoughts ?
> > > 
> > > Mathieu
> > > 
> > > 
> > > > 
> > > > 						Thanx, Paul
> > > 
> > > -- 
> > > Mathieu Desnoyers
> > > Operating System Efficiency R&D Consultant
> > > EfficiOS Inc.
> > > http://www.efficios.com
> 
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> On Mon, May 17, 2010 at 07:40:25PM -0400, Mathieu Desnoyers wrote:
> > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > On Mon, May 17, 2010 at 06:00:25PM -0400, Mathieu Desnoyers wrote:
> > > > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > > > On Mon, May 17, 2010 at 11:33:49PM +0300, Michael S. Tsirkin wrote:
> > > > > > On Thu, May 13, 2010 at 08:23:40AM -0700, Paul E. McKenney wrote:
> > > > > > > On Thu, May 13, 2010 at 03:07:23PM +0200, Peter Zijlstra wrote:
> > > > > > > > On Wed, 2010-05-12 at 16:00 -0700, Paul E. McKenney wrote:
> > > > > > > > > Any thoughts?  One approach would be to create a separate lockdep class
> > > > > > > > > for vhost workqueue state, similar to the approach used in instrument
> > > > > > > > > rcu_read_lock() and friends. 
> > > > > > > > 
> > > > > > > > workqueue_struct::lockdep_map, its held while executing worklets.
> > > > > > > > 
> > > > > > > > lock_is_held(&vhost_workqueue->lockdep_map), should do as you want.
> > > > > > > 
> > > > > > > Thank you, Peter!!!
> > > > > > > 
> > > > > > > 							Thanx, Paul
> > > > > > 
> > > > > > vhost in fact does flush_work rather than
> > > > > > flush_workqueue, so while for now everything runs
> > > > > > from vhost_workqueue in theory nothing would break
> > > > > > if we use some other workqueue or even a combination
> > > > > > thereof.
> > > > > > 
> > > > > > I guess when/if this happens, we could start by converting
> > > > > > to _raw and then devise a solution.
> > > > > 
> > > > > If there are a small finite number of work queues involved, we can
> > > > > easily do something like:
> > > > > 
> > > > > 	#ifdef CONFIG_PROVE_RCU
> > > > > 	int in_vhost_workqueue(void)
> > > > > 	{
> > > > > 		return in_workqueue_context(vhost_workqueue) ||
> > > > > 		       in_workqueue_context(vhost_other_workqueue) ||
> > > > > 		       in_workqueue_context(yet_another_vhost_workqueue);
> > > > > 	}
> > > > > 	#endif /* #ifdef CONFIG_PROVE_RCU */
> > > > > 
> > > > > Seem reasonable?
> > > > > 
> > > > > > By the way what would be really nice is if we had a way
> > > > > > to trap when rcu protected pointer is freed without a flush
> > > > > > while some reader is running. Current annotation does not
> > > > > > allow this, does it?
> > > > > 
> > > > > Right now, it does not, but I wonder if something like Thomas's and
> > > > > Mathieu's debugobjects work could be brought to bear on this problem?
> > > > > This would need to be implemented in vhost, as synchronize_rcu() has
> > > > > no way to know what memory it is flushing, nor does flush_work().
> > > > 
> > > > We can think of my recent debugobjects addition as a small state machine
> > > > that is described by the code that owns the objects. At each state
> > > > transition, the code passes the expected state as well as the next
> > > > state.
> > > > 
> > > > The current implementation can only keep track of a single "state" per
> > > > object at once. This should be extended to be able to count the number
> > > > RCU read side C.S. in flight that are accessing to an object.
> > > 
> > > Not a problem, as vhost doesn't use call_rcu().  So there won't be a
> > > conflict between different debugobjects views of the same memory.
> > 
> > Not quite sure I follow you here.
> 
> vhost uses only synchronize_rcu() and flush_work().  The existing
> debugobjects tagging would therefore be unaware of the actual object,
> instead tagging the rcu_head that synchronize_rcu() allocated on
> the stack, and being out of the picture completely in the case of
> flush_work().
> 
> Either way, RCU is completely unaware of exactly which structure is
> being pushed through a grace period, so RCU's debugobjects tagging cannot
> possibly conflict with any tagging that vhost does.

I see.

> 
> > > > We could use a hook in rcu_dereference (which knows about the object)
> > > > and a hook in rcu_read_unlock (which determines the end of valid object
> > > > use).
> > > > 
> > > > We should hook into rcu_assign_pointer() to detect RCU structure
> > > > privatization. It should put these objects in a "privatized" hash table.
> > > > 
> > > > We should also hook into synchronize_rcu/sched() to remove the
> > > > privatized structures from the privatized hash.
> > > > 
> > > > A hook in "kfree" (maybe a new rcu_free(void (fctptr*)(void *)) wrapper ?)
> > > > would call a debugobject hook that would lookup the "privatized" hash.
> > > > If it contains the object to free, we check if there are RCU read-side
> > > > C.S. in flight using this object at the same time, and show an error if
> > > > both are true.
> > > 
> > > I believe that we can't bury this into the RCU primitives, because
> > > rcu_read_unlock() doesn't know what objects were referenced in the
> > > RCU read-side critical section.
> > 
> > Well, if we can find a way to match a sequence of rcu_dereference
> > performed from a thread with the following rcu_read_unlock(), then we
> > might have the information we need. But we would have to somehow tie the
> > debugobject context to the thread context. That sounds too complex for
> > what we are trying to achieve here.
> 
> Indeed!  Especially given the fact that RCU read-side critical sections
> can be nested.  Which rcu_dereference() calls go with which RCU read-side
> critical section?

Good point.

> 
> > > But perhaps we should be simply treating this as a use-after-free
> > > problem, so that RCU is not directly involved. Isn't that the standard
> > > use of debugobjects anyway?
> > 
> > OK so we could tie "rcu_dereference" do debugobjects, and free would be
> > a standard free. Yes, I think it could be done. It looks a bit like the
> > memory allocation debugging code. If we know that a certain
> > rcu_dereference always access dynamically allocated memory, we could
> > probably add some checks there based on the memory allocator debug
> > objects.
> 
> We probably need vhost to add code at the end of the relevant RCU
> read-side critical section checking that the pointers returned by
> any rcu_dereference() calls still point to valid memory.  Don't get
> me wrong, your approach could find bugs in which someone forgot to
> remove the RCU-protected structure from a public list, but it could
> not detect failure to wait a grace period between the time of removal
> and the time of freeing.

Good point too. So something like a new rcu_unreference() (or feel free
to find any better name) ;) that would be compiled out normally, but
would call into debugobjects might do the trick. We would have to add
these annotations to match every rcu_dereference() though, might means a
lot of new lines of code. On the plus side, that looks like a good audit
of RCU read-side use. ;)

Thanks,

Mathieu

> 
> 							Thanx, Paul
> 
> > Thanks,
> > 
> > Mathieu
> > 
> > 
> > > 
> > > 							Thanx, Paul
> > > 
> > > > Thoughts ?
> > > > 
> > > > Mathieu
> > > > 
> > > > 
> > > > > 
> > > > > 						Thanx, Paul
> > > > 
> > > > -- 
> > > > Mathieu Desnoyers
> > > > Operating System Efficiency R&D Consultant
> > > > EfficiOS Inc.
> > > > http://www.efficios.com
> > 
> > -- 
> > Mathieu Desnoyers
> > Operating System Efficiency R&D Consultant
> > EfficiOS Inc.
> > http://www.efficios.com

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

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

On Mon, May 17, 2010 at 09:35:28PM -0400, Mathieu Desnoyers wrote:
> * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > On Mon, May 17, 2010 at 07:40:25PM -0400, Mathieu Desnoyers wrote:
> > > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > > On Mon, May 17, 2010 at 06:00:25PM -0400, Mathieu Desnoyers wrote:

[ . . . ]

> > > > But perhaps we should be simply treating this as a use-after-free
> > > > problem, so that RCU is not directly involved. Isn't that the standard
> > > > use of debugobjects anyway?
> > > 
> > > OK so we could tie "rcu_dereference" do debugobjects, and free would be
> > > a standard free. Yes, I think it could be done. It looks a bit like the
> > > memory allocation debugging code. If we know that a certain
> > > rcu_dereference always access dynamically allocated memory, we could
> > > probably add some checks there based on the memory allocator debug
> > > objects.
> > 
> > We probably need vhost to add code at the end of the relevant RCU
> > read-side critical section checking that the pointers returned by
> > any rcu_dereference() calls still point to valid memory.  Don't get
> > me wrong, your approach could find bugs in which someone forgot to
> > remove the RCU-protected structure from a public list, but it could
> > not detect failure to wait a grace period between the time of removal
> > and the time of freeing.
> 
> Good point too. So something like a new rcu_unreference() (or feel free
> to find any better name) ;) that would be compiled out normally, but
> would call into debugobjects might do the trick. We would have to add
> these annotations to match every rcu_dereference() though, might means a
> lot of new lines of code. On the plus side, that looks like a good audit
> of RCU read-side use. ;)

My first thought is that we have added quite a bit of RCU consistency
check code in the past few months, so we should see what bugs they find
and what bugs escape.  It is all too easy to create consistency check
code that is more trouble than it is worth.

But in the meantime, let's see what would be required to check for
failures to insert grace-period delays:

o	There would need to be something like rcu_unreference(),
	rcu_no_more_readers() or some such after the grace period.
	The update side would then become something like the following:

		oldp = rcu_dereference_protected(gp, &mylock);
		rcu_assign_pointer(gp, newp);
		synchronize_rcu();
		rcu_no_more_readers(oldp);
		kfree(oldp);

o	There would need to be something to check all of the pointers
	traversed in the read-side critical sections:

		rcu_read_lock();
		...
		p1 = rcu_dereference(gp1->field1);
		...
		p2 = rcu_dereference(gp2->field2);
		...

		rcu_validate(p1);
		rcu_validate(p2);
		rcu_read_unlock();

One thing that bothers me about this is that we are forcing the developer
to do a lot of extra typing.  For example, rcu_no_more_readers() is in
a truth-and-beauty sense redundant with kfree() -- why type both?  The
same could be said about rcu_validate() and rcu_read_unlock(), but nested
RCU read-side critical sections make this difficult.

Or am I misunderstanding what you are suggesting?

							Thanx, Paul

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Michael S. Tsirkin]

On Tue, May 18, 2010 at 07:20:08AM -0700, Paul E. McKenney wrote:
> On Mon, May 17, 2010 at 09:35:28PM -0400, Mathieu Desnoyers wrote:
> > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > On Mon, May 17, 2010 at 07:40:25PM -0400, Mathieu Desnoyers wrote:
> > > > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > > > On Mon, May 17, 2010 at 06:00:25PM -0400, Mathieu Desnoyers wrote:
> 
> [ . . . ]
> 
> > > > > But perhaps we should be simply treating this as a use-after-free
> > > > > problem, so that RCU is not directly involved. Isn't that the standard
> > > > > use of debugobjects anyway?
> > > > 
> > > > OK so we could tie "rcu_dereference" do debugobjects, and free would be
> > > > a standard free. Yes, I think it could be done. It looks a bit like the
> > > > memory allocation debugging code. If we know that a certain
> > > > rcu_dereference always access dynamically allocated memory, we could
> > > > probably add some checks there based on the memory allocator debug
> > > > objects.
> > > 
> > > We probably need vhost to add code at the end of the relevant RCU
> > > read-side critical section checking that the pointers returned by
> > > any rcu_dereference() calls still point to valid memory.  Don't get
> > > me wrong, your approach could find bugs in which someone forgot to
> > > remove the RCU-protected structure from a public list, but it could
> > > not detect failure to wait a grace period between the time of removal
> > > and the time of freeing.
> > 
> > Good point too. So something like a new rcu_unreference() (or feel free
> > to find any better name) ;) that would be compiled out normally, but
> > would call into debugobjects might do the trick. We would have to add
> > these annotations to match every rcu_dereference() though, might means a
> > lot of new lines of code. On the plus side, that looks like a good audit
> > of RCU read-side use. ;)
> 
> My first thought is that we have added quite a bit of RCU consistency
> check code in the past few months, so we should see what bugs they find
> and what bugs escape.  It is all too easy to create consistency check
> code that is more trouble than it is worth.

Right. Do the patches that started this discussion catch anything BTW?

> But in the meantime, let's see what would be required to check for
> failures to insert grace-period delays:
> 
> o	There would need to be something like rcu_unreference(),
> 	rcu_no_more_readers() or some such after the grace period.
> 	The update side would then become something like the following:
> 
> 		oldp = rcu_dereference_protected(gp, &mylock);
> 		rcu_assign_pointer(gp, newp);
> 		synchronize_rcu();
> 		rcu_no_more_readers(oldp);
> 		kfree(oldp);
> 
> o	There would need to be something to check all of the pointers
> 	traversed in the read-side critical sections:
> 
> 		rcu_read_lock();
> 		...
> 		p1 = rcu_dereference(gp1->field1);
> 		...
> 		p2 = rcu_dereference(gp2->field2);
> 		...
> 
> 		rcu_validate(p1);
> 		rcu_validate(p2);
> 		rcu_read_unlock();
> 

what does rcu_validate do?

> One thing that bothers me about this is that we are forcing the developer
> to do a lot of extra typing.  For example, rcu_no_more_readers() is in
> a truth-and-beauty sense redundant with kfree() -- why type both?

With kfree, yes. We could stick rcu_no_more_readers in kfree I guess?

> The
> same could be said about rcu_validate() and rcu_read_unlock(), but nested
> RCU read-side critical sections make this difficult.
> Or am I misunderstanding what you are suggesting?
> 
> 							Thanx, Paul

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

On Tue, May 18, 2010 at 05:25:57PM +0300, Michael S. Tsirkin wrote:
> On Tue, May 18, 2010 at 07:20:08AM -0700, Paul E. McKenney wrote:
> > On Mon, May 17, 2010 at 09:35:28PM -0400, Mathieu Desnoyers wrote:
> > > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > > On Mon, May 17, 2010 at 07:40:25PM -0400, Mathieu Desnoyers wrote:
> > > > > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > > > > On Mon, May 17, 2010 at 06:00:25PM -0400, Mathieu Desnoyers wrote:
> > 
> > [ . . . ]
> > 
> > > > > > But perhaps we should be simply treating this as a use-after-free
> > > > > > problem, so that RCU is not directly involved. Isn't that the standard
> > > > > > use of debugobjects anyway?
> > > > > 
> > > > > OK so we could tie "rcu_dereference" do debugobjects, and free would be
> > > > > a standard free. Yes, I think it could be done. It looks a bit like the
> > > > > memory allocation debugging code. If we know that a certain
> > > > > rcu_dereference always access dynamically allocated memory, we could
> > > > > probably add some checks there based on the memory allocator debug
> > > > > objects.
> > > > 
> > > > We probably need vhost to add code at the end of the relevant RCU
> > > > read-side critical section checking that the pointers returned by
> > > > any rcu_dereference() calls still point to valid memory.  Don't get
> > > > me wrong, your approach could find bugs in which someone forgot to
> > > > remove the RCU-protected structure from a public list, but it could
> > > > not detect failure to wait a grace period between the time of removal
> > > > and the time of freeing.
> > > 
> > > Good point too. So something like a new rcu_unreference() (or feel free
> > > to find any better name) ;) that would be compiled out normally, but
> > > would call into debugobjects might do the trick. We would have to add
> > > these annotations to match every rcu_dereference() though, might means a
> > > lot of new lines of code. On the plus side, that looks like a good audit
> > > of RCU read-side use. ;)
> > 
> > My first thought is that we have added quite a bit of RCU consistency
> > check code in the past few months, so we should see what bugs they find
> > and what bugs escape.  It is all too easy to create consistency check
> > code that is more trouble than it is worth.
> 
> Right. Do the patches that started this discussion catch anything BTW?

All three approaches have found some bugs.

> > But in the meantime, let's see what would be required to check for
> > failures to insert grace-period delays:
> > 
> > o	There would need to be something like rcu_unreference(),
> > 	rcu_no_more_readers() or some such after the grace period.
> > 	The update side would then become something like the following:
> > 
> > 		oldp = rcu_dereference_protected(gp, &mylock);
> > 		rcu_assign_pointer(gp, newp);
> > 		synchronize_rcu();
> > 		rcu_no_more_readers(oldp);
> > 		kfree(oldp);
> > 
> > o	There would need to be something to check all of the pointers
> > 	traversed in the read-side critical sections:
> > 
> > 		rcu_read_lock();
> > 		...
> > 		p1 = rcu_dereference(gp1->field1);
> > 		...
> > 		p2 = rcu_dereference(gp2->field2);
> > 		...
> > 
> > 		rcu_validate(p1);
> > 		rcu_validate(p2);
> > 		rcu_read_unlock();
> > 
> 
> what does rcu_validate do?

It checks to make sure that the pointer still points to something valid.

> > One thing that bothers me about this is that we are forcing the developer
> > to do a lot of extra typing.  For example, rcu_no_more_readers() is in
> > a truth-and-beauty sense redundant with kfree() -- why type both?
> 
> With kfree, yes. We could stick rcu_no_more_readers in kfree I guess?

But why not just use the existing debugobjects?  You can just use
something like this:

	debug_check_no_obj_freed(p1, sizeof(*p1));

in place of:

	rcu_validate(p1);

Of course, if you are using your own custom allocator, you will need
to put the allocation/free checks in, same as slab and the others
currently do.

							Thanx, Paul

> > The
> > same could be said about rcu_validate() and rcu_read_unlock(), but nested
> > RCU read-side critical sections make this difficult.
> > Or am I misunderstanding what you are suggesting?
> > 
> > 							Thanx, Paul

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> On Mon, May 17, 2010 at 09:35:28PM -0400, Mathieu Desnoyers wrote:
> > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > On Mon, May 17, 2010 at 07:40:25PM -0400, Mathieu Desnoyers wrote:
> > > > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > > > On Mon, May 17, 2010 at 06:00:25PM -0400, Mathieu Desnoyers wrote:
> 
> [ . . . ]
> 
> > > > > But perhaps we should be simply treating this as a use-after-free
> > > > > problem, so that RCU is not directly involved. Isn't that the standard
> > > > > use of debugobjects anyway?
> > > > 
> > > > OK so we could tie "rcu_dereference" do debugobjects, and free would be
> > > > a standard free. Yes, I think it could be done. It looks a bit like the
> > > > memory allocation debugging code. If we know that a certain
> > > > rcu_dereference always access dynamically allocated memory, we could
> > > > probably add some checks there based on the memory allocator debug
> > > > objects.
> > > 
> > > We probably need vhost to add code at the end of the relevant RCU
> > > read-side critical section checking that the pointers returned by
> > > any rcu_dereference() calls still point to valid memory.  Don't get
> > > me wrong, your approach could find bugs in which someone forgot to
> > > remove the RCU-protected structure from a public list, but it could
> > > not detect failure to wait a grace period between the time of removal
> > > and the time of freeing.
> > 
> > Good point too. So something like a new rcu_unreference() (or feel free
> > to find any better name) ;) that would be compiled out normally, but
> > would call into debugobjects might do the trick. We would have to add
> > these annotations to match every rcu_dereference() though, might means a
> > lot of new lines of code. On the plus side, that looks like a good audit
> > of RCU read-side use. ;)
> 
> My first thought is that we have added quite a bit of RCU consistency
> check code in the past few months, so we should see what bugs they find
> and what bugs escape.  It is all too easy to create consistency check
> code that is more trouble than it is worth.

Yes, although I expect that this new checking scheme will take some time
to implement and mainline anyway (implementation effort which I might
leave to someone else, as I have to focus on tracing at the moment).

> But in the meantime, let's see what would be required to check for
> failures to insert grace-period delays:
> 
> o	There would need to be something like rcu_unreference(),
> 	rcu_no_more_readers() or some such after the grace period.
> 	The update side would then become something like the following:
> 
> 		oldp = rcu_dereference_protected(gp, &mylock);
> 		rcu_assign_pointer(gp, newp);
> 		synchronize_rcu();
> 		rcu_no_more_readers(oldp);
> 		kfree(oldp);

Replacing a kfree with a rcu_free(kfree, oldp) call that would include
both could lessen the amount of typing:

#define rcu_free(freefct, ptr) \
do { \
  rcu_no_more_readers(ptr); \
  freefct(ptr); \
} while (0)


> o	There would need to be something to check all of the pointers
> 	traversed in the read-side critical sections:
> 
> 		rcu_read_lock();
> 		...
> 		p1 = rcu_dereference(gp1->field1);
> 		...
> 		p2 = rcu_dereference(gp2->field2);
> 		...
> 
> 		rcu_validate(p1);
> 		rcu_validate(p2);

Hrm, isn't the goal of this "rcu_validate(p1)" just to keep track of
"p1" liveness ? Or do you plan to add a check there also ? I'm not sure
I figure out what you are planning to validate here. I was thinking more
in terms of

                rcu_unreference(p1);
                rcu_unreference(p1);

that would be symmetric with the rcu_dereference.

> 		rcu_read_unlock();
> 
> One thing that bothers me about this is that we are forcing the developer
> to do a lot of extra typing.  For example, rcu_no_more_readers() is in
> a truth-and-beauty sense redundant with kfree() -- why type both?  The
> same could be said about rcu_validate() and rcu_read_unlock(), but nested
> RCU read-side critical sections make this difficult.

Ideally we'd like to add near-zero burden on developers, but I fear this
cannot be done easily for read-side C.S.. As for write-side, we have to
choose between tradeoff of genericity and less typing, e.g., between:

  rcu_free(kfree, ptr);
and
  rcu_kfree(ptr)

for the second, we would have to create a whole family of rcu_*free().

> 
> Or am I misunderstanding what you are suggesting?

I'm only unsure about the "validate" part.

Thanks,

Mathieu

> 
> 							Thanx, Paul

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

Re: [PATCH RFC tip/core/rcu 23/23] vhost: add __rcu annotations

[Paul E. McKenney]

On Tue, May 18, 2010 at 10:47:26AM -0400, Mathieu Desnoyers wrote:
> * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > On Mon, May 17, 2010 at 09:35:28PM -0400, Mathieu Desnoyers wrote:
> > > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > > On Mon, May 17, 2010 at 07:40:25PM -0400, Mathieu Desnoyers wrote:
> > > > > * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > > > > > On Mon, May 17, 2010 at 06:00:25PM -0400, Mathieu Desnoyers wrote:
> > 
> > [ . . . ]
> > 
> > > > > > But perhaps we should be simply treating this as a use-after-free
> > > > > > problem, so that RCU is not directly involved. Isn't that the standard
> > > > > > use of debugobjects anyway?
> > > > > 
> > > > > OK so we could tie "rcu_dereference" do debugobjects, and free would be
> > > > > a standard free. Yes, I think it could be done. It looks a bit like the
> > > > > memory allocation debugging code. If we know that a certain
> > > > > rcu_dereference always access dynamically allocated memory, we could
> > > > > probably add some checks there based on the memory allocator debug
> > > > > objects.
> > > > 
> > > > We probably need vhost to add code at the end of the relevant RCU
> > > > read-side critical section checking that the pointers returned by
> > > > any rcu_dereference() calls still point to valid memory.  Don't get
> > > > me wrong, your approach could find bugs in which someone forgot to
> > > > remove the RCU-protected structure from a public list, but it could
> > > > not detect failure to wait a grace period between the time of removal
> > > > and the time of freeing.
> > > 
> > > Good point too. So something like a new rcu_unreference() (or feel free
> > > to find any better name) ;) that would be compiled out normally, but
> > > would call into debugobjects might do the trick. We would have to add
> > > these annotations to match every rcu_dereference() though, might means a
> > > lot of new lines of code. On the plus side, that looks like a good audit
> > > of RCU read-side use. ;)
> > 
> > My first thought is that we have added quite a bit of RCU consistency
> > check code in the past few months, so we should see what bugs they find
> > and what bugs escape.  It is all too easy to create consistency check
> > code that is more trouble than it is worth.
> 
> Yes, although I expect that this new checking scheme will take some time
> to implement and mainline anyway (implementation effort which I might
> leave to someone else, as I have to focus on tracing at the moment).
> 
> > But in the meantime, let's see what would be required to check for
> > failures to insert grace-period delays:
> > 
> > o	There would need to be something like rcu_unreference(),
> > 	rcu_no_more_readers() or some such after the grace period.
> > 	The update side would then become something like the following:
> > 
> > 		oldp = rcu_dereference_protected(gp, &mylock);
> > 		rcu_assign_pointer(gp, newp);
> > 		synchronize_rcu();
> > 		rcu_no_more_readers(oldp);
> > 		kfree(oldp);
> 
> Replacing a kfree with a rcu_free(kfree, oldp) call that would include
> both could lessen the amount of typing:
> 
> #define rcu_free(freefct, ptr) \
> do { \
>   rcu_no_more_readers(ptr); \
>   freefct(ptr); \
> } while (0)

Or we could just rely on the existing debugobjects support that is
already in kfree().  ;-)

> > o	There would need to be something to check all of the pointers
> > 	traversed in the read-side critical sections:
> > 
> > 		rcu_read_lock();
> > 		...
> > 		p1 = rcu_dereference(gp1->field1);
> > 		...
> > 		p2 = rcu_dereference(gp2->field2);
> > 		...
> > 
> > 		rcu_validate(p1);
> > 		rcu_validate(p2);
> 
> Hrm, isn't the goal of this "rcu_validate(p1)" just to keep track of
> "p1" liveness ? Or do you plan to add a check there also ? I'm not sure
> I figure out what you are planning to validate here. I was thinking more
> in terms of
> 
>                 rcu_unreference(p1);
>                 rcu_unreference(p1);
> 
> that would be symmetric with the rcu_dereference.

My preference would be for people to just use the existing debugobjects
API, debug_check_no_obj_freed().  That is already in place, no need to
create RCU wrappers for it.

> > 		rcu_read_unlock();
> > 
> > One thing that bothers me about this is that we are forcing the developer
> > to do a lot of extra typing.  For example, rcu_no_more_readers() is in
> > a truth-and-beauty sense redundant with kfree() -- why type both?  The
> > same could be said about rcu_validate() and rcu_read_unlock(), but nested
> > RCU read-side critical sections make this difficult.
> 
> Ideally we'd like to add near-zero burden on developers, but I fear this
> cannot be done easily for read-side C.S.. As for write-side, we have to
> choose between tradeoff of genericity and less typing, e.g., between:
> 
>   rcu_free(kfree, ptr);
> and
>   rcu_kfree(ptr)
> 
> for the second, we would have to create a whole family of rcu_*free().
> 
> > 
> > Or am I misunderstanding what you are suggesting?
> 
> I'm only unsure about the "validate" part.

Again, we should just rely on the existing debugobjects function, letting
developers use it as they see fit.

							Thanx, Paul

> Thanks,
> 
> Mathieu
> 
> > 
> > 							Thanx, Paul
> 
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com