On 20/01/2017 15:02, Radim Krcmar wrote:
> 2017-01-20 14:36+0100, Paolo Bonzini:
>> On 20/01/2017 14:07, Marcelo Tosatti wrote:
>>> On Fri, Jan 20, 2017 at 01:55:27PM +0100, Paolo Bonzini wrote:
>>>>
>>>>
>>>> On 20/01/2017 13:20, Marcelo Tosatti wrote:
>>>>> kernel/time/timekeeping.c | 79 +++++++++++++++++++++++++++++++++++++++
>>>>
>>>> Why not leave this in drivers/ptp/ptp_chardev.c?
>>>
>>> timekeeper_lock
>>
>> Why does emulate_ptp_sys_offset need it, if the current PTP_SYS_OFFSET
>> code doesn't? Is the latency acceptable (considering this is a raw spin
>> lock) or is there a seqlock that we can use instead (such as tk_core.seq
>> like in get_device_system_crosststamp)?
>
> The spinlock prevents writers to take the tk_core.seq, which means that
> time cannot be changed during that.
>
> The simplest alternative would be to use tk_core.seq for all our reads,
> but that would increse the chance of re-reading, even infinitely.
How much? If a hypercall takes 1 microsecond, and PTP_MAX_SAMPLES is
25, we should be done in less than 50 microseconds. If update_wall-time
is called with 250 Hz frequency (sounds like a lot), that's still 4000
microseconds so the probability of even 3-4 consecutive retries should
be very low.
> But we don't need to read everything with the same time base -- if the
> time is changed (by NTP/user/...) between our reads, then the value will
> be off, but if writer took tk_core.seq just to accumulate current time,
> then the time after accumulation stays the same and it would work as if
> we had the tk_core.seq for the whole time.
You mean only check seqlock separately for each sample, but restart the
entire loop upon changes to cs_was_changed_seq or clock_was_set_seq?
That would work too.
> Another solution would be to do just one one read and set it to all
> saples -- the difference between t[i] and t[i+2] would be 0. We are
> quite sure the just one read is enough, this hack could be even better.
I'd be afraid of messing up chrony's stats...
Paolo
>>>>> + if (ptp->info->emulate_ptp_sys_offset_mean) {
>>>>> + err = emulate_ptp_sys_offset(ptp->info, sysoff, arg);
>>>>> + break;
>>>>> + }
>>>>
>>>> I think this should be simply "if (!ptp->info->gettime64)" and,
>>>> likewise, there should be an emulation based getcrosststamp in
>>>> ptp_clock_gettime.
>>>>
>>>> Paolo
>>>
>>> gettime64 is called directly via ptp_clock_gettime.
>>
>> Yes, but ptp_clock_gettime can be taught to use getcrosststamp instead.
>
> I agree,
>
> if (!gettime64)
> use getcrosststamp
>
> and KVM PTP device will not implement gettime64().
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
