[PATCH tip/core/rcu 0/4] Array-based RCU removals for 4.2

[PATCH tip/core/rcu 0/4] Array-based RCU removals for 4.2

[Paul E. McKenney]

Hello!

This series removes the sole use of RCU-protected array indexes.
It turns out that this use is in x86 code, so smp_load_acquire() works
just as well, and this allows removing a troublesome aspect of the
RCU API.  This removal should in turn allow compilers to better grok RCU
dependency chains.  For example, expressions like (x-x), (x*0), and (x%1)
can result in valid array indexes, but cannot produce valid pointers.

1.	Make x86's MCE code use smp_load_acuire() and READ_ONCE() in
	place of the RCU API members intended to handle RCU-protected
	array indexes.

2.	Eliminates the RCU API members supporting RCU-protected array
	indexes, namely rcu_access_index() and rcu_dereference_index_check().

3.	Updates the RCU documentation to reflect #2 above.

4.	Makes one of the modified functions be static, courtesy of
	Fengguang Wu.

							Thanx, Paul

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

 b/Documentation/RCU/arrayRCU.txt        |   20 ++++++++++--
 b/Documentation/RCU/lockdep.txt         |   10 ------
 b/Documentation/RCU/rcu_dereference.txt |   33 +++++++--------------
 b/Documentation/RCU/whatisRCU.txt       |    2 -
 b/arch/x86/kernel/cpu/mcheck/mce.c      |   15 +++++----
 b/include/linux/rcupdate.h              |   50 --------------------------------
 6 files changed, 37 insertions(+), 93 deletions(-)

[PATCH tip/core/rcu 1/4] mce: Stop using array-index-based RCU primitives

[Paul E. McKenney]

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

Because mce is arch-specific x86 code, there is little or no
performance benefit of using rcu_dereference_index_check() over using
smp_load_acquire().  It also turns out that mce is the only place that
array-index-based RCU is used, and it would be convenient to drop
this portion of the RCU API.

This patch therefore changes rcu_dereference_index_check() uses to
smp_load_acquire(), but keeping the lockdep diagnostics, and also
changes rcu_access_index() uses to READ_ONCE().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: linux-edac@vger.kernel.org
Cc: Tony Luck <tony.luck@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
---
 arch/x86/kernel/cpu/mcheck/mce.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index e535533d5ab8..1fd04a02d90a 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -53,9 +53,12 @@
 static DEFINE_MUTEX(mce_chrdev_read_mutex);
 
 #define rcu_dereference_check_mce(p) \
-	rcu_dereference_index_check((p), \
-			      rcu_read_lock_sched_held() || \
-			      lockdep_is_held(&mce_chrdev_read_mutex))
+({ \
+	rcu_lockdep_assert(rcu_read_lock_sched_held() || \
+			   lockdep_is_held(&mce_chrdev_read_mutex), \
+			   "suspicious rcu_dereference_check_mce() usage"); \
+	smp_load_acquire(&(p)); \
+})
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/mce.h>
@@ -1884,7 +1887,7 @@ out:
 static unsigned int mce_chrdev_poll(struct file *file, poll_table *wait)
 {
 	poll_wait(file, &mce_chrdev_wait, wait);
-	if (rcu_access_index(mcelog.next))
+	if (READ_ONCE(mcelog.next))
 		return POLLIN | POLLRDNORM;
 	if (!mce_apei_read_done && apei_check_mce())
 		return POLLIN | POLLRDNORM;
-- 
1.8.1.5

[PATCH tip/core/rcu 2/4] rcu: Eliminate array-index-based RCU primitives

[Paul E. McKenney]

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

Now that rcu_access_index() and rcu_dereference_index_check() are no
longer used, the commit removes them from the RCU API.  This means that
RCU's data dependencies now involve only pointers, give or take the
occasional cast to and then back from an integer type to do pointer
arithmetic.  This in turn eliminates the need for a number of operations
on values carrying RCU data dependencies.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: linux-edac@vger.kernel.org
Cc: Tony Luck <tony.luck@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
---
 include/linux/rcupdate.h | 50 ------------------------------------------------
 1 file changed, 50 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 87bb0eee665b..b97842ff71d2 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -628,21 +628,6 @@ static inline void rcu_preempt_sleep_check(void)
 	((typeof(*p) __force __kernel *)(p)); \
 })
 
-#define __rcu_access_index(p, space) \
-({ \
-	typeof(p) _________p1 = READ_ONCE(p); \
-	rcu_dereference_sparse(p, space); \
-	(_________p1); \
-})
-#define __rcu_dereference_index_check(p, c) \
-({ \
-	/* Dependency order vs. p above. */ \
-	typeof(p) _________p1 = lockless_dereference(p); \
-	rcu_lockdep_assert(c, \
-			   "suspicious rcu_dereference_index_check() usage"); \
-	(_________p1); \
-})
-
 /**
  * RCU_INITIALIZER() - statically initialize an RCU-protected global variable
  * @v: The value to statically initialize with.
@@ -787,41 +772,6 @@ static inline void rcu_preempt_sleep_check(void)
 #define rcu_dereference_raw_notrace(p) __rcu_dereference_check((p), 1, __rcu)
 
 /**
- * rcu_access_index() - fetch RCU index with no dereferencing
- * @p: The index to read
- *
- * Return the value of the specified RCU-protected index, but omit the
- * smp_read_barrier_depends() and keep the READ_ONCE().  This is useful
- * when the value of this index is accessed, but the index is not
- * dereferenced, for example, when testing an RCU-protected index against
- * -1.  Although rcu_access_index() may also be used in cases where
- * update-side locks prevent the value of the index from changing, you
- * should instead use rcu_dereference_index_protected() for this use case.
- */
-#define rcu_access_index(p) __rcu_access_index((p), __rcu)
-
-/**
- * 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
-- 
1.8.1.5

[PATCH tip/core/rcu 3/4] documentation: RCU-protected array indexes no longer supported

[Paul E. McKenney]

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

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 Documentation/RCU/arrayRCU.txt        | 20 ++++++++++++++++----
 Documentation/RCU/lockdep.txt         | 10 ----------
 Documentation/RCU/rcu_dereference.txt | 33 ++++++++++++---------------------
 Documentation/RCU/whatisRCU.txt       |  2 --
 4 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/Documentation/RCU/arrayRCU.txt b/Documentation/RCU/arrayRCU.txt
index 453ebe6953ee..f05a9afb2c39 100644
--- a/Documentation/RCU/arrayRCU.txt
+++ b/Documentation/RCU/arrayRCU.txt
@@ -10,7 +10,19 @@ also be used to protect arrays.  Three situations are as follows:
 
 3.  Resizeable Arrays
 
-Each of these situations are discussed below.
+Each of these three situations involves an RCU-protected pointer to an
+array that is separately indexed.  It might be tempting to consider use
+of RCU to instead protect the index into an array, however, this use
+case is -not- supported.  The problem with RCU-protected indexes into
+arrays is that compilers can play way too many optimization games with
+integers, which means that the rules governing handling of these indexes
+are far more trouble than they are worth.  If RCU-protected indexes into
+arrays prove to be particularly valuable (which they have not thus far),
+explicit cooperation from the compiler will be required to permit them
+to be safely used.
+
+That aside, each of the three RCU-protected pointer situations are
+described in the following sections.
 
 
 Situation 1: Hash Tables
@@ -36,9 +48,9 @@ Quick Quiz:  Why is it so important that updates be rare when
 Situation 3: Resizeable Arrays
 
 Use of RCU for resizeable arrays is demonstrated by the grow_ary()
-function used by the System V IPC code.  The array is used to map from
-semaphore, message-queue, and shared-memory IDs to the data structure
-that represents the corresponding IPC construct.  The grow_ary()
+function formerly used by the System V IPC code.  The array is used
+to map from semaphore, message-queue, and shared-memory IDs to the data
+structure that represents the corresponding IPC construct.  The grow_ary()
 function does not acquire any locks; instead its caller must hold the
 ids->sem semaphore.
 
diff --git a/Documentation/RCU/lockdep.txt b/Documentation/RCU/lockdep.txt
index cd83d2348fef..da51d3068850 100644
--- a/Documentation/RCU/lockdep.txt
+++ b/Documentation/RCU/lockdep.txt
@@ -47,11 +47,6 @@ checking of rcu_dereference() primitives:
 		Use explicit check expression "c" along with
 		srcu_read_lock_held()().  This is useful in code that
 		is invoked by both SRCU readers and updaters.
-	rcu_dereference_index_check(p, c):
-		Use explicit check expression "c", but the caller
-		must supply one of the rcu_read_lock_held() functions.
-		This is useful in code that uses RCU-protected arrays
-		that is invoked by both RCU readers and updaters.
 	rcu_dereference_raw(p):
 		Don't check.  (Use sparingly, if at all.)
 	rcu_dereference_protected(p, c):
@@ -64,11 +59,6 @@ checking of rcu_dereference() primitives:
 		but retain the compiler constraints that prevent duplicating
 		or coalescsing.  This is useful when when testing the
 		value of the pointer itself, for example, against NULL.
-	rcu_access_index(idx):
-		Return the value of the index and omit all barriers, but
-		retain the compiler constraints that prevent duplicating
-		or coalescsing.  This is useful when when testing the
-		value of the index itself, for example, against -1.
 
 The rcu_dereference_check() check expression can be any boolean
 expression, but would normally include a lockdep expression.  However,
diff --git a/Documentation/RCU/rcu_dereference.txt b/Documentation/RCU/rcu_dereference.txt
index ceb05da5a5ac..66864d2a7f60 100644
--- a/Documentation/RCU/rcu_dereference.txt
+++ b/Documentation/RCU/rcu_dereference.txt
@@ -25,17 +25,6 @@ o	You must use one of the rcu_dereference() family of primitives
 	for an example where the compiler can in fact deduce the exact
 	value of the pointer, and thus cause misordering.
 
-o	Do not use single-element RCU-protected arrays.  The compiler
-	is within its right to assume that the value of an index into
-	such an array must necessarily evaluate to zero.  The compiler
-	could then substitute the constant zero for the computation, so
-	that the array index no longer depended on the value returned
-	by rcu_dereference().  If the array index no longer depends
-	on rcu_dereference(), then both the compiler and the CPU
-	are within their rights to order the array access before the
-	rcu_dereference(), which can cause the array access to return
-	garbage.
-
 o	Avoid cancellation when using the "+" and "-" infix arithmetic
 	operators.  For example, for a given variable "x", avoid
 	"(x-x)".  There are similar arithmetic pitfalls from other
@@ -76,14 +65,15 @@ o	Do not use the results from the boolean "&&" and "||" when
 	dereferencing.	For example, the following (rather improbable)
 	code is buggy:
 
-		int a[2];
-		int index;
-		int force_zero_index = 1;
+		int *p;
+		int *q;
 
 		...
 
-		r1 = rcu_dereference(i1)
-		r2 = a[r1 && force_zero_index];  /* BUGGY!!! */
+		p = rcu_dereference(gp)
+		q = &global_q;
+		q += p != &oom_p1 && p != &oom_p2;
+		r1 = *q;  /* BUGGY!!! */
 
 	The reason this is buggy is that "&&" and "||" are often compiled
 	using branches.  While weak-memory machines such as ARM or PowerPC
@@ -94,14 +84,15 @@ o	Do not use the results from relational operators ("==", "!=",
 	">", ">=", "<", or "<=") when dereferencing.  For example,
 	the following (quite strange) code is buggy:
 
-		int a[2];
-		int index;
-		int flip_index = 0;
+		int *p;
+		int *q;
 
 		...
 
-		r1 = rcu_dereference(i1)
-		r2 = a[r1 != flip_index];  /* BUGGY!!! */
+		p = rcu_dereference(gp)
+		q = &global_q;
+		q += p > &oom_p;
+		r1 = *q;  /* BUGGY!!! */
 
 	As before, the reason this is buggy is that relational operators
 	are often compiled using branches.  And as before, although
diff --git a/Documentation/RCU/whatisRCU.txt b/Documentation/RCU/whatisRCU.txt
index 88dfce182f66..b201d4cd77f9 100644
--- a/Documentation/RCU/whatisRCU.txt
+++ b/Documentation/RCU/whatisRCU.txt
@@ -879,9 +879,7 @@ SRCU:	Initialization/cleanup
 
 All:  lockdep-checked RCU-protected pointer access
 
-	rcu_access_index
 	rcu_access_pointer
-	rcu_dereference_index_check
 	rcu_dereference_raw
 	rcu_lockdep_assert
 	rcu_sleep_check
-- 
1.8.1.5

[PATCH tip/core/rcu 4/4] mce: mce_chrdev_write() can be static

[Paul E. McKenney]

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

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 1fd04a02d90a..d4298feaa6fb 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1932,8 +1932,8 @@ void register_mce_write_callback(ssize_t (*fn)(struct file *filp,
 }
 EXPORT_SYMBOL_GPL(register_mce_write_callback);
 
-ssize_t mce_chrdev_write(struct file *filp, const char __user *ubuf,
-			 size_t usize, loff_t *off)
+static ssize_t mce_chrdev_write(struct file *filp, const char __user *ubuf,
+				size_t usize, loff_t *off)
 {
 	if (mce_write)
 		return mce_write(filp, ubuf, usize, off);
-- 
1.8.1.5

Re: [PATCH tip/core/rcu 1/4] mce: Stop using array-index-based RCU primitives

[Borislav Petkov]

On Tue, May 12, 2015 at 02:18:41PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> Because mce is arch-specific x86 code, there is little or no
> performance benefit of using rcu_dereference_index_check() over using
> smp_load_acquire().  It also turns out that mce is the only place that
> array-index-based RCU is used, and it would be convenient to drop
> this portion of the RCU API.
> 
> This patch therefore changes rcu_dereference_index_check() uses to
> smp_load_acquire(), but keeping the lockdep diagnostics, and also
> changes rcu_access_index() uses to READ_ONCE().
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: linux-edac@vger.kernel.org
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>

Looks good to me.

If it is easier for you carrying it in your tree:

Acked-by: Borislav Petkov <bp@suse.de>

Otherwise, let me know.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH tip/core/rcu 1/4] mce: Stop using array-index-based RCU primitives

[Paul E. McKenney]

On Wed, May 13, 2015 at 12:31:57PM +0200, Borislav Petkov wrote:
> On Tue, May 12, 2015 at 02:18:41PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > Because mce is arch-specific x86 code, there is little or no
> > performance benefit of using rcu_dereference_index_check() over using
> > smp_load_acquire().  It also turns out that mce is the only place that
> > array-index-based RCU is used, and it would be convenient to drop
> > this portion of the RCU API.
> > 
> > This patch therefore changes rcu_dereference_index_check() uses to
> > smp_load_acquire(), but keeping the lockdep diagnostics, and also
> > changes rcu_access_index() uses to READ_ONCE().
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: linux-edac@vger.kernel.org
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> 
> Looks good to me.
> 
> If it is easier for you carrying it in your tree:
> 
> Acked-by: Borislav Petkov <bp@suse.de>
> 
> Otherwise, let me know.

Thank you, I have applied your Acked-by.  Probably easier for me
to carry this change, given that it needs to be applied before the
later patches that remove array-index support from RCU.


							Thanx, Paul

Re: [PATCH tip/core/rcu 2/4] rcu: Eliminate array-index-based RCU primitives

[Borislav Petkov]

On Tue, May 12, 2015 at 02:18:42PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> Now that rcu_access_index() and rcu_dereference_index_check() are no
> longer used, the commit removes them from the RCU API.  This means that
> RCU's data dependencies now involve only pointers, give or take the
> occasional cast to and then back from an integer type to do pointer
> arithmetic.  This in turn eliminates the need for a number of operations
> on values carrying RCU data dependencies.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: linux-edac@vger.kernel.org
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> ---
>  include/linux/rcupdate.h | 50 ------------------------------------------------
>  1 file changed, 50 deletions(-)

Diffstats like this one are my favourite ones!

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH tip/core/rcu 2/4] rcu: Eliminate array-index-based RCU primitives

[Paul E. McKenney]

On Wed, May 13, 2015 at 03:08:45PM +0200, Borislav Petkov wrote:
> On Tue, May 12, 2015 at 02:18:42PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > Now that rcu_access_index() and rcu_dereference_index_check() are no
> > longer used, the commit removes them from the RCU API.  This means that
> > RCU's data dependencies now involve only pointers, give or take the
> > occasional cast to and then back from an integer type to do pointer
> > arithmetic.  This in turn eliminates the need for a number of operations
> > on values carrying RCU data dependencies.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: linux-edac@vger.kernel.org
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > ---
> >  include/linux/rcupdate.h | 50 ------------------------------------------------
> >  1 file changed, 50 deletions(-)
> 
> Diffstats like this one are my favourite ones!

Me too!

> Acked-by: Borislav Petkov <bp@suse.de>

Applied, thank you!

								Thanx, Paul