lkml.org 
[lkml]   [2012]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] perf report: auto-detect branch stack sampling mode
From
On Fri, Feb 24, 2012 at 4:24 PM, David Ahern <dsahern@gmail.com> 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@google.com>
>> ---
>>
>> 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.

> David
>
>
>> +
>>        if (!rec->file_new) {
>>                err = perf_session__read_header(session, output);
>>                if (err<  0)
>> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
>> index 528789f..edd4289 100644
>> --- a/tools/perf/builtin-report.c
>> +++ b/tools/perf/builtin-report.c
>> @@ -306,21 +306,14 @@ static int __cmd_report(struct perf_report *rep)
>>  {
>>        int ret = -EINVAL;
>>        u64 nr_samples;
>> -       struct perf_session *session;
>>        struct perf_evsel *pos;
>> +       struct perf_session *session = rep->session;
>>        struct map *kernel_map;
>>        struct kmap *kernel_kmap;
>>        const char *help = "For a higher level overview, try: perf report
>> --sort comm,dso";
>>
>>        signal(SIGINT, sig_handler);
>>
>> -       session = perf_session__new(rep->input_name, O_RDONLY,
>> -                                   rep->force, false,&rep->tool);
>>
>> -       if (session == NULL)
>> -               return -ENOMEM;
>> -
>> -       rep->session = session;
>> -
>>        if (rep->cpu_list) {
>>                ret = perf_session__cpu_bitmap(session, rep->cpu_list,
>>                                               rep->cpu_bitmap);
>> @@ -489,7 +482,10 @@ parse_callchain_opt(const struct option *opt, const
>> char *arg, int unset)
>>
>>  int cmd_report(int argc, const char **argv, const char *prefix __used)
>>  {
>> +       struct perf_session *session;
>>        struct stat st;
>> +       bool has_br_stack;
>> +       int ret = -1;
>>        char callchain_default_opt[] = "fractal,0.5,callee";
>>        const char * const report_usage[] = {
>>                "perf report [<options>]",
>> @@ -600,7 +596,23 @@ int cmd_report(int argc, const char **argv, const
>> char *prefix __used)
>>                        report.input_name = "perf.data";
>>        }
>>
>> -       if (sort__branch_mode) {
>> +       session = perf_session__new(report.input_name, O_RDONLY,
>> +                                   report.force, false,&report.tool);
>> +       if (session == NULL)
>> +               return -ENOMEM;
>> +
>> +       report.session = session;
>> +
>> +       has_br_stack = perf_header__has_feat(&session->header,
>> +                                            HEADER_BRANCH_STACK);
>> +
>> +       /*
>> +        * if branch mode set by user via -b or --branch-stack
>> +        * or not forced off by user (-no-branch-stack) user and present
>> +        * in the file then we set branch mode
>> +        */
>> +       if (sort__branch_mode || (sort__branch_mode == -1&&
>>  has_br_stack)) {
>>
>> +               sort__branch_mode = true;
>>                if (use_browser)
>>                        fprintf(stderr, "Warning: TUI interface not
>> supported"
>>                                        " in branch mode\n");
>> @@ -657,13 +669,13 @@ int cmd_report(int argc, const char **argv, const
>> char *prefix __used)
>>        }
>>
>>        if (symbol__init()<  0)
>> -               return -1;
>> +               goto error;
>>
>>        setup_sorting(report_usage, options);
>>
>>        if (parent_pattern != default_parent_pattern) {
>>                if (sort_dimension__add("parent")<  0)
>> -                       return -1;
>> +                       goto error;
>>
>>                /*
>>                 * Only show the parent fields if we explicitly
>> @@ -685,5 +697,8 @@ int cmd_report(int argc, const char **argv, const char
>> *prefix __used)
>>        sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm",
>> stdout);
>>        sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol",
>> stdout);
>>
>> -       return __cmd_report(&report);
>> +       ret = __cmd_report(&report);
>> +error:
>> +       perf_session__delete(session);
>> +       return ret;
>>  }
>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>> index c851495..c22491e 100644
>> --- a/tools/perf/util/header.c
>> +++ b/tools/perf/util/header.c
>> @@ -1023,6 +1023,12 @@ static int write_cpuid(int fd, struct perf_header
>> *h __used,
>>        return do_write_string(fd, buffer);
>>  }
>>
>> +static int write_branch_stack(int fd __used, struct perf_header *h
>> __used,
>> +                      struct perf_evlist *evlist __used)
>> +{
>> +       return 0;
>> +}
>> +
>>  static void print_hostname(struct perf_header *ph, int fd, FILE *fp)
>>  {
>>        char *str = do_read_string(fd, ph);
>> @@ -1315,6 +1321,12 @@ static void print_cpuid(struct perf_header *ph, int
>> fd, FILE *fp)
>>        free(str);
>>  }
>>
>> +static void print_branch_stack(struct perf_header *ph __used, int fd
>> __used,
>> +                              FILE *fp)
>> +{
>> +       fprintf(fp, "# contains samples with branch stacks\n");
>> +}
>> +
>>  static int __event_process_build_id(struct build_id_event *bev,
>>                                    char *filename,
>>                                    struct perf_session *session)
>> @@ -1519,6 +1531,7 @@ static const struct feature_ops
>> feat_ops[HEADER_LAST_FEATURE] = {
>>        FEAT_OPA(HEADER_CMDLINE,        cmdline),
>>        FEAT_OPF(HEADER_CPU_TOPOLOGY,   cpu_topology),
>>        FEAT_OPF(HEADER_NUMA_TOPOLOGY,  numa_topology),
>> +       FEAT_OPA(HEADER_BRANCH_STACK,   branch_stack),
>>  };
>>
>>  struct header_print_data {
>> diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
>> index e68f617..21a6be0 100644
>> --- a/tools/perf/util/header.h
>> +++ b/tools/perf/util/header.h
>> @@ -27,7 +27,7 @@ enum {
>>        HEADER_EVENT_DESC,
>>        HEADER_CPU_TOPOLOGY,
>>        HEADER_NUMA_TOPOLOGY,
>> -
>> +       HEADER_BRANCH_STACK,
>>        HEADER_LAST_FEATURE,
>>        HEADER_FEAT_BITS        = 256,
>>  };
>> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
>> index 2739ed1..69d50c0 100644
>> --- a/tools/perf/util/sort.c
>> +++ b/tools/perf/util/sort.c
>> @@ -8,7 +8,7 @@ const char      default_sort_order[] = "comm,dso,symbol";
>>  const char    *sort_order = default_sort_order;
>>  int           sort__need_collapse = 0;
>>  int           sort__has_parent = 0;
>> -bool           sort__branch_mode;
>> +bool           sort__branch_mode = -1; /* -1 = means not set */
>>
>>  enum sort_type        sort__first_dimension;
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2012-02-24 16:31    [W:0.100 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site