Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

perf annotate: Remove duplicate 'name' field from disasm_line

The disasm_line::name field is always equal to ins::name, being used
just to locate the instruction's ins_ops from the per-arch instructions
table.

Eliminate this duplication, nuking that field and instead make
ins__find() return an ins_ops, store it in disasm_line::ins.ops, and
keep just in disasm_line::ins.name what was in disasm_line::name, this
way we end up not keeping a reference to entries in the per-arch
instructions table.

This in turn will help supporting multiple ways to manage the per-arch
instructions table, allowing resorting that array, for instance, when
the entries will move after references to its addresses were made. The
same problem is avoided when one grows the array with realloc.

So architectures simply keeping a constant array will work as well as
architectures building the table using regular expressions or other
logic that involves resorting the table.

Reviewed-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Chris Riyder <chris.ryder@arm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kim Phillips <kim.phillips@arm.com>
Cc: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Taeung Song <treeze.taeung@gmail.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-vr899azvabnw9gtuepuqfd9t@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

+51 -57
+9 -9
tools/perf/ui/browsers/annotate.c
··· 213 213 ui_browser__write_nstring(browser, bf, printed); 214 214 if (change_color) 215 215 ui_browser__set_color(browser, color); 216 - if (dl->ins && dl->ins->ops->scnprintf) { 217 - if (ins__is_jump(dl->ins)) { 216 + if (dl->ins.ops && dl->ins.ops->scnprintf) { 217 + if (ins__is_jump(&dl->ins)) { 218 218 bool fwd = dl->ops.target.offset > (u64)dl->offset; 219 219 220 220 ui_browser__write_graph(browser, fwd ? SLSMG_DARROW_CHAR : 221 221 SLSMG_UARROW_CHAR); 222 222 SLsmg_write_char(' '); 223 - } else if (ins__is_call(dl->ins)) { 223 + } else if (ins__is_call(&dl->ins)) { 224 224 ui_browser__write_graph(browser, SLSMG_RARROW_CHAR); 225 225 SLsmg_write_char(' '); 226 - } else if (ins__is_ret(dl->ins)) { 226 + } else if (ins__is_ret(&dl->ins)) { 227 227 ui_browser__write_graph(browser, SLSMG_LARROW_CHAR); 228 228 SLsmg_write_char(' '); 229 229 } else { ··· 243 243 244 244 static bool disasm_line__is_valid_jump(struct disasm_line *dl, struct symbol *sym) 245 245 { 246 - if (!dl || !dl->ins || !ins__is_jump(dl->ins) 246 + if (!dl || !dl->ins.ops || !ins__is_jump(&dl->ins) 247 247 || !disasm_line__has_offset(dl) 248 248 || dl->ops.target.offset >= symbol__size(sym)) 249 249 return false; ··· 492 492 }; 493 493 char title[SYM_TITLE_MAX_SIZE]; 494 494 495 - if (!ins__is_call(dl->ins)) 495 + if (!ins__is_call(&dl->ins)) 496 496 return false; 497 497 498 498 if (map_groups__find_ams(&target) || ··· 545 545 struct disasm_line *dl = browser->selection; 546 546 s64 idx; 547 547 548 - if (!ins__is_jump(dl->ins)) 548 + if (!ins__is_jump(&dl->ins)) 549 549 return false; 550 550 551 551 dl = annotate_browser__find_offset(browser, dl->ops.target.offset, &idx); ··· 841 841 ui_helpline__puts("Huh? No selection. Report to linux-kernel@vger.kernel.org"); 842 842 else if (browser->selection->offset == -1) 843 843 ui_helpline__puts("Actions are only available for assembly lines."); 844 - else if (!browser->selection->ins) 844 + else if (!browser->selection->ins.ops) 845 845 goto show_sup_ins; 846 - else if (ins__is_ret(browser->selection->ins)) 846 + else if (ins__is_ret(&browser->selection->ins)) 847 847 goto out; 848 848 else if (!(annotate_browser__jump(browser) || 849 849 annotate_browser__callq(browser, evsel, hbt))) {
+34 -39
tools/perf/util/annotate.c
··· 28 28 const char *objdump_path; 29 29 static regex_t file_lineno; 30 30 31 - static struct ins *ins__find(struct arch *arch, const char *name); 32 - static int disasm_line__parse(char *line, char **namep, char **rawp); 31 + static struct ins_ops *ins__find(struct arch *arch, const char *name); 32 + static int disasm_line__parse(char *line, const char **namep, char **rawp); 33 33 34 34 struct arch { 35 35 const char *name; ··· 218 218 219 219 static int lock__parse(struct arch *arch, struct ins_operands *ops, struct map *map) 220 220 { 221 - char *name; 222 - 223 221 ops->locked.ops = zalloc(sizeof(*ops->locked.ops)); 224 222 if (ops->locked.ops == NULL) 225 223 return 0; 226 224 227 - if (disasm_line__parse(ops->raw, &name, &ops->locked.ops->raw) < 0) 225 + if (disasm_line__parse(ops->raw, &ops->locked.ins.name, &ops->locked.ops->raw) < 0) 228 226 goto out_free_ops; 229 227 230 - ops->locked.ins = ins__find(arch, name); 231 - free(name); 228 + ops->locked.ins.ops = ins__find(arch, ops->locked.ins.name); 232 229 233 - if (ops->locked.ins == NULL) 230 + if (ops->locked.ins.ops == NULL) 234 231 goto out_free_ops; 235 232 236 - if (!ops->locked.ins->ops) 237 - return 0; 238 - 239 - if (ops->locked.ins->ops->parse && 240 - ops->locked.ins->ops->parse(arch, ops->locked.ops, map) < 0) 233 + if (ops->locked.ins.ops->parse && 234 + ops->locked.ins.ops->parse(arch, ops->locked.ops, map) < 0) 241 235 goto out_free_ops; 242 236 243 237 return 0; ··· 246 252 { 247 253 int printed; 248 254 249 - if (ops->locked.ins == NULL) 255 + if (ops->locked.ins.ops == NULL) 250 256 return ins__raw_scnprintf(ins, bf, size, ops); 251 257 252 258 printed = scnprintf(bf, size, "%-6.6s ", ins->name); 253 - return printed + ins__scnprintf(ops->locked.ins, bf + printed, 259 + return printed + ins__scnprintf(&ops->locked.ins, bf + printed, 254 260 size - printed, ops->locked.ops); 255 261 } 256 262 257 263 static void lock__delete(struct ins_operands *ops) 258 264 { 259 - struct ins *ins = ops->locked.ins; 265 + struct ins *ins = &ops->locked.ins; 260 266 261 - if (ins && ins->ops->free) 267 + if (ins->ops && ins->ops->free) 262 268 ins->ops->free(ops->locked.ops); 263 269 else 264 270 ins__delete(ops->locked.ops); ··· 419 425 qsort(arch->instructions, nmemb, sizeof(struct ins), ins__cmp); 420 426 } 421 427 422 - static struct ins *ins__find(struct arch *arch, const char *name) 428 + static struct ins_ops *ins__find(struct arch *arch, const char *name) 423 429 { 430 + struct ins *ins; 424 431 const int nmemb = arch->nr_instructions; 425 432 426 433 if (!arch->sorted_instructions) { ··· 429 434 arch->sorted_instructions = true; 430 435 } 431 436 432 - return bsearch(name, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp); 437 + ins = bsearch(name, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp); 438 + return ins ? ins->ops : NULL; 433 439 } 434 440 435 441 static int arch__key_cmp(const void *name, const void *archp) ··· 687 691 688 692 static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, struct map *map) 689 693 { 690 - dl->ins = ins__find(arch, dl->name); 694 + dl->ins.ops = ins__find(arch, dl->ins.name); 691 695 692 - if (dl->ins == NULL) 696 + if (!dl->ins.ops) 693 697 return; 694 698 695 - if (!dl->ins->ops) 696 - return; 697 - 698 - if (dl->ins->ops->parse && dl->ins->ops->parse(arch, &dl->ops, map) < 0) 699 - dl->ins = NULL; 699 + if (dl->ins.ops->parse && dl->ins.ops->parse(arch, &dl->ops, map) < 0) 700 + dl->ins.ops = NULL; 700 701 } 701 702 702 - static int disasm_line__parse(char *line, char **namep, char **rawp) 703 + static int disasm_line__parse(char *line, const char **namep, char **rawp) 703 704 { 704 705 char *name = line, tmp; 705 706 ··· 729 736 return 0; 730 737 731 738 out_free_name: 732 - zfree(namep); 739 + free((void *)namep); 740 + *namep = NULL; 733 741 return -1; 734 742 } 735 743 ··· 749 755 goto out_delete; 750 756 751 757 if (offset != -1) { 752 - if (disasm_line__parse(dl->line, &dl->name, &dl->ops.raw) < 0) 758 + if (disasm_line__parse(dl->line, &dl->ins.name, &dl->ops.raw) < 0) 753 759 goto out_free_line; 754 760 755 761 disasm_line__init_ins(dl, arch, map); ··· 768 774 void disasm_line__free(struct disasm_line *dl) 769 775 { 770 776 zfree(&dl->line); 771 - zfree(&dl->name); 772 - if (dl->ins && dl->ins->ops->free) 773 - dl->ins->ops->free(&dl->ops); 777 + if (dl->ins.ops && dl->ins.ops->free) 778 + dl->ins.ops->free(&dl->ops); 774 779 else 775 780 ins__delete(&dl->ops); 781 + free((void *)dl->ins.name); 782 + dl->ins.name = NULL; 776 783 free(dl); 777 784 } 778 785 779 786 int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool raw) 780 787 { 781 - if (raw || !dl->ins) 782 - return scnprintf(bf, size, "%-6.6s %s", dl->name, dl->ops.raw); 788 + if (raw || !dl->ins.ops) 789 + return scnprintf(bf, size, "%-6.6s %s", dl->ins.name, dl->ops.raw); 783 790 784 - return ins__scnprintf(dl->ins, bf, size, &dl->ops); 791 + return ins__scnprintf(&dl->ins, bf, size, &dl->ops); 785 792 } 786 793 787 794 static void disasm__add(struct list_head *head, struct disasm_line *line) ··· 1138 1143 map__rip_2objdump(map, sym->start); 1139 1144 1140 1145 /* kcore has no symbols, so add the call target name */ 1141 - if (dl->ins && ins__is_call(dl->ins) && !dl->ops.target.name) { 1146 + if (dl->ins.ops && ins__is_call(&dl->ins) && !dl->ops.target.name) { 1142 1147 struct addr_map_symbol target = { 1143 1148 .map = map, 1144 1149 .addr = dl->ops.target.addr, ··· 1168 1173 while (!list_empty(list)) { 1169 1174 dl = list_entry(list->prev, struct disasm_line, node); 1170 1175 1171 - if (dl->ins && dl->ins->ops) { 1172 - if (dl->ins->ops != &nop_ops) 1176 + if (dl->ins.ops) { 1177 + if (dl->ins.ops != &nop_ops) 1173 1178 return; 1174 1179 } else { 1175 1180 if (!strstr(dl->line, " nop ") && ··· 1762 1767 if (dl->offset == -1) 1763 1768 return fprintf(fp, "%s\n", dl->line); 1764 1769 1765 - printed = fprintf(fp, "%#" PRIx64 " %s", dl->offset, dl->name); 1770 + printed = fprintf(fp, "%#" PRIx64 " %s", dl->offset, dl->ins.name); 1766 1771 1767 1772 if (dl->ops.raw[0] != '\0') { 1768 1773 printed += fprintf(fp, "%.*s %s\n", 6 - (int)printed, " ",
+8 -9
tools/perf/util/annotate.h
··· 11 11 #include <linux/rbtree.h> 12 12 #include <pthread.h> 13 13 14 - struct ins; 14 + struct ins_ops; 15 + 16 + struct ins { 17 + const char *name; 18 + struct ins_ops *ops; 19 + }; 15 20 16 21 struct ins_operands { 17 22 char *raw; ··· 33 28 u64 addr; 34 29 } source; 35 30 struct { 36 - struct ins *ins; 31 + struct ins ins; 37 32 struct ins_operands *ops; 38 33 } locked; 39 34 }; ··· 48 43 struct ins_operands *ops); 49 44 }; 50 45 51 - struct ins { 52 - const char *name; 53 - struct ins_ops *ops; 54 - }; 55 - 56 46 bool ins__is_jump(const struct ins *ins); 57 47 bool ins__is_call(const struct ins *ins); 58 48 bool ins__is_ret(const struct ins *ins); ··· 59 59 struct list_head node; 60 60 s64 offset; 61 61 char *line; 62 - char *name; 63 - struct ins *ins; 62 + struct ins ins; 64 63 int line_nr; 65 64 float ipc; 66 65 u64 cycles;