Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757256AbZAVDxW (ORCPT ); Wed, 21 Jan 2009 22:53:22 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756447AbZAVDxM (ORCPT ); Wed, 21 Jan 2009 22:53:12 -0500 Received: from e35.co.us.ibm.com ([32.97.110.153]:33096 "EHLO e35.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756431AbZAVDxL (ORCPT ); Wed, 21 Jan 2009 22:53:11 -0500 Subject: [PATCH][RFC] Sanity check sysfs clocksource changes From: john stultz To: Thomas Gleixner Cc: Ingo Molnar , Andrew Morton , lkml In-Reply-To: References: <20090121145236.04c17ed7.akpm@linux-foundation.org> <20090121225547.GA16127@elte.hu> <20090121231006.GA9967@elte.hu> Content-Type: text/plain Date: Wed, 21 Jan 2009 19:53:06 -0800 Message-Id: <1232596386.6997.39.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3981 Lines: 111 Hey all, Thomas, Andrew and Ingo pointed out that we don't have any safety checks in the clocksource sysfs entries to make sure sysadmins don't try to change the clocksource to a non high-res timer capable clocksource (such as jiffies) when high-res timers (HRT) is enabled. Doing so will likely hang a system. This patch tries to correct this by filtering non HRT clocksources from available_clocksources and not accepting non HRT clocksources with HRT is is enabled. This has been lightly tested, and seems to work, but there may be some drawbacks. One issue I realized was that when TSCs disqualified, they are marked as not CLOCK_SOURCE_VALID_FOR_HRES. This means on boxes with unsycned TSCs, the only available clocksource may be the slower acpi_pm and the user will not be able to override it with the TSC as is currently possible. (even if that seems ill-advised). So Thomas, what do you think? Should we just use CLOCK_SOURCE_IS_CONTINUOUS flag or is CLOCK_SOURCE_VALID_FOR_HRES really what we want? Signed-off-by: John Stultz diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h index bd37078..05f67f0 100644 --- a/include/linux/hrtimer.h +++ b/include/linux/hrtimer.h @@ -305,7 +305,7 @@ static inline int hrtimer_is_hres_active(struct hrtimer *timer) extern ktime_t ktime_get(void); extern ktime_t ktime_get_real(void); - +extern int hrtimer_hres_active(void); DECLARE_PER_CPU(struct tick_device, tick_cpu_device); diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 2dc30c5..fa4abdc 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -476,7 +476,7 @@ static inline int hrtimer_is_hres_enabled(void) /* * Is the high resolution mode active ? */ -static inline int hrtimer_hres_active(void) +int hrtimer_hres_active(void) { return __get_cpu_var(hrtimer_bases).hres_active; } @@ -689,7 +689,7 @@ static int hrtimer_switch_to_hres(void) #else -static inline int hrtimer_hres_active(void) { return 0; } +int hrtimer_hres_active(void) { return 0; } static inline int hrtimer_is_hres_enabled(void) { return 0; } static inline int hrtimer_switch_to_hres(void) { return 0; } static inline void hrtimer_force_reprogram(struct hrtimer_cpu_base *base) { } diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c index ca89e15..1bb1161 100644 --- a/kernel/time/clocksource.c +++ b/kernel/time/clocksource.c @@ -30,6 +30,7 @@ #include #include /* for spin_unlock_irq() using preempt_count() m68k */ #include +#include /* XXX - Would like a better way for initializing curr_clocksource */ extern struct clocksource clocksource_jiffies; @@ -436,6 +437,17 @@ static ssize_t sysfs_override_clocksource(struct sys_device *dev, } } + /* Check to make sure we don't switch to a non-HRT usable + * clocksource if HRT is enabled and running + */ + if (hrtimer_hres_active() && + !(ovr->flags & CLOCK_SOURCE_VALID_FOR_HRES)) { + printk(KERN_WARNING "%s clocksource is not HRT compatible. " + "Cannot switch while in HRT mode\n", ovr->name); + ovr = NULL; + override_name[0] = 0; + } + /* Reselect, when the override name has changed */ if (ovr != clocksource_override) { clocksource_override = ovr; @@ -464,7 +476,10 @@ sysfs_show_available_clocksources(struct sys_device *dev, spin_lock_irq(&clocksource_lock); list_for_each_entry(src, &clocksource_list, list) { - count += snprintf(buf + count, + /* Don't show non-HRES clocksource if HRES is enabled */ + if (!hrtimer_hres_active() || + (src->flags & CLOCK_SOURCE_VALID_FOR_HRES)) + count += snprintf(buf + count, max((ssize_t)PAGE_SIZE - count, (ssize_t)0), "%s ", src->name); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/