From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752614AbdATO4P (ORCPT ); Fri, 20 Jan 2017 09:56:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56124 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751276AbdATO4M (ORCPT ); Fri, 20 Jan 2017 09:56:12 -0500 Date: Fri, 20 Jan 2017 11:52:58 -0200 From: Marcelo Tosatti To: Paolo Bonzini Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Radim Krcmar , Richard Cochran , Miroslav Lichvar Subject: Re: [patch 4/5] PTP: add PTP_SYS_OFFSET emulation via cross timestamps infrastructure Message-ID: <20170120135254.GA6350@amt.cnet> 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> User-Agent: Mutt/1.5.21 (2010-09-15) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 20 Jan 2017 14:55:53 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 20, 2017 at 02:36:40PM +0100, Paolo Bonzini wrote: > > > 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? Because if time is adjusted while you are taking the samples, the mean can return non existant values: 1) take sample1 (realtime = 2000) 2) userspace changes realtime (realtime = 100) 3) 2100/2 = 1050 However that 1050 value never existed, before or after userspace changed realtime. Such behaviour does not exist with PTP_SYS_OFFSET, because taking getnstimeofday64 is serialized against time changes. I am not sure whether returning such bizzare values is fine, to drop the lock. Hum... i think it must be because userspace will consider the new values after realtime is changed as correct. > 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)? Well can move it after the ->getcrosststamp loop. I'll just drop the spinlock and document the behaviour.