Re: [PATCH v2 1/6] perf annotate: Remove privsize from symbol__annotate() args

From: Arnaldo Carvalho de Melo
Date: Thu Jan 30 2020 - 06:17:04 EST


Em Fri, Jan 24, 2020 at 01:34:27PM +0530, Ravi Bangoria escreveu:
> privsize is passed as 0 from all the symbol__annotate() callers.
> Remove it from argument list.

Right, trying to figure out when was it that this became unnecessary to
see if this in fact is hiding some other problem...

It all starts in the following change, re-reading those patches...

- Arnaldo


commit c835e1914c4bcfdd41f43d270cafc6d8119d7782
Author: Jiri Olsa <jolsa@xxxxxxxxxx>
Date: Wed Oct 11 17:01:37 2017 +0200

perf annotate: Add annotation_line__(new|delete) functions

Changing the way the annotation lines are allocated and adding
annotation_line__(new|delete) functions to deal with this.

Before the allocation schema was as follows:

-----------------------------------------------------------
struct disasm_line | struct annotation_line | private space
-----------------------------------------------------------

Where the private space is used in TUI code to store computed
annotation data for events. The stdio code computes the data
on the fly.

The goal is to compute and store annotation line's data directly
in the struct annotation_line itself, so this patch changes the
line allocation schema as follows:

------------------------------------------------------------
privsize space | struct disasm_line | struct annotation_line
------------------------------------------------------------

Moving struct annotation_line to the end, because in following
changes we will move here the non-fixed length event's data.

> Signed-off-by: Ravi Bangoria <ravi.bangoria@xxxxxxxxxxxxx>
> ---
> tools/perf/builtin-top.c | 2 +-
> tools/perf/ui/gtk/annotate.c | 2 +-
> tools/perf/util/annotate.c | 7 ++++---
> tools/perf/util/annotate.h | 2 +-
> 4 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 8affcab75604..3e37747364e0 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -143,7 +143,7 @@ static int perf_top__parse_source(struct perf_top *top, struct hist_entry *he)
> return err;
> }
>
> - err = symbol__annotate(&he->ms, evsel, 0, &top->annotation_opts, NULL);
> + err = symbol__annotate(&he->ms, evsel, &top->annotation_opts, NULL);
> if (err == 0) {
> top->sym_filter_entry = he;
> } else {
> diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
> index 22cc240f7371..35f9641bf670 100644
> --- a/tools/perf/ui/gtk/annotate.c
> +++ b/tools/perf/ui/gtk/annotate.c
> @@ -174,7 +174,7 @@ static int symbol__gtk_annotate(struct map_symbol *ms, struct evsel *evsel,
> if (ms->map->dso->annotate_warned)
> return -1;
>
> - err = symbol__annotate(ms, evsel, 0, &annotation__default_options, NULL);
> + err = symbol__annotate(ms, evsel, &annotation__default_options, NULL);
> if (err) {
> char msg[BUFSIZ];
> symbol__strerror_disassemble(ms, err, msg, sizeof(msg));
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index ca73fb74ad03..ea70bc050bce 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -2149,9 +2149,10 @@ void symbol__calc_percent(struct symbol *sym, struct evsel *evsel)
> annotation__calc_percent(notes, evsel, symbol__size(sym));
> }
>
> -int symbol__annotate(struct map_symbol *ms, struct evsel *evsel, size_t privsize,
> +int symbol__annotate(struct map_symbol *ms, struct evsel *evsel,
> struct annotation_options *options, struct arch **parch)
> {
> + size_t privsize = 0;
> struct symbol *sym = ms->sym;
> struct annotation *notes = symbol__annotation(sym);
> struct annotate_args args = {
> @@ -2790,7 +2791,7 @@ int symbol__tty_annotate(struct map_symbol *ms, struct evsel *evsel,
> struct symbol *sym = ms->sym;
> struct rb_root source_line = RB_ROOT;
>
> - if (symbol__annotate(ms, evsel, 0, opts, NULL) < 0)
> + if (symbol__annotate(ms, evsel, opts, NULL) < 0)
> return -1;
>
> symbol__calc_percent(sym, evsel);
> @@ -3070,7 +3071,7 @@ int symbol__annotate2(struct map_symbol *ms, struct evsel *evsel,
> if (perf_evsel__is_group_event(evsel))
> nr_pcnt = evsel->core.nr_members;
>
> - err = symbol__annotate(ms, evsel, 0, options, parch);
> + err = symbol__annotate(ms, evsel, options, parch);
> if (err)
> goto out_free_offsets;
>
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 455403e8fede..dade64781670 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -352,7 +352,7 @@ struct annotated_source *symbol__hists(struct symbol *sym, int nr_hists);
> void symbol__annotate_zero_histograms(struct symbol *sym);
>
> int symbol__annotate(struct map_symbol *ms,
> - struct evsel *evsel, size_t privsize,
> + struct evsel *evsel,
> struct annotation_options *options,
> struct arch **parch);
> int symbol__annotate2(struct map_symbol *ms,
> --
> 2.24.1
>

--

- Arnaldo