RE: [Xen-devel] [PATCH] Adjust time init sequence

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

>From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx] 
>Sent: Wednesday, December 10, 2008 9:49 PM
>
>Is it really safe to use NOW() before init_percpu_time()? Seems dodgy.

Where did you mean by using NOW before init_percpu_time?
I moved do_settime earlier but with a zero system stamp now
which matches the line behind to init stime_platform_time to zero.
To me there's no difference to initialize wallclock at zero point
or sometime after with a NOW() drift, which should cause similar
result to wc_sec/wc_nsec. 

>
>Since calibration points are sync'ed across all CPUs on Xen 3.3 and
>unstable, trying to ensure that the first calibration period 
>across all CPUs
>is equal is a lost cause isn't it?

Yes, it's not a strong cause as I also mentioned in patch
description. But if it can be addressed without hurting others,
I think it still welcomed. Especially considering that only 2nd or
3rd calibration fully sync across all CPUs, that normally means
dom0 will suffer a short bump in 1-2s at early boot phase. It's
better to eliminish such unconformtable factor as we don't know
whether some corner case is not considered. :-)

Thanks,
Kevin

>
> -- Keir
>
>On 10/12/2008 13:10, "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote:
>
>> Adjust time init sequence
>> 
>> percpu timer init for BSP happens within init_xen_time,
>> while CMOS access at end may take up to 1s. This may
>> make 1st trigger to calibration timer happens >1s interval
>> and elapsed local stime for BSP also exceed 1s. However
>> percpu timer init happens after that point for APs, which
>> will then have a <1s elapsed local stime at 1st calibration.
>> That leads to distinct mul_frac among cores, which can
>> cause up to 6ms system time skew in the start. This is
>> not big issue, since gradually xen calibration framework
>> closes that gap. But it's still better to avoid that big
>> skew in early boot phase.
>> 
>> Signed-off-by Kevin Tian <kevin.tian@xxxxxxxxx>
>> 
>> diff -r 85b2f4aafea4 xen/arch/x86/time.c
>> --- a/xen/arch/x86/time.c Tue Dec 09 20:56:23 2008 -0500
>> +++ b/xen/arch/x86/time.c Tue Dec 09 22:21:07 2008 -0500
>> @@ -1095,7 +1095,7 @@
>>  
>>      local_irq_save(flags);
>>      rdtscll(t->local_tsc_stamp);
>> -    now = !plt_src.read_counter ? 0 : read_platform_stime();
>> +    now = read_platform_stime();
>>      local_irq_restore(flags);
>>  
>>      t->stime_master_stamp = now;
>> @@ -1118,12 +1118,13 @@
>>  
>>      open_softirq(TIME_CALIBRATE_SOFTIRQ, local_time_calibration);
>>  
>> -    init_percpu_time();
>> +    do_settime(get_cmos_time(), 0, 0);
>>  
>>      stime_platform_stamp = 0;
>>      init_platform_timer();
>>  
>> -    do_settime(get_cmos_time(), 0, NOW());
>> +    /* init bsp percpu time after platform timer 
>initialized, similar as AP
>> */
>> +    init_percpu_time();
>>  
>>      return 0;
>>  }
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel