Messages in this thread | | | Date | Tue, 10 Dec 2013 12:44:42 -0800 | From | John Stultz <> | Subject | Re: [PATCH] perf: POSIX CLOCK_PERF to report current time value |
| |
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
| |