Miscellaneous fixes for 3.18
cover Miscellaneous fixes for 3.18
Hello!
This series contains miscellaneous fixes for RCU:
1. Remove more read-modify-write ACCESS_ONCE() calls, courtesy of
Pranith Kumar.
2. Fix sparse warning for rcu_batches_completed_preempt(), courtesy
of Pranith Kumar.
3. Use bool type for variable holding rcu_is_watching() return
value, courtesy of Pranith Kumar.
4. Use bool constant for bool return in rcu_try_advance_all_cbs(),
courtesy of Pranith Kumar.
5. Use bool constant for bool return in rcu_lockdep_current_cpu_online(),
courtesy of Pranith Kumar.
6. Use bool constant for assignments to bool variable isidle,
courtesy of Pranith Kumar.
7. Shrink the kernel a bit by uninlining rcu_read_lock_held(),
courtesy of Oleg Nesterov.
8. Shrink the kernel a bit more by defining tracepoint strings only
if CONFIG_TRACING is set, courtesy of Ard Biesheuvel.
9. Fix outdated pathnames in comments, courtesy of Pranith Kumar.
10. Remove stale comment, courtesy of Pranith Kumar.
11. Break additional call_rcu() deadlock involving scheduler and perf.
12. Shrink the TINY_RCU side of the kernel by putting error checks
under #ifdef.
13. Eliminate unnecessary self-wakeups, courtesy of Pranith Kumar.
14. Replace flush_signals() with WARN_ON(signal_pending()).
15. Add ACCESS_ONCE() for RCU_INIT_POINTER().
Thanx, Paul
------------------------------------------------------------------------
b/include/linux/rcupdate.h | 40 ++------------------------------------- b/include/trace/events/rcu.h | 3 ++ b/kernel/rcu/tiny.c | 8 +++---- b/kernel/rcu/tree.c | 39 +++++++++++++++++++++++--------------- b/kernel/rcu/tree.h | 9 ++++++-- b/kernel/rcu/tree_plugin.h | 44 +++++++++++++++++++++++++++---------------- b/kernel/rcu/update.c | 32 +++++++++++++++++++++++++++++++ 7 files changed, 101 insertions(+), 74 deletions(-)
PATCH 1/15 rcu: Remove remaining read-modify-write ACCESS_ONCE() calls
From: Pranith Kumar <[email protected]>
Change the remaining uses of ACCESS_ONCE() so that each ACCESS_ONCE() either does a load or a store, but not both.
--- kernel/rcu/tree.c | 6 ++++-- kernel/rcu/tree_plugin.h | 8 +++++--- 2 files changed, 9 insertions(+), 5 deletions(-)
kernel/rcu/tree.c +4 -2
@@ -1684,7 +1684,8 @@ static int rcu_gp_fqs(struct rcu_state *rsp, int fqs_state_in)if (ACCESS_ONCE (rsp->gp_flags) &RCU_GP_FLAG_FQS ) {raw_spin_lock_irq (&rnp->lock);smp_mb__after_unlock_lock (); -ACCESS_ONCE (rsp->gp_flags) &= ~RCU_GP_FLAG_FQS ; +ACCESS_ONCE (rsp->gp_flags) = +ACCESS_ONCE (rsp->gp_flags) & ~RCU_GP_FLAG_FQS ;raw_spin_unlock_irq (&rnp->lock); }return fqs_state;
@@ -2505,7 +2506,8 @@ static void force_quiescent_state(struct rcu_state *rsp)raw_spin_unlock_irqrestore (&rnp_old->lock, flags);return ;/* Someone beat us to it. */ } -ACCESS_ONCE (rsp->gp_flags) |=RCU_GP_FLAG_FQS ; +ACCESS_ONCE (rsp->gp_flags) = +ACCESS_ONCE (rsp->gp_flags) |RCU_GP_FLAG_FQS ;raw_spin_unlock_irqrestore (&rnp_old->lock, flags);wake_up (&rsp->gp_wq);/* Memory barrier implied by wake_up() path. */ }
kernel/rcu/tree_plugin.h +5 -3
@@ -897,7 +897,8 @@ void synchronize_rcu_expedited(void)/* Clean up and exit. */ smp_mb ();/* ensure expedited GP seen before counter increment. */ -ACCESS_ONCE (sync_rcu_preempt_exp_count)++; +ACCESS_ONCE (sync_rcu_preempt_exp_count) = + sync_rcu_preempt_exp_count +1 ; unlock_mb_ret:mutex_unlock (&sync_rcu_preempt_exp_mutex); mb_ret:
@@ -2428,8 +2429,9 @@ static int rcu_nocb_kthread(void *arg) list = next; }trace_rcu_batch_end (rdp->rsp->name, c, !!list,0 ,0 ,1 ); -ACCESS_ONCE (rdp->nocb_p_count) -= c; -ACCESS_ONCE (rdp->nocb_p_count_lazy) -= cl; +ACCESS_ONCE (rdp->nocb_p_count) = rdp->nocb_p_count - c; +ACCESS_ONCE (rdp->nocb_p_count_lazy) = + rdp->nocb_p_count_lazy - cl; rdp->n_nocbs_invoked += c; }return 0 ;
Signature · 1 line
1.8.1.5
PATCH 2/15 rcu: Fix sparse warning about rcu_batches_completed_preempt() being non-static
From: Pranith Kumar <[email protected]>
fix sparse warning about rcu_batches_completed_preempt() being non-static by marking it as static
--- kernel/rcu/tree_plugin.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/rcu/tree_plugin.h +1 -1
@@ -134,7 +134,7 @@ static void __init rcu_bootup_announce(void)* Return the number of RCU-preempt batches processed thus far * for debug and statistics. */ -long rcu_batches_completed_preempt (void ) +static long rcu_batches_completed_preempt (void ) {return rcu_preempt_state.completed; }
Signature · 1 line
1.8.1.5
PATCH 3/15 rcu: Use bool type for return value in rcu_is_watching()
In reply to Paul E. McKenneyFrom: Pranith Kumar <[email protected]>
Use a bool type for return in rcu_is_watching().
--- kernel/rcu/tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/rcu/tree.c +1 -1
@@ -819,7 +819,7 @@ bool notrace __rcu_is_watching(void)*/ bool notrace rcu_is_watching(void) { -int ret; +bool ret;preempt_disable (); ret =__rcu_is_watching ();
Signature · 1 line
1.8.1.5
PATCH 4/15 rcu: Return bool type for rcu_try_advance_all_cbs()
In reply to Paul E. McKenneyFrom: Pranith Kumar <[email protected]>
Return a bool type instead of 0 in rcu_try_advance_all_cbs().
--- kernel/rcu/tree_plugin.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/rcu/tree_plugin.h +1 -1
@@ -1626,7 +1626,7 @@ static bool __maybe_unused rcu_try_advance_all_cbs(void)/* Exit early if we advanced recently. */ if (jiffies == rdtp->last_advance_all) -return 0 ; +return false; rdtp->last_advance_all = jiffies;for_each_rcu_flavor (rsp) {
Signature · 1 line
1.8.1.5
PATCH 5/15 rcu: Return bool type in rcu_lockdep_current_cpu_online()
In reply to Paul E. McKenneyFrom: Pranith Kumar <[email protected]>
Return true instead of 1 in rcu_lockdep_current_cpu_online() as this has bool as return type.
--- include/linux/rcupdate.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/rcupdate.h +1 -1
@@ -349,7 +349,7 @@ bool rcu_lockdep_current_cpu_online(void);#else /* #if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PROVE_RCU) */ static inline bool rcu_lockdep_current_cpu_online (void ) { -return 1 ; +return true; }#endif /* #else #if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PROVE_RCU) */
Signature · 1 line
1.8.1.5
PATCH 6/15 rcu: Use true/false instead of 1/0 for a bool type
In reply to Paul E. McKenneyFrom: Pranith Kumar <[email protected]>
This commit uses true/false instead of 1/0 for bool types in rcu_gp_fqs() and force_qs_rnp().
--- kernel/rcu/tree.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
kernel/rcu/tree.c +3 -3
@@ -1668,7 +1668,7 @@ static int rcu_gp_fqs(struct rcu_state *rsp, int fqs_state_in)if (fqs_state ==RCU_SAVE_DYNTICK ) {/* Collect dyntick-idle snapshots. */ if (is_sysidle_rcu_state (rsp)) { - isidle =1 ; + isidle = true; maxj = jiffies -ULONG_MAX /4 ; } force_qs_rnp(rsp, dyntick_save_progress_counter,
@@ -1677,7 +1677,7 @@ static int rcu_gp_fqs(struct rcu_state *rsp, int fqs_state_in) fqs_state =RCU_FORCE_QS ; }else {/* Handle dyntick-idle and offline CPUs. */ - isidle =0 ; + isidle = false;force_qs_rnp (rsp, rcu_implicit_dynticks_qs, &isidle, &maxj); }/* Clear flag to prevent immediate re-entry. */
@@ -2450,7 +2450,7 @@ static void force_qs_rnp(struct rcu_state *rsp,for (; cpu <= rnp->grphi; cpu++, bit <<=1 ) {if ((rnp->qsmask & bit) !=0 ) {if ((rnp->qsmaskinit & bit) !=0 ) - *isidle =0 ; + *isidle = false;if (f (per_cpu_ptr (rsp->rda, cpu), isidle, maxj)) mask |= bit; }
Signature · 1 line
1.8.1.5
PATCH 7/15 rcu: Uninline rcu_read_lock_held()
In reply to Paul E. McKenneyFrom: Oleg Nesterov <[email protected]>
This commit uninlines rcu_read_lock_held(). According to "size vmlinux" this saves 28549 in .text:
- 5541731 3014560 14757888 23314179
+ 5513182 3026848 14757888 23297918
Note: it looks as if the data grows by 12288 bytes but this is not true, it does not actually grow. But .data starts with ALIGN(THREAD_SIZE) and since .text shrinks the padding grows, and thus .data grows too as it seen by /bin/size. diff System.map:
- ffffffff81510000 D _sdata
- ffffffff81510000 D init_thread_union
+ ffffffff81509000 D _sdata
+ ffffffff8150c000 D init_thread_union
Perhaps we can change vmlinux.lds.S to .data itself, so that /bin/size can't "wrongly" report that .data grows if .text shinks.
--- include/linux/rcupdate.h | 36 +----------------------------------- kernel/rcu/update.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 35 deletions(-)
include/linux/rcupdate.h +1 -35
@@ -371,41 +371,7 @@ extern struct lockdep_map rcu_sched_lock_map;extern struct lockdep_map rcu_callback_map;int debug_lockdep_rcu_enabled (void ); -/** -* rcu_read_lock_held() - might we be in RCU read-side critical section? -* -* If CONFIG_DEBUG_LOCK_ALLOC is selected, returns nonzero iff in an RCU -* read-side critical section. In absence of CONFIG_DEBUG_LOCK_ALLOC, -* this assumes we are in an RCU read-side critical section unless it can -* prove otherwise. This is useful for debug checks in functions that -* require that they be called within an RCU read-side critical section. -* -* Checks debug_lockdep_rcu_enabled() to prevent false positives during boot -* and while lockdep is disabled. -* -* Note that rcu_read_lock() and the matching rcu_read_unlock() must -* occur in the same context, for example, it is illegal to invoke -* rcu_read_unlock() in process context if the matching rcu_read_lock() -* was invoked from within an irq handler. -* -* Note that rcu_read_lock() is disallowed if the CPU is either idle or -* offline from an RCU perspective, so check for those as well. -*/ -static inline int rcu_read_lock_held (void ) -{ -if (!debug_lockdep_rcu_enabled ()) -return 1 ; -if (!rcu_is_watching ()) -return 0 ; -if (!rcu_lockdep_current_cpu_online ()) -return 0 ; -return lock_is_held (&rcu_lock_map); -} - -/* -* rcu_read_lock_bh_held() is defined out of line to avoid #include-file -* hell. -*/ +int rcu_read_lock_held (void );int rcu_read_lock_bh_held (void );/**
kernel/rcu/update.c +32 -0
@@ -137,6 +137,38 @@ int notrace debug_lockdep_rcu_enabled(void)EXPORT_SYMBOL_GPL (debug_lockdep_rcu_enabled);/** +* rcu_read_lock_held() - might we be in RCU read-side critical section? +* +* If CONFIG_DEBUG_LOCK_ALLOC is selected, returns nonzero iff in an RCU +* read-side critical section. In absence of CONFIG_DEBUG_LOCK_ALLOC, +* this assumes we are in an RCU read-side critical section unless it can +* prove otherwise. This is useful for debug checks in functions that +* require that they be called within an RCU read-side critical section. +* +* Checks debug_lockdep_rcu_enabled() to prevent false positives during boot +* and while lockdep is disabled. +* +* Note that rcu_read_lock() and the matching rcu_read_unlock() must +* occur in the same context, for example, it is illegal to invoke +* rcu_read_unlock() in process context if the matching rcu_read_lock() +* was invoked from within an irq handler. +* +* Note that rcu_read_lock() is disallowed if the CPU is either idle or +* offline from an RCU perspective, so check for those as well. +*/ +int rcu_read_lock_held (void ) +{ +if (!debug_lockdep_rcu_enabled ()) +return 1 ; +if (!rcu_is_watching ()) +return 0 ; +if (!rcu_lockdep_current_cpu_online ()) +return 0 ; +return lock_is_held (&rcu_lock_map); +} +EXPORT_SYMBOL_GPL (rcu_read_lock_held); + +/** * rcu_read_lock_bh_held() - might we be in RCU-bh read-side critical section? * * Check for bottom half being disabled, which covers both the
Signature · 1 line
1.8.1.5
PATCH 8/15 rcu: Define tracepoint strings only if CONFIG_TRACING is set
In reply to Paul E. McKenneyFrom: Ard Biesheuvel <[email protected]>
Commit f7f7bac9cb1c ("rcu: Have the RCU tracepoints use the tracepoint_string infrastructure") unconditionally populates the __tracepoint_str input section, but this section is not assigned an output section if CONFIG_TRACING is not set. This results in the __tracepoint_str turning up in unexpected places, i.e., after _edata.
--- kernel/rcu/tree.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
kernel/rcu/tree.c +12 -3
@@ -79,9 +79,18 @@ static struct lock_class_key rcu_fqs_class[RCU_NUM_LVLS];* the tracing userspace tools to be able to decipher the string * address to the matching string. */ -#define RCU_STATE_INITIALIZER (sname, sabbr, cr) \ +#ifdef CONFIG_TRACING +# define DEFINE_RCU_TPS (sname) \ static char sname##_varname[] = #sname; \ -static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname; \ +static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname; +# define RCU_STATE_NAME (sname) sname##_varname +#else +# define DEFINE_RCU_TPS (sname) +# define RCU_STATE_NAME (sname) __stringify(sname) +#endif + +#define RCU_STATE_INITIALIZER (sname, sabbr, cr) \ +DEFINE_RCU_TPS(sname) \ struct rcu_state sname##_state = { \ .level = { &sname##_state.node[0] }, \ .call = cr, \
@@ -93,7 +102,7 @@ struct rcu_state sname##_state = { \ .orphan_donetail = &sname##_state.orphan_donelist, \ .barrier_mutex = __MUTEX_INITIALIZER(sname##_state.barrier_mutex), \ .onoff_mutex = __MUTEX_INITIALIZER(sname##_state.onoff_mutex), \ - .name = sname##_varname, \ + .name = RCU_STATE_NAME(sname), \ .abbr = sabbr, \ }; \ DEFINE_PER_CPU(struct rcu_data, sname##_data)
Signature · 1 line
1.8.1.5
PATCH 9/15 rcu: Update tiny.c references to tree.c
In reply to Paul E. McKenneyFrom: Pranith Kumar <[email protected]>
This commit updates the references to rcutree.c which is now rcu/tree.c
--- kernel/rcu/tiny.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/rcu/tiny.c +2 -2
@@ -51,7 +51,7 @@ static long long rcu_dynticks_nesting = DYNTICK_TASK_EXIT_IDLE;#include "tiny_plugin.h" -/* Common code for rcu_idle_enter() and rcu_irq_exit(), see kernel/rcutree.c. */ +/* Common code for rcu_idle_enter() and rcu_irq_exit(), see kernel/rcu/tree.c. */ static void rcu_idle_enter_common (long long newval) {if (newval) {
@@ -114,7 +114,7 @@ void rcu_irq_exit(void) }EXPORT_SYMBOL_GPL (rcu_irq_exit); -/* Common code for rcu_idle_exit() and rcu_irq_enter(), see kernel/rcutree.c. */ +/* Common code for rcu_idle_exit() and rcu_irq_enter(), see kernel/rcu/tree.c. */ static void rcu_idle_exit_common (long long oldval) {if (oldval) {
Signature · 1 line
1.8.1.5
PATCH 10/15 rcu: Remove stale comment in tree.c
In reply to Paul E. McKenneyFrom: Pranith Kumar <[email protected]>
This commit removes a stale comment in rcu/tree.c which was left out when some code was moved around previously in commit 2036d94a7b61 ("rcu: Rework detection of use of RCU by offline CPUs") For reference, the following updated comment exists a few lines below this which means the same:
/* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */
--- kernel/rcu/tree.c | 2 -- 1 file changed, 2 deletions(-)
kernel/rcu/tree.c +0 -2
@@ -2220,8 +2220,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp)/* Adjust any no-longer-needed kthreads. */ rcu_boost_kthread_setaffinity (rnp,-1 ); -/* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */ -/* Exclude any attempts to start a new grace period. */ mutex_lock (&rsp->onoff_mutex);raw_spin_lock_irqsave (&rsp->orphan_lock, flags);
Signature · 1 line
1.8.1.5
PATCH 11/15 rcu: Break more call_rcu() deadlock involving scheduler and perf
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
Commit 96d3fd0d315a9 (rcu: Break call_rcu() deadlock involving scheduler and perf) covered the case where __call_rcu_nocb_enqueue() needs to wake the rcuo kthread due to the queue being initially empty, but did not do anything for the case where the queue was overflowing. This commit therefore also defers wakeup for the overflow case.
--- include/trace/events/rcu.h | 3 +++ kernel/rcu/tree.h | 9 +++++++-- kernel/rcu/tree_plugin.h | 26 ++++++++++++++++++-------- 3 files changed, 28 insertions(+), 10 deletions(-)
include/trace/events/rcu.h +3 -0
@@ -180,9 +180,12 @@ TRACE_EVENT(rcu_grace_period_init,* argument is a string as follows: * * "WakeEmpty": Wake rcuo kthread, first CB to empty list. +* "WakeEmptyIsDeferred": Wake rcuo kthread later, first CB to empty list. * "WakeOvf": Wake rcuo kthread, CB list is huge. +* "WakeOvfIsDeferred": Wake rcuo kthread later, CB list is huge. * "WakeNot": Don't wake rcuo kthread. * "WakeNotPoll": Don't wake rcuo kthread because it is polling. +* "DeferredWake": Carried out the "IsDeferred" wakeup. * "Poll": Start of new polling cycle for rcu_nocb_poll. * "Sleep": Sleep waiting for CBs for !rcu_nocb_poll. * "WokeEmpty": rcuo kthread woke to find empty list.
kernel/rcu/tree.h +7 -2
@@ -350,7 +350,7 @@ struct rcu_data {int nocb_p_count_lazy;/* (approximate). */ wait_queue_head_t nocb_wq;/* For nocb kthreads to sleep on. */ struct task_struct *nocb_kthread; -bool nocb_defer_wakeup;/* Defer wakeup of nocb_kthread. */ +int nocb_defer_wakeup;/* Defer wakeup of nocb_kthread. */ /* The following fields are used by the leader, hence own cacheline. */ struct rcu_head *nocb_gp_head____cacheline_internodealigned_in_smp ;
@@ -383,6 +383,11 @@ struct rcu_data {#define RCU_FORCE_QS 3/* Need to force quiescent state. */ #define RCU_SIGNAL_INIT RCU_SAVE_DYNTICK +/* Values for nocb_defer_wakeup field in struct rcu_data. */ +#define RCU_NOGP_WAKE_NOT 0 +#define RCU_NOGP_WAKE 1 +#define RCU_NOGP_WAKE_FORCE 2 +#define RCU_JIFFIES_TILL_FORCE_QS (1 + (HZ > 250) + (HZ > 500))/* For jiffies_till_first_fqs and */ /* and jiffies_till_next_fqs. */
@@ -589,7 +594,7 @@ static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,static bool rcu_nocb_adopt_orphan_cbs (struct rcu_state *rsp,struct rcu_data *rdp,unsigned long flags); -static bool rcu_nocb_need_deferred_wakeup (struct rcu_data *rdp); +static int rcu_nocb_need_deferred_wakeup (struct rcu_data *rdp);static void do_nocb_deferred_wakeup (struct rcu_data *rdp);static void rcu_boot_init_nocb_percpu_data (struct rcu_data *rdp);static void rcu_spawn_nocb_kthreads (struct rcu_state *rsp);
kernel/rcu/tree_plugin.h +18 -8
@@ -2121,16 +2121,23 @@ static void __call_rcu_nocb_enqueue(struct rcu_data *rdp,trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeEmpty" )); }else { - rdp->nocb_defer_wakeup = true; + rdp->nocb_defer_wakeup =RCU_NOGP_WAKE ;trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeEmptyIsDeferred" )); } rdp->qlen_last_fqs_check =0 ; }else if (len > rdp->qlen_last_fqs_check + qhimark) {/* ... or if many callbacks queued. */ -wake_nocb_leader (rdp, true); +if (!irqs_disabled_flags (flags)) { +wake_nocb_leader (rdp, true); +trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu, +TPS ("WakeOvf" )); + }else { + rdp->nocb_defer_wakeup =RCU_NOGP_WAKE_FORCE ; +trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu, +TPS ("WakeOvfIsDeferred" )); + } rdp->qlen_last_fqs_check =LONG_MAX /2 ; -trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeOvf" )); }else {trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeNot" )); }
@@ -2438,7 +2445,7 @@ static int rcu_nocb_kthread(void *arg) }/* Is a deferred wakeup of rcu_nocb_kthread() required? */ -static bool rcu_nocb_need_deferred_wakeup (struct rcu_data *rdp) +static int rcu_nocb_need_deferred_wakeup (struct rcu_data *rdp) {return ACCESS_ONCE (rdp->nocb_defer_wakeup); }
@@ -2446,11 +2453,14 @@ static bool rcu_nocb_need_deferred_wakeup(struct rcu_data *rdp)/* Do a deferred wakeup of rcu_nocb_kthread(). */ static void do_nocb_deferred_wakeup (struct rcu_data *rdp) { +int ndw; +if (!rcu_nocb_need_deferred_wakeup (rdp))return ; -ACCESS_ONCE (rdp->nocb_defer_wakeup) = false; -wake_nocb_leader (rdp, false); -trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("DeferredWakeEmpty" )); + ndw =ACCESS_ONCE (rdp->nocb_defer_wakeup); +ACCESS_ONCE (rdp->nocb_defer_wakeup) =RCU_NOGP_WAKE_NOT ; +wake_nocb_leader (rdp, ndw ==RCU_NOGP_WAKE_FORCE ); +trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("DeferredWake" )); }/* Initialize per-rcu_data variables for no-CBs CPUs. */
@@ -2557,7 +2567,7 @@ static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp) { } -static bool rcu_nocb_need_deferred_wakeup (struct rcu_data *rdp) +static int rcu_nocb_need_deferred_wakeup (struct rcu_data *rdp) {return false; }
Signature · 1 line
1.8.1.5
Re: [PATCH tip/core/rcu 11/15] rcu: Break more call_rcu() deadlock involving scheduler and perf
Quoted 21 lines · Paul E. McKenney
kernel/rcu/tree_plugin.h +5
-2@@ -2121,16 +2121,23 @@ static void __call_rcu_nocb_enqueue(struct rcu_data *rdp,trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeEmpty" )); }else { - rdp->nocb_defer_wakeup = true; + rdp->nocb_defer_wakeup =RCU_NOGP_WAKE ;trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeEmptyIsDeferred" )); } rdp->qlen_last_fqs_check =0 ; }else if (len > rdp->qlen_last_fqs_check + qhimark) {/* ... or if many callbacks queued. */ -wake_nocb_leader (rdp, true); +if (!irqs_disabled_flags (flags)) { +wake_nocb_leader (rdp, true); +trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu, +TPS ("WakeOvf" ));
kernel/rcu/tree_plugin.h static void __call_rcu_nocb_enqueue(struct rcu_data *rdp, · line 2143
Quoted 10 lines
kernel/rcu/tree_plugin.h +5
-1+ }else { + rdp->nocb_defer_wakeup =RCU_NOGP_WAKE_FORCE ; +trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu, +TPS ("WakeOvfIsDeferred" )); + } rdp->qlen_last_fqs_check =LONG_MAX /2 ; -trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeOvf" )); }else {trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeNot" )); }
Is it possible for the RCU_NOCP_WAKE write to overwrite a WAKE_FORCE ? If not, why not? (Would make a good comment thereabouts).
Re: [PATCH tip/core/rcu 11/15] rcu: Break more call_rcu() deadlock involving scheduler and perf
Quoted 28 lines · Peter Zijlstra
Quoted 26 lines · Paul E. McKenney
kernel/rcu/tree_plugin.h +10
-2@@ -2121,16 +2121,23 @@ static void __call_rcu_nocb_enqueue(struct rcu_data *rdp,trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeEmpty" )); }else { - rdp->nocb_defer_wakeup = true; + rdp->nocb_defer_wakeup =RCU_NOGP_WAKE ;trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeEmptyIsDeferred" )); } rdp->qlen_last_fqs_check =0 ; }else if (len > rdp->qlen_last_fqs_check + qhimark) {/* ... or if many callbacks queued. */ -wake_nocb_leader (rdp, true); +if (!irqs_disabled_flags (flags)) { +wake_nocb_leader (rdp, true); +trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu, +TPS ("WakeOvf" )); + }else { + rdp->nocb_defer_wakeup =RCU_NOGP_WAKE_FORCE ; +trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu, +TPS ("WakeOvfIsDeferred" )); + }
Quoted 8 lines
kernel/rcu/tree_plugin.h static void __call_rcu_nocb_enqueue(struct rcu_data *rdp, · line 2143
Quoted 5 lines
kernel/rcu/tree_plugin.h +0
-1rdp->qlen_last_fqs_check =LONG_MAX /2 ; -trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeOvf" )); }else {trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,TPS ("WakeNot" )); }Is it possible for the
RCU_NOCP_WAKEwrite to overwrite aWAKE_FORCE? If not, why not? (Would make a good comment thereabouts).
Good point.
So RCU_NOGP_WAKE only happens on the empty-to-non-empty transition, and that RCU_NOGP_WAKE_FORCE only happens once the queue has a large number of entries (10,000 by default). Because this code runs with irqs disabled (and must because you can do call_rcu() from interrupt handlers), RCU_NOGP_WAKE_FORCE could overwrite RCU_NOGP_WAKE, but not vice versa.
However, the same effect can be obtained by having the setting of RCU_NOGP_WAKE_FORCE race with the call to do_nocb_deferred_wakeup(). However, this would require that the call to do_nocb_deferred_wakeup() was delayed long enough for 9,999 additional callbacks be registered, and the next 10,000 callbacks will do another RCU_NOGP_WAKE_FORCE. In addition, the RCU_NOGP_WAKE_FORCE shortens delays in the rcuo kthreads rather than waking a rcuo kthread that would otherwise sleep indefinitely. So I didn't see the point of closing this window.
Thanx, Paul
PATCH 12/15 rcu: Make TINY_RCU tinier by putting error checks under #ifdef
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
The rcu_idle_enter_common() and rcu_idle_exit_common() functions contain error checks that have to the best of my knowledge have never triggered over the past several years. These are nevertheless valuable when creating new architectures or doing other low-level changes, so the checks should not be deleted. This commit instead places these checks under #ifdef CONFIG_RCU_TRACE so that they are executed only when specifically requested.
The savings are significant:
Before: text data bss dec hex filename 1749 39 0 1788 6fc /tmp/b/kernel/rcu/tiny.o 632 152 0 784 310 /tmp/b/kernel/rcu/update.o ---- 2572 After: text data bss dec hex filename 1281 37 0 1318 526 /tmp/b/kernel/rcu/tiny.o 632 152 0 784 310 /tmp/b/kernel/rcu/update.o ---- 2102
This amounts to 470 bytes, or 18% of the original.
Switched from #ifdef to IS_ENABLED() on Josh Triplett's advice.
--- kernel/rcu/tiny.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/rcu/tiny.c +2 -2
@@ -62,7 +62,7 @@ static void rcu_idle_enter_common(long long newval) }RCU_TRACE (trace_rcu_dyntick (TPS ("Start" ), rcu_dynticks_nesting, newval)); -if (!is_idle_task (current)) { +if (IS_ENABLED (CONFIG_RCU_TRACE ) && !is_idle_task (current)) {struct task_struct *idle__maybe_unused =idle_task (smp_processor_id ()); RCU_TRACE(trace_rcu_dyntick(TPS("Entry error: not idle task"),
@@ -123,7 +123,7 @@ static void rcu_idle_exit_common(long long oldval)return ; }RCU_TRACE (trace_rcu_dyntick (TPS ("End" ), oldval, rcu_dynticks_nesting)); -if (!is_idle_task (current)) { +if (IS_ENABLED (CONFIG_RCU_TRACE ) && !is_idle_task (current)) {struct task_struct *idle__maybe_unused =idle_task (smp_processor_id ()); RCU_TRACE(trace_rcu_dyntick(TPS("Exit error: not idle task"),
Signature · 1 line
1.8.1.5
PATCH 13/15 rcu: Use rcu_gp_kthread_wake() to wake up grace period kthreads
In reply to Paul E. McKenneyFrom: Pranith Kumar <[email protected]>
The rcu_gp_kthread_wake() function checks for three conditions before waking up grace period kthreads:
* Is the thread we are trying to wake up the current thread? * Are the gp_flags zero? (all threads wait on non-zero gp_flags condition) * Is there no thread created for this flavour, hence nothing to wake up?
If any one of these condition is true, we do not call wake_up(). It was found that there are quite a few avoidable wake ups both during idle time and under stress induced by rcutorture.
Idle:
Total:66000, unnecessary:66000, case1:61827, case2:66000, case3:0 Total:68000, unnecessary:68000, case1:63696, case2:68000, case3:0
rcutorture:
Total:254000, unnecessary:254000, case1:199913, case2:254000, case3:0 Total:256000, unnecessary:256000, case1:201784, case2:256000, case3:0
Here case{1-3} are the cases listed above. We can avoid these wake ups by using rcu_gp_kthread_wake() to conditionally wake up the grace period kthreads.
There is a comment about an implied barrier supplied by the wake_up() logic. This barrier is necessary for the awakened thread to see the updated ->gp_flags. This flag is always being updated with the root node lock held. Also, the awakened thread tries to acquire the root node lock before reading ->gp_flags because of which there is proper ordering.
Hence this commit tries to avoid calling wake_up() whenever we can by using rcu_gp_kthread_wake() function.
--- kernel/rcu/tree.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/rcu/tree.c +2 -2
@@ -1938,7 +1938,7 @@ static void rcu_report_qs_rsp(struct rcu_state *rsp, unsigned long flags) {WARN_ON_ONCE (!rcu_gp_in_progress (rsp));raw_spin_unlock_irqrestore (&rcu_get_root (rsp)->lock, flags); -wake_up (&rsp->gp_wq);/* Memory barrier implied by wake_up() path. */ +rcu_gp_kthread_wake (rsp); }/*
@@ -2516,7 +2516,7 @@ static void force_quiescent_state(struct rcu_state *rsp)ACCESS_ONCE (rsp->gp_flags) =ACCESS_ONCE (rsp->gp_flags) |RCU_GP_FLAG_FQS ;raw_spin_unlock_irqrestore (&rnp_old->lock, flags); -wake_up (&rsp->gp_wq);/* Memory barrier implied by wake_up() path. */ +rcu_gp_kthread_wake (rsp); }/*
Signature · 1 line
1.8.1.5
PATCH 14/15 rcu: Replace flush_signals() with WARN_ON(signal_pending())
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
Currently, when RCU awakens from a wait_event_interruptible() that might have awakened prematurely, it does a flush_signals(). This is done on the off-chance that someone figured out how to deliver a signal to a kthread, which is supposed to be impossible. Given that this is supposed to be impossible, this commit changes the flush_signals() calls into WARN_ON(signal_pending()).
--- kernel/rcu/tree.c | 4 ++-- kernel/rcu/tree_plugin.h | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-)
kernel/rcu/tree.c +2 -2
@@ -1796,7 +1796,7 @@ static int __noreturn rcu_gp_kthread(void *arg)if (rcu_gp_init (rsp))break ;cond_resched (); -flush_signals (current); +WARN_ON (signal_pending (current));trace_rcu_grace_period (rsp->name,ACCESS_ONCE (rsp->gpnum),TPS ("reqwaitsig" ));
@@ -1842,7 +1842,7 @@ static int __noreturn rcu_gp_kthread(void *arg) }else {/* Deal with stray signal. */ cond_resched (); -flush_signals (current); +WARN_ON (signal_pending (current));trace_rcu_grace_period (rsp->name,ACCESS_ONCE (rsp->gpnum),TPS ("fqswaitsig" ));
kernel/rcu/tree_plugin.h +3 -3
@@ -2237,7 +2237,7 @@ static void rcu_nocb_wait_gp(struct rcu_data *rdp) (d = ULONG_CMP_GE(ACCESS_ONCE(rnp->completed), c)));if (likely (d))break ; -flush_signals (current); +WARN_ON (signal_pending (current));trace_rcu_future_gp (rnp, rdp, c,TPS ("ResumeWait" )); }trace_rcu_future_gp (rnp, rdp, c,TPS ("EndWait" ));
@@ -2296,7 +2296,7 @@ wait_again:if (!rcu_nocb_poll)trace_rcu_nocb_wake (my_rdp->rsp->name, my_rdp->cpu,"WokeEmpty" ); -flush_signals (current); +WARN_ON (signal_pending (current));schedule_timeout_interruptible (1 );/* Rescan in case we were a victim of memory ordering. */
@@ -2375,7 +2375,7 @@ static void nocb_follower_wait(struct rcu_data *rdp)if (!rcu_nocb_poll)trace_rcu_nocb_wake (rdp->rsp->name, rdp->cpu,"WokeEmpty" ); -flush_signals (current); +WARN_ON (signal_pending (current));schedule_timeout_interruptible (1 ); } }
Signature · 1 line
1.8.1.5
PATCH 15/15 rcu: Add ACCESS_ONCE() for RCU_INIT_POINTER()
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
It is permissible to use RCU_INIT_POINTER() instead of rcu_assign_pointer() in a couple of cases where readers might be concurrently accessing the pointer being assigned to: (1) When assigning NULL and (2) When assigning a pointer that is already visible to readers. In these two cases, we should really be using ACCESS_ONCE() to prevent compiler mischief. This commit therefore adds the ACCESS_ONCE().
--- include/linux/rcupdate.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/rcupdate.h +1 -1
@@ -989,7 +989,7 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)*/ #define RCU_INIT_POINTER (p, v) \ do { \ - p = RCU_INITIALIZER(v); \ + ACCESS_ONCE(p) = (v); \ } while (0)/**
Signature · 1 line
1.8.1.5