Re: [PATCH RFC] xen-time: decreasing the rating of the xen clocksource below that of the tsc clocksource for dom0's

[Imre Palik <imrep.amz@gmail.com>]

On 01/07/15 17:30, Ian Campbell wrote:
> On Wed, 2015-01-07 at 17:16 +0100, Imre Palik wrote:
>> From: "Palik, Imre" <imrep@amazon.de>
>>
>> In Dom0's the use of the TSC clocksource (whenever it is stable enough to
>> be used) instead of the Xen clocksource should not cause any issues, as
>> Dom0 VMs never live-migrated.
> 
> Is this still true given that dom0's vcpus are migrated amongst pcpus on
> the host? The tsc are not synchronised on some generations of hardware
> so the result there would be the TSC appearing to do very odd things
> under dom0's feet. Does Linux cope with that or does it not matter for
> some other reason?

First of all, if the initial pcpus are not synchronised, linux won't consider
TSC as a viable clocksource.

If the initial pcpus are synchronised, but then the dom0 vcpus are migrated to
unsynchronised pcpus, then hopefully the tsc watchdog catches the issue, and
the kernel falls back to the xen clocksource.  The issue here is that the
watchdog can only detect clock skews bigger than 0.0625s/0.5s.  Hopefully this
is enough to avoid the weirdest things.

Also, some parts of the kernel (e.g., scheduling) will always use the paravirt
clock.  No matter what priorities are set.

So it should be safe for some definition of safe.
But I was unable to test it on a hardware platform that would hit the problematic
case described above.

> 
>>
>> Cc: Anthony Liguori <aliguori@amazon.com>
>> Signed-off-by: Imre Palik <imrep@amazon.de>
>> ---
>>  arch/x86/xen/time.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
>> index f473d26..c768726 100644
>> --- a/arch/x86/xen/time.c
>> +++ b/arch/x86/xen/time.c
>> @@ -487,6 +487,10 @@ static void __init xen_time_init(void)
>>  	int cpu = smp_processor_id();
>>  	struct timespec tp;
>>  
>> +	/* As Dom0 is never moved, no penalty on using TSC there */
>> +	if (xen_initial_domain())
>> +		xen_clocksource.rating = 275;
>> +
>>  	clocksource_register_hz(&xen_clocksource, NSEC_PER_SEC);
>>  
>>  	if (HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, cpu, NULL) == 0) {
> 
>