Re: [PATCH v2 3/6] x86/time: implement tsc as clocksource

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

On 04/05/2016 04:12 PM, Jan Beulich wrote:
>>>> On 05.04.16 at 16:56, <joao.m.martins@oracle.com> wrote:
>> On 04/05/2016 11:43 AM, Jan Beulich wrote:
>>>>>> On 29.03.16 at 15:44, <joao.m.martins@oracle.com> wrote:
>>>> @@ -541,6 +613,10 @@ static int __init try_platform_timer(struct platform_timesource *pts)
>>>>      if ( rc <= 0 )
>>>>          return rc;
>>>>  
>>>> +    /* We have a platform timesource already so reset it */
>>>> +    if ( plt_src.counter_bits != 0 )
>>>> +        reset_platform_timer();
>>>
>>> What if any of the time functions get used between here and the
>>> setting of the new clock source? For example, what will happen to
>>> log messages when "console_timestamps=..." is in effect?
>> time functions will use NOW() (i.e. get_s_time) which uses cpu_time structs
>> being updated on local_time_calibration() or cpu frequency changes.
>> reset_platform_timer() will zero out some of the variables used by the
>> plt_overflow and read_platform_stime(). The overflow and calibration isn't an
>> issue as the timers are previously killed so any further calls will use what's
>> on cpu_time while plt_src is being changed.
>>
>> The case I could see this being different is if cpu_frequency_change is called
>> between reset_platform_time() and the next update of stime_platform_stamp. In
>> this situation the calculation would end up a bit different meaning subsequent
>> calls of read_platform_stime() would return the equivalent to
>> scale_delta(plt_src->read_counter(), plt_scale), as opposed to computing with a
>> previous taken stime_platform_stamp and incrementing a difference with a counter
>> read. But can this situation actually happen?
> 
> No matter which CPU freq model is being used (right now, may
> change when the Intel P-state driver finally arrives), Dom0 is
> required to be executing already, so no issue here.
Cool.

> Since you didn't go into any detail on the specific example of log
> time stamps - am I to imply that they're completely unaffected by
> this (i.e. there's also no risk of them going backwards for a brief
> period of time)?
log timestamps uses wallclock_time() function which uses NOW() and follows the
first paragraph I explained. But I need to correct that statement as get_s_time
uses a previously seeded value from stime_local_stamp (which is set through
read_platform_time()). Time going backwards could happen only if TSC is behind
the currently-in-use clocksource. During that brief period it uses the values
taken from cpu_time but after I set the new clocksource it would go backwards as
I zeroed the previous stime/stamp snapshots in reset_platform_time. In this case
with I propose I don't have any other timestamps (platform_timer_stamp =
(stime_platform_stamp = 0)) to calculate the difference with, so it would go
backwards if the TSC falls behind. I could prevent this situation from happening
by having an offset which would be used until the new clocksource catches up to
the old one? I am also searching in the manual, if that can situation can happen.

Joao

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