From: Ian Munsie <imunsie@au1.ibm.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
Ingo Molnar <mingo@elte.hu>,
Frederic Weisbecker <fweisbec@gmail.com>,
Mike Galbraith <efault@gmx.de>, Paul Mackerras <paulus@samba.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 3/3] perf record/report: Process events in order
Date: Mon, 06 Dec 2010 23:42:16 +1100 [thread overview]
Message-ID: <1291637998-sup-4601@au1.ibm.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1012061014180.2653@localhost6.localdomain6>
Excerpts from Thomas Gleixner's message of Mon Dec 06 20:20:06 +1100 2010:
> See https://lkml.org/lkml/2010/12/5/45
>
> Slightly different, but the same idea :)
Yeah I noted that in my cover email. I meant to get my patches out
before the weekend, but hadn't quite finished cleaning them up to ensure
they didn't break perf when running on a kernel without sample_id_all.
> > + case PERF_RECORD_HEADER_ATTR:
> > + return ops->attr(event, s);
> > + case PERF_RECORD_HEADER_EVENT_TYPE:
> > + return ops->event_type(event, s);
> > + case PERF_RECORD_HEADER_TRACING_DATA:
> > + return ops->tracing_data(event, s);
> > + case PERF_RECORD_HEADER_BUILD_ID:
> > + return ops->build_id(event, s);
>
> These can be processed unordered.
I just moved them into this routine to keep all the dispatching in one
place, whether delayed or not. These particular events will still be
processed immediately when encountered in the file. Only >=
PERF_RECORD_MMAP && <= PERF_RECORD_SAMPLE will be delayed via the
perf_session__process_timed function.
> > + case PERF_RECORD_FINISHED_ROUND:
> > + return ops->finished_round(event, s, ops);
>
> You don't want to delay this unless you have unlimited memory. See my
> patch.
As above, this is not delayed.
> > {
> > + if (ops->ordered_samples && sample->time == -1ULL) {
> > + dump_printf("Event missing timestamp, switching to unordered processing\n");
> > + flush_sample_queue(s, ops);
> > + ops->ordered_samples = false;
>
> Why ? The events injected by perf record itself have no timestamps and
> do not need them. So why disabling ordered_samples ?
For instance, suppose we ran this on an old kernel without support for
timestamps on every event (so timestamps are only on sample events):
perf record -T
perf report
If perf report tried to process the events in order, all the events
without timestamps would be processed first -- including the
PERF_RECORD_EXIT event, which would cause every sample not to be
attributed. Falling back means we should get no worse than the old
behaviour, while an upgraded kernel will provide the timestamps and
should not fall back.
Cheers,
-Ian
next prev parent reply other threads:[~2010-12-06 12:42 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-04 22:26 [GIT PULL 0/5] perf/core: Support for PERF_SAMPLE_ in all events Arnaldo Carvalho de Melo
2010-12-04 22:26 ` [PATCH 1/5] perf events: Fix event inherit fallout of precalculated headers Arnaldo Carvalho de Melo
2010-12-04 22:26 ` [PATCH 2/5] perf events: Separate the routines handling the PERF_SAMPLE_ identity fields Arnaldo Carvalho de Melo
2010-12-04 22:26 ` [PATCH 3/5] perf events: Make sample_type identity fields available in all PERF_RECORD_ events Arnaldo Carvalho de Melo
2010-12-04 22:26 ` [PATCH 4/5] perf session: Parse sample earlier Arnaldo Carvalho de Melo
2010-12-04 22:26 ` [PATCH 5/5] perf tools: Ask for ID PERF_SAMPLE_ info on all PERF_RECORD_ events Arnaldo Carvalho de Melo
2010-12-05 13:32 ` [Patch] perf: session: Sort all events if ordered_samples=true Thomas Gleixner
2010-12-06 2:37 ` Process events in order if recording from multiple CPUs Ian Munsie
2010-12-06 2:37 ` [PATCH 1/3] perf tool: Better displaying of unresolved DSOs and symbols Ian Munsie
2010-12-07 6:56 ` [tip:perf/core] perf hist: " tip-bot for Ian Munsie
2010-12-06 2:37 ` [PATCH 2/3] perf: Move all output for perf report -D into trace_event Ian Munsie
2010-12-06 2:37 ` [PATCH 3/3] perf record/report: Process events in order Ian Munsie
2010-12-06 9:20 ` Thomas Gleixner
2010-12-06 12:42 ` Ian Munsie [this message]
2010-12-06 13:04 ` Thomas Gleixner
2010-12-06 13:11 ` Ian Munsie
2010-12-06 14:41 ` Thomas Gleixner
2010-12-07 1:15 ` Ian Munsie
2010-12-07 10:47 ` Thomas Gleixner
2010-12-07 10:54 ` Arnaldo Carvalho de Melo
2010-12-07 12:24 ` Thomas Gleixner
2010-12-07 6:57 ` [tip:perf/core] perf session: Sort all events if ordered_samples=true tip-bot for Thomas Gleixner
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=1291637998-sup-4601@au1.ibm.com \
--to=imunsie@au1.ibm.com \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@ghostprotocols.net \
--cc=efault@gmx.de \
--cc=eranian@google.com \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=paulus@samba.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