Re: [PATCH v3 2/6] KVM: x86: switch to masterclock update using timekeeper functionality

[Denis Plotnikov <dplotnikov@virtuozzo.com>]

On 01.08.2017 15:41, Paolo Bonzini wrote:
> On 01/08/2017 14:11, Denis Plotnikov wrote:
>> In fact, this "cycles_valid" is going to be used for deciding whether to
>> use KVM masterclock or not. And if it's not we still want to know
>> cycles_stamp value to use it in KVM.
> 
> Why?  Neither pvclock_update_vm_gtod_copy nor kvm_pv_clock_pairing do
> anything with the two variables that are passed by reference, if the
> read returns false.  Hence my suggestion of calling it cycles_valid.
giving up
> 
>> So the cycles is valid, but clocksource is not reliable (why? let decide
>> to a clocksource, by default assume they are all not stable), thus we
>> must calculate time values for a guest each time its needed.
>> So, my proposal is to name the variable sightly differently: cs_reliable
>> and go like:
>>      if (clock->read_clock_with_stamp) {
>>          systime_snapshot->cs_reliable =
>>              clock->read_clock_with_stamp(
>>                  &now, &systime_snapshot->cycles);
>>      } else {
>>          now = tk_clock_read(&tk->tkr_mono);
>>          systime_snapshot->cs_reliable = false;
>>          systime_snapshot->cycles = now;
>>      }
>> What do you think?
> 
> I'm afraid you still have to define the meaning of "reliable".  (Though
> I agree that the right default is false, that was a thinko on my side.
> This also means that you need to define read_clock_with_stamp for the
> TSC clocksource too).
> 
> Paolo
> 
agree
   name: cycles_valid
   default: false
   read_clock_with_stamp: defined for tsc returning true

Thanks!

Patches are in progress.
They well be sent soon, nice and shiny!

Denis