From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754341AbZBIVrS (ORCPT ); Mon, 9 Feb 2009 16:47:18 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752240AbZBIVrA (ORCPT ); Mon, 9 Feb 2009 16:47:00 -0500 Received: from mga11.intel.com ([192.55.52.93]:56854 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752150AbZBIVq7 (ORCPT ); Mon, 9 Feb 2009 16:46:59 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.37,406,1231142400"; d="scan'208";a="429560026" Subject: Re: [PATCH NET-NEXT 02/10] time sync: generic infrastructure to map between time stamps generated by a time counter and system time From: Patrick Ohly To: John Stultz Cc: "linux-kernel@vger.kernel.org" , "netdev@vger.kernel.org" , David Miller , Thomas Gleixner In-Reply-To: <1234207667.10457.53.camel@jstultz-laptop> References: <1233752517-30010-1-git-send-email-patrick.ohly@intel.com> <1233752517-30010-2-git-send-email-patrick.ohly@intel.com> <1233752517-30010-3-git-send-email-patrick.ohly@intel.com> <1233776680.6994.30.camel@localhost.localdomain> <1233829262.20435.37.camel@pohly-MOBL> <1234198922.20325.170.camel@ecld0pohly> <1234207667.10457.53.camel@jstultz-laptop> Content-Type: text/plain Date: Mon, 09 Feb 2009 22:46:53 +0100 Message-Id: <1234216013.8213.18.camel@pohly-MOBL> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2009-02-09 at 21:27 +0200, John Stultz wrote: > On Mon, 2009-02-09 at 18:02 +0100, Patrick Ohly wrote: > > Is this revision of the patch okay? How should the two patches get > > included in the main kernel - via netdev-next-2.6? > > Small comment below, but otherwise it looks ok to me. I usually push > patches through Andrew, so I'd probably go that way. But I'd leave it to > Dave if he's comfortable pushing them to Linus. As you don't mind, I suggest to push through Dave as part of the complete patch series. That way we don't need to worry about coordinating two subtrees. > Acked-by: John Stultz Thanks! Will add that. > > + /* ignore negative durations */ > > + sample.duration_target = ktime_to_ns(ktime_sub(end, start)); > > + if (sample.duration_target >= 0) { > > You may also want to checking the bounds on the duration_target. If > preemption hits and the values are too out of whack, the symetric delay > assumption below might be quite invalid. > > I guess the outliers removal probably covers this as well, but seems > some sanity checking might be good. That would require more information, like "duration_target is usually in the xxx-yyy range". This could be determined based on past measurements or the median of the current sample set, but is this really better than the current "remove longest 25%"? In practice I haven't seen such a problem, therefore I'd prefer to keep the code simple and not change it. It was tested under load conditions (both CPU and network). -- Bye, Patrick Ohly -- Patrick.Ohly@gmx.de http://www.estamos.de/