Re: [PATCH 12/25] perf symbols: Add nr_events to symbol_conf
From: David Ahern
Date: Mon Nov 28 2011 - 23:44:56 EST
On 11/28/2011 04:14 PM, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
>
> Since symbol__alloc_hists need it, to avoid passing it around in many
> functions have it in the symbol_conf struct.
I get why you want to do this, but symbol_conf seems to be an odd
choice. This is number of events, not how to resolve or display symbols.
David
>
> Cc: David Ahern <dsahern@xxxxxxxxx>
> Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> Cc: Mike Galbraith <efault@xxxxxx>
> Cc: Paul Mackerras <paulus@xxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Stephane Eranian <eranian@xxxxxxxxxx>
> Link: http://lkml.kernel.org/n/tip-cwv8ysvpywzjq4v3xtbd4zwv@xxxxxxxxxxxxxx
> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> ---
> tools/perf/builtin-annotate.c | 22 ++++++++--------------
> tools/perf/builtin-report.c | 3 +--
> tools/perf/builtin-top.c | 7 ++++---
> tools/perf/util/annotate.c | 6 +++---
> tools/perf/util/annotate.h | 5 ++---
> tools/perf/util/header.c | 2 ++
> tools/perf/util/hist.h | 3 +--
> tools/perf/util/symbol.h | 1 +
> tools/perf/util/ui/browsers/annotate.c | 16 +++++++---------
> tools/perf/util/ui/browsers/hists.c | 2 +-
> 10 files changed, 30 insertions(+), 37 deletions(-)
>
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 46b4c24..8b9091b 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -43,10 +43,9 @@ static const char *sym_hist_filter;
> static const char *cpu_list;
> static DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
>
> -static int perf_evlist__add_sample(struct perf_evlist *evlist,
> - struct perf_sample *sample,
> - struct perf_evsel *evsel,
> - struct addr_location *al)
> +static int perf_evsel__add_sample(struct perf_evsel *evsel,
> + struct perf_sample *sample,
> + struct addr_location *al)
> {
> struct hist_entry *he;
> int ret;
> @@ -69,8 +68,7 @@ static int perf_evlist__add_sample(struct perf_evlist *evlist,
> ret = 0;
> if (he->ms.sym != NULL) {
> struct annotation *notes = symbol__annotation(he->ms.sym);
> - if (notes->src == NULL &&
> - symbol__alloc_hist(he->ms.sym, evlist->nr_entries) < 0)
> + if (notes->src == NULL && symbol__alloc_hist(he->ms.sym) < 0)
> return -ENOMEM;
>
> ret = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
> @@ -98,8 +96,7 @@ static int process_sample_event(union perf_event *event,
> if (cpu_list && !test_bit(sample->cpu, cpu_bitmap))
> return 0;
>
> - if (!al.filtered &&
> - perf_evlist__add_sample(session->evlist, sample, evsel, &al)) {
> + if (!al.filtered && perf_evsel__add_sample(evsel, sample, &al)) {
> pr_warning("problem incrementing symbol count, "
> "skipping event\n");
> return -1;
> @@ -114,8 +111,7 @@ static int hist_entry__tty_annotate(struct hist_entry *he, int evidx)
> print_line, full_paths, 0, 0);
> }
>
> -static void hists__find_annotations(struct hists *self, int evidx,
> - int nr_events)
> +static void hists__find_annotations(struct hists *self, int evidx)
> {
> struct rb_node *nd = rb_first(&self->entries), *next;
> int key = K_RIGHT;
> @@ -138,8 +134,7 @@ find_next:
> }
>
> if (use_browser > 0) {
> - key = hist_entry__tui_annotate(he, evidx, nr_events,
> - NULL, NULL, 0);
> + key = hist_entry__tui_annotate(he, evidx, NULL, NULL, 0);
> switch (key) {
> case K_RIGHT:
> next = rb_next(nd);
> @@ -217,8 +212,7 @@ static int __cmd_annotate(void)
> total_nr_samples += nr_samples;
> hists__collapse_resort(hists);
> hists__output_resort(hists);
> - hists__find_annotations(hists, pos->idx,
> - session->evlist->nr_entries);
> + hists__find_annotations(hists, pos->idx);
> }
> }
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 4d7c834..758a287 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -92,8 +92,7 @@ static int perf_session__add_hist_entry(struct perf_session *session,
> assert(evsel != NULL);
>
> err = -ENOMEM;
> - if (notes->src == NULL &&
> - symbol__alloc_hist(he->ms.sym, session->evlist->nr_entries) < 0)
> + if (notes->src == NULL && symbol__alloc_hist(he->ms.sym) < 0)
> goto out;
>
> err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index c9cdedb..04288ee 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -170,7 +170,7 @@ static int parse_source(struct hist_entry *he)
>
> pthread_mutex_lock(¬es->lock);
>
> - if (symbol__alloc_hist(sym, top.evlist->nr_entries) < 0) {
> + if (symbol__alloc_hist(sym) < 0) {
> pthread_mutex_unlock(¬es->lock);
> pr_err("Not enough memory for annotating '%s' symbol!\n",
> sym->name);
> @@ -210,8 +210,7 @@ static void record_precise_ip(struct hist_entry *he, int counter, u64 ip)
> if (pthread_mutex_trylock(¬es->lock))
> return;
>
> - if (notes->src == NULL &&
> - symbol__alloc_hist(sym, top.evlist->nr_entries) < 0) {
> + if (notes->src == NULL && symbol__alloc_hist(sym) < 0) {
> pthread_mutex_unlock(¬es->lock);
> pr_err("Not enough memory for annotating '%s' symbol!\n",
> sym->name);
> @@ -1215,6 +1214,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
> return -ENOMEM;
> }
>
> + symbol_conf.nr_events = top.evlist->nr_entries;
> +
> if (top.delay_secs < 1)
> top.delay_secs = 1;
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 119e996..376e643 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -25,17 +25,17 @@ int symbol__annotate_init(struct map *map __used, struct symbol *sym)
> return 0;
> }
>
> -int symbol__alloc_hist(struct symbol *sym, int nevents)
> +int symbol__alloc_hist(struct symbol *sym)
> {
> struct annotation *notes = symbol__annotation(sym);
> size_t sizeof_sym_hist = (sizeof(struct sym_hist) +
> (sym->end - sym->start) * sizeof(u64));
>
> - notes->src = zalloc(sizeof(*notes->src) + nevents * sizeof_sym_hist);
> + notes->src = zalloc(sizeof(*notes->src) + symbol_conf.nr_events * sizeof_sym_hist);
> if (notes->src == NULL)
> return -1;
> notes->src->sizeof_sym_hist = sizeof_sym_hist;
> - notes->src->nr_histograms = nevents;
> + notes->src->nr_histograms = symbol_conf.nr_events;
> INIT_LIST_HEAD(¬es->src->source);
> return 0;
> }
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index d907252..efa5dc8 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -72,7 +72,7 @@ static inline struct annotation *symbol__annotation(struct symbol *sym)
>
> int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
> int evidx, u64 addr);
> -int symbol__alloc_hist(struct symbol *sym, int nevents);
> +int symbol__alloc_hist(struct symbol *sym);
> void symbol__annotate_zero_histograms(struct symbol *sym);
>
> int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize);
> @@ -99,8 +99,7 @@ static inline int symbol__tui_annotate(struct symbol *sym __used,
> }
> #else
> int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
> - int nr_events, void(*timer)(void *arg), void *arg,
> - int delay_secs);
> + void(*timer)(void *arg), void *arg, int delay_secs);
> #endif
>
> extern const char *disassembler_style;
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index bcd05d0..41424a1 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2041,6 +2041,8 @@ int perf_session__read_header(struct perf_session *session, int fd)
> lseek(fd, tmp, SEEK_SET);
> }
>
> + symbol_conf.nr_events = nr_attrs;
> +
> if (f_header.event_types.size) {
> lseek(fd, f_header.event_types.offset, SEEK_SET);
> events = malloc(f_header.event_types.size);
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index c86c1d2..6676d55 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -119,7 +119,6 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __used,
>
> static inline int hist_entry__tui_annotate(struct hist_entry *self __used,
> int evidx __used,
> - int nr_events __used,
> void(*timer)(void *arg) __used,
> void *arg __used,
> int delay_secs __used)
> @@ -130,7 +129,7 @@ static inline int hist_entry__tui_annotate(struct hist_entry *self __used,
> #define K_RIGHT -2
> #else
> #include "ui/keysyms.h"
> -int hist_entry__tui_annotate(struct hist_entry *he, int evidx, int nr_events,
> +int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
> void(*timer)(void *arg), void *arg, int delay_secs);
>
> int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 29f8d74..123c2e1 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -68,6 +68,7 @@ struct strlist;
>
> struct symbol_conf {
> unsigned short priv_size;
> + unsigned short nr_events;
> bool try_vmlinux_path,
> use_modules,
> sort_by_name,
> diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
> index 0575905..295a9c9 100644
> --- a/tools/perf/util/ui/browsers/annotate.c
> +++ b/tools/perf/util/ui/browsers/annotate.c
> @@ -224,7 +224,7 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> }
>
> static int annotate_browser__run(struct annotate_browser *self, int evidx,
> - int nr_events, void(*timer)(void *arg),
> + void(*timer)(void *arg),
> void *arg, int delay_secs)
> {
> struct rb_node *nd = NULL;
> @@ -328,8 +328,7 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
> notes = symbol__annotation(target);
> pthread_mutex_lock(¬es->lock);
>
> - if (notes->src == NULL &&
> - symbol__alloc_hist(target, nr_events) < 0) {
> + if (notes->src == NULL && symbol__alloc_hist(target) < 0) {
> pthread_mutex_unlock(¬es->lock);
> ui__warning("Not enough memory for annotating '%s' symbol!\n",
> target->name);
> @@ -337,7 +336,7 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
> }
>
> pthread_mutex_unlock(¬es->lock);
> - symbol__tui_annotate(target, ms->map, evidx, nr_events,
> + symbol__tui_annotate(target, ms->map, evidx,
> timer, arg, delay_secs);
> }
> continue;
> @@ -358,15 +357,15 @@ out:
> return key;
> }
>
> -int hist_entry__tui_annotate(struct hist_entry *he, int evidx, int nr_events,
> +int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
> void(*timer)(void *arg), void *arg, int delay_secs)
> {
> - return symbol__tui_annotate(he->ms.sym, he->ms.map, evidx, nr_events,
> + return symbol__tui_annotate(he->ms.sym, he->ms.map, evidx,
> timer, arg, delay_secs);
> }
>
> int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
> - int nr_events, void(*timer)(void *arg), void *arg,
> + void(*timer)(void *arg), void *arg,
> int delay_secs)
> {
> struct objdump_line *pos, *n;
> @@ -419,8 +418,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
> browser.b.nr_entries = browser.nr_entries;
> browser.b.entries = ¬es->src->source,
> browser.b.width += 18; /* Percentage */
> - ret = annotate_browser__run(&browser, evidx, nr_events,
> - timer, arg, delay_secs);
> + ret = annotate_browser__run(&browser, evidx, timer, arg, delay_secs);
> list_for_each_entry_safe(pos, n, ¬es->src->source, node) {
> list_del(&pos->node);
> objdump_line__free(pos);
> diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
> index d0c94b4..1212a38 100644
> --- a/tools/perf/util/ui/browsers/hists.c
> +++ b/tools/perf/util/ui/browsers/hists.c
> @@ -1020,7 +1020,7 @@ do_annotate:
> * Don't let this be freed, say, by hists__decay_entry.
> */
> he->used = true;
> - err = hist_entry__tui_annotate(he, evsel->idx, nr_events,
> + err = hist_entry__tui_annotate(he, evsel->idx,
> timer, arg, delay_secs);
> he->used = false;
> ui_browser__update_nr_entries(&browser->b, browser->hists->nr_entries);
--
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/