Miscellaneous fixes for v4.16
Subsystems (3)
cover Miscellaneous fixes for v4.16
Hello!
This series provides miscellaneous fixes:
1. Prohibit call_srcu() use under raw spinlocks. This feature is
not currently used and causes trouble for the -rt guys.
2. Change printk() %p to %pK in SRCU.
3. Change printk() %p to %pK in RCU.
4. Hide trace event rcu_nocb_wake when not used, courtesy of Steven
Rostedt.
5. Add comment giving debug strategy for double call_rcu().
6. Remove have_rcu_nocb_mask from tree_plugin.h, courtesy of
Rakib Mullick.
Thanx, Paul
------------------------------------------------------------------------
include/linux/srcutree.h | 8 +-- include/trace/events/rcu.h | 2 kernel/rcu/srcutree.c | 111 +++++++++++++++++++++++++++------------------ kernel/rcu/tree.c | 9 ++- kernel/rcu/tree_plugin.h | 13 ++--- kernel/rcu/update.c | 2 6 files changed, 88 insertions(+), 57 deletions(-)
PATCH 2/6 srcu: Change printk() %p to %pK
This commit changes the %p printk() in rcutorture to %pK. This could be considered irrelevant, given that any user able to start rcutorture could inflict far heavier damage on the system, but it doesn't hurt to change it and doing so avoids script-generated noise.
--- kernel/rcu/srcutree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/rcu/srcutree.c +1 -1
@@ -386,7 +386,7 @@ void cleanup_srcu_struct(struct srcu_struct *sp)flush_delayed_work (&per_cpu_ptr (sp->sda, cpu)->work);if (WARN_ON (rcu_seq_state (READ_ONCE (sp->srcu_gp_seq)) !=SRCU_STATE_IDLE ) ||WARN_ON (srcu_readers_active (sp))) { -pr_info ("cleanup_srcu_struct: Active srcu_struct %p state: %d\n" , sp,rcu_seq_state (READ_ONCE (sp->srcu_gp_seq))); +pr_info ("cleanup_srcu_struct: Active srcu_struct %pK state: %d\n" , sp,rcu_seq_state (READ_ONCE (sp->srcu_gp_seq)));return ;/* Caller forgot to stop doing call_srcu()? */ }free_percpu (sp->sda);
Signature · 1 line
2.5.2
PATCH 5/6 rcu: Add comment giving debug strategy for double call_rcu()
In reply to Paul E. McKenneyThe following statement has for some reason proven non-intuitive:
WARN_ON_ONCE(rcu_segcblist_empty(&rdp->cblist) != (count == 0));
This commit therefore adds a comment that states that this warning usually triggers in response to a double call_rcu(), which is sort of like a double free. The comment also suggests building with CONFIG_DEBUG_OBJECTS_RCU_HEAD=y to track down the double call_rcu().
--- kernel/rcu/tree.c | 5 +++++ 1 file changed, 5 insertions(+)
kernel/rcu/tree.c +5 -0
@@ -2789,6 +2789,11 @@ static void rcu_do_batch(struct rcu_state *rsp, struct rcu_data *rdp) rdp->n_force_qs_snap = rsp->n_force_qs; }else if (count < rdp->qlen_last_fqs_check - qhimark) rdp->qlen_last_fqs_check = count; + +/* +* The following usually indicates a double call_rcu(). To track +* this down, try building with CONFIG_DEBUG_OBJECTS_RCU_HEAD=y. +*/ WARN_ON_ONCE (rcu_segcblist_empty (&rdp->cblist) != (count ==0 ));local_irq_restore (flags);
Signature · 1 line
2.5.2
PATCH 1/6 srcu: Prohibit call_srcu() use under raw spinlocks
In reply to Paul E. McKenneyInvoking queue_delayed_work() while holding a raw spinlock is forbidden in -rt kernels, which is exactly what __call_srcu() does, indirectly via srcu_funnel_gp_start(). This commit therefore downgrades Tree SRCU's locking from raw to non-raw spinlocks, which works because call_srcu() is not ever called while holding a raw spinlock.
--- include/linux/srcutree.h | 8 ++-- kernel/rcu/srcutree.c | 109 +++++++++++++++++++++++++++++------------------ 2 files changed, 72 insertions(+), 45 deletions(-)
include/linux/srcutree.h +4 -4
@@ -40,7 +40,7 @@ struct srcu_data {unsigned long srcu_unlock_count[2 ];/* Unlocks per CPU. */ /* Update-side state. */ -raw_spinlock_t __private lock____cacheline_internodealigned_in_smp ; +spinlock_t __private lock____cacheline_internodealigned_in_smp ;struct rcu_segcblist srcu_cblist;/* List of callbacks.*/ unsigned long srcu_gp_seq_needed;/* Furthest future GP needed. */ unsigned long srcu_gp_seq_needed_exp;/* Furthest future exp GP. */
@@ -58,7 +58,7 @@ struct srcu_data {* Node in SRCU combining tree, similar in function to rcu_data. */ struct srcu_node { -raw_spinlock_t __private lock; +spinlock_t __private lock;unsigned long srcu_have_cbs[4 ];/* GP seq for children */ /* having CBs, but only */ /* is > ->srcu_gq_seq. */
@@ -78,7 +78,7 @@ struct srcu_struct {struct srcu_node *level[RCU_NUM_LVLS +1 ];/* First node at each level. */ struct mutex srcu_cb_mutex;/* Serialize CB preparation. */ -raw_spinlock_t __private lock;/* Protect counters */ +spinlock_t __private lock;/* Protect counters */ struct mutex srcu_gp_mutex;/* Serialize GP work. */ unsigned int srcu_idx;/* Current rdr array element. */ unsigned long srcu_gp_seq;/* Grace-period seq #. */
@@ -107,7 +107,7 @@ struct srcu_struct {#define __SRCU_STRUCT_INIT (name) \ { \ .sda = &name##_srcu_data, \ - .lock = __RAW_SPIN_LOCK_UNLOCKED(name.lock), \ + .lock = __SPIN_LOCK_UNLOCKED(name.lock), \ .srcu_gp_seq_needed = 0 - 1, \ __SRCU_DEP_MAP_INIT(name) \ }
kernel/rcu/srcutree.c +68 -41
317 lines not on this page · raw message
Signature · 1 line
2.5.2
PATCH 6/6 rcu: Remove have_rcu_nocb_mask from tree_plugin.h
In reply to Paul E. McKenneyFrom: Rakib Mullick <[email protected]>
Currently have_rcu_nocb_mask is used to avoid double allocation of rcu_nocb_mask during boot up. Due to different representation of cpumask_var_t on different kernel config CPUMASK=y(or n) it was okay. But now we have a helper cpumask_available(), which can be utilized to check whether rcu_nocb_mask has been allocated or not without using a variable.
Removing the variable also reduces vmlinux size.
Unpatched version:
text data bss dec hex filename
13050393 7852470 14543408 35446271 21cddff vmlinux
Patched version:
text data bss dec hex filename
13050390 7852438 14543408 35446236 21cdddc vmlinux
--- kernel/rcu/tree_plugin.h | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
kernel/rcu/tree_plugin.h +4 -7
@@ -61,7 +61,6 @@ DEFINE_PER_CPU(char, rcu_cpu_has_work);#ifdef CONFIG_RCU_NOCB_CPU static cpumask_var_t rcu_nocb_mask;/* CPUs to have callbacks offloaded. */ -static bool have_rcu_nocb_mask;/* Was rcu_nocb_mask allocated? */ static bool __read_mostly rcu_nocb_poll;/* Offload kthread are to poll. */ #endif /* #ifdef CONFIG_RCU_NOCB_CPU */
@@ -1752,7 +1751,6 @@ static void increment_cpu_stall_ticks(void)static int __init rcu_nocb_setup (char *str) {alloc_bootmem_cpumask_var (&rcu_nocb_mask); - have_rcu_nocb_mask = true;cpulist_parse (str, rcu_nocb_mask);return 1 ; }
@@ -1801,7 +1799,7 @@ static void rcu_init_one_nocb(struct rcu_node *rnp)/* Is the specified CPU a no-CBs CPU? */ bool rcu_is_nocb_cpu (int cpu) { -if (have_rcu_nocb_mask) +if (cpumask_available (rcu_nocb_mask))return cpumask_test_cpu (cpu, rcu_nocb_mask);return false; }
@@ -2295,14 +2293,13 @@ void __init rcu_init_nohz(void) need_rcu_nocb_mask = true;#endif /* #if defined(CONFIG_NO_HZ_FULL) */ -if (!have_rcu_nocb_mask && need_rcu_nocb_mask) { +if (!cpumask_available (rcu_nocb_mask) && need_rcu_nocb_mask) {if (!zalloc_cpumask_var (&rcu_nocb_mask,GFP_KERNEL )) {pr_info ("rcu_nocb_mask allocation failed, callback offloading disabled.\n" );return ; } - have_rcu_nocb_mask = true; } -if (!have_rcu_nocb_mask) +if (!cpumask_available (rcu_nocb_mask))return ;#if defined(CONFIG_NO_HZ_FULL )
@@ -2428,7 +2425,7 @@ static void __init rcu_organize_nocb_kthreads(struct rcu_state *rsp)struct rcu_data *rdp_leader =NULL ;/* Suppress misguided gcc warn. */ struct rcu_data *rdp_prev =NULL ; -if (!have_rcu_nocb_mask) +if (!cpumask_available (rcu_nocb_mask))return ;if (ls ==-1 ) { ls =int_sqrt (nr_cpu_ids);
Signature · 1 line
2.5.2
PATCH 3/6 rcu: Change printk() %p to %pK
In reply to Paul E. McKenneyThis commit changes the %p printk() in rcutorture to %pK. This could be considered irrelevant, given that any user able to start rcutorture could inflict far heavier damage on the system, but it doesn't hurt to change it and doing so avoids script-generated noise.
--- kernel/rcu/tree.c | 4 ++-- kernel/rcu/tree_plugin.h | 2 +- kernel/rcu/update.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
kernel/rcu/tree.c +2 -2
@@ -3103,7 +3103,7 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func,* Use rcu:rcu_callback trace event to find the previous * time callback was passed to __call_rcu(). */ -WARN_ONCE (1 ,"__call_rcu(): Double-freed CB %p->%pF()!!!\n" , +WARN_ONCE (1 ,"__call_rcu(): Double-freed CB %pK->%pF()!!!\n" , head, head->func);WRITE_ONCE (head->func, rcu_leak_callback);return ;
@@ -3985,7 +3985,7 @@ static void rcu_migrate_callbacks(int cpu, struct rcu_state *rsp)raw_spin_unlock_irqrestore_rcu_node (rnp_root, flags);WARN_ONCE (rcu_segcblist_n_cbs (&rdp->cblist) !=0 || !rcu_segcblist_empty (&rdp->cblist), -"rcu_cleanup_dead_cpu: Callbacks on offline CPU %d: qlen=%lu, 1stCB=%p\n" , +"rcu_cleanup_dead_cpu: Callbacks on offline CPU %d: qlen=%lu, 1stCB=%pK\n" , cpu,rcu_segcblist_n_cbs (&rdp->cblist),rcu_segcblist_first_cb (&rdp->cblist)); }
kernel/rcu/tree_plugin.h +1 -1
@@ -1899,7 +1899,7 @@ static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu)if (!READ_ONCE (rdp->nocb_kthread) && rhp && rcu_scheduler_fully_active) {/* RCU callback enqueued before CPU first came online??? */ -pr_err ("RCU: Never-onlined no-CBs CPU %d has CB %p\n" , +pr_err ("RCU: Never-onlined no-CBs CPU %d has CB %pK\n" , cpu, rhp->func);WARN_ON_ONCE (1 ); }
kernel/rcu/update.c +1 -1
@@ -696,7 +696,7 @@ static void check_holdout_task(struct task_struct *t, *firstreport = false; } cpu =task_cpu (t); - pr_alert("%p: %c%c nvcsw: %lu/%lu holdout: %d idle_cpu: %d/%d\n", + pr_alert("%pK: %c%c nvcsw: %lu/%lu holdout: %d idle_cpu: %d/%d\n", t, ".I"[is_idle_task(t)], "N."[cpu < 0 || !tick_nohz_full_cpu(cpu)], t->rcu_tasks_nvcsw, t->nvcsw, t->rcu_tasks_holdout,
Signature · 1 line
2.5.2
PATCH 4/6 tracing, rcu: Hide trace event rcu_nocb_wake when not used
In reply to Paul E. McKenneyFrom: "Steven Rostedt (VMware)" <[email protected]>
The trace event rcu_nocb_wake is only used when CONFIG_RCU_NOCB_CPU is defined. But the trace event is defined regardless. As defined trace events take up memory, it is a waste to have it defined when not used. Surround the trace event with an #ifdef to have it only defined when it is used.
--- include/trace/events/rcu.h | 2 ++ 1 file changed, 2 insertions(+)
include/trace/events/rcu.h +2 -0
@@ -243,6 +243,7 @@ TRACE_EVENT(rcu_exp_funnel_lock, __entry->grphi, __entry->gpevent) ); +#ifdef CONFIG_RCU_NOCB_CPU /* * Tracepoint for RCU no-CBs CPU callback handoffs. This event is intended * to assist debugging of these handoffs.
@@ -285,6 +286,7 @@ TRACE_EVENT(rcu_nocb_wake,TP_printk ("%s %d %s" , __entry->rcuname, __entry->cpu, __entry->reason) ); +#endif /* * Tracepoint for tasks blocking within preemptible-RCU read-side
Signature · 1 line
2.5.2
Re: [PATCH tip/core/rcu 2/6] srcu: Change printk() %p to %pK
In reply to Paul E. McKenneyQuoted 7 lines · Paul E. McKenney
This commit changes the %p
printk()in rcutorture to %pK. This could be considered irrelevant, given that any user able to start rcutorture could inflict far heavier damage on the system, but it doesn't hurt to change it and doing so avoids script-generated noise.Reported-by Tobin C. Harding <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>
Quoted 20 lines
--- kernel/rcu/srcutree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)kernel/rcu/srcutree.c +1
-2@@ -386,7 +386,7 @@ void cleanup_srcu_struct(struct srcu_struct *sp)flush_delayed_work (&per_cpu_ptr (sp->sda, cpu)->work);if (WARN_ON (rcu_seq_state (READ_ONCE (sp->srcu_gp_seq)) !=SRCU_STATE_IDLE ) ||WARN_ON (srcu_readers_active (sp))) { -pr_info ("cleanup_srcu_struct: Active srcu_struct %p state: %d\n" , sp,rcu_seq_state (READ_ONCE (sp->srcu_gp_seq))); +pr_info ("cleanup_srcu_struct: Active srcu_struct %pK state: %d\n" , sp,rcu_seq_state (READ_ONCE (sp->srcu_gp_seq)));return ;/* Caller forgot to stop doing call_srcu()? */ }free_percpu (sp->sda); --2.5.2
Paul E. McKenney
AppliedRe: [PATCH tip/core/rcu 2/6] srcu: Change printk() %p to %pK
Quoted 11 lines · Lai Jiangshan
Quoted 7 lines · Paul E. McKenney
This commit changes the %p
printk()in rcutorture to %pK. This could be considered irrelevant, given that any user able to start rcutorture could inflict far heavier damage on the system, but it doesn't hurt to change it and doing so avoids script-generated noise.Reported-by Tobin C. Harding <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>Reviewed-by Lai Jiangshan <[email protected]>
Applied, thank you for the review!
Thanx, Paul
Quoted 21 lines
Quoted 20 lines
--- kernel/rcu/srcutree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)kernel/rcu/srcutree.c +1
-2@@ -386,7 +386,7 @@ void cleanup_srcu_struct(struct srcu_struct *sp)flush_delayed_work (&per_cpu_ptr (sp->sda, cpu)->work);if (WARN_ON (rcu_seq_state (READ_ONCE (sp->srcu_gp_seq)) !=SRCU_STATE_IDLE ) ||WARN_ON (srcu_readers_active (sp))) { -pr_info ("cleanup_srcu_struct: Active srcu_struct %p state: %d\n" , sp,rcu_seq_state (READ_ONCE (sp->srcu_gp_seq))); +pr_info ("cleanup_srcu_struct: Active srcu_struct %pK state: %d\n" , sp,rcu_seq_state (READ_ONCE (sp->srcu_gp_seq)));return ;/* Caller forgot to stop doing call_srcu()? */ }free_percpu (sp->sda); --2.5.2