Message-ID: <4D6E5413.6060500@cisco.com>
Date:	Wed, 02 Mar 2011 07:28:35 -0700
From:	David Ahern <daahern@...co.com>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	linux-perf-users@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>, acme@...stprotocols.net,
	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <peterz@...radead.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Paul Mackerras <paulus@...ba.org>,
	John Stultz <johnstul@...ibm.com>
Subject: Re: [PATCH 3/6] perf record: add time-of-day option



On 03/02/2011 07:16 AM, Thomas Gleixner wrote:
> On Sun, 27 Feb 2011, David Ahern wrote:
> 
>> Enable data collection for generating time-of-day strings when
>> printing individual perf samples. This is done by sampling the
>> realtime clock event with the perf_clock time stamps.
>>
>> If the realtime-clock event is not available (e.g, older kernels)
>> fallback to a synthesized event. (I realize there is resistance
>> to new synthesized events, but it is a simple way to gain this
>> feature on older kernels without the need to modify the kernel
>> code).
> 
> ...
>   
>> +/* add a sample to the event stream based on user request */
>> +static int perf_event_generate_sample(struct perf_event *event)
>> +{
>> +	struct perf_sample_data data;
>> +	struct pt_regs regs;
>> +
>> +	perf_fetch_caller_regs(&regs);
>> +	event->pmu->read(event);
>> +	perf_sample_data_init(&data, 0);
>> +	data.period = event->hw.last_period;
>> +	perf_event_output(event, 0, &data, &regs);
>> +
>> +	return 0;
>> +}
> 
> Errm. This is a generic ioctl to inject random events from user space.
> 
> First of all this wants to be a separate patch and not burried into
> some 100 lines of changes to tools/perf.
> 
> Secondly adding such an ioctl needs a pretty good reason, and the use
> case at hand is not really one.

Peter has made it clear he objects to the ioctl.

> 
> Reading through the other patches, your goal is to correlate
> CLOCK_REALTIME based logs to CLOCK_PERF based events.
> 
> To achieve that you inject cyclic clock_realtime samples into the
> event stream and want to have this initial event inserted via the
> ioctl.

I still need a solution. There is no means for requesting a sample
immediately when a counter is opened, and there is a desire not to add
any more synthesized events. How then do I get the initial correlation
without scanning a potentially large file looking for a single event
type occurrence.

> 
> How does all that deal with CLOCK_REALTIME being affected by NTP and
> settimeofday? Not really, as far as I can tell. It somehow works, but
> that depends on the frequency of your event injection.

It is sampled at some periodic rate to get NTP changes. Right now it is
hardcoded at once an hour. The frequency option can be added to the
--tod parameter.

> 
> To be honest, that's just too much churn for a feature which is single
> purpose and questionable functionality.
> 
> It would be far more interesting to have trace points in the
> timekeeping code, which are useful for other things as well
> (e.g. precise monitoring of NTP) and provide always a correct
> association with the trace clock. That would cover everything from
> frequency adjustments, clock setting including the information about
> the relation to clock monotonic.

I'm not looking to track ntp changes. I want time-of-day strings on
sample dumps. That is not a questionable feature request; it is a
usability enhancement.

We are users of perf. Fantastic tool, but of limited functionality to
the engineers on the team if they cannot correlate event samples with
application logs.

David


> 
> Thanks,
> 
> 	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists