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: Tue, 07 Dec 2010 00:11:28 +1100 [thread overview]
Message-ID: <1291640985-sup-4443@au1.ibm.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1012061359140.2653@localhost6.localdomain6>
Excerpts from Thomas Gleixner's message of Tue Dec 07 00:04:20 +1100 2010:
> > 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.
>
> Gah. This is nasty. I really prefer the explicit split of instant
> processed and possibly delayed events. That makes the code clear and
> easy to extend. I just want to add a new event type at the right place
> and not worry about magic comparisions in some other place.
Fair enough, I'll split them back out.
> > 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.
>
> Ok, but you'll break existing code which does only care about sample
> ordering if you do that at the session level unconditionally.
Good point. I'll give this some thought overnight.
Cheers,
-Ian
next prev parent reply other threads:[~2010-12-06 13:11 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
2010-12-06 13:04 ` Thomas Gleixner
2010-12-06 13:11 ` Ian Munsie [this message]
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=1291640985-sup-4443@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.