linux-kernel - Re: [PATCH 02/35] perf annotate: Add annotation

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: <20171011152942.GD3503@kernel.org>
Date:   Wed, 11 Oct 2017 12:29:42 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Jiri Olsa <jolsa@...nel.org>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        David Ahern <dsahern@...il.com>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH 02/35] perf annotate: Add annotation_line struct

Em Wed, Oct 11, 2017 at 05:01:25PM +0200, Jiri Olsa escreveu:
> In order to make the annotation support generic, I'm adding
> 'struct annotation_line', which will hold all generic data
> common to any annotation source (it's coming on following
> patches). Having this, we can add different annotation
> line support than objdump disasm.

Such as? What other "annotation line support" other than "objdump
disasm"?

- Arnaldo
 
> Link: http://lkml.kernel.org/n/tip-d7wk2e18blnq2e22wvw946ol@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
>  tools/perf/ui/browsers/annotate.c | 34 +++++++++++++++++-----------------
>  tools/perf/ui/gtk/annotate.c      |  6 +++---
>  tools/perf/util/annotate.c        | 20 ++++++++++----------
>  tools/perf/util/annotate.h        | 20 ++++++++++++--------
>  4 files changed, 42 insertions(+), 38 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 786fecaf578e..78ecfc86fb3d 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -83,7 +83,7 @@ static bool disasm_line__filter(struct ui_browser *browser __maybe_unused,
>  				void *entry)
>  {
>  	if (annotate_browser__opts.hide_src_code) {
> -		struct disasm_line *dl = list_entry(entry, struct disasm_line, node);
> +		struct disasm_line *dl = list_entry(entry, struct disasm_line, al.node);
>  		return dl->offset == -1;
>  	}
>  
> @@ -122,7 +122,7 @@ static int annotate_browser__cycles_width(struct annotate_browser *ab)
>  static void annotate_browser__write(struct ui_browser *browser, void *entry, int row)
>  {
>  	struct annotate_browser *ab = container_of(browser, struct annotate_browser, b);
> -	struct disasm_line *dl = list_entry(entry, struct disasm_line, node);
> +	struct disasm_line *dl = list_entry(entry, struct disasm_line, al.node);
>  	struct browser_disasm_line *bdl = disasm_line__browser(dl);
>  	bool current_entry = ui_browser__is_current_entry(browser, row);
>  	bool change_color = (!annotate_browser__opts.hide_src_code &&
> @@ -285,7 +285,7 @@ static bool disasm_line__is_valid_jump(struct disasm_line *dl, struct symbol *sy
>  
>  static bool is_fused(struct annotate_browser *ab, struct disasm_line *cursor)
>  {
> -	struct disasm_line *pos = list_prev_entry(cursor, node);
> +	struct disasm_line *pos = list_prev_entry(cursor, al.node);
>  	const char *name;
>  
>  	if (!pos)
> @@ -403,16 +403,16 @@ static void annotate_browser__set_top(struct annotate_browser *browser,
>  	browser->b.top_idx = browser->b.index = idx;
>  
>  	while (browser->b.top_idx != 0 && back != 0) {
> -		pos = list_entry(pos->node.prev, struct disasm_line, node);
> +		pos = list_entry(pos->al.node.prev, struct disasm_line, al.node);
>  
> -		if (disasm_line__filter(&browser->b, &pos->node))
> +		if (disasm_line__filter(&browser->b, &pos->al.node))
>  			continue;
>  
>  		--browser->b.top_idx;
>  		--back;
>  	}
>  
> -	browser->b.top = pos;
> +	browser->b.top = &pos->al;
>  	browser->b.navkeypressed = true;
>  }
>  
> @@ -445,7 +445,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
>  
>  	pthread_mutex_lock(&notes->lock);
>  
> -	list_for_each_entry(pos, &notes->src->source, node) {
> +	list_for_each_entry(pos, &notes->src->source, al.node) {
>  		struct browser_disasm_line *bpos = disasm_line__browser(pos);
>  		const char *path = NULL;
>  		double max_percent = 0.0;
> @@ -491,7 +491,7 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
>  	off_t offset = browser->b.index - browser->b.top_idx;
>  
>  	browser->b.seek(&browser->b, offset, SEEK_CUR);
> -	dl = list_entry(browser->b.top, struct disasm_line, node);
> +	dl = list_entry(browser->b.top, struct disasm_line, al.node);
>  	bdl = disasm_line__browser(dl);
>  
>  	if (annotate_browser__opts.hide_src_code) {
> @@ -588,10 +588,10 @@ struct disasm_line *annotate_browser__find_offset(struct annotate_browser *brows
>  	struct disasm_line *pos;
>  
>  	*idx = 0;
> -	list_for_each_entry(pos, &notes->src->source, node) {
> +	list_for_each_entry(pos, &notes->src->source, al.node) {
>  		if (pos->offset == offset)
>  			return pos;
> -		if (!disasm_line__filter(&browser->b, &pos->node))
> +		if (!disasm_line__filter(&browser->b, &pos->al.node))
>  			++*idx;
>  	}
>  
> @@ -629,8 +629,8 @@ struct disasm_line *annotate_browser__find_string(struct annotate_browser *brows
>  	struct disasm_line *pos = browser->selection;
>  
>  	*idx = browser->b.index;
> -	list_for_each_entry_continue(pos, &notes->src->source, node) {
> -		if (disasm_line__filter(&browser->b, &pos->node))
> +	list_for_each_entry_continue(pos, &notes->src->source, al.node) {
> +		if (disasm_line__filter(&browser->b, &pos->al.node))
>  			continue;
>  
>  		++*idx;
> @@ -668,8 +668,8 @@ struct disasm_line *annotate_browser__find_string_reverse(struct annotate_browse
>  	struct disasm_line *pos = browser->selection;
>  
>  	*idx = browser->b.index;
> -	list_for_each_entry_continue_reverse(pos, &notes->src->source, node) {
> -		if (disasm_line__filter(&browser->b, &pos->node))
> +	list_for_each_entry_continue_reverse(pos, &notes->src->source, al.node) {
> +		if (disasm_line__filter(&browser->b, &pos->al.node))
>  			continue;
>  
>  		--*idx;
> @@ -1133,7 +1133,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
>  	notes = symbol__annotation(sym);
>  	browser.start = map__rip_2objdump(map, sym->start);
>  
> -	list_for_each_entry(pos, &notes->src->source, node) {
> +	list_for_each_entry(pos, &notes->src->source, al.node) {
>  		struct browser_disasm_line *bpos;
>  		size_t line_len = strlen(pos->line);
>  
> @@ -1173,8 +1173,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
>  	annotate_browser__update_addr_width(&browser);
>  
>  	ret = annotate_browser__run(&browser, evsel, hbt);
> -	list_for_each_entry_safe(pos, n, &notes->src->source, node) {
> -		list_del(&pos->node);
> +	list_for_each_entry_safe(pos, n, &notes->src->source, al.node) {
> +		list_del(&pos->al.node);
>  		disasm_line__free(pos);
>  	}
>  
> diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
> index 02176193f427..87b138b318e6 100644
> --- a/tools/perf/ui/gtk/annotate.c
> +++ b/tools/perf/ui/gtk/annotate.c
> @@ -118,7 +118,7 @@ static int perf_gtk__annotate_symbol(GtkWidget *window, struct symbol *sym,
>  	gtk_tree_view_set_model(GTK_TREE_VIEW(view), GTK_TREE_MODEL(store));
>  	g_object_unref(GTK_TREE_MODEL(store));
>  
> -	list_for_each_entry(pos, &notes->src->source, node) {
> +	list_for_each_entry(pos, &notes->src->source, al.node) {
>  		GtkTreeIter iter;
>  		int ret = 0;
>  
> @@ -147,8 +147,8 @@ static int perf_gtk__annotate_symbol(GtkWidget *window, struct symbol *sym,
>  
>  	gtk_container_add(GTK_CONTAINER(window), view);
>  
> -	list_for_each_entry_safe(pos, n, &notes->src->source, node) {
> -		list_del(&pos->node);
> +	list_for_each_entry_safe(pos, n, &notes->src->source, al.node) {
> +		list_del(&pos->al.node);
>  		disasm_line__free(pos);
>  	}
>  
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 08164162c345..88dcf999f4d3 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -921,12 +921,12 @@ int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool r
>  
>  static void disasm__add(struct list_head *head, struct disasm_line *line)
>  {
> -	list_add_tail(&line->node, head);
> +	list_add_tail(&line->al.node, head);
>  }
>  
>  struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disasm_line *pos)
>  {
> -	list_for_each_entry_continue(pos, head, node)
> +	list_for_each_entry_continue(pos, head, al.node)
>  		if (pos->offset >= 0)
>  			return pos;
>  
> @@ -1112,7 +1112,7 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>  			return 1;
>  
>  		if (queue != NULL) {
> -			list_for_each_entry_from(queue, &notes->src->source, node) {
> +			list_for_each_entry_from(queue, &notes->src->source, al.node) {
>  				if (queue == dl)
>  					break;
>  				disasm_line__print(queue, sym, start, evsel, len,
> @@ -1295,7 +1295,7 @@ static void delete_last_nop(struct symbol *sym)
>  	struct disasm_line *dl;
>  
>  	while (!list_empty(list)) {
> -		dl = list_entry(list->prev, struct disasm_line, node);
> +		dl = list_entry(list->prev, struct disasm_line, al.node);
>  
>  		if (dl->ins.ops) {
>  			if (dl->ins.ops != &nop_ops)
> @@ -1307,7 +1307,7 @@ static void delete_last_nop(struct symbol *sym)
>  				return;
>  		}
>  
> -		list_del(&dl->node);
> +		list_del(&dl->al.node);
>  		disasm_line__free(dl);
>  	}
>  }
> @@ -1834,7 +1834,7 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map,
>  	if (verbose > 0)
>  		symbol__annotate_hits(sym, evsel);
>  
> -	list_for_each_entry(pos, &notes->src->source, node) {
> +	list_for_each_entry(pos, &notes->src->source, al.node) {
>  		if (context && queue == NULL) {
>  			queue = pos;
>  			queue_len = 0;
> @@ -1864,7 +1864,7 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map,
>  			if (!context)
>  				break;
>  			if (queue_len == context)
> -				queue = list_entry(queue->node.next, typeof(*queue), node);
> +				queue = list_entry(queue->al.node.next, typeof(*queue), al.node);
>  			else
>  				++queue_len;
>  			break;
> @@ -1901,8 +1901,8 @@ void disasm__purge(struct list_head *head)
>  {
>  	struct disasm_line *pos, *n;
>  
> -	list_for_each_entry_safe(pos, n, head, node) {
> -		list_del(&pos->node);
> +	list_for_each_entry_safe(pos, n, head, al.node) {
> +		list_del(&pos->al.node);
>  		disasm_line__free(pos);
>  	}
>  }
> @@ -1929,7 +1929,7 @@ size_t disasm__fprintf(struct list_head *head, FILE *fp)
>  	struct disasm_line *pos;
>  	size_t printed = 0;
>  
> -	list_for_each_entry(pos, head, node)
> +	list_for_each_entry(pos, head, al.node)
>  		printed += disasm_line__fprintf(pos, fp);
>  
>  	return printed;
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 9ce575c25fd9..66008347c52b 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -58,15 +58,19 @@ bool ins__is_fused(struct arch *arch, const char *ins1, const char *ins2);
>  
>  struct annotation;
>  
> +struct annotation_line {
> +	struct list_head	 node;
> +};
> +
>  struct disasm_line {
> -	struct list_head    node;
> -	s64		    offset;
> -	char		    *line;
> -	struct ins	    ins;
> -	int		    line_nr;
> -	float		    ipc;
> -	u64		    cycles;
> -	struct ins_operands ops;
> +	struct annotation_line	 al;
> +	s64			 offset;
> +	char			*line;
> +	struct ins		 ins;
> +	int			 line_nr;
> +	float			 ipc;
> +	u64			 cycles;
> +	struct ins_operands	 ops;
>  };
>  
>  static inline bool disasm_line__has_offset(const struct disasm_line *dl)
> -- 
> 2.13.6