Re: [PATCH 2/9] x86, perf: Add support for PEBSv3 profiling

[Peter Zijlstra <peterz@infradead.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.