From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752944AbbEHMG6 (ORCPT ); Fri, 8 May 2015 08:06:58 -0400 Received: from casper.infradead.org ([85.118.1.10]:37692 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751558AbbEHMG4 (ORCPT ); Fri, 8 May 2015 08:06:56 -0400 Date: Fri, 8 May 2015 14:06:43 +0200 From: Peter Zijlstra To: Andi Kleen Cc: kan.liang@intel.com, eranian@google.com, acme@infradead.org, linux-kernel@vger.kernel.org, Andi Kleen Subject: Re: [PATCH 2/9] x86, perf: Add support for PEBSv3 profiling Message-ID: <20150508120643.GG27504@twins.programming.kicks-ass.net> References: <1431039392-12589-1-git-send-email-andi@firstfloor.org> <1431039392-12589-3-git-send-email-andi@firstfloor.org> <20150508105958.GD27504@twins.programming.kicks-ass.net> <20150508115954.GT2366@two.firstfloor.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150508115954.GT2366@two.firstfloor.org> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 08, 2015 at 01:59:54PM +0200, Andi Kleen wrote: > > That's wrong. It does not respect perf_event_attr::clock_id. > > Ok. > > Are we really defaulting to real time now? No, we still default to the thing you implemented. However, when the event is configured differently, you'll write the wrong stamps in. This code needs to consider that. > (CLOCK_REALTIME == 0) > What happens when ntpd is active and lets the time drift? That > sounds very bad for event accuracy. NTP slew rate adjustment should still keep the thing monotonic, so its still useful to order events. But as is, CLOCK_REALTIME is not supported for hardware events, only CLOCK_MONOTONIC and CLOCK_MONOTONIC_RAW are. > The clockid patch looks broken to me. At least it should default > to MONOTONIC time, not just the time that happens to be 0 by > accident. You've not looked good enough, the Changelog states and the code matches that the default is unchanged. > Also I question we really need that many different kinds of > time stamps. CLOCK_MONOTONIC is useful for cluster wide timestamps, CLOCK_MONOTONIC_RAW is useful for things that need/want to correlate to fixed rate clocks. Most of the other clocks are just a side effect of using the generic clockid API for selecting clocks.