Miscellaneous fixes for 4.11

This commit switches RCU suspicious-access splats use pr_err() instead of the current INFO printk()s. This change makes it easier to automatically classify splats.

Reported-by Dmitry Vyukov <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

 kernel/locking/lockdep.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

kernel/locking/lockdep.c +0 -0

index 7c38f8f3d97b..844cd04bb453 100644

kernel/locking/lockdep.c +4 -4

 	print_kernel_ident();
-	printk("-------------------------------\n");
-	printk("%s:%d %s!\n", file, line, s);
-	printk("\nother info that might help us debug this:\n\n");
-	printk("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
+	pr_err("-------------------------------\n");
+	pr_err("%s:%d %s!\n", file, line, s);
+	pr_err("\nother info that might help us debug this:\n\n");
+	pr_err("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
 	       !rcu_lockdep_current_cpu_online()
 			? "RCU used illegally from offline CPU!\n"
 			: !rcu_is_watching()

Signature · 1 line

2.5.2

Quoted 6 lines · Paul E. McKenney

This commit switches RCU suspicious-access splats use pr_err() instead of the current INFO printk()s. This change makes it easier to automatically classify splats.

Reported-by Dmitry Vyukov <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

One comment below.

Quoted 8 lines

 kernel/locking/lockdep.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

kernel/locking/lockdep.c +0 -0

index 7c38f8f3d97b..844cd04bb453 100644

kernel/locking/lockdep.c void lockdep_rcu_suspicious(const char *file, const int line, const char *s) · line 4416

Quoted 7 lines

kernel/locking/lockdep.c +2 -2

@@ -4412,13 +4412,13 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
 #endif /* #ifdef CONFIG_PROVE_RCU_REPEATEDLY */
 	/* Note: the following can be executed concurrently, so be careful. */
 	printk("\n");
-	printk("===============================\n");
-	printk("[ INFO: suspicious RCU usage. ]\n");
+	pr_err("===============================\n");
+	pr_err("suspicious RCU usage. ]\n");

This message change seems unrelated, and potentially unintentional.

Quoted 15 lines

kernel/locking/lockdep.c +4 -4

 	print_kernel_ident();
-	printk("-------------------------------\n");
-	printk("%s:%d %s!\n", file, line, s);
-	printk("\nother info that might help us debug this:\n\n");
-	printk("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
+	pr_err("-------------------------------\n");
+	pr_err("%s:%d %s!\n", file, line, s);
+	pr_err("\nother info that might help us debug this:\n\n");
+	pr_err("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
 	       !rcu_lockdep_current_cpu_online()
 			? "RCU used illegally from offline CPU!\n"
 			: !rcu_is_watching()

Signature · 1 line

2.5.2

This commit switches RCU suspicious-access splats use pr_err() instead of the current INFO printk()s. This change makes it easier to automatically classify splats.

Reported-by Dmitry Vyukov <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/locking/lockdep.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

kernel/locking/lockdep.c +2 -3

index 7c38f8f3d97b..844cd04bb453 100644
@@ -4412,13 +4412,13 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
 #endif /* #ifdef CONFIG_PROVE_RCU_REPEATEDLY */
 	/* Note: the following can be executed concurrently, so be careful. */
 	printk("\n");
-	printk("===============================\n");
-	printk("[ INFO: suspicious RCU usage. ]\n");
+	pr_err("===============================\n");
+	pr_err("suspicious RCU usage. ]\n");
 	print_kernel_ident();
-	printk("-------------------------------\n");

Quoted 30 lines · Paul E. McKenney

This commit switches RCU suspicious-access splats use pr_err() instead of the current INFO printk()s. This change makes it easier to automatically classify splats.

Reported-by Dmitry Vyukov <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/locking/lockdep.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

kernel/locking/lockdep.c +5 -6

index 7c38f8f3d97b..844cd04bb453 100644
@@ -4412,13 +4412,13 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
 #endif /* #ifdef CONFIG_PROVE_RCU_REPEATEDLY */
 	/* Note: the following can be executed concurrently, so be careful. */
 	printk("\n");
-	printk("===============================\n");
-	printk("[ INFO: suspicious RCU usage. ]\n");
+	pr_err("===============================\n");
+	pr_err("suspicious RCU usage. ]\n");
 	print_kernel_ident();
-	printk("-------------------------------\n");
-	printk("%s:%d %s!\n", file, line, s);
-	printk("\nother info that might help us debug this:\n\n");
-	printk("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
+	pr_err("-------------------------------\n");
+	pr_err("%s:%d %s!\n", file, line, s);
+	pr_err("\nother info that might help us debug this:\n\n");

kernel/locking/lockdep.c void lockdep_rcu_suspicious(const char *file, const int line, const char *s) · line 4424

Quoted 4 lines

kernel/locking/lockdep.c +1 -0

+	pr_err("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
 	       !rcu_lockdep_current_cpu_online()
 			? "RCU used illegally from offline CPU!\n"
 			: !rcu_is_watching()

This makes it inconsistent with the rest of lockdep; why are these more important?

Commit d2db185bfee8 ("rcu: Remove short-term CPU kicking") removed frequent calls to resched_cpu(), which means that the only time resched_cpu() is invoked is after an RCU CPU stall warning. Although this is good from an avoid-IPIs perspective, we should try to break things loose -before- splatting. This commit therefore starts invoking resched_cpu() for each holdout at each force-quiescent-state interval that is more than halfway through the stall-warning interval.

Signed-off-by Paul E. McKenney <[email protected]>

 kernel/rcu/tree.c | 7 +++++++
 1 file changed, 7 insertions(+)

kernel/rcu/tree.c +7 -0

index 3c0d6c129718..5a4aaad75e76 100644
@@ -1220,6 +1220,13 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
 		rdp->rsp->jiffies_resched += 5; /* Re-enable beating. */
 	}
 
+	/*
+	 * If more than halfway to RCU CPU stall-warning time, do
+	 * a resched_cpu() to try to loosen things up a bit.
+	 */
+	if (jiffies - rdp->rsp->gp_start > rcu_jiffies_till_stall_check() / 2)
+		resched_cpu(rdp->cpu);
+
 	return 0;
 }
 

Signature · 1 line

2.5.2

Quoted 8 lines · Paul E. McKenney

Commit d2db185bfee8 ("rcu: Remove short-term CPU kicking") removed frequent calls to resched_cpu(), which means that the only time resched_cpu() is invoked is after an RCU CPU stall warning. Although this is good from an avoid-IPIs perspective, we should try to break things loose -before- splatting. This commit therefore starts invoking resched_cpu() for each holdout at each force-quiescent-state interval that is more than halfway through the stall-warning interval.

Quoted 1 line

Signed-off-by Paul E. McKenney <[email protected]>

Could you please reverse the order of patches 7 and 8, so that unnecessary splats don't occur between the two?

For the patch itself:

Reviewed-by Josh Triplett <[email protected]>

Quoted 24 lines

 kernel/rcu/tree.c | 7 +++++++
 1 file changed, 7 insertions(+)

kernel/rcu/tree.c +7 -0

index 3c0d6c129718..5a4aaad75e76 100644
@@ -1220,6 +1220,13 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
 		rdp->rsp->jiffies_resched += 5; /* Re-enable beating. */
 	}
 
+	/*
+	 * If more than halfway to RCU CPU stall-warning time, do
+	 * a resched_cpu() to try to loosen things up a bit.
+	 */
+	if (jiffies - rdp->rsp->gp_start > rcu_jiffies_till_stall_check() / 2)
+		resched_cpu(rdp->cpu);
+
 	return 0;
 }
 

Signature · 1 line

2.5.2

Under some circumstances, RCU grace periods are zero cost. For RCU-preempt, this is the case during boot, and for RCU-bh and RCU-sched, this is the case if there is only one CPU. This means that RCU users might wish to dispense with grace-period-avoidance strategies when grace periods are zero cost, so this commit adds rcu_trivial_gp(), rcu_bh_trivial_gp(), and rcu_sched_trivial_gp() to test for these conditions. Because the conditions leading to zero-cost grace periods can change at any time (for example, when a second CPU is onlined), these functions should be used as performance hints, and must not be relied on for correctness. For example, even if rcu_trivial_gp() returns true, you are required to invoke synchronize_rcu().

Signed-off-by Paul E. McKenney <[email protected]>

 include/linux/rcupdate.h |  8 ++++++++
 include/linux/rcutiny.h  | 10 ++++++++++
 include/linux/rcutree.h  |  2 ++
 kernel/rcu/tree.c        | 24 ++++++++++++++++++++++++
 kernel/rcu/tree_plugin.h | 11 +++++++++++
 5 files changed, 55 insertions(+)

include/linux/rcupdate.h +8 -0

index 01f71e1d2e94..a6222478b87d 100644
@@ -293,6 +293,7 @@ void __rcu_read_lock(void);
 void __rcu_read_unlock(void);
 void rcu_read_unlock_special(struct task_struct *t);
 void synchronize_rcu(void);
+bool rcu_trivial_gp(void);
 
 /*
  * Defined as a macro as it is a very low level header included from

@@ -448,6 +449,13 @@ bool __rcu_is_watching(void);
 #define RCU_SCHEDULER_INIT	1
 #define RCU_SCHEDULER_RUNNING	2
 
+#ifndef CONFIG_PREEMPT_RCU
+static inline bool rcu_trivial_gp(void)
+{
+	return rcu_sched_trivial_gp();
+}
+#endif /* #ifndef CONFIG_PREEMPT_RCU */
+
 /*
  * init_rcu_head_on_stack()/destroy_rcu_head_on_stack() are needed for dynamic
  * initialization and destruction of rcu_head on the stack. rcu_head structures

include/linux/rcutiny.h +10 -0

index ac81e4063b40..a77dafe79813 100644
@@ -82,6 +82,16 @@ static inline void synchronize_sched_expedited(void)
 	synchronize_sched();
 }
 
+static inline bool rcu_sched_trivial_gp(void)
+{
+	return true;
+}
+
+static inline bool rcu_bh_trivial_gp(void)
+{
+	return true;
+}
+
 static inline void kfree_call_rcu(struct rcu_head *head,
 				  rcu_callback_t func)
 {

include/linux/rcutree.h +2 -0

index 63a4e4cf40a5..fcd61cb08851 100644
@@ -47,6 +47,8 @@ static inline void rcu_virt_note_context_switch(int cpu)
 void synchronize_rcu_bh(void);
 void synchronize_sched_expedited(void);
 void synchronize_rcu_expedited(void);
+bool rcu_sched_trivial_gp(void);
+bool rcu_bh_trivial_gp(void);
 
 void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
 

kernel/rcu/tree.c +24 -0

index d7b63b88434b..ed5a17aca281 100644
@@ -3293,6 +3293,18 @@ void synchronize_sched(void)
 EXPORT_SYMBOL_GPL(synchronize_sched);
 
 /**
+ * rcu_sched_trivial_gp - Are RCU-sched grace periods trivially zero cost?
+ *
+ * Returns true if RCU-sched grace periods are currently zero cost, which
+ * they are if there is only one CPU.  Note that unless you take steps to
+ * prevent it, the number of CPUs might change at any time.
+ */
+bool rcu_sched_trivial_gp(void)
+{
+	return rcu_blocking_is_gp();
+}
+
+/**
  * synchronize_rcu_bh - wait until an rcu_bh grace period has elapsed.
  *
  * Control will return to the caller some time after a full rcu_bh grace

@@ -3320,6 +3332,18 @@ void synchronize_rcu_bh(void)
 EXPORT_SYMBOL_GPL(synchronize_rcu_bh);
 
 /**
+ * rcu_bh_trivial_gp - Are RCU-bh grace periods trivially zero cost?
+ *
+ * Returns true if RCU-bh grace periods are currently zero cost, which
+ * they are if there is only one CPU.  Note that unless you take steps to
+ * prevent it, the number of CPUs might change at any time.
+ */
+bool rcu_bh_trivial_gp(void)
+{
+	return rcu_blocking_is_gp();
+}
+
+/**
  * get_state_synchronize_rcu - Snapshot current RCU state
  *
  * Returns a cookie that is used by a later call to cond_synchronize_rcu()

kernel/rcu/tree_plugin.h +11 -0

index 56583e764ebf..e92d67a3fad2 100644
@@ -680,6 +680,17 @@ void synchronize_rcu(void)
 EXPORT_SYMBOL_GPL(synchronize_rcu);
 
 /**
+ * rcu_trivial_gp - Are RCU grace periods trivially zero cost?
+ *
+ * Returns true if RCU grace periods are currently zero cost, which
+ * they are during boot.
+ */
+bool rcu_trivial_gp(void)
+{
+	return rcu_scheduler_active == RCU_SCHEDULER_INACTIVE;
+}
+
+/**
  * rcu_barrier - Wait until all in-flight call_rcu() callbacks complete.
  *
  * Note that this primitive does not necessarily wait for an RCU grace period

Signature · 1 line

2.5.2

Quoted 12 lines · Paul E. McKenney

Under some circumstances, RCU grace periods are zero cost. For RCU-preempt, this is the case during boot, and for RCU-bh and RCU-sched, this is the case if there is only one CPU. This means that RCU users might wish to dispense with grace-period-avoidance strategies when grace periods are zero cost, so this commit adds rcu_trivial_gp(), rcu_bh_trivial_gp(), and rcu_sched_trivial_gp() to test for these conditions. Because the conditions leading to zero-cost grace periods can change at any time (for example, when a second CPU is onlined), these functions should be used as performance hints, and must not be relied on for correctness. For example, even if rcu_trivial_gp() returns true, you are required to invoke synchronize_rcu().

Quoted 1 line

Signed-off-by Paul E. McKenney <[email protected]>

Do you have anything planned that uses these functions?

Exposing this information, rather than just letting callers call synchronize_rcu() and sometimes get "free" grace periods, seems potentially error-prone.

If you keep these functions, please expand the comments above them to explicitly include the explanation in the commit message, and specifically that you must always call the corresponding synchronize function even if these functions return true.

Quoted 134 lines

 include/linux/rcupdate.h |  8 ++++++++
 include/linux/rcutiny.h  | 10 ++++++++++
 include/linux/rcutree.h  |  2 ++
 kernel/rcu/tree.c        | 24 ++++++++++++++++++++++++
 kernel/rcu/tree_plugin.h | 11 +++++++++++
 5 files changed, 55 insertions(+)

include/linux/rcupdate.h +8 -0

index 01f71e1d2e94..a6222478b87d 100644
@@ -293,6 +293,7 @@ void __rcu_read_lock(void);
 void __rcu_read_unlock(void);
 void rcu_read_unlock_special(struct task_struct *t);
 void synchronize_rcu(void);
+bool rcu_trivial_gp(void);
 
 /*
  * Defined as a macro as it is a very low level header included from

@@ -448,6 +449,13 @@ bool __rcu_is_watching(void);
 #define RCU_SCHEDULER_INIT	1
 #define RCU_SCHEDULER_RUNNING	2
 
+#ifndef CONFIG_PREEMPT_RCU
+static inline bool rcu_trivial_gp(void)
+{
+	return rcu_sched_trivial_gp();
+}
+#endif /* #ifndef CONFIG_PREEMPT_RCU */
+
 /*
  * init_rcu_head_on_stack()/destroy_rcu_head_on_stack() are needed for dynamic
  * initialization and destruction of rcu_head on the stack. rcu_head structures

include/linux/rcutiny.h +10 -0

index ac81e4063b40..a77dafe79813 100644
@@ -82,6 +82,16 @@ static inline void synchronize_sched_expedited(void)
 	synchronize_sched();
 }
 
+static inline bool rcu_sched_trivial_gp(void)
+{
+	return true;
+}
+
+static inline bool rcu_bh_trivial_gp(void)
+{
+	return true;
+}
+
 static inline void kfree_call_rcu(struct rcu_head *head,
 				  rcu_callback_t func)
 {

include/linux/rcutree.h +2 -0

index 63a4e4cf40a5..fcd61cb08851 100644
@@ -47,6 +47,8 @@ static inline void rcu_virt_note_context_switch(int cpu)
 void synchronize_rcu_bh(void);
 void synchronize_sched_expedited(void);
 void synchronize_rcu_expedited(void);
+bool rcu_sched_trivial_gp(void);
+bool rcu_bh_trivial_gp(void);
 
 void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
 

kernel/rcu/tree.c +24 -0

index d7b63b88434b..ed5a17aca281 100644
@@ -3293,6 +3293,18 @@ void synchronize_sched(void)
 EXPORT_SYMBOL_GPL(synchronize_sched);
 
 /**
+ * rcu_sched_trivial_gp - Are RCU-sched grace periods trivially zero cost?
+ *
+ * Returns true if RCU-sched grace periods are currently zero cost, which
+ * they are if there is only one CPU.  Note that unless you take steps to
+ * prevent it, the number of CPUs might change at any time.
+ */
+bool rcu_sched_trivial_gp(void)
+{
+	return rcu_blocking_is_gp();
+}
+
+/**
  * synchronize_rcu_bh - wait until an rcu_bh grace period has elapsed.
  *
  * Control will return to the caller some time after a full rcu_bh grace

@@ -3320,6 +3332,18 @@ void synchronize_rcu_bh(void)
 EXPORT_SYMBOL_GPL(synchronize_rcu_bh);
 
 /**
+ * rcu_bh_trivial_gp - Are RCU-bh grace periods trivially zero cost?
+ *
+ * Returns true if RCU-bh grace periods are currently zero cost, which
+ * they are if there is only one CPU.  Note that unless you take steps to
+ * prevent it, the number of CPUs might change at any time.
+ */
+bool rcu_bh_trivial_gp(void)
+{
+	return rcu_blocking_is_gp();
+}
+
+/**
  * get_state_synchronize_rcu - Snapshot current RCU state
  *
  * Returns a cookie that is used by a later call to cond_synchronize_rcu()

kernel/rcu/tree_plugin.h +11 -0

index 56583e764ebf..e92d67a3fad2 100644
@@ -680,6 +680,17 @@ void synchronize_rcu(void)
 EXPORT_SYMBOL_GPL(synchronize_rcu);
 
 /**
+ * rcu_trivial_gp - Are RCU grace periods trivially zero cost?
+ *
+ * Returns true if RCU grace periods are currently zero cost, which
+ * they are during boot.
+ */
+bool rcu_trivial_gp(void)
+{
+	return rcu_scheduler_active == RCU_SCHEDULER_INACTIVE;
+}
+
+/**
  * rcu_barrier - Wait until all in-flight call_rcu() callbacks complete.
  *
  * Note that this primitive does not necessarily wait for an RCU grace period

Signature · 1 line

2.5.2

There is some confusion as to which of cond_resched() or cond_resched_rcu_qs() should be added to long in-kernel loops. This commit therefore eliminates the decision by adding RCU quiescent states to cond_resched().

Warning: This is a prototype. For example, it does not correctly handle Tasks RCU. Which is OK for the moment, given that no one actually uses Tasks RCU yet.

Reported-by Michal Hocko <[email protected]>

Not-yet-signed-off-by Paul E. McKenney <[email protected]>

Cc Peter Zijlstra <[email protected]>

 include/linux/sched.h | 3 ++-
 kernel/sched/core.c   | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

include/linux/sched.h +2 -1

index 4d1905245c7a..1531c48f56e2 100644
@@ -3352,10 +3352,11 @@ static inline int signal_pending_state(long state, struct task_struct *p)
  * cond_resched_lock() will drop the spinlock before scheduling,
  * cond_resched_softirq() will enable bhs before scheduling.
  */
+void rcu_all_qs(void);
 #ifndef CONFIG_PREEMPT
 extern int _cond_resched(void);
 #else
-static inline int _cond_resched(void) { return 0; }
+static inline int _cond_resched(void) { rcu_all_qs(); return 0; }
 #endif
 
 #define cond_resched() ({			\

kernel/sched/core.c +1 -0

index c56fb57f2991..b442f8918eb9 100644
@@ -4907,6 +4907,7 @@ int __sched _cond_resched(void)
 		preempt_schedule_common();
 		return 1;
 	}
+	rcu_all_qs();
 	return 0;
 }
 EXPORT_SYMBOL(_cond_resched);

Signature · 1 line

2.5.2

Quoted 5 lines · Paul E. McKenney

There is some confusion as to which of cond_resched() or cond_resched_rcu_qs() should be added to long in-kernel loops. This commit therefore eliminates the decision by adding RCU quiescent states to cond_resched().

Quoted 7 lines

Warning: This is a prototype. For example, it does not correctly handle Tasks RCU. Which is OK for the moment, given that no one actually uses Tasks RCU yet.

Reported-by Michal Hocko <[email protected]>

Not-yet-signed-off-by Paul E. McKenney <[email protected]>

Cc Peter Zijlstra <[email protected]>

Do you intend to merge this patch, or have you just posted it for review?

You may want to remove it from this 20-patch series and post it as a separate RFC PATCH, to ensure that it doesn't get merged while still a prototype.

Quoted 36 lines

 include/linux/sched.h | 3 ++-
 kernel/sched/core.c   | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

include/linux/sched.h +2 -1

index 4d1905245c7a..1531c48f56e2 100644
@@ -3352,10 +3352,11 @@ static inline int signal_pending_state(long state, struct task_struct *p)
  * cond_resched_lock() will drop the spinlock before scheduling,
  * cond_resched_softirq() will enable bhs before scheduling.
  */
+void rcu_all_qs(void);
 #ifndef CONFIG_PREEMPT
 extern int _cond_resched(void);
 #else
-static inline int _cond_resched(void) { return 0; }
+static inline int _cond_resched(void) { rcu_all_qs(); return 0; }
 #endif
 
 #define cond_resched() ({			\

kernel/sched/core.c +1 -0

index c56fb57f2991..b442f8918eb9 100644
@@ -4907,6 +4907,7 @@ int __sched _cond_resched(void)
 		preempt_schedule_common();
 		return 1;
 	}
+	rcu_all_qs();
 	return 0;
 }
 EXPORT_SYMBOL(_cond_resched);

Signature · 1 line

2.5.2

There is some confusion as to which of cond_resched() or cond_resched_rcu_qs() should be added to long in-kernel loops. This commit therefore eliminates the decision by adding RCU quiescent states to cond_resched().

Quoted 4 lines · Paul E. McKenney

There is some confusion as to which of cond_resched() or cond_resched_rcu_qs() should be added to long in-kernel loops. This commit therefore eliminates the decision by adding RCU quiescent states to cond_resched().

Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); invalid under preemptible RCU. Is it already?

Quoted 3 lines

Warning: This is a prototype. For example, it does not correctly handle Tasks RCU. Which is OK for the moment, given that no one actually uses Tasks RCU yet.

kernel/sched/core.c int __sched _cond_resched(void) · line 4912

Quoted 9 lines

kernel/sched/core.c +1 -0

@@ -4907,6 +4907,7 @@ int __sched _cond_resched(void)
 		preempt_schedule_common();
 		return 1;
 	}
+	rcu_all_qs();
 	return 0;
 }

Still not a real fan of this, it does make cond_resched() touch a bunch more cachelines, also, I suppose that if we're going to do this, we should make __cond_resched_lock() and __cond_resched_softirq() act similarly.

Quoted 8 lines · Peter Zijlstra

Quoted 4 lines · Paul E. McKenney

There is some confusion as to which of cond_resched() or cond_resched_rcu_qs() should be added to long in-kernel loops. This commit therefore eliminates the decision by adding RCU quiescent states to cond_resched().

Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); invalid under preemptible RCU. Is it already?

In theory, yes. In practice, I just tested it with preemption and lockdep enabled, and it didn't complain. If further testing finds complaints, we can either fix those uses (preferred) or revert this patch.

Quoted 4 lines

Quoted 3 lines

Warning: This is a prototype. For example, it does not correctly handle Tasks RCU. Which is OK for the moment, given that no one actually uses Tasks RCU yet.

Quoted 14 lines

kernel/sched/core.c int __sched _cond_resched(void) · line 4912

Quoted 9 lines

kernel/sched/core.c +1 -0

@@ -4907,6 +4907,7 @@ int __sched _cond_resched(void)
 		preempt_schedule_common();
 		return 1;
 	}
+	rcu_all_qs();
 	return 0;
 }

Still not a real fan of this, it does make cond_resched() touch a bunch more cachelines, also, I suppose that if we're going to do this, we should make __cond_resched_lock() and __cond_resched_softirq() act similarly.

Michal (now CCed) argues that having to distinguish between cond_resched() and cond_resched_rcu_qs() is overly burdensome. Michal?

Any thoughts on how we might remove this burden without the additional cache misses? I will take another look as well to see what could make it lower cost. There are probably ways... Would it make sense to have RCU maintain a need-rcu_all_qs() flage in the same cacheline as the __preempt_count? Perhaps throttling the writes to this flag from the RCU grace-period kthreads to once per 100 milliseconds or so?

Quoted 34 lines · Paul E. McKenney

Quoted 8 lines · Peter Zijlstra

Quoted 4 lines · Paul E. McKenney

There is some confusion as to which of cond_resched() or cond_resched_rcu_qs() should be added to long in-kernel loops. This commit therefore eliminates the decision by adding RCU quiescent states to cond_resched().

Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); invalid under preemptible RCU. Is it already?

In theory, yes. In practice, I just tested it with preemption and lockdep enabled, and it didn't complain. If further testing finds complaints, we can either fix those uses (preferred) or revert this patch.

Quoted 4 lines

Quoted 3 lines

Warning: This is a prototype. For example, it does not correctly handle Tasks RCU. Which is OK for the moment, given that no one actually uses Tasks RCU yet.

Quoted 14 lines

kernel/sched/core.c int __sched _cond_resched(void) · line 4912

Quoted 9 lines

kernel/sched/core.c +1 -0

@@ -4907,6 +4907,7 @@ int __sched _cond_resched(void)
 		preempt_schedule_common();
 		return 1;
 	}
+	rcu_all_qs();
 	return 0;
 }

Still not a real fan of this, it does make cond_resched() touch a bunch more cachelines, also, I suppose that if we're going to do this, we should make __cond_resched_lock() and __cond_resched_softirq() act similarly.

Quoted 2 lines

Michal (now CCed) argues that having to distinguish between cond_resched() and cond_resched_rcu_qs() is overly burdensome. Michal?

Yes, it is really not clear which one is meant to be in which context. I really do not see which cond_resched should be turned intto cond_resched_rcu_qs.

Quoted 6 lines

Any thoughts on how we might remove this burden without the additional cache misses? I will take another look as well to see what could make it lower cost. There are probably ways... Would it make sense to have RCU maintain a need-rcu_all_qs() flage in the same cacheline as the __preempt_count? Perhaps throttling the writes to this flag from the RCU grace-period kthreads to once per 100 milliseconds or so?

Can the stall detector simply request rescheduling when it gets dangerously close to the timeout?

Quoted 49 lines · Michal Hocko

Quoted 34 lines · Paul E. McKenney

Quoted 8 lines · Peter Zijlstra

Quoted 4 lines · Paul E. McKenney

There is some confusion as to which of cond_resched() or cond_resched_rcu_qs() should be added to long in-kernel loops. This commit therefore eliminates the decision by adding RCU quiescent states to cond_resched().

Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); invalid under preemptible RCU. Is it already?

In theory, yes. In practice, I just tested it with preemption and lockdep enabled, and it didn't complain. If further testing finds complaints, we can either fix those uses (preferred) or revert this patch.

Quoted 4 lines

Quoted 3 lines

Warning: This is a prototype. For example, it does not correctly handle Tasks RCU. Which is OK for the moment, given that no one actually uses Tasks RCU yet.

Quoted 14 lines

kernel/sched/core.c int __sched _cond_resched(void) · line 4912

Quoted 9 lines

kernel/sched/core.c +1 -0

@@ -4907,6 +4907,7 @@ int __sched _cond_resched(void)
 		preempt_schedule_common();
 		return 1;
 	}
+	rcu_all_qs();
 	return 0;
 }

Still not a real fan of this, it does make cond_resched() touch a bunch more cachelines, also, I suppose that if we're going to do this, we should make __cond_resched_lock() and __cond_resched_softirq() act similarly.

Quoted 2 lines

Michal (now CCed) argues that having to distinguish between cond_resched() and cond_resched_rcu_qs() is overly burdensome. Michal?

Yes, it is really not clear which one is meant to be in which context. I really do not see which cond_resched should be turned intto cond_resched_rcu_qs.

Quoted 6 lines

Any thoughts on how we might remove this burden without the additional cache misses? I will take another look as well to see what could make it lower cost. There are probably ways... Would it make sense to have RCU maintain a need-rcu_all_qs() flage in the same cacheline as the __preempt_count? Perhaps throttling the writes to this flag from the RCU grace-period kthreads to once per 100 milliseconds or so?

Quoted 2 lines

Can the stall detector simply request rescheduling when it gets dangerously close to the timeout?

It is quite possible that half of the stall timeout would be a better choice than my 100 milliseconds, but either way, there would be need for a flag or some such.

Quoted 50 lines · Paul E. McKenney

Quoted 49 lines · Michal Hocko

Quoted 34 lines · Paul E. McKenney

Quoted 8 lines · Peter Zijlstra

Quoted 4 lines · Paul E. McKenney

There is some confusion as to which of cond_resched() or cond_resched_rcu_qs() should be added to long in-kernel loops. This commit therefore eliminates the decision by adding RCU quiescent states to cond_resched().

Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); invalid under preemptible RCU. Is it already?

In theory, yes. In practice, I just tested it with preemption and lockdep enabled, and it didn't complain. If further testing finds complaints, we can either fix those uses (preferred) or revert this patch.

Quoted 4 lines

Quoted 3 lines

Warning: This is a prototype. For example, it does not correctly handle Tasks RCU. Which is OK for the moment, given that no one actually uses Tasks RCU yet.

Quoted 14 lines

kernel/sched/core.c int __sched _cond_resched(void) · line 4912

Quoted 9 lines

kernel/sched/core.c +1 -0

@@ -4907,6 +4907,7 @@ int __sched _cond_resched(void)
 		preempt_schedule_common();
 		return 1;
 	}
+	rcu_all_qs();
 	return 0;
 }

Still not a real fan of this, it does make cond_resched() touch a bunch more cachelines, also, I suppose that if we're going to do this, we should make __cond_resched_lock() and __cond_resched_softirq() act similarly.

Quoted 2 lines

Michal (now CCed) argues that having to distinguish between cond_resched() and cond_resched_rcu_qs() is overly burdensome. Michal?

Yes, it is really not clear which one is meant to be in which context. I really do not see which cond_resched should be turned intto cond_resched_rcu_qs.

Quoted 6 lines

Any thoughts on how we might remove this burden without the additional cache misses? I will take another look as well to see what could make it lower cost. There are probably ways... Would it make sense to have RCU maintain a need-rcu_all_qs() flage in the same cacheline as the __preempt_count? Perhaps throttling the writes to this flag from the RCU grace-period kthreads to once per 100 milliseconds or so?

Quoted 6 lines

Quoted 2 lines

Can the stall detector simply request rescheduling when it gets dangerously close to the timeout?

It is quite possible that half of the stall timeout would be a better choice than my 100 milliseconds, but either way, there would be need for a flag or some such.

E.g. set_tsk_need_resched() on the task currently running on a cpu which is preventing the rcu grace period for too long?

That would only require change to the stall detector and the cond_resched could be left alone completely.

Hello!

This series provides miscellaneous fixes:

1. Make RCU_EXPEDITE_BOOT be the default in order to speed up
boot, courtesy of Sebastian Andrzej Siewior.

2. Make RCU suspicious-access lockdep splats use pr_err().

3. Disable sys_membarrier when nohz_full is enabled, courtesy
of Mathieu Desnoyers.

4. Only dump stalled-tasks stacks if there was a real stall, courtesy
of Byungchul Park.

5. Remove unneeded rcu_process_callbacks() declarations.

6. Remove unused but set "mask" variable, courtesy of Tobias Klauser.

7. Remove short-term CPU kicking.

8. Add long-term CPU kicking.

9. Once again use NMI-based stack traces in stall warnings.

10. Add functions to test for trivial grace periods.

11. Make cond_resched() provide RCU quiescent state, which will
hopefully lead to the removal of cond_resched_rcu_qs().

12. Re-enable TASKS_RCU for User Mode Linux.

13. Don't wake rcuc/X kthreads on NOCB CPUs.

14. Add comment headers to expedited-grace-period counter functions.

15. Make rcu_cpu_starting() use its "cpu" argument.

16. Enable RCU tracepoints by default to aid in debugging, courtesy
of Matt Fleming.

17. Fix comment in rcu_organize_nocb_kthreads(), which no longer
spawns kthreads.

18. Clarify comments about when llist locking is needed, courtesy
of Joel Fernandes.

19. Eliminate unused expedited_normal counter.

20. Add lockdep checks to synchronous expedited primitives.

Quoted 47 lines · Paul E. McKenney

Hello!

This series provides miscellaneous fixes:

1. Make RCU_EXPEDITE_BOOT be the default in order to speed up
boot, courtesy of Sebastian Andrzej Siewior.

2. Make RCU suspicious-access lockdep splats use pr_err().

3. Disable sys_membarrier when nohz_full is enabled, courtesy
of Mathieu Desnoyers.

4. Only dump stalled-tasks stacks if there was a real stall, courtesy
of Byungchul Park.

5. Remove unneeded rcu_process_callbacks() declarations.

6. Remove unused but set "mask" variable, courtesy of Tobias Klauser.

7. Remove short-term CPU kicking.

8. Add long-term CPU kicking.

9. Once again use NMI-based stack traces in stall warnings.

10. Add functions to test for trivial grace periods.

11. Make cond_resched() provide RCU quiescent state, which will
hopefully lead to the removal of cond_resched_rcu_qs().

12. Re-enable TASKS_RCU for User Mode Linux.

13. Don't wake rcuc/X kthreads on NOCB CPUs.

14. Add comment headers to expedited-grace-period counter functions.

15. Make rcu_cpu_starting() use its "cpu" argument.

16. Enable RCU tracepoints by default to aid in debugging, courtesy
of Matt Fleming.

17. Fix comment in rcu_organize_nocb_kthreads(), which no longer
spawns kthreads.

18. Clarify comments about when llist locking is needed, courtesy
of Joel Fernandes.

Quoted 3 lines

19. Eliminate unused expedited_normal counter.

20. Add lockdep checks to synchronous expedited primitives.

I replied to patches 2, 8, 10, and 11 with comments. For the rest:

Reviewed-by Josh Triplett <[email protected]>

This commit switches RCU suspicious-access splats use pr_err() instead of the current INFO printk()s. This change makes it easier to automatically classify splats.

Reported-by Dmitry Vyukov <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/locking/lockdep.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

kernel/locking/lockdep.c +0 -0

index 7c38f8f3d97b..a74c0630172a 100644

kernel/locking/lockdep.c +4 -4

 	print_kernel_ident();
-	printk("-------------------------------\n");
-	printk("%s:%d %s!\n", file, line, s);
-	printk("\nother info that might help us debug this:\n\n");
-	printk("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
+	pr_err("-------------------------------\n");
+	pr_err("%s:%d %s!\n", file, line, s);
+	pr_err("\nother info that might help us debug this:\n\n");
+	pr_err("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
 	       !rcu_lockdep_current_cpu_online()
 			? "RCU used illegally from offline CPU!\n"
 			: !rcu_is_watching()

Signature · 1 line

2.5.2

Quoted 10 lines · Paul E. McKenney

This commit switches RCU suspicious-access splats use pr_err() instead of the current INFO printk()s. This change makes it easier to automatically classify splats.

Reported-by Dmitry Vyukov <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/locking/lockdep.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

kernel/locking/lockdep.c void lockdep_rcu_suspicious(const char *file, const int line, const char *s) · line 4416

Quoted 12 lines

kernel/locking/lockdep.c +2 -2

index 7c38f8f3d97b..a74c0630172a 100644
@@ -4412,13 +4412,13 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
 #endif /* #ifdef CONFIG_PROVE_RCU_REPEATEDLY */
 	/* Note: the following can be executed concurrently, so be careful. */
 	printk("\n");
-	printk("===============================\n");
-	printk("[ INFO: suspicious RCU usage. ]\n");
+	pr_err("===============================\n");
+	pr_err("[  suspicious RCU usage.      ]\n");

While re-adding the square bracket makes it symmetric, this change still seems odd, and unrelated to the switch to pr_err. You could change it to "ERR:" if you want, if "INFO:" feels inaccurate to you.

Quoted 15 lines

kernel/locking/lockdep.c +4 -4

 	print_kernel_ident();
-	printk("-------------------------------\n");
-	printk("%s:%d %s!\n", file, line, s);
-	printk("\nother info that might help us debug this:\n\n");
-	printk("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
+	pr_err("-------------------------------\n");
+	pr_err("%s:%d %s!\n", file, line, s);
+	pr_err("\nother info that might help us debug this:\n\n");
+	pr_err("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
 	       !rcu_lockdep_current_cpu_online()
 			? "RCU used illegally from offline CPU!\n"
 			: !rcu_is_watching()

Signature · 1 line

2.5.2

Quoted 11 lines · Josh Triplett

Quoted 10 lines · Paul E. McKenney

This commit switches RCU suspicious-access splats use pr_err() instead of the current INFO printk()s. This change makes it easier to automatically classify splats.

Reported-by Dmitry Vyukov <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/locking/lockdep.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Quoted 16 lines

kernel/locking/lockdep.c void lockdep_rcu_suspicious(const char *file, const int line, const char *s) · line 4416

Quoted 12 lines

kernel/locking/lockdep.c +2 -2

index 7c38f8f3d97b..a74c0630172a 100644
@@ -4412,13 +4412,13 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
 #endif /* #ifdef CONFIG_PROVE_RCU_REPEATEDLY */
 	/* Note: the following can be executed concurrently, so be careful. */
 	printk("\n");
-	printk("===============================\n");
-	printk("[ INFO: suspicious RCU usage. ]\n");
+	pr_err("===============================\n");
+	pr_err("[  suspicious RCU usage.      ]\n");

While re-adding the square bracket makes it symmetric, this change still seems odd, and unrelated to the switch to pr_err. You could change it to "ERR:" if you want, if "INFO:" feels inaccurate to you.

So this would be OK?

	pr_err("[ ERR: suspicious RCU usage.  ]\n");

(Changed to this as a best guess, but please let me know.)

Quoted 29 lines · Paul E. McKenney

Quoted 27 lines · Josh Triplett

Quoted 15 lines · Paul E. McKenney

This commit switches RCU suspicious-access splats use pr_err() instead of the current INFO printk()s. This change makes it easier to automatically classify splats.

Reported-by Dmitry Vyukov <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/locking/lockdep.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

kernel/locking/lockdep.c +0 -0

index 7c38f8f3d97b..a74c0630172a 100644

kernel/locking/lockdep.c void lockdep_rcu_suspicious(const char *file, const int line, const char *s) · line 4416

Quoted 7 lines

kernel/locking/lockdep.c +2 -2

@@ -4412,13 +4412,13 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
 #endif /* #ifdef CONFIG_PROVE_RCU_REPEATEDLY */
 	/* Note: the following can be executed concurrently, so be careful. */
 	printk("\n");
-	printk("===============================\n");
-	printk("[ INFO: suspicious RCU usage. ]\n");
+	pr_err("===============================\n");
+	pr_err("[  suspicious RCU usage.      ]\n");

While re-adding the square bracket makes it symmetric, this change still seems odd, and unrelated to the switch to pr_err. You could change it to "ERR:" if you want, if "INFO:" feels inaccurate to you.

Quoted 5 lines

So this would be OK?

	pr_err("[ ERR: suspicious RCU usage.  ]\n");

(Changed to this as a best guess, but please let me know.)

Yeah, that seems fine. Sorry to nitpick this; it just otherwise seemed entirely unrelated to the rest of the change.

Commit d2db185bfee8 ("rcu: Remove short-term CPU kicking") removed frequent calls to resched_cpu(), which means that the only time resched_cpu() is invoked is after an RCU CPU stall warning. Although this is good from an avoid-IPIs perspective, we should try to break things loose -before- splatting. This commit therefore starts invoking resched_cpu() for each holdout at each force-quiescent-state interval that is more than halfway through the stall-warning interval.

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcu/tree.c | 6 ++++++
 1 file changed, 6 insertions(+)

kernel/rcu/tree.c +6 -0

index 83bf054e194e..0e61b62e3f4a 100644
@@ -1225,6 +1225,12 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
 			 rdp->rsp->gp_start + 2 * jiffies_till_sched_qs) ||
 	    ULONG_CMP_GE(jiffies, rdp->rsp->gp_start + jiffies_till_sched_qs))
 		resched_cpu(rdp->cpu);  /* Force CPU into scheduler. */
+	/*
+	 * If more than halfway to RCU CPU stall-warning time, do
+	 * a resched_cpu() to try to loosen things up a bit.
+	 */
+	if (jiffies - rdp->rsp->gp_start > rcu_jiffies_till_stall_check() / 2)
+		resched_cpu(rdp->cpu);
 
 	return 0;
 }

Signature · 1 line

2.5.2

Commit d2db185bfee8 ("rcu: Remove short-term CPU kicking") removed frequent calls to resched_cpu(), which means that the only time resched_cpu() is invoked is after an RCU CPU stall warning. Although this is good from an avoid-IPIs perspective, we should try to break things loose -before- splatting. This commit therefore starts invoking resched_cpu() for each holdout at each force-quiescent-state interval that is more than halfway through the stall-warning interval.

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcu/tree.c | 6 ++++++
 1 file changed, 6 insertions(+)

kernel/rcu/tree.c +6 -0

index 83bf054e194e..0e61b62e3f4a 100644
@@ -1225,6 +1225,12 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
 			 rdp->rsp->gp_start + 2 * jiffies_till_sched_qs) ||
 	    ULONG_CMP_GE(jiffies, rdp->rsp->gp_start + jiffies_till_sched_qs))
 		resched_cpu(rdp->cpu);  /* Force CPU into scheduler. */
+	/*
+	 * If more than halfway to RCU CPU stall-warning time, do
+	 * a resched_cpu() to try to loosen things up a bit.
+	 */
+	if (jiffies - rdp->rsp->gp_start > rcu_jiffies_till_stall_check() / 2)
+		resched_cpu(rdp->cpu);
 
 	return 0;
 }

Signature · 1 line

2.5.2

Quoted 7 lines · Paul E. McKenney

Commit d2db185bfee8 ("rcu: Remove short-term CPU kicking") removed frequent calls to resched_cpu(), which means that the only time resched_cpu() is invoked is after an RCU CPU stall warning. Although this is good from an avoid-IPIs perspective, we should try to break things loose -before- splatting. This commit therefore starts invoking resched_cpu() for each holdout at each force-quiescent-state interval that is more than halfway through the stall-warning interval.

Just realized an issue with this commit message: you're referring to what now appears as patch 8 in the past tense as something already done, and with a commit ID that probably doesn't work anymore. You need to rephrase this to describe how it leads to a *subsequent* change rather than fixing something already changed.

With the commit message fixed:

Reviewed-by Josh Triplett <[email protected]>

Quoted 28 lines

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcu/tree.c | 6 ++++++
 1 file changed, 6 insertions(+)

kernel/rcu/tree.c +6 -0

index 83bf054e194e..0e61b62e3f4a 100644
@@ -1225,6 +1225,12 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
 			 rdp->rsp->gp_start + 2 * jiffies_till_sched_qs) ||
 	    ULONG_CMP_GE(jiffies, rdp->rsp->gp_start + jiffies_till_sched_qs))
 		resched_cpu(rdp->cpu);  /* Force CPU into scheduler. */
+	/*
+	 * If more than halfway to RCU CPU stall-warning time, do
+	 * a resched_cpu() to try to loosen things up a bit.
+	 */
+	if (jiffies - rdp->rsp->gp_start > rcu_jiffies_till_stall_check() / 2)
+		resched_cpu(rdp->cpu);
 
 	return 0;
 }

Signature · 1 line

2.5.2

Quoted 9 lines · Josh Triplett

Quoted 7 lines · Paul E. McKenney

Commit d2db185bfee8 ("rcu: Remove short-term CPU kicking") removed frequent calls to resched_cpu(), which means that the only time resched_cpu() is invoked is after an RCU CPU stall warning. Although this is good from an avoid-IPIs perspective, we should try to break things loose -before- splatting. This commit therefore starts invoking resched_cpu() for each holdout at each force-quiescent-state interval that is more than halfway through the stall-warning interval.

Quoted 8 lines

Just realized an issue with this commit message: you're referring to what now appears as patch 8 in the past tense as something already done, and with a commit ID that probably doesn't work anymore. You need to rephrase this to describe how it leads to a *subsequent* change rather than fixing something already changed.

With the commit message fixed:

Reviewed-by Josh Triplett <[email protected]>

Good point! It now reads as follows:

------------------------------------------------------------------------
This commit prepares for the removal of short-term CPU kicking (in a subsequent commit). It does so by starting to invoke resched_cpu() for each holdout at each force-quiescent-state interval that is more than halfway through the stall-warning interval.
------------------------------------------------------------------------

Does that work?

Thanx, Paul

Quoted 29 lines

Quoted 28 lines

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcu/tree.c | 6 ++++++
 1 file changed, 6 insertions(+)

kernel/rcu/tree.c +6 -0

index 83bf054e194e..0e61b62e3f4a 100644
@@ -1225,6 +1225,12 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
 			 rdp->rsp->gp_start + 2 * jiffies_till_sched_qs) ||
 	    ULONG_CMP_GE(jiffies, rdp->rsp->gp_start + jiffies_till_sched_qs))
 		resched_cpu(rdp->cpu);  /* Force CPU into scheduler. */
+	/*
+	 * If more than halfway to RCU CPU stall-warning time, do
+	 * a resched_cpu() to try to loosen things up a bit.
+	 */
+	if (jiffies - rdp->rsp->gp_start > rcu_jiffies_till_stall_check() / 2)
+		resched_cpu(rdp->cpu);
 
 	return 0;
 }

Signature · 1 line

2.5.2

Commit 4914950aaa12d ("rcu: Stop treating in-kernel CPU-bound workloads

as errors") added a (relatively) short-timeout call to resched_cpu(). This was inspired by as issue that was fixed by b7e7ade34e61 ("sched/core:
Fix remote wakeups"). But given that this issue was fixed, it is time for the current commit to remove this call to resched_cpu().

Reported-by Byungchul Park <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

Reviewed-by Josh Triplett <[email protected]>

---
 kernel/rcu/tree.c | 5 -----
 1 file changed, 5 deletions(-)

kernel/rcu/tree.c +0 -5

index 0e61b62e3f4a..5a4aaad75e76 100644
@@ -1220,11 +1220,6 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
 		rdp->rsp->jiffies_resched += 5; /* Re-enable beating. */
 	}
 
-	/* And if it has been a really long time, kick the CPU as well. */
-	if (ULONG_CMP_GE(jiffies,
-			 rdp->rsp->gp_start + 2 * jiffies_till_sched_qs) ||
-	    ULONG_CMP_GE(jiffies, rdp->rsp->gp_start + jiffies_till_sched_qs))
-		resched_cpu(rdp->cpu);  /* Force CPU into scheduler. */
 	/*
 	 * If more than halfway to RCU CPU stall-warning time, do
 	 * a resched_cpu() to try to loosen things up a bit.

Signature · 1 line

2.5.2

Quoted 1 line · Paul E. McKenney

Commit 4914950aaa12d ("rcu: Stop treating in-kernel CPU-bound workloads

Does this commit ID still match what you have in your tree, post-rebase?

Quoted 31 lines

as errors") added a (relatively) short-timeout call to resched_cpu(). This was inspired by as issue that was fixed by b7e7ade34e61 ("sched/core:
Fix remote wakeups"). But given that this issue was fixed, it is time for the current commit to remove this call to resched_cpu().

Reported-by Byungchul Park <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

Reviewed-by Josh Triplett <[email protected]>

---
 kernel/rcu/tree.c | 5 -----
 1 file changed, 5 deletions(-)

kernel/rcu/tree.c +0 -5

index 0e61b62e3f4a..5a4aaad75e76 100644
@@ -1220,11 +1220,6 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
 		rdp->rsp->jiffies_resched += 5; /* Re-enable beating. */
 	}
 
-	/* And if it has been a really long time, kick the CPU as well. */
-	if (ULONG_CMP_GE(jiffies,
-			 rdp->rsp->gp_start + 2 * jiffies_till_sched_qs) ||
-	    ULONG_CMP_GE(jiffies, rdp->rsp->gp_start + jiffies_till_sched_qs))
-		resched_cpu(rdp->cpu);  /* Force CPU into scheduler. */
 	/*
 	 * If more than halfway to RCU CPU stall-warning time, do
 	 * a resched_cpu() to try to loosen things up a bit.

Signature · 1 line

2.5.2