| 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
| ||
|
Message-ID: <20180313152059.GB29837@kernel.org>
Date: Tue, 13 Mar 2018 12:20:59 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Jin Yao <yao.jin@...ux.intel.com>
Cc: jolsa@...nel.org, peterz@...radead.org, mingo@...hat.com,
alexander.shishkin@...ux.intel.com, Linux-kernel@...r.kernel.org,
ak@...ux.intel.com, kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH v1 0/4] perf annotate: Create a new '--tui-dump' option
Em Tue, Mar 13, 2018 at 10:16:50PM +0800, Jin Yao escreveu:
> There is a requirement to let perf annotate support displaying the IPC/Cycle.
> In previous patch, this is supported in TUI mode. While it's not convenient
> for users since they have to take screen shots and copy/paste data.
>
> This patch series introduces a new option '--tui-dump' in perf annotate to
> dump the TUI output to stdio.
>
> User can easily use the command line like:
> 'perf annotate --tui-dump > /tmp/log.txt'
My first impression is that this pollutes the code with way too many
ifs, I was thinking more of a:
while (read parsed objdump line) {
ins__fprintf();
}
Going from the refresh routine, I started doing the conversion, but haven't
completed it, there are opportunities for more __scnprintf like routines, also
one to find the percent_max, etc then those would be used both in these two for
--stdio2, that eventually would become --stdio with the old one becoming
--stdio1, till we're satisfied with the new default.
static void annotation_line__fprintf(struct annotate_line *al, FILE *fp)
{
struct browser_line *bl = browser_line(al);
int i;
double percent_max = 0.0;
char bf[256];
for (i = 0; i < browser->nr_events; i++) {
if (al->samples[i].percent > percent_max)
percent_max = al->samples[i].percent;
}
/* the following if/else block should be transformed into a __scnprintf routine
that formats a buffer and then the TUI and --stdio2 use it */
if (al->offset != -1 && percent_max != 0.0) {
for (i = 0; i < ab->nr_events; i++) {
if (annotate_browser__opts.show_total_period) {
fprintf(fp, browser, "%11" PRIu64 " ", al->samples[i].he.period);
} else if (annotate_browser__opts.show_nr_samples) {
fprintf(fp, browser, "%6" PRIu64 " ", al->samples[i].he.nr_samples);
} else {
fprintf(fp, "%6.2f ", al->samples[i].percent);
}
}
} else {
ui_browser__printf(browser, "%*s", pcnt_width,
annotate_browser__opts.show_total_period ? "Period" :
annotate_browser__opts.show_nr_samples ? "Samples" : "Percent");
}
/* The ab->have_cycles should go to a separate struct, outside
* annotation_browser, and the rest should go to something that just does scnprintf on a buffer
* that then is printed on the TUI or with fprintf */
if (ab->have_cycles) {
if (al->ipc)
fprintf(fp, "%*.2f ", IPC_WIDTH - 1, al->ipc);
else if (show_title)
ui_browser__printf(browser, "%*s ", IPC_WIDTH - 1, "IPC");
if (al->cycles)
ui_browser__printf(browser, "%*" PRIu64 " ",
CYCLES_WIDTH - 1, al->cycles);
else if (!show_title)
ui_browser__write_nstring(browser, " ", CYCLES_WIDTH);
else
ui_browser__printf(browser, "%*s ", CYCLES_WIDTH - 1, "Cycle");
}
SLsmg_write_char(' ');
/* The scroll bar isn't being used */
if (!browser->navkeypressed)
width += 1;
if (!*al->line)
fprintf(fp, "\n");
else if (al->offset == -1) {
if (al->line_nr && annotate_browser__opts.show_linenr)
printed = scnprintf(bf, sizeof(bf), "%-*d ",
ab->addr_width + 1, al->line_nr);
else
printed = scnprintf(bf, sizeof(bf), "%*s ",
ab->addr_width, " ");
fprintf(fp, bf);
ui_browser__write_nstring(browser, al->line, width - printed - pcnt_width - cycles_width + 1);
} else {
u64 addr = al->offset;
int color = -1;
if (!annotate_browser__opts.use_offset)
addr += ab->start;
if (!annotate_browser__opts.use_offset) {
printed = scnprintf(bf, sizeof(bf), "%" PRIx64 ": ", addr);
} else {
if (bl->jump_sources) {
if (annotate_browser__opts.show_nr_jumps) {
int prev;
printed = scnprintf(bf, sizeof(bf), "%*d ",
ab->jumps_width,
bl->jump_sources);
prev = annotate_browser__set_jumps_percent_color(ab, bl->jump_sources,
current_entry);
ui_browser__write_nstring(browser, bf, printed);
ui_browser__set_color(browser, prev);
}
printed = scnprintf(bf, sizeof(bf), "%*" PRIx64 ": ",
ab->target_width, addr);
} else {
printed = scnprintf(bf, sizeof(bf), "%*s ",
ab->addr_width, " ");
}
}
fprintf(fp, bf);
disasm_line__write(disasm_line(al), browser, bf, sizeof(bf));
ui_browser__write_nstring(browser, bf, width - pcnt_width - cycles_width - 3 - printed);
}
}
unsigned int annotation_lines__fprintf(struct list_head *lines, FILE *fp)
{
struct list_head *line;
struct annotation_line *al;
list_for_each(line, lines) {
struct annotation_line *al = list_entry(line, struct annotation_line, node);
annotation_line__fprintf(al, line);
}
return row;
}
Then the main code would use the same code that creates the browser->b.entries
and would pass it to annpotation_lines__fprintf().
i.e. we would disentanble the formatting of strings and auxiliary routines to
obtain the max_percent, i.e. nothing of TUI is needed for --stdio2, just the
formatting of strings.
- Arnaldo
> For example:
> $ perf annotate compute_flag --tui-dump
>
> Percent IPC Cycle
>
> Disassembly of section .text:
>
> 0000000000400640 <compute_flag>:
> compute_flag():
> volatile int count;
> static unsigned int s_randseed;
>
> __attribute__((noinline))
> int compute_flag()
> {
> 23.00 1.16 sub $0x8,%rsp
> int i;
>
> i = rand() % 2;
> 23.06 1.16 1 callq rand@plt
>
> return i;
> 27.01 3.38 mov %eax,%edx
> }
> 3.38 add $0x8,%rsp
> {
> int i;
>
> i = rand() % 2;
>
> return i;
> 3.38 shr $0x1f,%edx
> 3.38 add %edx,%eax
> 3.38 and $0x1,%eax
> 3.38 sub %edx,%eax
> }
> 26.93 3.38 2 retq
>
> The '--stdio' option is still kept now. Maybe in future, we can only
> maintain the TUI routines and drop the lagacy stdio code. But right
> now we'd better keep it until the '--tui-dump' option is good enough.
>
> Jin Yao (4):
> perf browser: Add a new 'dump' flag
> perf browser: bypass ui_init if in tui dump mode
> perf annotate: Process the new switch flag tui_dump
> perf annotate: Enable the '--tui-dump' mode
>
> tools/perf/Documentation/perf-annotate.txt | 3 +++
> tools/perf/builtin-annotate.c | 12 +++++++--
> tools/perf/builtin-c2c.c | 2 +-
> tools/perf/builtin-report.c | 2 +-
> tools/perf/builtin-top.c | 2 +-
> tools/perf/ui/browser.c | 38 +++++++++++++++++++++++----
> tools/perf/ui/browser.h | 1 +
> tools/perf/ui/browsers/annotate.c | 41 +++++++++++++++++++++---------
> tools/perf/ui/browsers/hists.c | 2 +-
> tools/perf/ui/setup.c | 9 +++++--
> tools/perf/ui/ui.h | 2 +-
> tools/perf/util/annotate.h | 6 +++--
> tools/perf/util/hist.h | 11 +++++---
> 13 files changed, 99 insertions(+), 32 deletions(-)
>
> --
> 2.7.4
Powered by blists - more mailing lists