Re: [PATCH] Sanity check sysfs clocksource changes

[Thomas Gleixner <tglx@linutronix.de>]

On Mon, 4 May 2009, john stultz wrote:

> On Fri, 2009-05-01 at 13:10 -0700, Andrew Morton wrote:
> > What's the status of
> > clocksource-sanity-check-sysfs-clocksource-changes.patch, btw?  I have
> > it marked as "still an RFC", but it's been sitting here since January?
> 
> Thanks for reminding me! I lost track of that one. Still waiting on
> feedback from Thomas.
> 
> Thomas? What do you think? Original patch below, applies with fuzz and
> builds.
> 
> -john
> 
> 
> 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?

Hmm. If we switched to highres and discarded TSC already then we
should not go back to it. 
 
> Signed-off-by: John Stultz <johnstul@us.ibm.com>
> 
> 
> 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; }

We want to keep that static inline and move it to hrtimer.h to allow
gcc to optimize stuff out.

Thanks,

	tglx