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

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

[Imre Palik]

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.  The TSC clocksource is somewhat more
efficient than the Xen paravirtualised clocksource, thus it should have
higher rating.

This patch decreases the rating of the Xen clocksource in Dom0s to 275.
Which is half-way between the rating of the TSC clocksource (300) and the
hpet clocksource (250).

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) {
-- 
1.7.9.5

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

[David Vrabel]

On 13/01/15 08:14, 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.  The TSC clocksource is somewhat more
> efficient than the Xen paravirtualised clocksource, thus it should have
> higher rating.
> 
> This patch decreases the rating of the Xen clocksource in Dom0s to 275.
> Which is half-way between the rating of the TSC clocksource (300) and the
> hpet clocksource (250).

I'm happy with this but would like to see acks from those who objected
to v1.

David

> --- 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) {
> 

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

[Boris Ostrovsky]

On 01/13/2015 04:52 AM, David Vrabel wrote:
> On 13/01/15 08:14, 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.  The TSC clocksource is somewhat more
>> efficient than the Xen paravirtualised clocksource, thus it should have
>> higher rating.
>>
>> This patch decreases the rating of the Xen clocksource in Dom0s to 275.
>> Which is half-way between the rating of the TSC clocksource (300) and the
>> hpet clocksource (250).
> I'm happy with this but would like to see acks from those who objected
> to v1.
>
> David
>
>> --- 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 */

Again, why not any PV guest with TSC_MODE_NEVER_EMULATE?

And I am not sure I understood the argument about unsynchronized TSCs. 
You mentioned you can't find a platform to test on. I think you may be 
able to unsynchronize TSCs by writing something to MSR 0x10 early during 
Xen boot and then migrate dom0's vcpu to that pcpu later (making sure 
this vcpu was not running there while dom0 was booting)

-boris

>> +	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) {
>>

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

[David Vrabel]

On 13/01/15 15:42, Boris Ostrovsky wrote:
> On 01/13/2015 04:52 AM, David Vrabel wrote:
>> On 13/01/15 08:14, 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.  The TSC clocksource is somewhat more
>>> efficient than the Xen paravirtualised clocksource, thus it should have
>>> higher rating.
>>>
>>> This patch decreases the rating of the Xen clocksource in Dom0s to 275.
>>> Which is half-way between the rating of the TSC clocksource (300) and
>>> the
>>> hpet clocksource (250).
>> I'm happy with this but would like to see acks from those who objected
>> to v1.
>>
>> David
>>
>>> --- 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 */
> 
> Again, why not any PV guest with TSC_MODE_NEVER_EMULATE?

Surely if TSC_MODE_NEVER_EMULATE is set then the TSC is /not/ stable
across a guest save/restore thus the PV clocksource must be used?

I don't think we want to assume that TSC_MODE_NEVER_EMULATE => never
migrate.

David

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

[Boris Ostrovsky]

On 01/13/2015 11:07 AM, David Vrabel wrote:
> On 13/01/15 15:42, Boris Ostrovsky wrote:
>> On 01/13/2015 04:52 AM, David Vrabel wrote:
>>> On 13/01/15 08:14, 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.  The TSC clocksource is somewhat more
>>>> efficient than the Xen paravirtualised clocksource, thus it should have
>>>> higher rating.
>>>>
>>>> This patch decreases the rating of the Xen clocksource in Dom0s to 275.
>>>> Which is half-way between the rating of the TSC clocksource (300) and
>>>> the
>>>> hpet clocksource (250).
>>> I'm happy with this but would like to see acks from those who objected
>>> to v1.
>>>
>>> David
>>>
>>>> --- 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 */
>> Again, why not any PV guest with TSC_MODE_NEVER_EMULATE?
> Surely if TSC_MODE_NEVER_EMULATE is set then the TSC is /not/ stable
> across a guest save/restore thus the PV clocksource must be used?

TSC is declared stable when !d->disable_migrate && !d->arch.vtsc, with 
vtsc being 0 with TSC_MODE_NEVER_EMULATE (per domain_cpuid()).

And if TSC is not stable as seen by CPUID (which would be the case if 
disable_migrate is not set) then kernel won't use TSC as clocksource 
anyway, regardless of rating value, won't it?

-boris

>
> I don't think we want to assume that TSC_MODE_NEVER_EMULATE => never
> migrate.
>
> David

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

[Boris Ostrovsky]

On 01/13/2015 11:17 AM, Boris Ostrovsky wrote:
> On 01/13/2015 11:07 AM, David Vrabel wrote:
>> On 13/01/15 15:42, Boris Ostrovsky wrote:
>>> On 01/13/2015 04:52 AM, David Vrabel wrote:
>>>> On 13/01/15 08:14, 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.  The TSC clocksource is somewhat more
>>>>> efficient than the Xen paravirtualised clocksource, thus it should 
>>>>> have
>>>>> higher rating.
>>>>>
>>>>> This patch decreases the rating of the Xen clocksource in Dom0s to 
>>>>> 275.
>>>>> Which is half-way between the rating of the TSC clocksource (300) and
>>>>> the
>>>>> hpet clocksource (250).
>>>> I'm happy with this but would like to see acks from those who objected
>>>> to v1.
>>>>
>>>> David
>>>>
>>>>> --- 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 */
>>> Again, why not any PV guest with TSC_MODE_NEVER_EMULATE?
>> Surely if TSC_MODE_NEVER_EMULATE is set then the TSC is /not/ stable
>> across a guest save/restore thus the PV clocksource must be used?
>
> TSC is declared stable when !d->disable_migrate && !d->arch.vtsc, with 
> vtsc being 0 with TSC_MODE_NEVER_EMULATE (per domain_cpuid()).

I think I inverted domain_cpuid's logic here but my point was that we 
use'd use combination of TSC_MODE_NEVER_EMULATE and CPUID flag 
indicating TSC stability to pick the clocksource.


-boris

>
> And if TSC is not stable as seen by CPUID (which would be the case if 
> disable_migrate is not set) then kernel won't use TSC as clocksource 
> anyway, regardless of rating value, won't it?
>
> -boris
>
>>
>> I don't think we want to assume that TSC_MODE_NEVER_EMULATE => never
>> migrate.
>>
>> David
>

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

[Boris Ostrovsky]

On 01/13/2015 11:33 AM, Boris Ostrovsky wrote:
> On 01/13/2015 11:17 AM, Boris Ostrovsky wrote:
>> On 01/13/2015 11:07 AM, David Vrabel wrote:
>>> On 13/01/15 15:42, Boris Ostrovsky wrote:
>>>> On 01/13/2015 04:52 AM, David Vrabel wrote:
>>>>> On 13/01/15 08:14, 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.  The TSC clocksource is somewhat more
>>>>>> efficient than the Xen paravirtualised clocksource, thus it 
>>>>>> should have
>>>>>> higher rating.
>>>>>>
>>>>>> This patch decreases the rating of the Xen clocksource in Dom0s 
>>>>>> to 275.
>>>>>> Which is half-way between the rating of the TSC clocksource (300) 
>>>>>> and
>>>>>> the
>>>>>> hpet clocksource (250).
>>>>> I'm happy with this but would like to see acks from those who 
>>>>> objected
>>>>> to v1.
>>>>>
>>>>> David
>>>>>
>>>>>> --- 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 */
>>>> Again, why not any PV guest with TSC_MODE_NEVER_EMULATE?
>>> Surely if TSC_MODE_NEVER_EMULATE is set then the TSC is /not/ stable
>>> across a guest save/restore thus the PV clocksource must be used?
>>
>> TSC is declared stable when !d->disable_migrate && !d->arch.vtsc, 
>> with vtsc being 0 with TSC_MODE_NEVER_EMULATE (per domain_cpuid()).
>
> I think I inverted domain_cpuid's logic here but my point was that we 
> use'd use combination of TSC_MODE_NEVER_EMULATE and CPUID flag 
> indicating TSC stability to pick the clocksource.
>
>
> -boris
>
>>
>> And if TSC is not stable as seen by CPUID (which would be the case if 
>> disable_migrate is not set) then kernel won't use TSC as clocksource 
>> anyway, regardless of rating value, won't it?


So this is not going to work because early_init_intel() may set 
X86_FEATURE_CONSTANT_TSC unconditionally, without consulting CPUID.

Imre, you can ignore my question about non-dom0 guests then.

-boris

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

[David Vrabel]

On 13/01/15 08:14, 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.  The TSC clocksource is somewhat more
> efficient than the Xen paravirtualised clocksource, thus it should have
> higher rating.
> 
> This patch decreases the rating of the Xen clocksource in Dom0s to 275.
> Which is half-way between the rating of the TSC clocksource (300) and the
> hpet clocksource (250).

Applied to devel/for-linus-3.20, thanks.

David