[PATCH 0/3] tick: Fix softirq related warnings

[PATCH 0/3] tick: Fix softirq related warnings

[Frederic Weisbecker]

Some cleanups and debug rework.

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	timers/core

HEAD: 18b00369d58da4c73946d5f8ebed8e8c7ade89e2

Thanks,
	Frederic
---

Frederic Weisbecker (3):
      tick/rcu: Remove obsolete rcu_needs_cpu() parameters
      tick/rcu: Stop allowing RCU_SOFTIRQ in idle
      lib/irq_poll: Declare IRQ_POLL softirq vector as ksoftirqd-parking safe


 include/linux/interrupt.h | 11 ++++++++-
 include/linux/rcutiny.h   |  3 +--
 include/linux/rcutree.h   |  2 +-
 kernel/rcu/tree.c         |  3 +--
 kernel/time/tick-sched.c  | 60 ++++++++++++++++++++++++++++++++++-------------
 5 files changed, 57 insertions(+), 22 deletions(-)

[PATCH 1/3] tick/rcu: Remove obsolete rcu_needs_cpu() parameters

[Frederic Weisbecker]

With the removal of CONFIG_RCU_FAST_NO_HZ, the parameters in
rcu_needs_cpu() are not necessary anymore. Simply remove them.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>
---
 include/linux/rcutiny.h  |  3 +--
 include/linux/rcutree.h  |  2 +-
 kernel/rcu/tree.c        |  3 +--
 kernel/time/tick-sched.c | 10 ++++------
 4 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index 858f4d429946..5fed476f977f 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -64,9 +64,8 @@ static inline void rcu_softirq_qs(void)
 		rcu_tasks_qs(current, (preempt)); \
 	} while (0)
 
-static inline int rcu_needs_cpu(u64 basemono, u64 *nextevt)
+static inline int rcu_needs_cpu(void)
 {
-	*nextevt = KTIME_MAX;
 	return 0;
 }
 
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index 53209d669400..6cc91291d078 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -19,7 +19,7 @@
 
 void rcu_softirq_qs(void);
 void rcu_note_context_switch(bool preempt);
-int rcu_needs_cpu(u64 basem, u64 *nextevt);
+int rcu_needs_cpu(void);
 void rcu_cpu_stall_reset(void);
 
 /*
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index a4c25a6283b0..80faf2273ce9 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1086,9 +1086,8 @@ void rcu_irq_enter_irqson(void)
  * Just check whether or not this CPU has non-offloaded RCU callbacks
  * queued.
  */
-int rcu_needs_cpu(u64 basemono, u64 *nextevt)
+int rcu_needs_cpu(void)
 {
-	*nextevt = KTIME_MAX;
 	return !rcu_segcblist_empty(&this_cpu_ptr(&rcu_data)->cblist) &&
 		!rcu_rdp_is_offloaded(this_cpu_ptr(&rcu_data));
 }
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 17a283ce2b20..2abb5112feb9 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -768,7 +768,7 @@ static inline bool local_timer_softirq_pending(void)
 
 static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
 {
-	u64 basemono, next_tick, next_tmr, next_rcu, delta, expires;
+	u64 basemono, next_tick, delta, expires;
 	unsigned long basejiff;
 	unsigned int seq;
 
@@ -791,7 +791,7 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
 	 * minimal delta which brings us back to this place
 	 * immediately. Lather, rinse and repeat...
 	 */
-	if (rcu_needs_cpu(basemono, &next_rcu) || arch_needs_cpu() ||
+	if (rcu_needs_cpu() || arch_needs_cpu() ||
 	    irq_work_needs_cpu() || local_timer_softirq_pending()) {
 		next_tick = basemono + TICK_NSEC;
 	} else {
@@ -802,10 +802,8 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
 		 * disabled this also looks at the next expiring
 		 * hrtimer.
 		 */
-		next_tmr = get_next_timer_interrupt(basejiff, basemono);
-		ts->next_timer = next_tmr;
-		/* Take the next rcu event into account */
-		next_tick = next_rcu < next_tmr ? next_rcu : next_tmr;
+		next_tick = get_next_timer_interrupt(basejiff, basemono);
+		ts->next_timer = next_tick;
 	}
 
 	/*
-- 
2.25.1

[PATCH 2/3] tick/rcu: Stop allowing RCU_SOFTIRQ in idle

[Frederic Weisbecker]

RCU_SOFTIRQ used to be special in that it could be raised on purpose
within the idle path to prevent from stopping the tick. Some code still
prevents from unnecessary warnings related to this specific behaviour
while entering in dynticks-idle mode.

However the nohz layout has changed quite a bit in ten years, and the
removal of CONFIG_RCU_FAST_NO_HZ has been the final straw to this
safe-conduct. Now the RCU_SOFTIRQ vector is expected to be raised from
sane places.

A remaining corner case is admitted though when the vector is invoked
in fragile hotplug path.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>
---
 include/linux/interrupt.h |  8 ++++++-
 kernel/time/tick-sched.c  | 50 +++++++++++++++++++++++++++++++--------
 2 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 9367f1cb2e3c..9613326d2f8a 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -579,7 +579,13 @@ enum
 	NR_SOFTIRQS
 };
 
-#define SOFTIRQ_STOP_IDLE_MASK (~(1 << RCU_SOFTIRQ))
+/*
+ * Ignoring the RCU vector after ksoftirqd is parked is fine
+ * because:
+ * 	1) rcutree_migrate_callbacks() takes care of the queue.
+ * 	2) rcu_report_dead() reports the final quiescent states.
+ */
+#define SOFTIRQ_HOTPLUG_SAFE_MASK (BIT(RCU_SOFTIRQ))
 
 /* map softirq index to softirq name. update 'softirq_to_name' in
  * kernel/softirq.c when adding a new softirq.
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 2abb5112feb9..1e923f307f06 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -982,6 +982,45 @@ static void tick_nohz_full_update_tick(struct tick_sched *ts)
 	__tick_nohz_full_update_tick(ts, ktime_get());
 }
 
+/*
+ * A pending softirq outside an IRQ (or softirq disabled section) context
+ * should be waiting for ksoftirqd to handle it. Therefore we shouldn't
+ * reach here due to the need_resched() early check in can_stop_idle_tick().
+ *
+ * However if we are between CPUHP_AP_SMPBOOT_THREADS and CPU_TEARDOWN_CPU on the
+ * cpu_down() process, softirqs can still be raised while ksoftirqd is parked,
+ * triggering the below since wakep_softirqd() is ignored.
+ *
+ */
+static bool report_idle_softirq(void)
+{
+	static int ratelimit;
+	unsigned int pending = local_softirq_pending();
+
+	if (likely(!pending))
+		return false;
+
+	/* Some softirqs claim to be safe against hotplug and ksoftirqd parking */
+	if (!cpu_active(smp_processor_id())) {
+		pending &= ~SOFTIRQ_HOTPLUG_SAFE_MASK;
+		if (!pending)
+			return false;
+	}
+
+	if (ratelimit < 10)
+		return false;
+
+	/* On RT, softirqs handling may be waiting on some lock */
+	if (!local_bh_blocked())
+		return false;
+
+	pr_warn("NOHZ tick-stop error: local softirq work is pending, handler #%02x!!!\n",
+		pending);
+	ratelimit++;
+
+	return true;
+}
+
 static bool can_stop_idle_tick(int cpu, struct tick_sched *ts)
 {
 	/*
@@ -1008,17 +1047,8 @@ static bool can_stop_idle_tick(int cpu, struct tick_sched *ts)
 	if (need_resched())
 		return false;
 
-	if (unlikely(local_softirq_pending())) {
-		static int ratelimit;
-
-		if (ratelimit < 10 && !local_bh_blocked() &&
-		    (local_softirq_pending() & SOFTIRQ_STOP_IDLE_MASK)) {
-			pr_warn("NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #%02x!!!\n",
-				(unsigned int) local_softirq_pending());
-			ratelimit++;
-		}
+	if (unlikely(report_idle_softirq()))
 		return false;
-	}
 
 	if (tick_nohz_full_enabled()) {
 		/*
-- 
2.25.1

[PATCH 3/3] lib/irq_poll: Declare IRQ_POLL softirq vector as ksoftirqd-parking safe

[Frederic Weisbecker]

The following warning may appear while setting a CPU down:

	NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #20!!!

The IRQ_POLL_SOFTIRQ vector can be raised during the hotplug cpu_down()
path after ksoftirqd is parked and before the CPU actually dies. However
this is handled afterward at the CPUHP_IRQ_POLL_DEAD stage where the
queue gets migrated.

Hence this warning can be considered spurious and the vector can join
the "hotplug-safe" list.

Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>
---
 include/linux/interrupt.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 9613326d2f8a..f40754caaefa 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -580,12 +580,15 @@ enum
 };
 
 /*
- * Ignoring the RCU vector after ksoftirqd is parked is fine
- * because:
- * 	1) rcutree_migrate_callbacks() takes care of the queue.
+ * The following vectors can be safely ignored after ksoftirqd is parked:
+ *
+ * _ RCU:
+ * 	1) rcutree_migrate_callbacks() migrates the queue.
  * 	2) rcu_report_dead() reports the final quiescent states.
+ *
+ * _ IRQ_POLL: irq_poll_cpu_dead() migrates the queue
  */
-#define SOFTIRQ_HOTPLUG_SAFE_MASK (BIT(RCU_SOFTIRQ))
+#define SOFTIRQ_HOTPLUG_SAFE_MASK (BIT(RCU_SOFTIRQ) | BIT(IRQ_POLL_SOFTIRQ))
 
 /* map softirq index to softirq name. update 'softirq_to_name' in
  * kernel/softirq.c when adding a new softirq.
-- 
2.25.1

Re: [PATCH 0/3] tick: Fix softirq related warnings

[Paul E. McKenney]

On Tue, Feb 08, 2022 at 05:16:32PM +0100, Frederic Weisbecker wrote:
> Some cleanups and debug rework.
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> 	timers/core
> 
> HEAD: 18b00369d58da4c73946d5f8ebed8e8c7ade89e2
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (3):
>       tick/rcu: Remove obsolete rcu_needs_cpu() parameters
>       tick/rcu: Stop allowing RCU_SOFTIRQ in idle
>       lib/irq_poll: Declare IRQ_POLL softirq vector as ksoftirqd-parking safe

I have queued these for local testing, but I am guessing that you would
like them to go up some other path.

							Thanx, Paul

>  include/linux/interrupt.h | 11 ++++++++-
>  include/linux/rcutiny.h   |  3 +--
>  include/linux/rcutree.h   |  2 +-
>  kernel/rcu/tree.c         |  3 +--
>  kernel/time/tick-sched.c  | 60 ++++++++++++++++++++++++++++++++++-------------
>  5 files changed, 57 insertions(+), 22 deletions(-)

Re: [PATCH 0/3] tick: Fix softirq related warnings

[Paul Menzel]

Dear Frederic,


Am 08.02.22 um 17:16 schrieb Frederic Weisbecker:
> Some cleanups and debug rework.
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> 	timers/core
> 
> HEAD: 18b00369d58da4c73946d5f8ebed8e8c7ade89e2
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (3):
>        tick/rcu: Remove obsolete rcu_needs_cpu() parameters
>        tick/rcu: Stop allowing RCU_SOFTIRQ in idle
>        lib/irq_poll: Declare IRQ_POLL softirq vector as ksoftirqd-parking safe
> 
> 
>   include/linux/interrupt.h | 11 ++++++++-
>   include/linux/rcutiny.h   |  3 +--
>   include/linux/rcutree.h   |  2 +-
>   kernel/rcu/tree.c         |  3 +--
>   kernel/time/tick-sched.c  | 60 ++++++++++++++++++++++++++++++++++-------------
>   5 files changed, 57 insertions(+), 22 deletions(-)

I tested this series on the IBM S822LC with Ubuntu 20.10 and rcu/dev 
(commit 0ba8896d2fd7 (lib/irq_poll: Declare IRQ_POLL softirq vector as 
ksoftirqd-parking safe)). Running `sudo ppc64_cpu --smt=off` the 
warnings are gone.

Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

Re: [PATCH 0/3] tick: Fix softirq related warnings

[Paul E. McKenney]

On Mon, Feb 14, 2022 at 06:46:59AM +0100, Paul Menzel wrote:
> Dear Frederic,
> 
> 
> Am 08.02.22 um 17:16 schrieb Frederic Weisbecker:
> > Some cleanups and debug rework.
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> > 	timers/core
> > 
> > HEAD: 18b00369d58da4c73946d5f8ebed8e8c7ade89e2
> > 
> > Thanks,
> > 	Frederic
> > ---
> > 
> > Frederic Weisbecker (3):
> >        tick/rcu: Remove obsolete rcu_needs_cpu() parameters
> >        tick/rcu: Stop allowing RCU_SOFTIRQ in idle
> >        lib/irq_poll: Declare IRQ_POLL softirq vector as ksoftirqd-parking safe
> > 
> > 
> >   include/linux/interrupt.h | 11 ++++++++-
> >   include/linux/rcutiny.h   |  3 +--
> >   include/linux/rcutree.h   |  2 +-
> >   kernel/rcu/tree.c         |  3 +--
> >   kernel/time/tick-sched.c  | 60 ++++++++++++++++++++++++++++++++++-------------
> >   5 files changed, 57 insertions(+), 22 deletions(-)
> 
> I tested this series on the IBM S822LC with Ubuntu 20.10 and rcu/dev (commit
> 0ba8896d2fd7 (lib/irq_poll: Declare IRQ_POLL softirq vector as
> ksoftirqd-parking safe)). Running `sudo ppc64_cpu --smt=off` the warnings
> are gone.
> 
> Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>

I will apply on my next rebase, thank you!

And thanks to Peter and Christophe for chasing this down!

							Thanx, Paul