Messages in this thread | | | Date | Fri, 8 May 2015 14:06:43 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 2/9] x86, perf: Add support for PEBSv3 profiling |
| |
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.
| |