From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752121AbdATOEy (ORCPT ); Fri, 20 Jan 2017 09:04:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47454 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751966AbdATOEx (ORCPT ); Fri, 20 Jan 2017 09:04:53 -0500 Date: Fri, 20 Jan 2017 15:02:16 +0100 From: Radim Krcmar To: Paolo Bonzini Cc: Marcelo Tosatti , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Richard Cochran , Miroslav Lichvar Subject: Re: [patch 4/5] PTP: add PTP_SYS_OFFSET emulation via cross timestamps infrastructure Message-ID: <20170120140216.GA1358@potion> References: <20170120122025.665985919@redhat.com> <20170120122503.746158230@redhat.com> <48bb2650-ed00-ec07-31bf-8780d3ab5568@redhat.com> <20170120130711.GA27440@amt.cnet> <2d213ad9-fa40-1f1e-90a9-404764969d35@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2d213ad9-fa40-1f1e-90a9-404764969d35@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 20 Jan 2017 14:02:19 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. 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. 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. >>>> + 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().