public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: jolsa@redhat.com, namhyung@kernel.org, irogers@google.com,
	songliubraving@fb.com, yao.jin@linux.intel.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/6] perf annotate: Remove privsize from symbol__annotate() args
Date: Thu, 30 Jan 2020 12:16:53 +0100	[thread overview]
Message-ID: <20200130111653.GE3841@kernel.org> (raw)
In-Reply-To: <20200124080432.8065-2-ravi.bangoria@linux.ibm.com>

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@kernel.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@linux.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

  reply	other threads:[~2020-01-30 11:17 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-24  8:04 [PATCH v2 0/6] perf annotate: Misc fixes / improvements Ravi Bangoria
2020-01-24  8:04 ` [PATCH v2 1/6] perf annotate: Remove privsize from symbol__annotate() args Ravi Bangoria
2020-01-30 11:16   ` Arnaldo Carvalho de Melo [this message]
2020-02-03  4:54     ` Ravi Bangoria
2020-02-03 13:30       ` Jiri Olsa
2020-02-04  2:37         ` Ravi Bangoria
2020-01-24  8:04 ` [PATCH v2 2/6] perf annotate: Simplify disasm_line allocation and freeing code Ravi Bangoria
2020-01-27 11:59   ` Jiri Olsa
2020-01-28 13:57     ` Ravi Bangoria
2020-01-24  8:04 ` [PATCH v2 3/6] perf annotate: Align struct annotate_args Ravi Bangoria
2020-01-24  8:04 ` [PATCH v2 4/6] perf annotate: Fix segfault with source toggle Ravi Bangoria
2020-01-24  8:04 ` [PATCH v2 5/6] perf annotate: Make few functions static Ravi Bangoria
2020-01-24  8:04 ` [PATCH v2 6/6] perf annotate: Get rid of annotation->nr_jumps Ravi Bangoria

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200130111653.GE3841@kernel.org \
    --to=arnaldo.melo@gmail.com \
    --cc=irogers@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@kernel.org \
    --cc=ravi.bangoria@linux.ibm.com \
    --cc=songliubraving@fb.com \
    --cc=yao.jin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox