Re: [PATCH] perf report: auto-detect branch stack sampling mode

[David Ahern]

On 2/24/12 8:28 AM, Stephane Eranian wrote:
> On Fri, Feb 24, 2012 at 4:24 PM, David Ahern<dsahern@xxxxxxxxx>  wrote:
> > On 2/24/12 2:40 AM, Stephane Eranian wrote:
> > > This patch adds auto-detection of samples with taken branch stacks.
> > > The auto-detection avoids having to specify the -b or --branch-stack
> > > option on the cmdline.
> > >
> > > The patch adds a new feature bit HEADER_BRANCH_STACK to mark the
> > > presence of branch stacks in samples.
> > >
> > > You can now do:
> > > $ perf record -b any noploop 2
> > > $ perf report
> > > # Events: 8K cycles
> > > #
> > > # Overhead  Command  Source Shared Object        Source Symbol  Target
> > > Shared Object       Target Symbol
> > > # ........  .......  ....................  ...................
> > >   ....................  ..................
> > > #
> > >      91.56%  noploop  noploop               [.] noploop
> > >    noploop  [.] noploop
> > >       0.42%  noploop  [kernel.kallsyms]     [k] __lock_acquire
> > >   [kernel.kallsyms]  [k] __lock_acquire
> > >
> > >
> > > To force regular reporting based on the instruction address:
> > > $ perf report --no-branch-stack
> > > #
> > > # Events: 2K cycles
> > > #
> > > # Overhead  Command      Shared Object                           Symbol
> > > # ........  .......  .................  ...............................
> > > #
> > >      92.03%  noploop  noploop            [.] noploop
> > >       1.00%  noploop  [kernel.kallsyms]  [k] lock_acquire
> > >
> > >
> > > Signed-off-by: Stephane Eranian<eranian@xxxxxxxxxx>
> > > ---
> > >
> > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > > index 1c49d4e..5e833a2 100644
> > > --- a/tools/perf/builtin-record.c
> > > +++ b/tools/perf/builtin-record.c
> > > @@ -473,6 +473,9 @@ static int __cmd_record(struct perf_record *rec, int
> > > argc, const char **argv)
> > >         if (!have_tracepoints(&evsel_list->entries))
> > >                 perf_header__clear_feat(&session->header,
> > > HEADER_TRACE_INFO);
> > >
> > > +       if (!rec->opts.branch_stack)
> > > +               perf_header__clear_feat(&session->header,
> > > HEADER_BRANCH_STACK);
> >
> >
> > branch tracing is user requested on, so shouldn't feature default off and
> > only be enabled when requested?
> Well, what Ingo was suggesting is that perf report auto-detects whether or
> not branch mode is necessary by looking at the perf.data file. Most likely
> if you've recorded with -b, you are interested in a branch mode view rather
> that the instruction view (default). So all this does is elimintate the need
> to pass -b to perf report to enable branch mode.

Right. My point is that HEADER_BRANCH_STACK feature should be off by 
default and only enabled by builtin-record when branch stack is 
requested. You have the reverse -- enabled by default and off in record 
if not requested.

David

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