Add rcu_sync and implement percpu_rwsem in terms of it
Subsystems (3)
RFC PATCH 4/9 rcu_sync: Introduce rcu_sync_dtor()
In reply to Paul E. McKenneyFrom: Oleg Nesterov <[email protected]>
This commit allows rcu_sync structures to be safely deallocated, The trick is to add a new ->wait field to the gp_ops array. This field is a pointer to the rcu_barrier() function corresponding to the flavor of RCU in question. This allows a new rcu_sync_dtor() to wait for any outstanding callbacks before freeing the rcu_sync structure.
--- include/linux/rcu_sync.h | 1 + kernel/rcu/sync.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+)
include/linux/rcu_sync.h +1 -0
@@ -62,6 +62,7 @@ static inline bool rcu_sync_is_idle(struct rcu_sync *rsp)extern void rcu_sync_init (struct rcu_sync *,enum rcu_sync_type );extern void rcu_sync_enter (struct rcu_sync *);extern void rcu_sync_exit (struct rcu_sync *); +extern void rcu_sync_dtor (struct rcu_sync *);#define __RCU_SYNC_INITIALIZER (name, type) { \ .gp_state = 0, \
kernel/rcu/sync.c +22 -0
@@ -32,6 +32,7 @@ static const struct { void (*sync)(void); void (*call)(struct rcu_head *, void (*)(struct rcu_head *)); + void (*wait)(void); #ifdef CONFIG_PROVE_RCU int (*held)(void); #endif
@@ -39,16 +40,19 @@ static const struct { [RCU_SYNC ] = { .sync = synchronize_rcu, .call = call_rcu, + .wait = rcu_barrier,__INIT_HELD (rcu_read_lock_held) }, [RCU_SCHED_SYNC ] = { .sync = synchronize_sched, .call = call_rcu_sched, + .wait = rcu_barrier_sched,__INIT_HELD (rcu_read_lock_sched_held) }, [RCU_BH_SYNC ] = { .sync = synchronize_rcu_bh, .call = call_rcu_bh, + .wait = rcu_barrier_bh,__INIT_HELD (rcu_read_lock_bh_held) }, };
@@ -195,3 +199,21 @@ void rcu_sync_exit(struct rcu_sync *rsp) }spin_unlock_irq (&rsp->rss_lock); } + +void rcu_sync_dtor (struct rcu_sync *rsp) +{ +int cb_state; + +BUG_ON (rsp->gp_count); + +spin_lock_irq (&rsp->rss_lock); +if (rsp->cb_state ==CB_REPLAY ) + rsp->cb_state =CB_PENDING ; + cb_state = rsp->cb_state; +spin_unlock_irq (&rsp->rss_lock); + +if (cb_state !=CB_IDLE ) { + gp_ops[rsp->gp_type].wait (); +BUG_ON (rsp->cb_state !=CB_IDLE ); + } +}
Signature · 1 line
1.8.1.5
RFC PATCH 6/9 locking/percpu-rwsem: Make use of the rcu_sync infrastructure
In reply to Paul E. McKenneyFrom: Oleg Nesterov <[email protected]>
Currently down_write/up_write calls synchronize_sched_expedited() twice, which is evil. Change this code to rely on rcu-sync primitives. This avoids the _expedited "big hammer", and this can be faster in the contended case or even in the case when a single thread does down_write/up_write in a loop.
Of course, a single down_write() will take more time, but otoh it will be much more friendly to the whole system.
To simplify the review this patch doesn't update the comments, fixed by the next change.
--- include/linux/percpu-rwsem.h | 3 ++- kernel/locking/percpu-rwsem.c | 18 +++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-)
include/linux/percpu-rwsem.h +2 -1
@@ -5,11 +5,12 @@#include <linux/rwsem.h> #include <linux/percpu.h> #include <linux/wait.h> +#include <linux/rcu_sync.h> #include <linux/lockdep.h> struct percpu_rw_semaphore { +struct rcu_sync rss;unsigned int __percpu *fast_read_ctr; -atomic_t write_ctr;struct rw_semaphore rw_sem;atomic_t slow_read_ctr;wait_queue_head_t write_waitq;
kernel/locking/percpu-rwsem.c +7 -11
@@ -17,7 +17,7 @@ int __percpu_init_rwsem(struct percpu_rw_semaphore *brw,/* ->rw_sem represents the whole percpu_rw_semaphore for lockdep */ __init_rwsem (&brw->rw_sem, name, rwsem_key); -atomic_set (&brw->write_ctr,0 ); +rcu_sync_init (&brw->rss,RCU_SCHED_SYNC );atomic_set (&brw->slow_read_ctr,0 );init_waitqueue_head (&brw->write_waitq);return 0 ;
@@ -32,6 +32,7 @@ void percpu_free_rwsem(struct percpu_rw_semaphore *brw)if (!brw->fast_read_ctr)return ; +rcu_sync_dtor (&brw->rss);free_percpu (brw->fast_read_ctr); brw->fast_read_ctr =NULL ;/* catch use after free bugs */ }
@@ -61,13 +62,12 @@ void percpu_free_rwsem(struct percpu_rw_semaphore *brw)*/ static bool update_fast_ctr (struct percpu_rw_semaphore *brw,unsigned int val) { -bool success = false; +bool success;preempt_disable (); -if (likely (!atomic_read (&brw->write_ctr))) { + success =rcu_sync_is_idle (&brw->rss); +if (likely (success))__this_cpu_add (*brw->fast_read_ctr, val); - success = true; - }preempt_enable ();return success;
@@ -133,8 +133,6 @@ static int clear_fast_ctr(struct percpu_rw_semaphore *brw)*/ void percpu_down_write (struct percpu_rw_semaphore *brw) { -/* tell update_fast_ctr() there is a pending writer */ -atomic_inc (&brw->write_ctr);/* * 1. Ensures that write_ctr != 0 is visible to any down_read/up_read * so that update_fast_ctr() can't succeed.
@@ -146,7 +144,7 @@ void percpu_down_write(struct percpu_rw_semaphore *brw)* fast-path, it executes a full memory barrier before we return. * See R_W case in the comment above update_fast_ctr(). */ -synchronize_sched_expedited (); +rcu_sync_enter (&brw->rss);/* exclude other writers, and block the new readers completely */ down_write (&brw->rw_sem);
@@ -166,7 +164,5 @@ void percpu_up_write(struct percpu_rw_semaphore *brw)* Insert the barrier before the next fast-path in down_read, * see W_R case in the comment above update_fast_ctr(). */ -synchronize_sched_expedited (); -/* the last writer unblocks update_fast_ctr() */ -atomic_dec (&brw->write_ctr); +rcu_sync_exit (&brw->rss); }
Signature · 1 line
1.8.1.5
RFC PATCH 1/9 rcu: Create rcu_sync infrastructure
In reply to Paul E. McKenneyFrom: Oleg Nesterov <[email protected]>
The rcu_sync infrastructure can be thought of as infrastructure to be used to implement reader-writer primitives having extremely lightweight readers during times when there are no writers. The first use is in the percpu_rwsem used by the VFS subsystem.
This infrastructure is functionally equivalent to
struct rcu_sync_struct {
atomic_t counter;
};
/* Check possibility of fast-path read-side operations. */
static inline bool rcu_sync_is_idle(struct rcu_sync_struct *rss)
{
return atomic_read(&rss->counter) == 0;
}
/* Tell readers to use slowpaths. */
static inline void rcu_sync_enter(struct rcu_sync_struct *rss)
{
atomic_inc(&rss->counter);
synchronize_sched();
}
/* Allow readers to once again use fastpaths. */
static inline void rcu_sync_exit(struct rcu_sync_struct *rss)
{
synchronize_sched();
atomic_dec(&rss->counter);
}The main difference is that it records the state and only calls synchronize_sched() if required. At least some of the calls to synchronize_sched() will be optimized away when rcu_sync_enter() and rcu_sync_exit() are invoked repeatedly in quick succession.
--- include/linux/rcu_sync.h | 94 +++++++++++++++++++++++++ kernel/rcu/Makefile | 2 +- kernel/rcu/sync.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 270 insertions(+), 1 deletion(-) create mode 100644 include/linux/rcu_sync.h create mode 100644 kernel/rcu/sync.c
include/linux/rcu_sync.h new file +94 -0
@@ -0,0 +1,94 @@ +/* +* RCU-based infrastructure for lightweight reader-writer locking +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, you can access it online at +* http://www.gnu.org/licenses/gpl-2.0.html. +* +* Copyright (c) 2015, Red Hat, Inc. +* +* Author: Oleg Nesterov <[email protected]> +*/ + +#ifndef _LINUX_RCU_SYNC_H_ +#define _LINUX_RCU_SYNC_H_ + +#include <linux/wait.h> +#include <linux/rcupdate.h> + +/* Structure to mediate between updaters and fastpath-using readers. */ +struct rcu_sync { +int gp_state; +int gp_count; +wait_queue_head_t gp_wait; + +int cb_state; +struct rcu_head cb_head; + +void (*sync)(void ); +void (*call)(struct rcu_head *,void (*)(struct rcu_head *)); +}; + +#define ___RCU_SYNC_INIT (name) \ + .gp_state = 0, \ + .gp_count = 0, \ + .gp_wait = __WAIT_QUEUE_HEAD_INITIALIZER(name.gp_wait), \ + .cb_state = 0 + +#define __RCU_SCHED_SYNC_INIT (name) { \ + ___RCU_SYNC_INIT(name), \ + .sync = synchronize_sched, \ + .call = call_rcu_sched, \ +} + +#define __RCU_BH_SYNC_INIT (name) { \ + ___RCU_SYNC_INIT(name), \ + .sync = synchronize_rcu_bh, \ + .call = call_rcu_bh, \ +} + +#define __RCU_SYNC_INIT (name) { \ + ___RCU_SYNC_INIT(name), \ + .sync = synchronize_rcu, \ + .call = call_rcu, \ +} + +#define DEFINE_RCU_SCHED_SYNC (name) \ + struct rcu_sync name = __RCU_SCHED_SYNC_INIT(name) + +#define DEFINE_RCU_BH_SYNC (name) \ + struct rcu_sync name = __RCU_BH_SYNC_INIT(name) + +#define DEFINE_RCU_SYNC (name) \ + struct rcu_sync name = __RCU_SYNC_INIT(name) + +/** +* rcu_sync_is_idle() - Are readers permitted to use their fastpaths? +* @rsp: Pointer to rcu_sync structure to use for synchronization +* +* Returns true if readers are permitted to use their fastpaths. +* Must be invoked within an RCU read-side critical section whose +* flavor matches that of the rcu_sync struture. +*/ +static inline bool rcu_sync_is_idle (struct rcu_sync *rsp) +{ +return !rsp->gp_state;/* GP_IDLE */ +} + +enum rcu_sync_type {RCU_SYNC ,RCU_SCHED_SYNC ,RCU_BH_SYNC }; + +extern void rcu_sync_init (struct rcu_sync *,enum rcu_sync_type ); +extern void rcu_sync_enter (struct rcu_sync *); +extern void rcu_sync_exit (struct rcu_sync *); + +#endif /* _LINUX_RCU_SYNC_H_ */
kernel/rcu/Makefile +1 -1
@@ -1,4 +1,4 @@ -obj-y += update.o +obj-y += update.o sync.o obj-$(CONFIG_SRCU) += srcu.o obj-$(CONFIG_RCU_TORTURE_TEST) += rcutorture.o obj-$(CONFIG_TREE_RCU) += tree.o
kernel/rcu/sync.c new file +175 -0
175 lines not on this page · raw message
Signature · 1 line
1.8.1.5
RFC PATCH 9/9 rcu: Change _wait_rcu_gp() to work around GCC bug 67055
From: Oleg Nesterov <[email protected]>
Code like this in inline functions confuses some recent versions of gcc:
const int n = const-expr; whatever_t array[n];
For more details, see:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67055#c13
This compiler bug results in the following failure after 114b7fd4b (rcu:
Create rcu_sync infrastructure):
In file included from include/linux/rcupdate.h:429:0,
from include/linux/rcu_sync.h:5,
from kernel/rcu/sync.c:1:
include/linux/rcutiny.h: In function 'rcu_barrier_sched':
include/linux/rcutiny.h:55:20: internal compiler error: Segmentation fault
static inline void rcu_barrier_sched(void)
This commit therefore eliminates the constant local variable in favor of direct use of the expression.
--- include/linux/rcupdate.h | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
include/linux/rcupdate.h +5 -6
@@ -230,12 +230,11 @@ void __wait_rcu_gp(bool checktiny, int n, call_rcu_func_t *crcu_array, struct rcu_synchronize *rs_array);#define _wait_rcu_gp (checktiny, ...) \ -do { \ - call_rcu_func_t __crcu_array[] = { __VA_ARGS__ }; \ - const int __n = ARRAY_SIZE(__crcu_array); \ - struct rcu_synchronize __rs_array[__n]; \ - \ - __wait_rcu_gp(checktiny, __n, __crcu_array, __rs_array); \ +do { \ + call_rcu_func_t __crcu_array[] = { __VA_ARGS__ }; \ + struct rcu_synchronize __rs_array[ARRAY_SIZE(__crcu_array)]; \ + __wait_rcu_gp(checktiny, ARRAY_SIZE(__crcu_array), \ + __crcu_array, __rs_array); \ } while (0)#define wait_rcu_gp (...) _wait_rcu_gp(false, __VA_ARGS__)
Signature · 1 line
1.8.1.5
RFC PATCH 2/9 rcu_sync: Simplify rcu_sync using new rcu_sync_ops structure
In reply to Paul E. McKenneyFrom: Oleg Nesterov <[email protected]>
This commit adds the new struct rcu_sync_ops which holds sync/call methods, and turns the function pointers in rcu_sync_struct into an array of struct rcu_sync_ops. This simplifies the "init" helpers by collapsing a switch statement and explicit multiple definitions into a simple assignment and a helper macro, respectively.
--- include/linux/rcu_sync.h | 60 +++++++++++++++++++----------------------------- kernel/rcu/sync.c | 42 +++++++++++++++++---------------- 2 files changed, 45 insertions(+), 57 deletions(-)
include/linux/rcu_sync.h +23 -37
@@ -26,6 +26,8 @@#include <linux/wait.h> #include <linux/rcupdate.h> +enum rcu_sync_type {RCU_SYNC ,RCU_SCHED_SYNC ,RCU_BH_SYNC }; +/* Structure to mediate between updaters and fastpath-using readers. */ struct rcu_sync {int gp_state;
@@ -35,43 +37,9 @@ struct rcu_sync {int cb_state;struct rcu_head cb_head; -void (*sync)(void ); -void (*call)(struct rcu_head *,void (*)(struct rcu_head *)); +enum rcu_sync_type gp_type; }; -#define ___RCU_SYNC_INIT (name) \ - .gp_state = 0, \ - .gp_count = 0, \ - .gp_wait = __WAIT_QUEUE_HEAD_INITIALIZER(name.gp_wait), \ - .cb_state = 0 - -#define __RCU_SCHED_SYNC_INIT (name) { \ - ___RCU_SYNC_INIT(name), \ - .sync = synchronize_sched, \ - .call = call_rcu_sched, \ -} - -#define __RCU_BH_SYNC_INIT (name) { \ - ___RCU_SYNC_INIT(name), \ - .sync = synchronize_rcu_bh, \ - .call = call_rcu_bh, \ -} - -#define __RCU_SYNC_INIT (name) { \ - ___RCU_SYNC_INIT(name), \ - .sync = synchronize_rcu, \ - .call = call_rcu, \ -} - -#define DEFINE_RCU_SCHED_SYNC (name) \ - struct rcu_sync name = __RCU_SCHED_SYNC_INIT(name) - -#define DEFINE_RCU_BH_SYNC (name) \ - struct rcu_sync name = __RCU_BH_SYNC_INIT(name) - -#define DEFINE_RCU_SYNC (name) \ - struct rcu_sync name = __RCU_SYNC_INIT(name) -/** * rcu_sync_is_idle() - Are readers permitted to use their fastpaths? * @rsp: Pointer to rcu_sync structure to use for synchronization
@@ -85,10 +53,28 @@ static inline bool rcu_sync_is_idle(struct rcu_sync *rsp) return !rsp->gp_state; /* GP_IDLE */ } -enum rcu_sync_type {RCU_SYNC ,RCU_SCHED_SYNC ,RCU_BH_SYNC }; -extern void rcu_sync_init (struct rcu_sync *,enum rcu_sync_type );extern void rcu_sync_enter (struct rcu_sync *);extern void rcu_sync_exit (struct rcu_sync *); +#define __RCU_SYNC_INITIALIZER (name, type) { \ + .gp_state = 0, \ + .gp_count = 0, \ + .gp_wait = __WAIT_QUEUE_HEAD_INITIALIZER(name.gp_wait), \ + .cb_state = 0, \ + .gp_type = type, \ + } + +#define __DEFINE_RCU_SYNC (name, type) \ + struct rcu_sync_struct name = __RCU_SYNC_INITIALIZER(name, type) + +#define DEFINE_RCU_SYNC (name) \ + __DEFINE_RCU_SYNC(name, RCU_SYNC) + +#define DEFINE_RCU_SCHED_SYNC (name) \ + __DEFINE_RCU_SYNC(name, RCU_SCHED_SYNC) + +#define DEFINE_RCU_BH_SYNC (name) \ + __DEFINE_RCU_SYNC(name, RCU_BH_SYNC) +#endif /* _LINUX_RCU_SYNC_H_ */
kernel/rcu/sync.c +22 -20
72 lines not on this page · raw message
Signature · 1 line
1.8.1.5
cover Add rcu_sync and implement percpu_rwsem in terms of it
Hello!
This series implements an rcu_sync primitive and updates percpu_rwsem to be implemented in terms of it. This is an updated version of the series posted by Oleg Nesterov, responding to feedback from Ingo Molnar. The patches in this series, all courtesy of Oleg (and some in turn based on work by Peter Zijlstra), are as follows:
1. Create rcu_sync infrastructure.
2. Simplify rcu_sync using new rcu_sync_ops structure.
3. Add CONFIG_PROVE_RCU checks.
4. Introduce rcu_sync_dtor().
5. Make percpu_free_rwsem() after kzalloc() safe.
6. Make use of the rcu_sync infrastructure for percpu_rwsem.
7. Fix the percpu_rwsem comments outdated by rcu_sync.
8. Clean up the lockdep annotations in percpu_down_read().
9. Change _wait_rcu_gp() to work around GCC bug 67055.
Thanx, Paul
------------------------------------------------------------------------
b/include/linux/percpu-rwsem.h | 3 b/include/linux/rcu_sync.h | 161 +++++++++++++++++++----- b/include/linux/rcupdate.h | 11 - b/kernel/locking/percpu-rwsem.c | 85 ++++--------- b/kernel/rcu/Makefile | 2 b/kernel/rcu/sync.c | 259 ++++++++++++++++++++++++++++++++++++---- 6 files changed, 401 insertions(+), 120 deletions(-)
RFC PATCH 7/9 locking/percpu-rwsem: Fix the comments outdated by rcu_sync
In reply to Paul E. McKenneyFrom: Oleg Nesterov <[email protected]>
Update the comments broken by the previous change.
--- kernel/locking/percpu-rwsem.c | 50 ++++++++++--------------------------------- 1 file changed, 11 insertions(+), 39 deletions(-)
kernel/locking/percpu-rwsem.c +11 -39
@@ -38,27 +38,12 @@ void percpu_free_rwsem(struct percpu_rw_semaphore *brw) }/* -* This is the fast-path for down_read/up_read, it only needs to ensure -* there is no pending writer (atomic_read(write_ctr) == 0) and inc/dec the -* fast per-cpu counter. The writer uses synchronize_sched_expedited() to -* serialize with the preempt-disabled section below. -* -* The nontrivial part is that we should guarantee acquire/release semantics -* in case when -* -* R_W: down_write() comes after up_read(), the writer should see all -* changes done by the reader -* or -* W_R: down_read() comes after up_write(), the reader should see all -* changes done by the writer +* This is the fast-path for down_read/up_read. If it succeeds we rely +* on the barriers provided by rcu_sync_enter/exit; see the comments in +* percpu_down_write() and percpu_up_write(). * * If this helper fails the callers rely on the normal rw_semaphore and * atomic_dec_and_test(), so in this case we have the necessary barriers. -* -* But if it succeeds we do not have any barriers, atomic_read(write_ctr) or -* __this_cpu_add() below can be reordered with any LOAD/STORE done by the -* reader inside the critical section. See the comments in down_write and -* up_write below. */ static bool update_fast_ctr (struct percpu_rw_semaphore *brw,unsigned int val) {
@@ -120,29 +105,15 @@ static int clear_fast_ctr(struct percpu_rw_semaphore *brw)return sum; } -/* -* A writer increments ->write_ctr to force the readers to switch to the -* slow mode, note the atomic_read() check in update_fast_ctr(). -* -* After that the readers can only inc/dec the slow ->slow_read_ctr counter, -* ->fast_read_ctr is stable. Once the writer moves its sum into the slow -* counter it represents the number of active readers. -* -* Finally the writer takes ->rw_sem for writing and blocks the new readers, -* then waits until the slow counter becomes zero. -*/ void percpu_down_write (struct percpu_rw_semaphore *brw) {/* -* 1. Ensures that write_ctr != 0 is visible to any down_read/up_read -* so that update_fast_ctr() can't succeed. -* -* 2. Ensures we see the result of every previous this_cpu_add() in -* update_fast_ctr(). +* Make rcu_sync_is_idle() == F and thus disable the fast-path in +* percpu_down_read() and percpu_up_read(), and wait for gp pass. * -* 3. Ensures that if any reader has exited its critical section via -* fast-path, it executes a full memory barrier before we return. -* See R_W case in the comment above update_fast_ctr(). +* The latter synchronises us with the preceding readers which used +* the fast-past, so we can not miss the result of __this_cpu_add() +* or anything else inside their criticial sections. */ rcu_sync_enter (&brw->rss);
@@ -161,8 +132,9 @@ void percpu_up_write(struct percpu_rw_semaphore *brw)/* release the lock, but the readers can't use the fast-path */ up_write (&brw->rw_sem);/* -* Insert the barrier before the next fast-path in down_read, -* see W_R case in the comment above update_fast_ctr(). +* Enable the fast-path in percpu_down_read() and percpu_up_read() +* but only after another gp pass; this adds the necessary barrier +* to ensure the reader can't miss the changes done by us. */ rcu_sync_exit (&brw->rss); }
Signature · 1 line
1.8.1.5
RFC PATCH 5/9 locking/percpu-rwsem: Make percpu_free_rwsem() after kzalloc() safe
In reply to Paul E. McKenneyFrom: Oleg Nesterov <[email protected]>
This is the temporary ugly hack which will be reverted later. We only need it to ensure that the next patch will not break "change sb_writers to use percpu_rw_semaphore" patches routed via the VFS tree.
The alloc_super()->destroy_super() error path assumes that it is safe to call percpu_free_rwsem() after kzalloc() without percpu_init_rwsem(), so let's not disappoint it.
--- kernel/locking/percpu-rwsem.c | 7 +++++++ 1 file changed, 7 insertions(+)
kernel/locking/percpu-rwsem.c +7 -0
@@ -25,6 +25,13 @@ int __percpu_init_rwsem(struct percpu_rw_semaphore *brw,void percpu_free_rwsem (struct percpu_rw_semaphore *brw) { +/* +* XXX: temporary kludge. The error path in alloc_super() +* assumes that percpu_free_rwsem() is safe after kzalloc(). +*/ +if (!brw->fast_read_ctr) +return ; +free_percpu (brw->fast_read_ctr); brw->fast_read_ctr =NULL ;/* catch use after free bugs */ }
Signature · 1 line
1.8.1.5
RFC PATCH 3/9 rcu_sync: Add CONFIG_PROVE_RCU checks
In reply to Paul E. McKenneyFrom: Oleg Nesterov <[email protected]>
This commit validates that the caller of rcu_sync_is_idle() holds the corresponding type of RCU read-side lock, but only in kernels built with CONFIG_PROVE_RCU=y. This validation is carried out via a new rcu_sync_ops->held() method that is checked within rcu_sync_is_idle().
Note that although this does add code to the fast path, it only does so in kernels built with CONFIG_PROVE_RCU=y.
--- include/linux/rcu_sync.h | 6 ++++++ kernel/rcu/sync.c | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+)
include/linux/rcu_sync.h +6 -0
@@ -40,6 +40,8 @@ struct rcu_sync {enum rcu_sync_type gp_type; }; +extern bool __rcu_sync_is_idle (struct rcu_sync *); +/** * rcu_sync_is_idle() - Are readers permitted to use their fastpaths? * @rsp: Pointer to rcu_sync structure to use for synchronization
@@ -50,7 +52,11 @@ struct rcu_sync {*/ static inline bool rcu_sync_is_idle (struct rcu_sync *rsp) { +#ifdef CONFIG_PROVE_RCU +return __rcu_sync_is_idle (rss); +#else return !rsp->gp_state;/* GP_IDLE */ +#endif }extern void rcu_sync_init (struct rcu_sync *,enum rcu_sync_type );
kernel/rcu/sync.c +20 -0
@@ -23,21 +23,33 @@#include <linux/rcu_sync.h> #include <linux/sched.h> +#ifdef CONFIG_PROVE_RCU +#define __INIT_HELD (func) .held = func, +#else +#define __INIT_HELD (func) +#endif +static const struct {void (*sync)(void );void (*call)(struct rcu_head *,void (*)(struct rcu_head *)); +#ifdef CONFIG_PROVE_RCU +int (*held)(void ); +#endif } gp_ops[] = { [RCU_SYNC ] = { .sync = synchronize_rcu, .call = call_rcu, +__INIT_HELD (rcu_read_lock_held) }, [RCU_SCHED_SYNC ] = { .sync = synchronize_sched, .call = call_rcu_sched, +__INIT_HELD (rcu_read_lock_sched_held) }, [RCU_BH_SYNC ] = { .sync = synchronize_rcu_bh, .call = call_rcu_bh, +__INIT_HELD (rcu_read_lock_bh_held) }, };
@@ -46,6 +58,13 @@ enum { CB_IDLE = 0, CB_PENDING, CB_REPLAY };#define rss_lock gp_wait.lock +#ifdef CONFIG_PROVE_RCU +bool __rcu_sync_is_idle (struct rcu_sync *rsp) +{ +WARN_ON (!gp_ops[rsp->gp_type].held ()); +return rsp->gp_state ==GP_IDLE ; +} +/** * rcu_sync_init() - Initialize an rcu_sync structure * @rsp: Pointer to rcu_sync structure to be initialized
@@ -57,6 +76,7 @@ void rcu_sync_init(struct rcu_sync *rsp, enum rcu_sync_type type)init_waitqueue_head (&rsp->gp_wait); rsp->gp_type = type; } +#endif /** * rcu_sync_enter() - Force readers onto slowpath
Signature · 1 line
1.8.1.5
RFC PATCH 8/9 locking/percpu-rwsem: Clean up the lockdep annotations in percpu_down_read()
In reply to Paul E. McKenneyFrom: Oleg Nesterov <[email protected]>
Based on Peter Zijlstra's earlier patch.
Change percpu_down_read() to use __down_read(), this way we can do rwsem_acquire_read() unconditionally at the start to make this code more symmetric and clean.
Originally-From: Peter Zijlstra <[email protected]>
--- kernel/locking/percpu-rwsem.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
kernel/locking/percpu-rwsem.c +5 -5
@@ -69,14 +69,14 @@ static bool update_fast_ctr(struct percpu_rw_semaphore *brw, unsigned int val)void percpu_down_read (struct percpu_rw_semaphore *brw) {might_sleep (); -if (likely (update_fast_ctr (brw,+1 ))) { -rwsem_acquire_read (&brw->rw_sem.dep_map,0 ,0 , _RET_IP_); +rwsem_acquire_read (&brw->rw_sem.dep_map,0 ,0 , _RET_IP_); + +if (likely (update_fast_ctr (brw,+1 )))return ; - } -down_read (&brw->rw_sem); +/* Avoid rwsem_acquire_read() and rwsem_release() */ +__down_read (&brw->rw_sem);atomic_inc (&brw->slow_read_ctr); -/* avoid up_read()->rwsem_release() */ __up_read (&brw->rw_sem); }
Signature · 1 line
1.8.1.5
Re: [PATCH RFC tip/core/rcu 3/9] rcu_sync: Add CONFIG_PROVE_RCU checks
In reply to Paul E. McKenneyinclude/linux/rcu_sync.h struct rcu_sync { · line 56
Quoted 11 lines · Paul E. McKenney
include/linux/rcu_sync.h +4
-0+extern bool __rcu_sync_is_idle (struct rcu_sync *); +/** * rcu_sync_is_idle() - Are readers permitted to use their fastpaths? * @rsp: Pointer to rcu_sync structure to use for synchronization @@ -50,7 +52,11 @@ struct rcu_sync {*/ static inline bool rcu_sync_is_idle (struct rcu_sync *rsp) { +#ifdef CONFIG_PROVE_RCU +return __rcu_sync_is_idle (rss);
s/rss/rsp?
Quoted 4 lines
include/linux/rcu_sync.h +2
-0+#else return !rsp->gp_state;/* GP_IDLE */ +#endif }
Re: [PATCH RFC tip/core/rcu 3/9] rcu_sync: Add CONFIG_PROVE_RCU checks
Quoted 15 lines · Daniel Wagner
include/linux/rcu_sync.h struct rcu_sync { · line 56
Quoted 11 lines · Paul E. McKenney
include/linux/rcu_sync.h +4
-0+extern bool __rcu_sync_is_idle (struct rcu_sync *); +/** * rcu_sync_is_idle() - Are readers permitted to use their fastpaths? * @rsp: Pointer to rcu_sync structure to use for synchronization @@ -50,7 +52,11 @@ struct rcu_sync {*/ static inline bool rcu_sync_is_idle (struct rcu_sync *rsp) { +#ifdef CONFIG_PROVE_RCU +return __rcu_sync_is_idle (rss);s/rss/rsp?
Hmm, yes.
Paul, it seems that you renamed "rss" to "rsp" globally (I didn't even notice this change), but forgot about this one ;)
Should I send the patch or will you fix this yourself?
Oleg.
Re: [PATCH RFC tip/core/rcu 3/9] rcu_sync: Add CONFIG_PROVE_RCU checks
Quoted 17 lines · Oleg Nesterov
Quoted 15 lines · Daniel Wagner
include/linux/rcu_sync.h struct rcu_sync { · line 56
Quoted 11 lines · Paul E. McKenney
include/linux/rcu_sync.h +4
-0+extern bool __rcu_sync_is_idle (struct rcu_sync *); +/** * rcu_sync_is_idle() - Are readers permitted to use their fastpaths? * @rsp: Pointer to rcu_sync structure to use for synchronization @@ -50,7 +52,11 @@ struct rcu_sync {*/ static inline bool rcu_sync_is_idle (struct rcu_sync *rsp) { +#ifdef CONFIG_PROVE_RCU +return __rcu_sync_is_idle (rss);s/rss/rsp?
Quoted 6 lines
Hmm, yes.
Paul, it seems that you renamed "rss" to "rsp" globally (I didn't even notice this change), but forgot about this one ;)
Should I send the patch or will you fix this yourself?
I did fix it in -rcu, and a few other breakages as well. Most of them self-inflicted, as usual. However, I am holding off on this series in order to get it a bit more pre-merge testing. I expect to push this in v4.4.
Thanx, Paul