Re: [BUG] 2.6.21: Kernel won't boot with either/both of CONFIG_NO_HZ, CONFIG_HIGH_RES_TIMERS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

re: [PATCH] highres/dyntick: prevent xtime lock contention

|mark> I have a new notebook (Dell Inspiron 9400) with Core2-Duo T7400 @ 2.1Ghz.
|mark> When either/both of CONFIG_NO_HZ, CONFIG_HIGH_RES_TIMERS is used,
|mark> the 2.6.21 kernel hangs on startup just after printing one/both of these:
|mark>
|mark> kernel: switched to high resolution mode on cpu 1
|mark> kernel: switched to high resolution mode on cpu 0 | 
|thomas> Can you apply this patch please:
|thomas> http://lkml.org/lkml/2007/4/13/190
|thomas> It somehow did not make it into 2.6.21.
|
|andrew> Alas, poor me.  I ain't going to merge a contention-reduction patch when
|andrew> we're at -rc6.  If a patch fixes a bug, please tell me!

Okay, patch applied to 2.6.21.1, and I'm typing this email
from thunderbird while running the patched kernel (woo-hoo!).

So I guess it boots, works, and we need this patch in 2.6.21.2 & 2.6.22.


Subject [PATCH] highres/dyntick: prevent xtime lock contention
From	Thomas Gleixner <>
Date	Fri, 13 Apr 2007 21:05:57 +0200
Digg This

While the !highres/!dyntick code assigns the duty of the do_timer() call
to one specific CPU, this was dropped in the highres/dyntick part during
development.

Steven Rostedt discovered the xtime lock contention on highres/dyntick
due to several CPUs trying to update jiffies.

Add the single CPU assignment back. In the dyntick case this needs to
be handled carefully, as the CPU which has the do_timer() duty must drop
the assignement and let it be grabbed by another CPU, which is active.
Otherwise the do_timer() calls would not happen during the long sleep.


Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Ingo Molnar <[email protected]>
Acked-by: Mark Lord <[email protected]>

diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c index bfda3f7..a96ec9a 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -31,7 +31,7 @@ DEFINE_PER_CPU(struct tick_device, tick_cpu_device);
 */
ktime_t tick_next_period;
ktime_t tick_period;
-static int tick_do_timer_cpu = -1;
+int tick_do_timer_cpu __read_mostly = -1;
DEFINE_SPINLOCK(tick_device_lock);

/*
@@ -295,6 +295,12 @@ static void tick_shutdown(unsigned int *cpup)
		clockevents_exchange_device(dev, NULL);
		td->evtdev = NULL;
	}
+	/* Transfer the do_timer job away from this cpu */
+	if (*cpup == tick_do_timer_cpu) {
+		int cpu = first_cpu(cpu_online_map);
+
+		tick_do_timer_cpu = (cpu != NR_CPUS) ? cpu : -1;
+	}
	spin_unlock_irqrestore(&tick_device_lock, flags);
}

diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h index c9d203b..5645e6a 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -5,6 +5,7 @@ DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
extern spinlock_t tick_device_lock;
extern ktime_t tick_next_period;
extern ktime_t tick_period;
+extern int tick_do_timer_cpu __read_mostly;

extern void tick_setup_periodic(struct clock_event_device *dev, int broadcast);
extern void tick_handle_periodic(struct clock_event_device *dev);
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 51556b9..3a8e524 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -221,6 +221,18 @@ void tick_nohz_stop_sched_tick(void)
			ts->tick_stopped = 1;
			ts->idle_jiffies = last_jiffies;
		}
+
+		/*
+		 * If this cpu is the one which updates jiffies, then
+		 * give up the assignment and let it be taken by the
+		 * cpu which runs the tick timer next, which might be
+		 * this cpu as well. If we don't drop this here the
+		 * jiffies might be stale and do_timer() never
+		 * invoked.
+		 */
+		if (cpu == tick_do_timer_cpu)
+			tick_do_timer_cpu = -1;
+
		/*
		 * calculate the expiry time for the next timer wheel
		 * timer
@@ -338,12 +350,24 @@ static void tick_nohz_handler(struct clock_event_device *dev)
{
	struct tick_sched *ts = &__get_cpu_var(tick_cpu_sched);
	struct pt_regs *regs = get_irq_regs();
+	int cpu = smp_processor_id();
	ktime_t now = ktime_get();

	dev->next_event.tv64 = KTIME_MAX;

+	/*
+	 * Check if the do_timer duty was dropped. We don't care about
+	 * concurrency: This happens only when the cpu in charge went
+	 * into a long sleep. If two cpus happen to assign themself to
+	 * this duty, then the jiffies update is still serialized by
+	 * xtime_lock.
+	 */
+	if (unlikely(tick_do_timer_cpu == -1))
+		tick_do_timer_cpu = cpu;
+
	/* Check, if the jiffies need an update */
-	tick_do_update_jiffies64(now);
+	if (tick_do_timer_cpu == cpu)
+		tick_do_update_jiffies64(now);

	/*
	 * When we are idle and the tick is stopped, we have to touch
@@ -431,9 +455,23 @@ static enum hrtimer_restart tick_sched_timer(struct hrtimer *timer)
	struct hrtimer_cpu_base *base = timer->base->cpu_base;
	struct pt_regs *regs = get_irq_regs();
	ktime_t now = ktime_get();
+	int cpu = smp_processor_id();
+
+#ifdef CONFIG_NO_HZ
+	/*
+	 * Check if the do_timer duty was dropped. We don't care about
+	 * concurrency: This happens only when the cpu in charge went
+	 * into a long sleep. If two cpus happen to assign themself to
+	 * this duty, then the jiffies update is still serialized by
+	 * xtime_lock.
+	 */
+	if (unlikely(tick_do_timer_cpu == -1))
+		tick_do_timer_cpu = cpu;
+#endif

	/* Check, if the jiffies need an update */
-	tick_do_update_jiffies64(now);
+	if (tick_do_timer_cpu == cpu)
+		tick_do_update_jiffies64(now);

	/*
	 * Do not call, when we are not in irq context and have

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux