From: David Ahern <dsahern@gmail.com>
To: Frederic Weisbecker <fweisbec@gmail.com>, acme@ghostprotocols.net
Cc: linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
mingo@elte.hu, peterz@infradead.org, paulus@samba.org,
tglx@linutronix.de
Subject: Re: [PATCH 4/6] perf record: add time-of-day option
Date: Sun, 14 Aug 2011 22:24:33 -0600 [thread overview]
Message-ID: <4E489F81.7060007@gmail.com> (raw)
In-Reply-To: <20110617151510.GG25197@somewhere.redhat.com>
On 06/17/2011 09:15 AM, Frederic Weisbecker wrote:
> On Fri, Jun 17, 2011 at 08:23:01AM -0600, David Ahern wrote:
>> On 06/17/2011 08:14 AM, Frederic Weisbecker wrote:
>>>
>>> So I feel uncomfortable with this tod_sample_type hack. I think we can't really continue
>>> with this fixed sample_type per session given the kind of hacks that involves.
>>>
>>> One thing we could do is to split session->sample_type into an array with one sample
>>> type per event type (hardware, breakpoint, software, tracepoint).
>>>
>>> And then each builtin tool can provide their constraints on top of these values:
>>>
>>> - builtin-report wants sample_type[HARDWARE] == sample_type[SOFTWARE] == sample_type[TRACEPOINT] == sample_type[BREAKPOINT]
>>> although that may be tunable by the time but we can start with that.
>>> - builtin-script has no specific constraints, except that sample_type[i] meets what the user passed as a parameter
>>> - etc..
>>>
>>> Constraints can probably default to sample_type[i] == sample_type[i+1] to mimic the current behaviour. Then tools
>>> can override that.
>>>
>>> What do you think?
>>
>> I started working on sample_type refactoring right after sending this
>> patchset (though I got sidetracked). Each evsel in the list has a
>> perf_attr struct which has a sample_type. Why not use that which allows
>> events to have their own sample type - versus a type per event type?
>
> This can make sense, I can figure out some cases where such granularity can be
> useful. Branch recording doesn't care about recording period for example I think.
>
>>
>> I'll see if I can get back to it in the next few days and get a better
>> idea of the pain involved with the refactoring.
>
> Thanks a lot :)
Coming back to this one ....
From what I can see sample_type has to be a global per perf session and
all samples have to use the same sample_type or a change is needed to
the API/ABI.
The perf_event_header does not have any information that uniquely
associates it with a specific event type. Right now
perf_evlist__id2evsel() is used to associate a sample with a specific
event (evsel) in the list, but that function requires a parsed sample.
To parse a sample we need the sample_type. So, the sample_type has to be
a global and the same for all samples.
David
next prev parent reply other threads:[~2011-08-15 4:24 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-07 23:53 [PATCH 0/6] Add time-of-day option to perf events David Ahern
2011-06-07 23:55 ` [PATCH 1/6] trace: add tracepoints to timekeeping code - xtime changes David Ahern
2011-06-17 13:23 ` Frederic Weisbecker
2011-06-17 14:13 ` David Ahern
2011-06-17 14:19 ` Frederic Weisbecker
2011-08-15 4:03 ` David Ahern
2011-06-07 23:55 ` [PATCH 2/6] perf utils: export parse_single_tracepoint_event David Ahern
2011-06-07 23:55 ` [PATCH 3/6] perf: add reference time event David Ahern
2011-06-17 13:32 ` Frederic Weisbecker
2011-06-17 14:04 ` David Ahern
2011-06-17 14:17 ` Frederic Weisbecker
2011-06-17 14:28 ` David Ahern
2011-07-11 4:20 ` David Ahern
2011-07-12 14:30 ` Frederic Weisbecker
2011-07-12 16:35 ` David Ahern
2011-07-12 17:03 ` Frederic Weisbecker
2011-08-04 15:10 ` David Ahern
[not found] ` <20110808193033.GA2744@ghostprotocols.net>
2011-08-15 4:06 ` David Ahern
2011-06-07 23:56 ` [PATCH 4/6] perf record: add time-of-day option David Ahern
2011-06-17 14:14 ` Frederic Weisbecker
2011-06-17 14:23 ` David Ahern
2011-06-17 15:15 ` Frederic Weisbecker
2011-08-15 4:24 ` David Ahern [this message]
2011-06-07 23:56 ` [PATCH 5/6] perf script: pass trace event to print_trace_event David Ahern
2011-06-07 23:56 ` [PATCH 6/6] perf script: add time-of-day option for displaying events David Ahern
2011-06-14 18:42 ` [PATCH 0/6] Add time-of-day option to perf events David Ahern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E489F81.7060007@gmail.com \
--to=dsahern@gmail.com \
--cc=acme@ghostprotocols.net \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=paulus@samba.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).