Re: [PATCH] perf: POSIX CLOCK_PERF to report current time value

[John Stultz <john.stultz@linaro.org>]

On 12/10/2013 12:27 PM, David Ahern wrote:
> From: Pawel Moll <pawel.moll@arm.com>
>
> To co-relate user space events with the perf events stream
> a current (as in: "what time(stamp) is it now?") time value
> must be made available.
>
> This patch adds a POSIX clock returning the perf_clock()
> value and accesible from userspace:
>
>     #include <time.h>
>
>     struct timespec ts;
>
>     clock_gettime(CLOCK_PERF, &ts);

As stated earlier on a few occasions, I really don't like exposing the
perf clock directly to userland like this, because the perf clock is not
a well defined time domain. Instead its basically defined as "whatever
sched_clock does". Over the last few years, the behavior of sched_clock
has changed, so I'm not confident it won't change in the future.

I do understand the desire, but to export a poorly defined clockid to
userland has ABI stability concerns for me.  Particularly when the
performance characteristics will make it prone to abuse.

I'd much rather see perf export CLOCK_MONOTONIC_RAW timestamps, since
that clockid is well defined. Even if the underlying clocksource is
different, I suspect the perf clock could interpolate fairly closely to
CLOCK_MONOTONIC_RAW, as it already does similar interpolation on top of
CLOCK_MONOTONIC in some cases.

Other then this, I've said I'd not object other approaches, such as the
ioctl method or the dynamic clockid. These solutions also have the same
ABI stability concern, but the responsibility for that can be kept
outside the timekeeping code, so the perf folks can handle the potential
ABI stability downside. Additionally the few extra steps required to get
to the time stamping method make it less likely to be abused.

Again, I really do understand the need, and I don't mean to be a pain
here, but as a maintainer I have to push back here.

thanks
-john