Re: [PATCH] fib_trie: rcu_assign_pointer warning fix

Previous thread: Announce: Linux-next (Or Andrew's dream :-)) by Stephen Rothwell on Monday, February 11, 2008 - 9:02 pm. (215 messages)

Next thread: [PATCH] fib_trie: rcu_assign_pointer warning fix by Stephen Hemminger on Monday, February 11, 2008 - 9:18 pm. (1 message)
To: <shemminger@...>
Cc: <paulmck@...>, <netdev@...>, <linux-kernel@...>
Date: Monday, February 11, 2008 - 9:16 pm

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 11 Feb 2008 16:59:54 -0800

linux-kernel added to CC:, any change to generic kernel infrastructure
--

To: David Miller <davem@...>
Cc: <shemminger@...>, <paulmck@...>, <netdev@...>, <linux-kernel@...>
Date: Tuesday, February 12, 2008 - 4:57 am

...But, "If value is the NULL (constant 0)" we have:

Regards,
Jarek P.
--

To: Jarek Poplawski <jarkao2@...>
Cc: David Miller <davem@...>, <shemminger@...>, <netdev@...>, <linux-kernel@...>
Date: Tuesday, February 12, 2008 - 12:07 pm

"All programmers are blind, especially me."

You are right, Jarek. I ran this through gcc, and the following
comes close:

#define rcu_assign_pointer(p, v) \
({ \
if (!__builtin_constant_p(v) || (v)) \
smp_wmb(); \
(p) = (v); \
})

But I am concerned about the following case:

rcu_assign_pointer(global_index, 0);

. . .

x = global_array[rcu_dereference(global_index)];

Since arrays have a zero-th element, we would really want a memory
barrier in this case.

So how about leaving the index-unfriendly version of rcu_assign_pointer()
and adding an rcu_assign_index() as follows?

Thanx, Paul

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---

rcupdate.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff -urpNa -X dontdiff linux-2.6.24/include/linux/rcupdate.h linux-2.6.24-rai/include/linux/rcupdate.h
--- linux-2.6.24/include/linux/rcupdate.h 2008-01-24 14:58:37.000000000 -0800
+++ linux-2.6.24-rai/include/linux/rcupdate.h 2008-02-12 08:04:59.000000000 -0800
@@ -278,6 +278,24 @@ extern struct lockdep_map rcu_lock_map;
})

/**
+ * rcu_assign_index - assign (publicize) a index of a newly
+ * initialized array elementg that will be dereferenced by RCU
+ * read-side critical sections. Returns the value assigned.
+ *
+ * Inserts memory barriers on architectures that require them
+ * (pretty much all of them other than x86), and also prevents
+ * the compiler from reordering the code that initializes the
+ * structure after the index assignment. More importantly, this
+ * call documents which indexes will be dereferenced by RCU read-side
+ * code.
+ */
+
+#define rcu_assign_index(p, v) ({ \
+ smp_wmb(); \
+ (p) = (v); \
+ })
+
+/**
* synchronize_sched - block until all CPUs have exited any non-preemptive
* kernel code sequences.
*
--

To: Paul E. McKenney <paulmck@...>
Cc: David Miller <davem@...>, <shemminger@...>, <netdev@...>, <linux-kernel@...>
Date: Tuesday, February 12, 2008 - 3:32 pm

On Tue, Feb 12, 2008 at 08:07:29AM -0800, Paul E. McKenney wrote:

Hmm... I got it my way: you - superheroes - sometimes seem to be just
like us - common people... (Probably early in the morning, before

It seems the above version of this macro uses the barrier for 0, but
if I miss something, or for these other: documenting reasons, then of

---------------------------^
+ * initialized array element that will be dereferenced by RCU

Regards,
Jarek P.
--

To: Paul E. McKenney <paulmck@...>
Cc: David Miller <davem@...>, <shemminger@...>, <netdev@...>, <linux-kernel@...>
Date: Tuesday, February 12, 2008 - 3:46 pm

On Tue, Feb 12, 2008 at 08:32:18PM +0100, Jarek Poplawski wrote:

Jarek P.
--

To: Jarek Poplawski <jarkao2@...>
Cc: David Miller <davem@...>, <shemminger@...>, <netdev@...>, <linux-kernel@...>
Date: Wednesday, February 13, 2008 - 6:55 pm

Yep. For example:

elem[0].next = 1;
rcu_assign_index(global_index, 0);

--

Previous thread: Announce: Linux-next (Or Andrew's dream :-)) by Stephen Rothwell on Monday, February 11, 2008 - 9:02 pm. (215 messages)

Next thread: [PATCH] fib_trie: rcu_assign_pointer warning fix by Stephen Hemminger on Monday, February 11, 2008 - 9:18 pm. (1 message)