Re: [PATCH 06/10] timer: Report ignored local enqueue in nohz mode
On Tue, Mar 16, 2021 at 04:27:56PM +0100, Peter Zijlstra wrote: > On Thu, Mar 11, 2021 at 01:37:04PM +0100, Frederic Weisbecker wrote: > > Enqueuing a local timer after the tick has been stopped will result in > > the timer being ignored until the next random interrupt. > > > > Perform sanity checks to report these situations. > > > > Reviewed-by: Rafael J. Wysocki > > Signed-off-by: Frederic Weisbecker > > Cc: Peter Zijlstra > > Cc: Thomas Gleixner > > Cc: Ingo Molnar > > Cc: Paul E. McKenney > > --- > > kernel/sched/core.c | 20 +++- > > 1 file changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > > index ca2bb629595f..24552911f92b 100644 > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -674,6 +674,22 @@ int get_nohz_timer_target(void) > > return cpu; > > } > > > > +/* Make sure the timer won't be ignored in dynticks-idle case */ > > +static void wake_idle_assert_possible(void) > > +{ > > +#ifdef CONFIG_SCHED_DEBUG > > + /* > > +* Timers are re-evaluated after idle IRQs. In case of softirq, > > +* we assume IRQ tail. Ksoftirqd shouldn't reach here as the > > +* timer base wouldn't be idle. And inline softirq processing > > +* after a call to local_bh_enable() within idle loop sound too > > +* fun to be considered here. > > +*/ > > + WARN_ONCE(in_task(), > > + "Late timer enqueue may be ignored\n"); > > +#endif > > +} > > + > > /* > > * When add_timer_on() enqueues a timer into the timer wheel of an > > * idle CPU then this timer might expire before the next timer event > > @@ -688,8 +704,10 @@ static void wake_up_idle_cpu(int cpu) > > { > > struct rq *rq = cpu_rq(cpu); > > > > - if (cpu == smp_processor_id()) > > + if (cpu == smp_processor_id()) { > > + wake_idle_assert_possible(); > > return; > > + } > > > > if (set_nr_and_not_polling(rq->idle)) > > smp_send_reschedule(cpu); > > I'm not entirely sure I understand this one. What's the callchain that > leads to this? That's while calling add_timer*() or mod_timer() on an idle target. Now the issue is only relevant when these timer functions are called after cpuidle_select(), which arguably makes a small vulnerable window that could be spotted in the future if the timer functions are called after instrumentation_end()? Thanks.
Re: [PATCH 06/10] timer: Report ignored local enqueue in nohz mode
On Thu, Mar 11, 2021 at 01:37:04PM +0100, Frederic Weisbecker wrote: > Enqueuing a local timer after the tick has been stopped will result in > the timer being ignored until the next random interrupt. > > Perform sanity checks to report these situations. > > Reviewed-by: Rafael J. Wysocki > Signed-off-by: Frederic Weisbecker > Cc: Peter Zijlstra > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Paul E. McKenney > --- > kernel/sched/core.c | 20 +++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index ca2bb629595f..24552911f92b 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -674,6 +674,22 @@ int get_nohz_timer_target(void) > return cpu; > } > > +/* Make sure the timer won't be ignored in dynticks-idle case */ > +static void wake_idle_assert_possible(void) > +{ > +#ifdef CONFIG_SCHED_DEBUG > + /* > + * Timers are re-evaluated after idle IRQs. In case of softirq, > + * we assume IRQ tail. Ksoftirqd shouldn't reach here as the > + * timer base wouldn't be idle. And inline softirq processing > + * after a call to local_bh_enable() within idle loop sound too > + * fun to be considered here. > + */ > + WARN_ONCE(in_task(), > + "Late timer enqueue may be ignored\n"); > +#endif > +} > + > /* > * When add_timer_on() enqueues a timer into the timer wheel of an > * idle CPU then this timer might expire before the next timer event > @@ -688,8 +704,10 @@ static void wake_up_idle_cpu(int cpu) > { > struct rq *rq = cpu_rq(cpu); > > - if (cpu == smp_processor_id()) > + if (cpu == smp_processor_id()) { > + wake_idle_assert_possible(); > return; > + } > > if (set_nr_and_not_polling(rq->idle)) > smp_send_reschedule(cpu); I'm not entirely sure I understand this one. What's the callchain that leads to this?
[PATCH 06/10] timer: Report ignored local enqueue in nohz mode
Enqueuing a local timer after the tick has been stopped will result in the timer being ignored until the next random interrupt. Perform sanity checks to report these situations. Reviewed-by: Rafael J. Wysocki Signed-off-by: Frederic Weisbecker Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Paul E. McKenney --- kernel/sched/core.c | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index ca2bb629595f..24552911f92b 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -674,6 +674,22 @@ int get_nohz_timer_target(void) return cpu; } +/* Make sure the timer won't be ignored in dynticks-idle case */ +static void wake_idle_assert_possible(void) +{ +#ifdef CONFIG_SCHED_DEBUG + /* +* Timers are re-evaluated after idle IRQs. In case of softirq, +* we assume IRQ tail. Ksoftirqd shouldn't reach here as the +* timer base wouldn't be idle. And inline softirq processing +* after a call to local_bh_enable() within idle loop sound too +* fun to be considered here. +*/ + WARN_ONCE(in_task(), + "Late timer enqueue may be ignored\n"); +#endif +} + /* * When add_timer_on() enqueues a timer into the timer wheel of an * idle CPU then this timer might expire before the next timer event @@ -688,8 +704,10 @@ static void wake_up_idle_cpu(int cpu) { struct rq *rq = cpu_rq(cpu); - if (cpu == smp_processor_id()) + if (cpu == smp_processor_id()) { + wake_idle_assert_possible(); return; + } if (set_nr_and_not_polling(rq->idle)) smp_send_reschedule(cpu); -- 2.25.1
Re: timer: Report ignored local enqueue in nohz mode?
On Fri, Mar 05, 2021 at 02:41:11PM +0100, Frederic Weisbecker wrote: > On Wed, Mar 03, 2021 at 11:49:45AM -0800, Paul E. McKenney wrote: > > Hello, Frederic! > > > > I don't see the following commit in mainline, but figured I should > > check with you guys to see if the problem got solved in some other way. > > Unless I hear otherwise, I will continue to carry this patch in -rcu > > and will send it along for the v5.13 merge window. > > I have it included in a nohz series I'm about to post but since RCU is the > motivation behind doing this, it's fine if you carry it. Actually, please feel free to run this up the normal nohz path. I will remove my version once yours hits mainline, as I did with the others. I was just curious. ;-) Thanx, Paul > I've just modified it a bit after a review from Peter: > > --- > >From 7876725b8631147967bb9e65158ef1cb2bb94372 Mon Sep 17 00:00:00 2001 > From: Frederic Weisbecker > Date: Fri, 8 Jan 2021 13:50:12 +0100 > Subject: [PATCH] timer: Report ignored local enqueue in nohz mode > > Enqueuing a local timer after the tick has been stopped will result in > the timer being ignored until the next random interrupt. > > Perform sanity checks to report these situations. > > Signed-off-by: Frederic Weisbecker > Cc: Peter Zijlstra > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Paul E. McKenney > Cc: Rafael J. Wysocki > --- > kernel/sched/core.c | 20 +++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index ca2bb629595f..24552911f92b 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -674,6 +674,22 @@ int get_nohz_timer_target(void) > return cpu; > } > > +/* Make sure the timer won't be ignored in dynticks-idle case */ > +static void wake_idle_assert_possible(void) > +{ > +#ifdef CONFIG_SCHED_DEBUG > + /* > + * Timers are re-evaluated after idle IRQs. In case of softirq, > + * we assume IRQ tail. Ksoftirqd shouldn't reach here as the > + * timer base wouldn't be idle. And inline softirq processing > + * after a call to local_bh_enable() within idle loop sound too > + * fun to be considered here. > + */ > + WARN_ONCE(in_task(), > + "Late timer enqueue may be ignored\n"); > +#endif > +} > + > /* > * When add_timer_on() enqueues a timer into the timer wheel of an > * idle CPU then this timer might expire before the next timer event > @@ -688,8 +704,10 @@ static void wake_up_idle_cpu(int cpu) > { > struct rq *rq = cpu_rq(cpu); > > - if (cpu == smp_processor_id()) > + if (cpu == smp_processor_id()) { > + wake_idle_assert_possible(); > return; > + } > > if (set_nr_and_not_polling(rq->idle)) > smp_send_reschedule(cpu); > -- > 2.25.1 >
Re: timer: Report ignored local enqueue in nohz mode?
On Wed, Mar 03, 2021 at 11:49:45AM -0800, Paul E. McKenney wrote: > Hello, Frederic! > > I don't see the following commit in mainline, but figured I should > check with you guys to see if the problem got solved in some other way. > Unless I hear otherwise, I will continue to carry this patch in -rcu > and will send it along for the v5.13 merge window. I have it included in a nohz series I'm about to post but since RCU is the motivation behind doing this, it's fine if you carry it. I've just modified it a bit after a review from Peter: --- >From 7876725b8631147967bb9e65158ef1cb2bb94372 Mon Sep 17 00:00:00 2001 From: Frederic Weisbecker Date: Fri, 8 Jan 2021 13:50:12 +0100 Subject: [PATCH] timer: Report ignored local enqueue in nohz mode Enqueuing a local timer after the tick has been stopped will result in the timer being ignored until the next random interrupt. Perform sanity checks to report these situations. Signed-off-by: Frederic Weisbecker Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Paul E. McKenney Cc: Rafael J. Wysocki --- kernel/sched/core.c | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index ca2bb629595f..24552911f92b 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -674,6 +674,22 @@ int get_nohz_timer_target(void) return cpu; } +/* Make sure the timer won't be ignored in dynticks-idle case */ +static void wake_idle_assert_possible(void) +{ +#ifdef CONFIG_SCHED_DEBUG + /* +* Timers are re-evaluated after idle IRQs. In case of softirq, +* we assume IRQ tail. Ksoftirqd shouldn't reach here as the +* timer base wouldn't be idle. And inline softirq processing +* after a call to local_bh_enable() within idle loop sound too +* fun to be considered here. +*/ + WARN_ONCE(in_task(), + "Late timer enqueue may be ignored\n"); +#endif +} + /* * When add_timer_on() enqueues a timer into the timer wheel of an * idle CPU then this timer might expire before the next timer event @@ -688,8 +704,10 @@ static void wake_up_idle_cpu(int cpu) { struct rq *rq = cpu_rq(cpu); - if (cpu == smp_processor_id()) + if (cpu == smp_processor_id()) { + wake_idle_assert_possible(); return; + } if (set_nr_and_not_polling(rq->idle)) smp_send_reschedule(cpu); -- 2.25.1
Re: [PATCH tip/core/rcu 02/12] timer: Report ignored local enqueue in nohz mode
On Thu, Mar 04, 2021 at 12:58:54PM +0100, Rafael J. Wysocki wrote: > On 3/4/2021 1:23 AM, paul...@kernel.org wrote: > > From: Frederic Weisbecker > > > > Enqueuing a local timer after the tick has been stopped will result in > > the timer being ignored until the next random interrupt. > > > > Perform sanity checks to report these situations. > > > > Cc: Peter Zijlstra > > Cc: Thomas Gleixner > > Cc: Ingo Molnar > > Cc: Rafael J. Wysocki > > Signed-off-by: Frederic Weisbecker > > Signed-off-by: Paul E. McKenney > > Reviewed-by: Rafael J. Wysocki Applied, thank you! Thanx, Paul > > --- > > kernel/sched/core.c | 24 +++- > > 1 file changed, 23 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > > index ca2bb62..4822371 100644 > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -674,6 +674,26 @@ int get_nohz_timer_target(void) > > return cpu; > > } > > +static void wake_idle_assert_possible(void) > > +{ > > +#ifdef CONFIG_SCHED_DEBUG > > + /* Timers are re-evaluated after idle IRQs */ > > + if (in_hardirq()) > > + return; > > + /* > > +* Same as hardirqs, assuming they are executing > > +* on IRQ tail. Ksoftirqd shouldn't reach here > > +* as the timer base wouldn't be idle. And inline > > +* softirq processing after a call to local_bh_enable() > > +* within idle loop sound too fun to be considered here. > > +*/ > > + if (in_serving_softirq()) > > + return; > > + > > + WARN_ON_ONCE("Late timer enqueue may be ignored\n"); > > +#endif > > +} > > + > > /* > >* When add_timer_on() enqueues a timer into the timer wheel of an > >* idle CPU then this timer might expire before the next timer event > > @@ -688,8 +708,10 @@ static void wake_up_idle_cpu(int cpu) > > { > > struct rq *rq = cpu_rq(cpu); > > - if (cpu == smp_processor_id()) > > + if (cpu == smp_processor_id()) { > > + wake_idle_assert_possible(); > > return; > > + } > > if (set_nr_and_not_polling(rq->idle)) > > smp_send_reschedule(cpu); > >
Re: [PATCH tip/core/rcu 02/12] timer: Report ignored local enqueue in nohz mode
On 3/4/2021 1:23 AM, paul...@kernel.org wrote: From: Frederic Weisbecker Enqueuing a local timer after the tick has been stopped will result in the timer being ignored until the next random interrupt. Perform sanity checks to report these situations. Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Rafael J. Wysocki Signed-off-by: Frederic Weisbecker Signed-off-by: Paul E. McKenney Reviewed-by: Rafael J. Wysocki --- kernel/sched/core.c | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index ca2bb62..4822371 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -674,6 +674,26 @@ int get_nohz_timer_target(void) return cpu; } +static void wake_idle_assert_possible(void) +{ +#ifdef CONFIG_SCHED_DEBUG + /* Timers are re-evaluated after idle IRQs */ + if (in_hardirq()) + return; + /* +* Same as hardirqs, assuming they are executing +* on IRQ tail. Ksoftirqd shouldn't reach here +* as the timer base wouldn't be idle. And inline +* softirq processing after a call to local_bh_enable() +* within idle loop sound too fun to be considered here. +*/ + if (in_serving_softirq()) + return; + + WARN_ON_ONCE("Late timer enqueue may be ignored\n"); +#endif +} + /* * When add_timer_on() enqueues a timer into the timer wheel of an * idle CPU then this timer might expire before the next timer event @@ -688,8 +708,10 @@ static void wake_up_idle_cpu(int cpu) { struct rq *rq = cpu_rq(cpu); - if (cpu == smp_processor_id()) + if (cpu == smp_processor_id()) { + wake_idle_assert_possible(); return; + } if (set_nr_and_not_polling(rq->idle)) smp_send_reschedule(cpu);
[PATCH tip/core/rcu 02/12] timer: Report ignored local enqueue in nohz mode
From: Frederic Weisbecker Enqueuing a local timer after the tick has been stopped will result in the timer being ignored until the next random interrupt. Perform sanity checks to report these situations. Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Rafael J. Wysocki Signed-off-by: Frederic Weisbecker Signed-off-by: Paul E. McKenney --- kernel/sched/core.c | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index ca2bb62..4822371 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -674,6 +674,26 @@ int get_nohz_timer_target(void) return cpu; } +static void wake_idle_assert_possible(void) +{ +#ifdef CONFIG_SCHED_DEBUG + /* Timers are re-evaluated after idle IRQs */ + if (in_hardirq()) + return; + /* +* Same as hardirqs, assuming they are executing +* on IRQ tail. Ksoftirqd shouldn't reach here +* as the timer base wouldn't be idle. And inline +* softirq processing after a call to local_bh_enable() +* within idle loop sound too fun to be considered here. +*/ + if (in_serving_softirq()) + return; + + WARN_ON_ONCE("Late timer enqueue may be ignored\n"); +#endif +} + /* * When add_timer_on() enqueues a timer into the timer wheel of an * idle CPU then this timer might expire before the next timer event @@ -688,8 +708,10 @@ static void wake_up_idle_cpu(int cpu) { struct rq *rq = cpu_rq(cpu); - if (cpu == smp_processor_id()) + if (cpu == smp_processor_id()) { + wake_idle_assert_possible(); return; + } if (set_nr_and_not_polling(rq->idle)) smp_send_reschedule(cpu); -- 2.9.5
timer: Report ignored local enqueue in nohz mode?
Hello, Frederic! I don't see the following commit in mainline, but figured I should check with you guys to see if the problem got solved in some other way. Unless I hear otherwise, I will continue to carry this patch in -rcu and will send it along for the v5.13 merge window. Thanx, Paul commit 650c433b46ca9601378c9d170d5dc0e24dd42822 Author: Frederic Weisbecker Date: Fri Jan 8 13:50:12 2021 +0100 timer: Report ignored local enqueue in nohz mode Enqueuing a local timer after the tick has been stopped will result in the timer being ignored until the next random interrupt. Perform sanity checks to report these situations. Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Rafael J. Wysocki Signed-off-by: Frederic Weisbecker Signed-off-by: Paul E. McKenney diff --git a/kernel/sched/core.c b/kernel/sched/core.c index ca2bb62..4822371 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -674,6 +674,26 @@ int get_nohz_timer_target(void) return cpu; } +static void wake_idle_assert_possible(void) +{ +#ifdef CONFIG_SCHED_DEBUG + /* Timers are re-evaluated after idle IRQs */ + if (in_hardirq()) + return; + /* +* Same as hardirqs, assuming they are executing +* on IRQ tail. Ksoftirqd shouldn't reach here +* as the timer base wouldn't be idle. And inline +* softirq processing after a call to local_bh_enable() +* within idle loop sound too fun to be considered here. +*/ + if (in_serving_softirq()) + return; + + WARN_ON_ONCE("Late timer enqueue may be ignored\n"); +#endif +} + /* * When add_timer_on() enqueues a timer into the timer wheel of an * idle CPU then this timer might expire before the next timer event @@ -688,8 +708,10 @@ static void wake_up_idle_cpu(int cpu) { struct rq *rq = cpu_rq(cpu); - if (cpu == smp_processor_id()) + if (cpu == smp_processor_id()) { + wake_idle_assert_possible(); return; + } if (set_nr_and_not_polling(rq->idle)) smp_send_reschedule(cpu);
[RFC PATCH 8/8] timer: Report ignored local enqueue in nohz mode
Enqueuing a local timer after the tick has been stopped will result in the timer being ignored until the next random interrupt. Perform sanity checks to report these situations. Signed-off-by: Frederic Weisbecker Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Paul E. McKenney Cc: Rafael J. Wysocki --- kernel/sched/core.c | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 6056f0374674..6c8b04272a9a 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -715,6 +715,26 @@ int get_nohz_timer_target(void) return cpu; } +static void wake_idle_assert_possible(void) +{ +#ifdef CONFIG_SCHED_DEBUG + /* Timers are re-evaluated after idle IRQs */ + if (in_hardirq()) + return; + /* +* Same as hardirqs, assuming they are executing +* on IRQ tail. Ksoftirqd shouldn't reach here +* as the timer base wouldn't be idle. And inline +* softirq processing after a call to local_bh_enable() +* within idle loop sound too fun to be considered here. +*/ + if (in_serving_softirq()) + return; + + WARN_ON_ONCE("Late timer enqueue may be ignored\n"); +#endif +} + /* * When add_timer_on() enqueues a timer into the timer wheel of an * idle CPU then this timer might expire before the next timer event @@ -729,8 +749,10 @@ static void wake_up_idle_cpu(int cpu) { struct rq *rq = cpu_rq(cpu); - if (cpu == smp_processor_id()) + if (cpu == smp_processor_id()) { + wake_idle_assert_possible(); return; + } if (set_nr_and_not_polling(rq->idle)) smp_send_reschedule(cpu); -- 2.25.1