mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Gabriel C <crazy@frugalware.org>
Cc: Gabriel C <nix.or.die@googlemail.com>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	LKML <linux-kernel@vger.kernel.org>,
	Adrian Bunk <bunk@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Natalie Protasevich <protasnb@gmail.com>,
	andi-bz@firstfloor.org, Ingo Molnar <mingo@elte.hu>
Subject: Re: 2.6.25-rc5-git6: Reported regressions from 2.6.24
Date: Sat, 22 Mar 2008 15:30:00 +0100 (CET)	[thread overview]
Message-ID: <alpine.LFD.1.00.0803221441370.3781@apollo.tec.linutronix.de> (raw)
In-Reply-To: <47E50AF9.5070801@frugalware.org>

On Sat, 22 Mar 2008, Gabriel C wrote:
 > Now some time later CPU1 gets woken by an interrupt/IPI and runs the
> > timer wheel. At this point the pm_timer which is the reference clock
> > has already wrapped around, so the watchdog thinks that there is a
> > huge time difference and marks the TSC unstable.
> > 
> > Aside of that watchdog issue this also affects the other users of
> > add_timer_on(): e.g. queue_delayed_work_on().
> > 
> > Can you please apply the patch below and verify it with Andi's
> > watchdog patch applied ? 
> 
> 
> Did that , git head , Andi's + your patch but TSC is still marked unstable.

Doh, stupid me. We do not reevaluate the timer wheel, when we just
wake up via the smp_reschedule IPI when the resched flag on the other
CPU is not set. That's a separate vector which is not going through
irq_enter() / irq_exit(). 

Does the patch below solve the problem ?

Thanks,

	tglx

---
 include/linux/tick.h     |    4 +++
 kernel/time/tick-sched.c |   50 +++++++++++++++++++++++++++++++++++++++++++++++
 kernel/timer.c           |   14 ++++++++++++-
 3 files changed, 67 insertions(+), 1 deletion(-)

Index: linux-2.6/include/linux/tick.h
===================================================================
--- linux-2.6.orig/include/linux/tick.h
+++ linux-2.6/include/linux/tick.h
@@ -111,6 +111,8 @@ extern void tick_nohz_update_jiffies(voi
 extern ktime_t tick_nohz_get_sleep_length(void);
 extern void tick_nohz_stop_idle(int cpu);
 extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
+extern int tick_nohz_cpu_needs_wakeup(int cpu);
+extern void tick_nohz_rescan_timers_on(int cpu);
 # else
 static inline void tick_nohz_stop_sched_tick(void) { }
 static inline void tick_nohz_restart_sched_tick(void) { }
@@ -123,6 +125,8 @@ static inline ktime_t tick_nohz_get_slee
 }
 static inline void tick_nohz_stop_idle(int cpu) { }
 static inline u64 get_cpu_idle_time_us(int cpu, u64 *unused) { return 0; }
+static inline int tick_nohz_cpu_needs_wakeup(int cpu) { return 0; }
+static inline void tick_nohz_rescan_timers_on(int cpu) { }
 # endif /* !NO_HZ */
 
 #endif
Index: linux-2.6/kernel/time/tick-sched.c
===================================================================
--- linux-2.6.orig/kernel/time/tick-sched.c
+++ linux-2.6/kernel/time/tick-sched.c
@@ -183,6 +183,56 @@ u64 get_cpu_idle_time_us(int cpu, u64 *l
 }
 
 /**
+ * tick_nohz_cpu_needs_wakeup - check possible wakeup of cpu in add_timer_on()
+ *
+ * when add_timer_on() happens on a CPU which is in a long idle sleep,
+ * then we need to wake it up so the timer wheel gets reevaluated.
+ *
+ * Note: we use idle_cpu() which checks the idle state lockless, but
+ * we are ordered against the other cpu which might be on the way to
+ * idle by the timer base lock, which we hold.
+ */
+int tick_nohz_cpu_needs_wakeup(int cpu)
+{
+	return tick_nohz_enabled && idle_cpu(cpu) &&
+		(cpu != smp_processor_id());
+}
+
+/*
+ * Rescan the timer wheel, when
+ *
+ * - the CPU is idle
+ * - the CPU is not processing an interupt
+ * - the need_resched flag is off
+ */
+static void tick_nohz_rescan_timers(void *unused)
+{
+	int cpu = smp_processor_id();
+
+	if (!idle_cpu(cpu) || in_interrupt() || need_resched())
+		return;
+
+	tick_nohz_stop_idle(cpu);
+	tick_nohz_update_jiffies();
+	tick_nohz_stop_sched_tick();
+}
+
+/**
+ * tick_nohz_rescan_timers_on - reevaluate the idle sleep time of a CPU
+ *
+ * When a CPU is idle and a timer got added to this CPU timer wheel
+ * via add_timer_on() then we need to make sure that the CPU
+ * reevaluates the timer wheel. Otherwise the timer might be delayed
+ * for a real long time.
+ */
+void tick_nohz_rescan_timers_on(int cpu)
+{
+	if (tick_nohz_enabled && idle_cpu(cpu))
+		smp_call_function_single(cpu, tick_nohz_rescan_timers, NULL,
+					 0, 0);
+}
+
+/**
  * tick_nohz_stop_sched_tick - stop the idle tick from the idle task
  *
  * When the next event is more than a tick into the future, stop the idle tick
Index: linux-2.6/kernel/timer.c
===================================================================
--- linux-2.6.orig/kernel/timer.c
+++ linux-2.6/kernel/timer.c
@@ -445,15 +445,27 @@ void add_timer_on(struct timer_list *tim
 {
 	struct tvec_base *base = per_cpu(tvec_bases, cpu);
 	unsigned long flags;
+	int wakeidle;
 
 	timer_stats_timer_set_start_info(timer);
 	BUG_ON(timer_pending(timer) || !timer->function);
 	spin_lock_irqsave(&base->lock, flags);
 	timer_set_base(timer, base);
 	internal_add_timer(base, timer);
+	/*
+	 * Check whether the other CPU is idle and needs to be
+	 * triggered to reevaluate the timer wheel when nohz is
+	 * active. We are protected against the other CPU fiddling
+	 * with the timer by holding the timer base lock. This also
+	 * makes sure that a CPU on the way to idle can not evaluate
+	 * the timer wheel.
+	 */
+	wakeidle = tick_nohz_cpu_needs_wakeup(cpu);
 	spin_unlock_irqrestore(&base->lock, flags);
-}
 
+	if (wakeidle)
+		tick_nohz_rescan_timers_on(cpu);
+}
 
 /**
  * mod_timer - modify a timer's timeout

  reply	other threads:[~2008-03-22 14:31 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-16 23:18 Rafael J. Wysocki
2008-03-16 23:33 ` Linus Torvalds
2008-03-16 23:38   ` Rafael J. Wysocki
2008-03-17  0:20 ` Gabriel C
2008-03-17 16:17   ` Thomas Gleixner
2008-03-17 18:20     ` Gabriel C
2008-03-18  4:01       ` Gabriel C
2008-03-18  4:24         ` Gabriel C
2008-03-21 15:24         ` Gabriel C
2008-03-21 16:26           ` Thomas Gleixner
2008-03-21 16:46             ` Gabriel C
2008-03-21 18:11               ` Gabriel C
2008-03-21 18:49                 ` Thomas Gleixner
2008-03-21 19:23                   ` Gabriel C
2008-03-21 20:55                     ` Gabriel C
2008-03-21 21:15                       ` Thomas Gleixner
2008-03-21 21:59                         ` Gabriel C
2008-03-21 22:09                           ` Thomas Gleixner
2008-03-22 11:21                             ` Thomas Gleixner
2008-03-22 13:34                               ` Gabriel C
2008-03-22 14:30                                 ` Thomas Gleixner [this message]
2008-03-22 15:13                                   ` Gabriel C
2008-03-22 16:32                                     ` Thomas Gleixner
2008-03-22 21:55                                       ` Thomas Gleixner
2008-03-22 22:41                                         ` Gabriel C
2008-03-23 11:00                                           ` Gabriel C
2008-03-23 23:31                                             ` Gabriel C
2008-03-24 10:24                                               ` Thomas Gleixner
2008-03-24 22:33                                                 ` Gabriel C
2008-03-25  8:06                                                   ` Thomas Gleixner
2008-03-26 12:43                                                     ` Gabriel C
2008-03-26 14:51                                                       ` Thomas Gleixner
2008-03-22 14:25                               ` Andi Kleen
2008-03-22 14:41                                 ` Thomas Gleixner
2008-03-17  6:47 ` Jason Wu
2008-03-17 21:36   ` Rafael J. Wysocki
2008-03-23 19:01 ` Christian Kujau
2008-03-23 19:06   ` Rafael J. Wysocki
2008-03-23 19:40     ` Chr
2008-03-23 21:17 ` Christian Kujau
2008-03-23 21:29   ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.1.00.0803221441370.3781@apollo.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=akpm@linux-foundation.org \
    --cc=andi-bz@firstfloor.org \
    --cc=bunk@kernel.org \
    --cc=crazy@frugalware.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=nix.or.die@googlemail.com \
    --cc=protasnb@gmail.com \
    --cc=rjw@sisk.pl \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome