Re: [PATCH 2/3] perf events: Introduce realtime clock event

On Fri, 2011-02-18 at 07:39 -0700, David Ahern wrote:

On 02/18/11 04:14, Peter Zijlstra wrote:

quoted

On Thu, 2011-02-17 at 22:53 -0700, David Ahern wrote:

quoted

The motivation for this event is to convert perf_clock() time stamps
to wall-clock (gettimeofday()) equivalents, including adjustments made
by NTP (e.g., for comparing perf events to other log files).

quoted

This patch is based on the monotonic patch by Arnaldo Carvalho de Melo
[off-list ref].

(NOTE: Comments from the last review of the timehist patch series
suggested calling this a monotonic clock. I am not trying to be
dense here; since gettimeofday maps to realtime clock I think that
is the better name for it.)

Well, the idea was to use CLOCK_MONOTONIC, not to call CLOCK_REALTIME
monotonic.

I'm really not sure why you want CLOCK_REALTIME and I think
CLOCK_MONOTONIC is more useful (I'd argue you want your system logs to
contain both, every admin who's ever had to untangle what happened
during DST switches will agree)

I believe CLOCK_MONOTONIC is what perf_clock is tied to -- the
timestamps for PERF_SAMPLE_TIME -- so we already have that.

Its not (it mere _can_ be), it could be tied to the TSC which can
significantly drift wrt CLOCK_MONOTONIC.

Programs that generate time-of-day output are using gettimeofday which
is tied to CLOCK_REALTIME. We want to be able to correlate a perf sample
to an entry in an applications log file.

Well, you can argue those programs are broken :-), Imagine the joys of
trying to figure out wth happens when DST jumps the clock back an hour
and you have an hour of duplicate data.

quoted

quoted

@@ -5610,6 +5612,13 @@ static enum hrtimer_restart perf_swevent_hrtimer(struct hrtimer *hrtimer)
 
 	perf_sample_data_init(&data, 0);
 	data.period = event->hw.last_period;
+	if (event->attr.sample_type & PERF_SAMPLE_RAW) 
+	{
+		raw.size = sizeof(u64);
+		raw.data = &event->count;
+		data.raw = &raw;
+	}
+
 	regs = get_irq_regs();
 
 	if (regs && !perf_exclude_event(event, regs)) {

Why!? you already keep ->count = ktime_get_real(), so simply reading the
count value will get you the timestamp.. this is superfluous at best.

And that is a conundrum I was stuck on for a while. perf record does not
sample counters; it only creates sample events. I looked at having perf
record sample the clock event, but then I would have to synthesize an
event for the output file. Similarly perf record for hardware counters
does not show the value of the counter.

Well, but you can fix that, simply add PERF_SAMPLE_READ, no need to
abuse the raw crap.