lkml.org 
[lkml]   [2011]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH -tip v2 2/6] perf script: add magic word to indicate the failure of resolving symbols
    (2011/07/18 1:30), David Ahern wrote:
    > On 07/17/2011 10:07 AM, David Ahern wrote:
    >>
    >>
    >> On 07/17/2011 03:30 AM, Akihiro Nagai wrote:
    >>> Latest perf-script doesn't output symbol name when it failed to
    >>> resolve symbol name. This output is not friendly for external scripts
    >>> because it causes misaligment. So, this patch adds the magic word
    >>> "(unknown)" when perf-script failed to resolve symbols.
    >>
    >> Code has [unknown] with {}, not ().
    >
    > Evidently I had the shift key down -- s/{}/[]/. :-) And I see now, for
    > dsoname you have [unknown] and for symname (unknown). Use the same for
    > both. Since () is already used for separating dsoname, use [unknown] for
    > both
    At print_sample_start() in builtin-script.c, there is the code that print "(unknown)"
    when perf cannnot resolve the event name.

    printf("%s: ", evname ? evname : "(unknown)");

    On the other hand, at hist_entry__dso_snprintf() in util/sort.c, there is the code
    prints "[unknown]" for unresolvable DSO name.

    return repsep_snprintf(bf, size, "%-*s", width, "[unknown]");

    In addition, perf uses "[]" for DSO name for example "[kernel.kallsyms]".
    So, I chose it. However, this specification is not easy for user-scripts.
    Should I unify this expression?

    Thank you.
    >
    > David
    >
    >
    >>
    >>>
    >>> At the same time, this patch changes perf-script to output "[unknown]"
    >>> in the DSO field, when it failed to resolve the path of DSOs.
    >>>
    >>> Changes in v2:
    >>> - add this patch
    >>>
    >>> Signed-off-by: Akihiro Nagai<akihiro.nagai.hw@hitachi.com>
    >>> Cc: Peter Zijlstra<peterz@infradead.org>
    >>> Cc: Frederic Weisbecker<fweisbec@gmail.com>
    >>> Cc: Paul Mackerras<paulus@samba.org>
    >>> Cc: Ingo Molnar<mingo@elte.hu>
    >>> Cc: Arnaldo Carvalho de Melo<acme@infradead.org>
    >>> Cc: David Ahern<dsahern@gmail.com>
    >>> Cc: Masami Hiramatsu<masami.hiramatsu.pt@hitachi.com>
    >>> ---
    >>>
    >>> tools/perf/builtin-script.c | 18 +++++-------------
    >>> tools/perf/util/map.c | 12 ++++++++++++
    >>> tools/perf/util/map.h | 1 +
    >>> tools/perf/util/session.c | 35 ++++++++++-------------------------
    >>> tools/perf/util/symbol.c | 12 ++++++++++++
    >>> tools/perf/util/symbol.h | 1 +
    >>> 6 files changed, 41 insertions(+), 38 deletions(-)
    >>>
    >>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
    >>> index 09024ec..b3e0951 100644
    >>> --- a/tools/perf/builtin-script.c
    >>> +++ b/tools/perf/builtin-script.c
    >>> @@ -320,7 +320,6 @@ static void print_sample_addr(union perf_event *event,
    >>> {
    >>> struct addr_location al;
    >>> u8 cpumode = event->header.misc& PERF_RECORD_MISC_CPUMODE_MASK;
    >>> - const char *symname, *dsoname;
    >>>
    >>> printf("%16" PRIx64, sample->addr);
    >>>
    >>> @@ -340,21 +339,14 @@ static void print_sample_addr(union perf_event *event,
    >>> al.sym = map__find_symbol(al.map, al.addr, NULL);
    >>>
    >>> if (PRINT_FIELD(SYM)) {
    >>> - if (al.sym&& al.sym->name)
    >>> - symname = al.sym->name;
    >>> - else
    >>> - symname = "";
    >>> -
    >>> - printf(" %16s", symname);
    >>> + printf(" ");
    >>> + symbol__print_symname(al.sym);
    >>> }
    >>>
    >>> if (PRINT_FIELD(DSO)) {
    >>> - if (al.map&& al.map->dso&& al.map->dso->name)
    >>> - dsoname = al.map->dso->name;
    >>> - else
    >>> - dsoname = "";
    >>> -
    >>> - printf(" (%s)", dsoname);
    >>> + printf(" (");
    >>> + map__print_dsoname(al.map);
    >>> + printf(")");
    >>> }
    >>> }
    >>>
    >>> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
    >>> index a16ecab..dddc0f3 100644
    >>> --- a/tools/perf/util/map.c
    >>> +++ b/tools/perf/util/map.c
    >>> @@ -200,6 +200,18 @@ size_t map__fprintf(struct map *self, FILE *fp)
    >>> self->start, self->end, self->pgoff, self->dso->name);
    >>> }
    >>>
    >>> +void map__print_dsoname(struct map *self)
    >>> +{
    >>> + const char *dsoname;
    >>> +
    >>> + if (self&& self->dso&& self->dso->name)
    >>> + dsoname = self->dso->name;
    >>
    >> extra tab in the above line.
    >>
    >> Otherwise looks good to me.
    Oops! I fix it. Thank you.
    >>
    >> Acked-By: dsahern@gmail.com
    >>
    >>> + else
    >>> + dsoname = "[unknown]";
    >>> +
    >>> + printf("%s", dsoname);
    >>> +}
    >>> +
    >>> /*
    >>> * objdump wants/reports absolute IPs for ET_EXEC, and RIPs for ET_DYN.
    >>> * map->dso->adjust_symbols==1 for ET_EXEC-like cases.
    >>> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
    >>> index b397c03..6f452b9 100644
    >>> --- a/tools/perf/util/map.h
    >>> +++ b/tools/perf/util/map.h
    >>> @@ -112,6 +112,7 @@ void map__delete(struct map *self);
    >>> struct map *map__clone(struct map *self);
    >>> int map__overlap(struct map *l, struct map *r);
    >>> size_t map__fprintf(struct map *self, FILE *fp);
    >>> +void map__print_dsoname(struct map *self);
    >>>
    >>> int map__load(struct map *self, symbol_filter_t filter);
    >>> struct symbol *map__find_symbol(struct map *self,
    >>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
    >>> index 453a010..442be3a 100644
    >>> --- a/tools/perf/util/session.c
    >>> +++ b/tools/perf/util/session.c
    >>> @@ -1214,7 +1214,6 @@ void perf_session__print_ip(union perf_event *event,
    >>> int print_sym, int print_dso)
    >>> {
    >>> struct addr_location al;
    >>> - const char *symname, *dsoname;
    >>> struct callchain_cursor *cursor =&session->callchain_cursor;
    >>> struct callchain_cursor_node *node;
    >>>
    >>> @@ -1242,20 +1241,13 @@ void perf_session__print_ip(union perf_event *event,
    >>>
    >>> printf("\t%16" PRIx64, node->ip);
    >>> if (print_sym) {
    >>> - if (node->sym&& node->sym->name)
    >>> - symname = node->sym->name;
    >>> - else
    >>> - symname = "";
    >>> -
    >>> - printf(" %s", symname);
    >>> + printf(" ");
    >>> + symbol__print_symname(node->sym);
    >>> }
    >>> if (print_dso) {
    >>> - if (node->map&& node->map->dso&& node->map->dso->name)
    >>> - dsoname = node->map->dso->name;
    >>> - else
    >>> - dsoname = "";
    >>> -
    >>> - printf(" (%s)", dsoname);
    >>> + printf(" (");
    >>> + map__print_dsoname(al.map);
    >>> + printf(")");
    >>> }
    >>> printf("\n");
    >>>
    >>> @@ -1265,21 +1257,14 @@ void perf_session__print_ip(union perf_event *event,
    >>> } else {
    >>> printf("%16" PRIx64, sample->ip);
    >>> if (print_sym) {
    >>> - if (al.sym&& al.sym->name)
    >>> - symname = al.sym->name;
    >>> - else
    >>> - symname = "";
    >>> -
    >>> - printf(" %s", symname);
    >>> + printf(" ");
    >>> + symbol__print_symname(al.sym);
    >>> }
    >>>
    >>> if (print_dso) {
    >>> - if (al.map&& al.map->dso&& al.map->dso->name)
    >>> - dsoname = al.map->dso->name;
    >>> - else
    >>> - dsoname = "";
    >>> -
    >>> - printf(" (%s)", dsoname);
    >>> + printf(" (");
    >>> + map__print_dsoname(al.map);
    >>> + printf(")");
    >>> }
    >>> }
    >>> }
    >>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
    >>> index eec1963..85e19c4 100644
    >>> --- a/tools/perf/util/symbol.c
    >>> +++ b/tools/perf/util/symbol.c
    >>> @@ -175,6 +175,18 @@ static size_t symbol__fprintf(struct symbol *sym, FILE *fp)
    >>> sym->name);
    >>> }
    >>>
    >>> +void symbol__print_symname(const struct symbol *sym)
    >>> +{
    >>> + const char *symname;
    >>> +
    >>> + if (sym&& sym->name)
    >>> + symname = sym->name;
    >>> + else
    >>> + symname = "(unknown)";
    >>> +
    >>> + printf("%s", symname);
    >>> +}
    >>> +
    >>> void dso__set_long_name(struct dso *dso, char *name)
    >>> {
    >>> if (name == NULL)
    >>> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
    >>> index 325ee36..1ec17b4 100644
    >>> --- a/tools/perf/util/symbol.h
    >>> +++ b/tools/perf/util/symbol.h
    >>> @@ -238,6 +238,7 @@ void machines__destroy_guest_kernel_maps(struct rb_root *machines);
    >>>
    >>> int symbol__init(void);
    >>> void symbol__exit(void);
    >>> +void symbol__print_symname(const struct symbol *sym);
    >>> bool symbol_type__is_a(char symbol_type, enum map_type map_type);
    >>>
    >>> size_t machine__fprintf_vmlinux_path(struct machine *machine, FILE *fp);
    >>>


    \
     
     \ /
      Last update: 2011-07-21 11:39    [W:0.057 / U:1.536 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site