lkml.org 
[lkml]   [2008]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Skip tsc synchronization checks if CONSTANT_TSC bit is set.
On 10/24/2008 12:25 PM, Andi Kleen wrote:
>> BIOSes are also just software, and we have to deal with bugs in them
>> *all the time*. The reality is that we're going to have to deal with
>> both vendor and user reluctance to upgrade, and therefore have to deal
>> with brokenness in the field.
>
> In the field they will just continue using clock=pit, like they
> always did on vmware. And also they will not update the Linux kernel.
>
> This is strictly for new installations. And I frankly don't
> see why Linux needs to get white listed workarounds when the
> Hypervisor couldn't as well be fixed. We have the bizarre
> situation here where a HV vendor tries to add workarounds
> to Linux instead of fixing it on their products.
>

What exactly would you like vmware to fix? VMware fully virtualizes
x86. However, when running the kernel on virtual cpus, as compared to
running on a physical cpus, the timing characteristics are different --
virtual cpus have to time share physical cpus with each other.

So, timing assumptions that the kernel makes when running directly on
physical cpus no longer hold when running on virtual cpus.

Prior to clocksource/clockevents, the timing assumptions that the Linux
kernel made were terrible for hypervisors. Now, the assumptions are
much better. However, three *minor* assumptions that the kernel makes
are violated when running on a hypervisor:

1) The fast-path TSC calibration code makes assumptions about being able
to sample various counters in sequence in a set amount of time that are
not true when running virtualized. (Actually, it makes assumptions that
aren't really true 100% of the time on physical cpus, but in that case
the odds of violating the assumptions (by hitting an SMI at exactly the
right time and length) are really rare.

Note that accurate calibration of the TSC is extremely important in
clocksource kernels since any error will lead to long term drift of
wallclock time.

2) There is no guarantee that the acpi_pm timer will be sampled at least
every 4.68 seconds (the wrap interval), because the vcpu, in extreme
circumstances, may not have a chance to run in that time. Thus, the
acpi_pm timer is not suitable to be used as a clocksource watchdog when
running on a hypervisor.

3) Virtual TSCs can be kept nearly in sync, but because the virtual TSC
offset is set by software, it's not perfect. So, the TSC
synchronization test can fail. (Really, it can fail on native as well,
and that's why the tests for backwards TSC were added to
read_tsc()/vread_tsc()).

Clearly, #1 and #2 *cannot* be fixed in the hypervisor. These are cases
where the kernel is making assumptions that just are not true when
running on certain platforms (i.e. hypervisors). Let's fix them.

#3, as you have suggested below, can perhaps be fixed by loosening the
check a bit to allow some leeway for marginally offset TSCs.

> Now making generic code a little more flexible in what
> it accepts is fine though (like relaxing tsc_sync or
> checking and trusting UNSTABLE_TSC). That will scale at least
> and doesn't need significant new code.
>

I think everyone can agree that this is the preferred approach, in
general. And in fact it was the approach Alok first used for the TSC
frequency calibration problem (this is one reason why he merged the
32-bit and 64-bit TSC code -- to standardize on the more robust 64-bit
calibration code). But, in the end, folks wanted a "fast" TSC
calibration path, and that path makes assumptions that just won't be
true when running on a hypervisor, so we are left with skipping that
path if we are on a virtual cpu.

Also, with regards to your claim that users should continue to use
clock=pit like options on newer kernels: that is just plain *wrong*.
The reason for clock=pit (really clock=pmtmr) recommendation on
pre-clocksource kernels wasn't to avoid using the TSC, but it was simply
a workaround to avoid the kernel code that attempted to compensate for
lost ticks (but would do so incorrectly in the case of late, but not
lost, interrupts -- again, it was a kernel timing assumption that was
invalid on hypervisors).

Dan


\
 
 \ /
  Last update: 2008-10-24 22:19    [W:0.157 / U:0.616 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site