Re: [PATCH v3 3/6] x86/time: streamline platform time init on plt_update()

[Joao Martins <joao.m.martins@oracle.com>]

On 08/25/2016 11:13 AM, Jan Beulich wrote:
>>>> On 24.08.16 at 14:43, <joao.m.martins@oracle.com> wrote:
>> And use to initialize platform time solely for clocksource=tsc,
>> as opposed to initializing platform overflow timer, which would
>> only fire in ~180 years (on 2.2 Ghz Broadwell processor).
> 
> Do we really want to risk this time period going down by two orders
> of magnitude? Is there anything that's really expensive in setting the
> overflow timer in the far distant future?
It wasn't about cost but rather setting the timer in a so distant future. I could
decrease to an year time, month or day. But do you think we really need that overflow
handling for TSC?

>> Changes since v2:
>>  - Remove pointless intializer and replace it with the
>>  platform_time init return.
> 
> Does this really apply to this patch?
Oh no, The comment should have been something like:

"Remove clocksource_is_tsc in favor of comparing pts against plt_tsc"

> 
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -526,17 +526,31 @@ static s_time_t __read_platform_stime(u64 
>> platform_time)
>>      return (stime_platform_stamp + scale_delta(diff, &plt_scale));
>>  }
>>  
>> +static void __plt_update(void)
> 
> A single leading underscore only, please.
Fixed.

> 
>> @@ -630,10 +644,21 @@ static s64 __init try_platform_timer(struct platform_timesource *pts)
>>  
>>      set_time_scale(&plt_scale, pts->frequency);
>>  
>> -    plt_overflow_period = scale_delta(
>> -        1ull << (pts->counter_bits - 1), &plt_scale);
>>      plt_src = *pts;
>>  
>> +    if ( pts == &plt_tsc )
>> +    {
>> +        plt_update();
>> +    }
> 
> Unnecessary braces.
Fixed.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel