All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: "Liang, Kan" <kan.liang@intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Andi Kleen <ak@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH RFC V9 2/3] perf,tools: per-event callgraph support
Date: Mon, 10 Aug 2015 16:39:42 -0300	[thread overview]
Message-ID: <20150810193942.GF2521@kernel.org> (raw)
In-Reply-To: <37D7C6CF3E00A74B8858931C1DB2F077018DF6B7@SHSMSX103.ccr.corp.intel.com>

Em Mon, Aug 10, 2015 at 06:57:20PM +0000, Liang, Kan escreveu:
> > <SNIP>

> > > > I get:

> > > > Samples: 2K of event 'cpu/instructions,call-
> > > > graph=no,time=0,period=20000/p', Event count (approx.): 46956518
> > > >   Children      Self  Command          Shared Object Symbol                  ◆
> > > > -   67.56%     0.00%  qemu-system-x86  [unknown]     [.]
> > > > 0xad5e258d4c544155  ▒
> > > >      0xad5e258d4c544155                                                      ▒
> > > > -   67.56%     0.00%  qemu-system-x86  libc-2.20.so  [.] __libc_start_main
> > > >      __libc_start_main                                                       ▒
> > > >      0xad5e258d4c544155                                                      ▒

> > > > This is in the 'perf report' TUI, why, for an event with
> > > > 'callgraph=no', we get callchains? How come?

> > > That's the design.
> > > For sampling multiple events, it may not be needed to collect
> > > callgraphs for all of them. Because the sample sites are usually
> > > nearby. It's enough to collect the callgraphs on a reference event.
> > > For other events, it can still show callgraphs according to the callgraphs on
> > a reference event.

> > So, "call-graph=no" doesn't mean you don't want callchains for a particular
> > events _if_ there is another event in the group for which callchains is
> > available.

> > But if "call-graph=no" for all events, then, yes, "no" means really "no". :-)

> > I think we should use "call-graph=ref" to mean that no callchains should be
> > requested to the kernel infrastructure for that particular event, but that
> > when doing the report, use callchains available in some other event
> > (perhaps would be good to specify which one), while "call-graph=no"
> > really means "no", i.e. no callchains asked from the kernel for this event,
> > and _no_ callchains to appear on report.

> > If "ref" is used and no callchains are available anywhere, that is a bug as
> > well, i.e. I asked for callchains up to a event to be used, by getting that info
> > from another event, but no event has callchains:
> > error.
 
> If we use " call-graph=ref", it means "ref" is a new callchain mode. But it's not.
> I think the "ref" thing should only impact the perf report.

I don't have much of a problem with that, but using "ref" to make the
intention, i.e. use reference callchains, documented, clear, makes sense to me.

I.e. when you ask for two events, one with callchains and the other without it
doesn't necessarily means we want callchains appearing on the ones we have not
enabled them.

> So we may introduce a new option "--show-callchain-ref" for that purpose.
> If it applied, the available callchain information from other event will be
> printed for "call-graph=no" event.

Ok, if the user explicitely asked for "--show-callchain-ref", then
he/she will not get confused seeing callchains for an event with
"call-graph=no".

Ah, probably --show-ref-call-graph should be better, to keep it consistent with
all the other options dealing with call-graph stuff.

> If not, no callchain information is printed for "call-graph=no" event.
> The default is no print.

Agreed, I think this almost completely reduces the possible source of
confusion.

> Is it OK?

Ok.

One possible improvement to your proposal: When showing callchains in
reference mode, make that extra explicit by adding some marker on the
side of the event name.

I.e. right now we will see callchains, when this is with another event with
callchains:

  Samples: 24  of event 'cpu/instructions,call-graph=no,time=0,period=20000/p', Event count (approx.): 480000
  Overhead  Command  Shared Object     Symbol
    12.50%  usleep   libc-2.20.so      [.] _dl_addr

My suggestion is to have something like:

  Samples: 24  of event 'cpu/instructions,call-graph=no,time=0,period=20000/p', ref cg, Event count (approx.): 480000
  Overhead  Command  Shared Object     Symbol
    12.50%  usleep   libc-2.20.so      [.] _dl_addr

See that ", ref cg"?

But would be just to remove the confusion of seeing, on the same screen,
"call-graph=no" when one _sees_ call graphs.

- Arnaldo

  reply	other threads:[~2015-08-10 19:39 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-06 19:44 [PATCH RFC V9 1/3] perf,tools: move callchain option parse code to util.c kan.liang
2015-08-06 19:44 ` [PATCH RFC V9 2/3] perf,tools: per-event callgraph support kan.liang
2015-08-07 10:54   ` Jiri Olsa
2015-08-07 15:38   ` Arnaldo Carvalho de Melo
2015-08-07 15:49     ` Arnaldo Carvalho de Melo
2015-08-08 16:45       ` Jiri Olsa
2015-08-08 17:35         ` Arnaldo Carvalho de Melo
2015-08-10 13:46         ` Arnaldo Carvalho de Melo
2015-08-10 12:56       ` Liang, Kan
2015-08-10 15:39         ` Arnaldo Carvalho de Melo
2015-08-10 18:57           ` Liang, Kan
2015-08-10 19:39             ` Arnaldo Carvalho de Melo [this message]
2015-08-08 17:12     ` [PATCH] perf tools: Unset perf_event_attr::freq when period term is set Jiri Olsa
2015-08-12 12:29       ` [tip:perf/core] perf tools: Unset perf_event_attr:: freq " tip-bot for Jiri Olsa
2015-08-06 19:44 ` [PATCH RFC V9 3/3] perf,tests: Add tests to callgraph and time parse kan.liang
2015-08-07 10:54   ` Jiri Olsa
2015-08-07 10:51 ` [PATCH] perf tools: Move perf_counts struct and functions into separate object Jiri Olsa
2015-08-07 13:14   ` Arnaldo Carvalho de Melo
2015-08-12 12:28   ` [tip:perf/core] perf stat: " tip-bot for Jiri Olsa
2015-08-12 12:29 ` [tip:perf/core] perf callchain: Move option parsing code to util.c tip-bot for Kan Liang

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=20150810193942.GF2521@kernel.org \
    --to=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@kernel.org \
    /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.