SRCU updates for v4.21/v5.0
Subsystems (2)
cover SRCU updates for v4.21/v5.0
Hello!
This series contains SRCU updates:
1. Fix kernel-doc missing notation, courtesy of Randy Dunlap.
2. Prevent __call_srcu() counter wrap with read-side critical section.
3. Lock srcu_data structure in srcu_gp_start(), fixing a an extremely
rare but also extremely embarrassing concurrency bug, courtesy of Dennis Krein.
4. Use "ssp" instead of "sp" for srcu_struct pointer, in the hope
that this prevents confusion about which lock is held.
Thanx, Paul
------------------------------------------------------------------------
include/linux/srcu.h | 81 ++++--- include/linux/srcutiny.h | 24 +- include/linux/srcutree.h | 8 kernel/rcu/srcutiny.c | 120 +++++------ kernel/rcu/srcutree.c | 493 +++++++++++++++++++++++------------------------ 5 files changed, 367 insertions(+), 359 deletions(-)
PATCH 1/4 srcu: Fix kernel-doc missing notation
From: Randy Dunlap <[email protected]>
Fix kernel-doc warnings for missing parameter descriptions:
../include/linux/srcu.h:175: warning: Function parameter or member 'p' not described in 'srcu_dereference_notrace' ../include/linux/srcu.h:175: warning: Function parameter or member 'sp' not described in 'srcu_dereference_notrace'
Fixes 0b764a6e4e19d ("srcu: Add notrace variant of srcu_dereference")
Signed-off-by Randy Dunlap <[email protected]>
Cc Lai Jiangshan <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc Josh Triplett <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Mathieu Desnoyers <[email protected]>
Cc Joel Fernandes (Google) <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>
--- include/linux/srcu.h | 3 +++ 1 file changed, 3 insertions(+)
include/linux/srcu.h +3 -0
@@ -171,6 +171,9 @@ static inline int srcu_read_lock_held(const struct srcu_struct *sp)/** * srcu_dereference_notrace - no tracing and no lockdep calls from here +* @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. */ #define srcu_dereference_notrace (p, sp) srcu_dereference_check((p), (sp), 1)
Signature · 1 line
2.17.1
PATCH 2/4 srcu: Prevent __call_srcu() counter wrap with read-side critical section
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
Ever since cdf7abc4610a ("srcu: Allow use of Tiny/Tree SRCU from both process and interrupt context"), it has been permissible to use SRCU read-side critical sections in interrupt context. This allows __call_srcu() to use SRCU read-side critical sections to prevent a new SRCU grace period from ending before the call to either srcu_funnel_gp_start() or srcu_funnel_exp_start completes, thus preventing SRCU grace-period counter overflow during that time.
Note that this does not permit removal of the counter-wrap checks in srcu_gp_end(). These check are necessary to handle the case where a given CPU does not interact at all with SRCU for an extended time period.
This commit therefore adds an SRCU read-side critical section to __call_srcu() in order to prevent grace period counter wrap during the funnel-locking process.
Signed-off-by Paul E. McKenney <[email protected]>
--- kernel/rcu/srcutree.c | 3 +++ 1 file changed, 3 insertions(+)
kernel/rcu/srcutree.c +3 -0
@@ -858,6 +858,7 @@ void __call_srcu(struct srcu_struct *sp, struct rcu_head *rhp, rcu_callback_t func, bool do_norm) {unsigned long flags; +int idx;bool needexp = false;bool needgp = false;unsigned long s;
@@ -871,6 +872,7 @@ void __call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,return ; } rhp->func = func; + idx =srcu_read_lock (sp);local_irq_save (flags); sdp =this_cpu_ptr (sp->sda);spin_lock_rcu_node (sdp);
@@ -892,6 +894,7 @@ void __call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,srcu_funnel_gp_start (sp, sdp, s, do_norm); else if (needexp) srcu_funnel_exp_start(sp, sdp->mynode, s); +srcu_read_unlock (sp ,idx ); }/**
Signature · 1 line
2.17.1
PATCH 3/4 srcu: Lock srcu_data structure in srcu_gp_start()
In reply to Paul E. McKenneyFrom: Dennis Krein <[email protected]>
The srcu_gp_start() function is called with the srcu_struct structure's ->lock held, but not with the srcu_data structure's ->lock. This is problematic because this function accesses and updates the srcu_data structure's ->srcu_cblist, which is protected by that lock. Failing to hold this lock can result in corruption of the SRCU callback lists, which in turn can result in arbitrarily bad results.
This commit therefore makes srcu_gp_start() acquire the srcu_data structure's ->lock across the calls to rcu_segcblist_advance() and rcu_segcblist_accelerate(), thus preventing this corruption.
Reported-by Bart Van Assche <[email protected]>
Reported-by Christoph Hellwig <[email protected]>
Reported-by Sebastian Kuzminsky <[email protected]>
Signed-off-by Dennis Krein <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>
Tested-by Dennis Krein <[email protected]>
Cc <[email protected]> # 4.12.x
--- kernel/rcu/srcutree.c | 2 ++ 1 file changed, 2 insertions(+)
kernel/rcu/srcutree.c +2 -0
@@ -451,10 +451,12 @@ static void srcu_gp_start(struct srcu_struct *sp)lockdep_assert_held (&ACCESS_PRIVATE (sp, lock));WARN_ON_ONCE (ULONG_CMP_GE (sp->srcu_gp_seq, sp->srcu_gp_seq_needed)); +spin_lock_rcu_node (sdp);/* Interrupts already disabled. */ rcu_segcblist_advance (&sdp->srcu_cblist,rcu_seq_current (&sp->srcu_gp_seq)); (void )rcu_segcblist_accelerate (&sdp->srcu_cblist,rcu_seq_snap (&sp->srcu_gp_seq)); +spin_unlock_rcu_node (sdp);/* Interrupts remain disabled. */ smp_mb ();/* Order prior store to ->srcu_gp_seq_needed vs. GP start. */ rcu_seq_start (&sp->srcu_gp_seq); state =rcu_seq_state (READ_ONCE (sp->srcu_gp_seq));
Signature · 1 line
2.17.1
PATCH 4/4 srcu: Use "ssp" instead of "sp" for srcu_struct pointer
In reply to Paul E. McKenneyIn RCU, the distinction between "rsp", "rnp", and "rdp" has served well for a great many years, but in SRCU, "sp" vs. "sdp" has proven confusing. This commit therefore renames SRCU's "sp" pointers to "ssp", so that there is "ssp" for srcu_struct pointer, "snp" for srcu_node pointer, and "sdp" for srcu_data pointer.
Signed-off-by Paul E. McKenney <[email protected]>
--- include/linux/srcu.h | 78 +++---- include/linux/srcutiny.h | 24 +- include/linux/srcutree.h | 8 +- kernel/rcu/srcutiny.c | 120 +++++----- kernel/rcu/srcutree.c | 488 +++++++++++++++++++-------------------- 5 files changed, 359 insertions(+), 359 deletions(-)
include/linux/srcu.h +39 -39
@@ -38,20 +38,20 @@ struct srcu_struct;#ifdef CONFIG_DEBUG_LOCK_ALLOC -int __init_srcu_struct (struct srcu_struct *sp,const char *name, +int __init_srcu_struct (struct srcu_struct *ssp,const char *name,struct lock_class_key *key); -#define init_srcu_struct (sp) \ +#define init_srcu_struct (ssp) \ ({ \ static struct lock_class_key __srcu_key; \ \ - __init_srcu_struct((sp), #sp, &__srcu_key); \ + __init_srcu_struct((ssp), #ssp, &__srcu_key); \ })#define __SRCU_DEP_MAP_INIT (srcu_name) .dep_map = { .name = #srcu_name },#else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */ -int init_srcu_struct (struct srcu_struct *sp); +int init_srcu_struct (struct srcu_struct *ssp);#define __SRCU_DEP_MAP_INIT (srcu_name)#endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
@@ -67,28 +67,28 @@ int init_srcu_struct(struct srcu_struct *sp);struct srcu_struct { };#endif -void call_srcu (struct srcu_struct *sp,struct rcu_head *head, +void call_srcu (struct srcu_struct *ssp,struct rcu_head *head,void (*func)(struct rcu_head *head)); -void _cleanup_srcu_struct (struct srcu_struct *sp,bool quiesced); -int __srcu_read_lock(struct srcu_struct *sp) __acquires(sp); -void __srcu_read_unlock(struct srcu_struct *sp, int idx) __releases(sp); -void synchronize_srcu (struct srcu_struct *sp); +void _cleanup_srcu_struct (struct srcu_struct *ssp,bool quiesced); +int __srcu_read_lock(struct srcu_struct *ssp) __acquires(ssp); +void __srcu_read_unlock(struct srcu_struct *ssp, int idx) __releases(ssp); +void synchronize_srcu (struct srcu_struct *ssp);/** * cleanup_srcu_struct - deconstruct a sleep-RCU structure -* @sp: structure to clean up. +* @ssp: structure to clean up. * * Must invoke this after you are finished using a given srcu_struct that * was initialized via init_srcu_struct(), else you leak memory. */ -static inline void cleanup_srcu_struct (struct srcu_struct *sp) +static inline void cleanup_srcu_struct (struct srcu_struct *ssp) { -_cleanup_srcu_struct (sp, false); +_cleanup_srcu_struct (ssp, false); }/** * cleanup_srcu_struct_quiesced - deconstruct a quiesced sleep-RCU structure -* @sp: structure to clean up. +* @ssp: structure to clean up. * * Must invoke this after you are finished using a given srcu_struct that * was initialized via init_srcu_struct(), else you leak memory. Also,
@@ -103,16 +103,16 @@ static inline void cleanup_srcu_struct(struct srcu_struct *sp)* (with high probability, anyway), and will also cause the srcu_struct * to be leaked. */ -static inline void cleanup_srcu_struct_quiesced (struct srcu_struct *sp) +static inline void cleanup_srcu_struct_quiesced (struct srcu_struct *ssp) { -_cleanup_srcu_struct (sp, true); +_cleanup_srcu_struct (ssp, true); }#ifdef CONFIG_DEBUG_LOCK_ALLOC /** * srcu_read_lock_held - might we be in SRCU read-side critical section? -* @sp: The srcu_struct structure to check +* @ssp: The srcu_struct structure to check * * If CONFIG_DEBUG_LOCK_ALLOC is selected, returns nonzero iff in an SRCU * read-side critical section. In absence of CONFIG_DEBUG_LOCK_ALLOC,
@@ -126,16 +126,16 @@ static inline void cleanup_srcu_struct_quiesced(struct srcu_struct *sp)* relies on normal RCU, it can be called from the CPU which * is in the idle loop from an RCU point of view or offline. */ -static inline int srcu_read_lock_held (const struct srcu_struct *sp) +static inline int srcu_read_lock_held (const struct srcu_struct *ssp) {if (!debug_lockdep_rcu_enabled ())return 1 ; -return lock_is_held (&sp->dep_map); +return lock_is_held (&ssp->dep_map); }#else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */ -static inline int srcu_read_lock_held (const struct srcu_struct *sp) +static inline int srcu_read_lock_held (const struct srcu_struct *ssp) {return 1 ; }
@@ -145,7 +145,7 @@ static inline int srcu_read_lock_held(const struct srcu_struct *sp)/** * 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 +* @ssp: 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 *
@@ -154,32 +154,32 @@ static inline int srcu_read_lock_held(const struct srcu_struct *sp)* to 1. The @c argument will normally be a logical expression containing * lockdep_is_held() calls. */ -#define srcu_dereference_check (p, sp, c) \ - __rcu_dereference_check((p), (c) || srcu_read_lock_held(sp), __rcu) +#define srcu_dereference_check (p, ssp, c) \ + __rcu_dereference_check((p), (c) || srcu_read_lock_held(ssp), __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 +* @ssp: 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) +#define srcu_dereference (p, ssp) srcu_dereference_check((p), (ssp), 0)/** * srcu_dereference_notrace - no tracing and no lockdep calls from here * @p: the pointer to fetch and protect for later dereferencing -* @sp: pointer to the srcu_struct, which is used to check that we +* @ssp: pointer to the srcu_struct, which is used to check that we * really are in an SRCU read-side critical section. */ -#define srcu_dereference_notrace (p, sp) srcu_dereference_check((p), (sp), 1) +#define srcu_dereference_notrace (p, ssp) srcu_dereference_check((p), (ssp), 1)/** * srcu_read_lock - register a new reader for an SRCU-protected structure. -* @sp: srcu_struct in which to register the new reader. +* @ssp: srcu_struct in which to register the new reader. * * Enter an SRCU read-side critical section. Note that SRCU read-side * critical sections may be nested. However, it is illegal to
@@ -194,44 +194,44 @@ static inline int srcu_read_lock_held(const struct srcu_struct *sp)* srcu_read_unlock() in an irq handler if the matching srcu_read_lock() * was invoked in process context. */ -static inline int srcu_read_lock (struct srcu_struct *sp)__acquires (sp) +static inline int srcu_read_lock (struct srcu_struct *ssp)__acquires (ssp) {int retval; - retval =__srcu_read_lock (sp); -rcu_lock_acquire (&(sp)->dep_map); + retval =__srcu_read_lock (ssp); +rcu_lock_acquire (&(ssp)->dep_map);return retval; }/* Used by tracing, cannot be traced and cannot invoke lockdep. */ static inline notrace int -srcu_read_lock_notrace (struct srcu_struct *sp)__acquires (sp) +srcu_read_lock_notrace (struct srcu_struct *ssp)__acquires (ssp) {int retval; - retval =__srcu_read_lock (sp); + retval =__srcu_read_lock (ssp);return retval; }/** * srcu_read_unlock - unregister a old reader from an SRCU-protected structure. -* @sp: srcu_struct in which to unregister the old reader. +* @ssp: srcu_struct in which to unregister the old reader. * @idx: return value from corresponding srcu_read_lock(). * * Exit an SRCU read-side critical section. */ -static inline void srcu_read_unlock (struct srcu_struct *sp,int idx) -__releases (sp) +static inline void srcu_read_unlock (struct srcu_struct *ssp,int idx) +__releases (ssp) { -rcu_lock_release (&(sp)->dep_map); -__srcu_read_unlock (sp, idx); +rcu_lock_release (&(ssp)->dep_map); +__srcu_read_unlock (ssp, idx); }/* Used by tracing, cannot be traced and cannot call lockdep. */ static inline notrace void -srcu_read_unlock_notrace (struct srcu_struct *sp,int idx)__releases (sp) +srcu_read_unlock_notrace (struct srcu_struct *ssp,int idx)__releases (ssp) { -__srcu_read_unlock (sp, idx); +__srcu_read_unlock (ssp, idx); }/**
include/linux/srcutiny.h +12 -12
@@ -60,7 +60,7 @@ void srcu_drive_gp(struct work_struct *wp);#define DEFINE_STATIC_SRCU (name) \ static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name) -void synchronize_srcu (struct srcu_struct *sp); +void synchronize_srcu (struct srcu_struct *ssp);/* * Counts the new reader in the appropriate per-CPU element of the
@@ -68,36 +68,36 @@ void synchronize_srcu(struct srcu_struct *sp);* __srcu_read_unlock() must be in the same handler instance. Returns an * index that must be passed to the matching srcu_read_unlock(). */ -static inline int __srcu_read_lock (struct srcu_struct *sp) +static inline int __srcu_read_lock (struct srcu_struct *ssp) {int idx; - idx =READ_ONCE (sp->srcu_idx); -WRITE_ONCE (sp->srcu_lock_nesting[idx], sp->srcu_lock_nesting[idx] +1 ); + idx =READ_ONCE (ssp->srcu_idx); +WRITE_ONCE (ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] +1 );return idx; } -static inline void synchronize_srcu_expedited (struct srcu_struct *sp) +static inline void synchronize_srcu_expedited (struct srcu_struct *ssp) { -synchronize_srcu (sp); +synchronize_srcu (ssp); } -static inline void srcu_barrier (struct srcu_struct *sp) +static inline void srcu_barrier (struct srcu_struct *ssp) { -synchronize_srcu (sp); +synchronize_srcu (ssp); }/* Defined here to avoid size increase for non-torture kernels. */ -static inline void srcu_torture_stats_print (struct srcu_struct *sp, +static inline void srcu_torture_stats_print (struct srcu_struct *ssp,char *tt,char *tf) {int idx; - idx =READ_ONCE (sp->srcu_idx) &0x1 ; + idx =READ_ONCE (ssp->srcu_idx) &0x1 ;pr_alert ("%s%s Tiny SRCU per-CPU(idx=%d): (%hd,%hd)\n" , tt, tf, idx, -READ_ONCE (sp->srcu_lock_nesting[!idx]), -READ_ONCE (sp->srcu_lock_nesting[idx])); +READ_ONCE (ssp->srcu_lock_nesting[!idx]), +READ_ONCE (ssp->srcu_lock_nesting[idx])); }#endif
include/linux/srcutree.h +4 -4
@@ -51,7 +51,7 @@ struct srcu_data {unsigned long grpmask;/* Mask for leaf srcu_node */ /* ->srcu_data_have_cbs[]. */ int cpu; -struct srcu_struct *sp; +struct srcu_struct *ssp; };/*
@@ -138,8 +138,8 @@ struct srcu_struct { #define DEFINE_SRCU(name) __DEFINE_SRCU(name, /* not static */)#define DEFINE_STATIC_SRCU (name) __DEFINE_SRCU(name, static) -void synchronize_srcu_expedited (struct srcu_struct *sp); -void srcu_barrier (struct srcu_struct *sp); -void srcu_torture_stats_print (struct srcu_struct *sp,char *tt,char *tf); +void synchronize_srcu_expedited (struct srcu_struct *ssp); +void srcu_barrier (struct srcu_struct *ssp); +void srcu_torture_stats_print (struct srcu_struct *ssp,char *tt,char *tf);#endif
kernel/rcu/srcutiny.c +60 -60
220 lines not on this page · raw message
kernel/rcu/srcutree.c +244 -244
1,084 lines not on this page · raw message
Signature · 1 line
2.17.1