PATCH: Allow user to force 'tsc' to be treated as stable.

PATCH: Allow user to force 'tsc' to be treated as stable.

[Ben Greear]

[-- Attachment #1: Type: text/plain, Size: 316 bytes --]

Allow user to force TSC as stable clock-source.
Works around BIOS issues in the FWA-7304 (Via CN700 chipset)
system, and possibly others.

This is against 2.6.29-rc3.

Signed-Off-By:  Ben Greear<greearb@candelatech.com>

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


[-- Attachment #2: patch0.patch --]
[-- Type: text/x-patch, Size: 1731 bytes --]

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index ca89e15..0578fcb 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -54,6 +54,7 @@ static LIST_HEAD(clocksource_list);
 static DEFINE_SPINLOCK(clocksource_lock);
 static char override_name[32];
 static int finished_booting;
+static int force_tsc_stable;
 
 /* clocksource_done_booting - Called near the end of core bootup
  *
@@ -82,6 +83,14 @@ static unsigned long watchdog_resumed;
 #define WATCHDOG_INTERVAL (HZ >> 1)
 #define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 4)
 
+static int __init parse_force_tsc_stable(char *arg)
+{
+	force_tsc_stable = 1;
+	return 0;
+}
+early_param("force_tsc_stable", parse_force_tsc_stable);
+
+
 static void clocksource_ratewd(struct clocksource *cs, int64_t delta)
 {
 	if (delta > -WATCHDOG_THRESHOLD && delta < WATCHDOG_THRESHOLD)
@@ -89,9 +98,16 @@ static void clocksource_ratewd(struct clocksource *cs, int64_t delta)
 
 	printk(KERN_WARNING "Clocksource %s unstable (delta = %Ld ns)\n",
 	       cs->name, delta);
-	cs->flags &= ~(CLOCK_SOURCE_VALID_FOR_HRES | CLOCK_SOURCE_WATCHDOG);
-	clocksource_change_rating(cs, 0);
-	list_del(&cs->wd_list);
+	if (force_tsc_stable && (strcmp(cs->name, "tsc") == 0))
+		printk(KERN_WARNING "Forcing tsc to be treated as stable due to force_tsc_stable=1\n");
+	else {
+		printk(KERN_WARNING "  This clock is no longer valid for WATCHDOG or HIGH-RES.\n");
+		if (strcmp(cs->name, "tsc") == 0)
+			printk(KERN_WARNING "  Use force_tsc_stable=1 to override.\n");
+		cs->flags &= ~(CLOCK_SOURCE_VALID_FOR_HRES | CLOCK_SOURCE_WATCHDOG);
+		clocksource_change_rating(cs, 0);
+		list_del(&cs->wd_list);
+	}
 }
 
 static void clocksource_watchdog(unsigned long data)

Re: PATCH: Allow user to force 'tsc' to be treated as stable.

[Ingo Molnar]

* Ben Greear <greearb@candelatech.com> wrote:

> Allow user to force TSC as stable clock-source.
> Works around BIOS issues in the FWA-7304 (Via CN700 chipset)
> system, and possibly others.
>
> This is against 2.6.29-rc3.

What 'issues' does that work around, exactly?

	Ingo

Re: PATCH: Allow user to force 'tsc' to be treated as stable.

[Ben Greear]

Ingo Molnar wrote:
> * Ben Greear <greearb@candelatech.com> wrote:
> 
>> Allow user to force TSC as stable clock-source.
>> Works around BIOS issues in the FWA-7304 (Via CN700 chipset)
>> system, and possibly others.
>>
>> This is against 2.6.29-rc3.
> 
> What 'issues' does that work around, exactly?

Without this, my system comes up with no stable high-res
clock, and the getnstimeofday seems to return something
with around 1ms granularity.  I have HZ set to 1000, so
its probably just using the 'jiffy' clocksource.
This in turn screws up my patched version of pktgen and
likely is sub-optimal for other things as well.

It seems the TSC on this system is stable, but I got lost
trying to understand the watchdog testing code, so I'm
not certain if it is wrong or not.  With this force in
place, the system seems stable and ran overnight under
network load fine.

Also, the BIOS on my system seems quite bad about filling out
tables, so that may be part of the root cause.

Thanks,
Ben


> 
> 	Ingo
> --
> 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/


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: PATCH: Allow user to force 'tsc' to be treated as stable.

[Ingo Molnar]

* Ben Greear <greearb@candelatech.com> wrote:

> Ingo Molnar wrote:
>> * Ben Greear <greearb@candelatech.com> wrote:
>>
>>> Allow user to force TSC as stable clock-source.
>>> Works around BIOS issues in the FWA-7304 (Via CN700 chipset)
>>> system, and possibly others.
>>>
>>> This is against 2.6.29-rc3.
>>
>> What 'issues' does that work around, exactly?
>
> Without this, my system comes up with no stable high-res
> clock, and the getnstimeofday seems to return something
> with around 1ms granularity.  I have HZ set to 1000, so
> its probably just using the 'jiffy' clocksource.
> This in turn screws up my patched version of pktgen and
> likely is sub-optimal for other things as well.
>
> It seems the TSC on this system is stable, but I got lost
> trying to understand the watchdog testing code, so I'm
> not certain if it is wrong or not.  With this force in
> place, the system seems stable and ran overnight under
> network load fine.

What does this tool output:

 http://people.redhat.com/mingo/time-warp-test/time-warp-test.c

? (you can run it both the patched or on the unpatched system)

	Ingo

Re: PATCH: Allow user to force 'tsc' to be treated as stable.

[Frans Pop]

Ben Greear wrote:
> +       if (force_tsc_stable && (strcmp(cs->name, "tsc") == 0))
> +               printk(KERN_WARNING "Forcing tsc to be treated as stable due to force_tsc_stable=1\n");
> +       else { 
> +               printk(KERN_WARNING "  This clock is no longer valid for WATCHDOG or HIGH-RES.\n");
> +               if (strcmp(cs->name, "tsc") == 0)
> +                       printk(KERN_WARNING "  Use force_tsc_stable=1 to override.\n");

Do we really want to be so verbose?

Loads of users get these messages (I get it on both my laptops) and IIUC
the workaround is only valid for a very limited group of users.

I'd suggest dropping the "else" branch. It seems inadvisable to encourage
workarounds that may not be valid (or even safe).

Cheers,
FJP

Re: PATCH: Allow user to force 'tsc' to be treated as stable.

[Jiri Kosina]

On Thu, 5 Feb 2009, Ben Greear wrote:

> Allow user to force TSC as stable clock-source. Works around BIOS issues 
> in the FWA-7304 (Via CN700 chipset) system, and possibly others.

How do you know that the TSC is stable on such system? What was the reason 
for kernel marking it unstable?

> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index ca89e15..0578fcb 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -54,6 +54,7 @@ static LIST_HEAD(clocksource_list);
>  static DEFINE_SPINLOCK(clocksource_lock);
>  static char override_name[32];
>  static int finished_booting;
> +static int force_tsc_stable;
>  
>  /* clocksource_done_booting - Called near the end of core bootup
>   *
> @@ -82,6 +83,14 @@ static unsigned long watchdog_resumed;
>  #define WATCHDOG_INTERVAL (HZ >> 1)
>  #define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 4)
>  
> +static int __init parse_force_tsc_stable(char *arg)
> +{
> +	force_tsc_stable = 1;
> +	return 0;
> +}
> +early_param("force_tsc_stable", parse_force_tsc_stable);

Documentation/kernel-parameters.txt would also like to know about this 
option.

-- 
Jiri Kosina
SUSE Labs

Re: PATCH: Allow user to force 'tsc' to be treated as stable.

[Ben Greear]

Jiri Kosina wrote:
> On Thu, 5 Feb 2009, Ben Greear wrote:
> 
>> Allow user to force TSC as stable clock-source. Works around BIOS issues 
>> in the FWA-7304 (Via CN700 chipset) system, and possibly others.
> 
> How do you know that the TSC is stable on such system? What was the reason 
> for kernel marking it unstable?

I've used it for a timing source in my own modules and it has
worked well.  It could still be broken in some way, but if so,
I'm not sure how to detect it.  Ingo sent me a program to test
the tsc, and at least for short runs, it showed no problem.

Another person suggested the system might think it is unstable
because I have ACPI disabled in my BIOS and that might cause
the kernel not to have a 'PM timer' that it uses to check the
tsc?

Other than that, I have no idea why the kernel thinks the tsc
is unstable.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: PATCH: Allow user to force 'tsc' to be treated as stable.

[Thomas Gleixner]

On Fri, 6 Feb 2009, Ben Greear wrote:

> Jiri Kosina wrote:
> > On Thu, 5 Feb 2009, Ben Greear wrote:
> > 
> > > Allow user to force TSC as stable clock-source. Works around BIOS issues
> > > in the FWA-7304 (Via CN700 chipset) system, and possibly others.
> > 
> > How do you know that the TSC is stable on such system? What was the reason
> > for kernel marking it unstable?
> 
> I've used it for a timing source in my own modules and it has
> worked well.  It could still be broken in some way, but if so,
> I'm not sure how to detect it.  Ingo sent me a program to test
> the tsc, and at least for short runs, it showed no problem.
> 
> Another person suggested the system might think it is unstable
> because I have ACPI disabled in my BIOS and that might cause
> the kernel not to have a 'PM timer' that it uses to check the
> tsc?
> 
> Other than that, I have no idea why the kernel thinks the tsc
> is unstable.

Can you please provide the dmesg output, so I can see in which context
this happens.

Thanks,

	tglx

Re: PATCH: Allow user to force 'tsc' to be treated as stable.

[Jiri Kosina]

On Fri, 6 Feb 2009, Ben Greear wrote:

> > How do you know that the TSC is stable on such system? What was the 
> > reason for kernel marking it unstable?
> I've used it for a timing source in my own modules and it has worked 
> well.  It could still be broken in some way, but if so, I'm not sure how 
> to detect it.  Ingo sent me a program to test the tsc, and at least for 
> short runs, it showed no problem.
> Another person suggested the system might think it is unstable because I 
> have ACPI disabled in my BIOS and that might cause the kernel not to 
> have a 'PM timer' that it uses to check the tsc?
> Other than that, I have no idea why the kernel thinks the tsc is 
> unstable.

Normally the reason should be indicated by kernel in dmesg output, could 
you please provide that?

Thanks,

-- 
Jiri Kosina
SUSE Labs