Array-based RCU removals for 4.2
Subsystems (3)
cover Array-based RCU removals for 4.2
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 1/4 mce: Stop using array-index-based RCU primitives
From: "Paul E. McKenney" <[email protected]>
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().
--- arch/x86/kernel/cpu/mcheck/mce.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +7 -4
@@ -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 ;
Signature · 1 line
1.8.1.5
PATCH 3/4 documentation: RCU-protected array indexes no longer supported
From: "Paul E. McKenney" <[email protected]>
--- 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(-)
Documentation/RCU/arrayRCU.txt +16 -4
@@ -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.
Documentation/RCU/lockdep.txt +0 -10
@@ -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,
Documentation/RCU/rcu_dereference.txt +12 -21
@@ -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
Documentation/RCU/whatisRCU.txt +0 -2
@@ -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
Signature · 1 line
1.8.1.5
PATCH 4/4 mce: mce_chrdev_write() can be static
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
--- arch/x86/kernel/cpu/mcheck/mce.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +2 -2
@@ -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);
Signature · 1 line
1.8.1.5
PATCH 2/4 rcu: Eliminate array-index-based RCU primitives
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
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.
--- include/linux/rcupdate.h | 50 ------------------------------------------------ 1 file changed, 50 deletions(-)
include/linux/rcupdate.h +0 -50
@@ -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
Signature · 1 line
1.8.1.5
Re: [PATCH tip/core/rcu 1/4] mce: Stop using array-index-based RCU primitives
In reply to Paul E. McKenneyQuoted 8 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
Because mce is arch-specific x86 code, there is little or no performance benefit of using
rcu_dereference_index_check()over usingsmp_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.
Quoted 8 lines
This patch therefore changes
rcu_dereference_index_check()uses tosmp_load_acquire(), but keeping the lockdep diagnostics, and also changesrcu_access_index()uses toREAD_ONCE().Signed-off-by Paul E. McKenney <[email protected]>Cc Tony Luck <[email protected]>Cc Borislav Petkov <[email protected]>
Looks good to me.
If it is easier for you carrying it in your tree:
Otherwise, let me know.
Thanks.
Signature · 5 lines
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
Quoted 18 lines · Borislav Petkov
Quoted 8 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
Because mce is arch-specific x86 code, there is little or no performance benefit of using
rcu_dereference_index_check()over usingsmp_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.Quoted 8 lines
This patch therefore changes
rcu_dereference_index_check()uses tosmp_load_acquire(), but keeping the lockdep diagnostics, and also changesrcu_access_index()uses toREAD_ONCE().Signed-off-by Paul E. McKenney <[email protected]>Cc Tony Luck <[email protected]>Cc Borislav Petkov <[email protected]>
Quoted 7 lines
Looks good to me.
If it is easier for you carrying it in your tree:
Acked-by Borislav Petkov <[email protected]>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
In reply to Paul E. McKenneyQuoted 9 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
Now that
rcu_access_index()andrcu_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.
Quoted 7 lines
Signed-off-by Paul E. McKenney <[email protected]>Cc Tony Luck <[email protected]>Cc Borislav Petkov <[email protected]>--- include/linux/rcupdate.h | 50 ------------------------------------------------ 1 file changed, 50 deletions(-)
Diffstats like this one are my favourite ones!
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Paul E. McKenney
AppliedRe: [PATCH tip/core/rcu 2/4] rcu: Eliminate array-index-based RCU primitives
Quoted 18 lines · Borislav Petkov
Quoted 9 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
Now that
rcu_access_index()andrcu_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.Quoted 7 lines
Signed-off-by Paul E. McKenney <[email protected]>Cc Tony Luck <[email protected]>Cc Borislav Petkov <[email protected]>--- include/linux/rcupdate.h | 50 ------------------------------------------------ 1 file changed, 50 deletions(-)
Quoted 1 line
Diffstats like this one are my favourite ones!
Me too!
Quoted 1 line
Acked-by Borislav Petkov <[email protected]>
Applied, thank you!
Thanx, Paul