linux-kernel - Re: [PATCH v2 1/6] perf annotate: Remove privsize from symbol_

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200130111653.GE3841@kernel.org>
Date:   Thu, 30 Jan 2020 12:16:53 +0100
From:   Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To:     Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
Cc:     jolsa@...hat.com, namhyung@...nel.org, irogers@...gle.com,
        songliubraving@...com, yao.jin@...ux.intel.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/6] perf annotate: Remove privsize from
 symbol__annotate() args

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@...nel.org>
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@...ux.ibm.com>
> ---
>  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