| 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
| ||
|
Date: Mon, 20 Jan 2020 23:11:12 +0800
From: "Jin, Yao" <yao.jin@...ux.intel.com>
To: acme@...nel.org, jolsa@...nel.org, peterz@...radead.org,
mingo@...hat.com, alexander.shishkin@...ux.intel.com
Cc: Linux-kernel@...r.kernel.org, ak@...ux.intel.com,
kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH v6 2/4] perf report: Change sort order by a specified
event in group
Hi Arnaldo,
I see the patch 1/4 has been accpeted. Can the remaining 3 patches be
picked up? :)
Thanks
Jin Yao
On 12/20/2019 9:37 AM, Jin Yao wrote:
> When performing "perf report --group", it shows the event group information
> together. By default, the output is sorted by the first event in group.
>
> It would be nice for user to select any event for sorting. This patch
> introduces a new option "--group-sort-idx" to sort the output by the
> event at the index n in event group.
>
> For example,
>
> Before:
>
> # perf report --group --stdio
>
> # To display the perf.data header info, please use --header/--header-only options.
> #
> #
> # Total Lost Samples: 0
> #
> # Samples: 12K of events 'cpu/instructions,period=2000003/, cpu/cpu-cycles,period=200003/, BR_MISP_RETIRED.ALL_BRANCHES:pp, cpu/event=0xc0,umask=1,cmask=1,
> # Event count (approx.): 6451235635
> #
> # Overhead Command Shared Object Symbol
> # ................................ ......... ....................... ...................................
> #
> 92.19% 98.68% 0.00% 93.30% mgen mgen [.] LOOP1
> 3.12% 0.29% 0.00% 0.16% gsd-color libglib-2.0.so.0.5600.4 [.] 0x0000000000049515
> 1.56% 0.03% 0.00% 0.04% gsd-color libglib-2.0.so.0.5600.4 [.] 0x00000000000494b7
> 1.56% 0.01% 0.00% 0.00% gsd-color libglib-2.0.so.0.5600.4 [.] 0x00000000000494ce
> 1.56% 0.00% 0.00% 0.00% mgen [kernel.kallsyms] [k] task_tick_fair
> 0.00% 0.15% 0.00% 0.04% perf [kernel.kallsyms] [k] smp_call_function_single
> 0.00% 0.13% 0.00% 6.08% swapper [kernel.kallsyms] [k] intel_idle
> 0.00% 0.03% 0.00% 0.00% gsd-color libglib-2.0.so.0.5600.4 [.] g_main_context_check
> 0.00% 0.03% 0.00% 0.00% swapper [kernel.kallsyms] [k] apic_timer_interrupt
> ...
>
> After:
>
> # perf report --group --stdio --group-sort-idx 3
>
> # To display the perf.data header info, please use --header/--header-only options.
> #
> #
> # Total Lost Samples: 0
> #
> # Samples: 12K of events 'cpu/instructions,period=2000003/, cpu/cpu-cycles,period=200003/, BR_MISP_RETIRED.ALL_BRANCHES:pp, cpu/event=0xc0,umask=1,cmask=1,
> # Event count (approx.): 6451235635
> #
> # Overhead Command Shared Object Symbol
> # ................................ ......... ....................... ...................................
> #
> 92.19% 98.68% 0.00% 93.30% mgen mgen [.] LOOP1
> 0.00% 0.13% 0.00% 6.08% swapper [kernel.kallsyms] [k] intel_idle
> 3.12% 0.29% 0.00% 0.16% gsd-color libglib-2.0.so.0.5600.4 [.] 0x0000000000049515
> 0.00% 0.00% 0.00% 0.06% swapper [kernel.kallsyms] [k] hrtimer_start_range_ns
> 1.56% 0.03% 0.00% 0.04% gsd-color libglib-2.0.so.0.5600.4 [.] 0x00000000000494b7
> 0.00% 0.15% 0.00% 0.04% perf [kernel.kallsyms] [k] smp_call_function_single
> 0.00% 0.00% 0.00% 0.02% mgen [kernel.kallsyms] [k] update_curr
> 0.00% 0.00% 0.00% 0.02% mgen [kernel.kallsyms] [k] apic_timer_interrupt
> 0.00% 0.00% 0.00% 0.02% mgen [kernel.kallsyms] [k] native_apic_msr_eoi_write
> 0.00% 0.00% 0.00% 0.02% mgen [kernel.kallsyms] [k] __update_load_avg_se
> 0.00% 0.00% 0.00% 0.02% mgen [kernel.kallsyms] [k] scheduler_tick
>
> Now the output is sorted by the fourth event in group.
>
> v6:
> ---
> No change.
>
> v5:
> ---
> No change in v5.
> v4 has been ACKed by Jiri.
>
> v4:
> ---
> 1. Update Documentation/perf-report.txt to mention
> '--group-sort-idx' support multiple groups with different
> amount of events and it should be used on grouped events.
>
> 2. Update __hpp__group_sort_idx(), just return when the
> idx is out of limit.
>
> 3. Return failure on symbol_conf.group_sort_idx && !session->evlist->nr_groups.
> So now we don't need to use together with --group.
>
> v3:
> ---
> Refine the code in __hpp__group_sort_idx().
>
> Before:
> for (i = 1; i < nr_members; i++) {
> if (i == idx) {
> ret = field_cmp(fields_a[i], fields_b[i]);
> if (ret)
> goto out;
> }
> }
>
> After:
> if (idx >= 1 && idx < nr_members) {
> ret = field_cmp(fields_a[idx], fields_b[idx]);
> if (ret)
> goto out;
> }
>
> v2:
> ---
> No change
>
> Signed-off-by: Jin Yao <yao.jin@...ux.intel.com>
> ---
> tools/perf/Documentation/perf-report.txt | 5 ++
> tools/perf/builtin-report.c | 10 +++
> tools/perf/ui/hist.c | 104 +++++++++++++++++++----
> tools/perf/util/symbol_conf.h | 1 +
> 4 files changed, 105 insertions(+), 15 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index 8dbe2119686a..fbb715bcb512 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -371,6 +371,11 @@ OPTIONS
> Show event group information together. It forces group output also
> if there are no groups defined in data file.
>
> +--group-sort-idx::
> + Sort the output by the event at the index n in group. If n is invalid,
> + sort by the first event. It can support multiple groups with different
> + amount of events. WARNING: This should be used on grouped events.
> +
> --demangle::
> Demangle symbol names to human readable form. It's enabled by default,
> disable with --no-demangle.
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index de988589d99b..4c80a3ba73c3 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -1211,6 +1211,10 @@ int cmd_report(int argc, const char **argv)
> "Show a column with the sum of periods"),
> OPT_BOOLEAN_SET(0, "group", &symbol_conf.event_group, &report.group_set,
> "Show event group information together"),
> + OPT_INTEGER(0, "group-sort-idx", &symbol_conf.group_sort_idx,
> + "Sort the output by the event at the index n in group. "
> + "If n is invalid, sort by the first event. "
> + "WARNING: should be used on grouped events."),
> OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "",
> "use branch records for per branch histogram filling",
> parse_branch_mode),
> @@ -1350,6 +1354,12 @@ int cmd_report(int argc, const char **argv)
>
> setup_forced_leader(&report, session->evlist);
>
> + if (symbol_conf.group_sort_idx && !session->evlist->nr_groups) {
> + parse_options_usage(NULL, options, "group-sort-idx", 0);
> + ret = -EINVAL;
> + goto error;
> + }
> +
> if (itrace_synth_opts.last_branch)
> has_br_stack = true;
>
> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index f73675500061..35224b366305 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c
> @@ -151,15 +151,100 @@ static int field_cmp(u64 field_a, u64 field_b)
> return 0;
> }
>
> +static int pair_fields_alloc(struct hist_entry *a, struct hist_entry *b,
> + hpp_field_fn get_field, int nr_members,
> + u64 **fields_a, u64 **fields_b)
> +{
> + struct evsel *evsel;
> + struct hist_entry *pair;
> + u64 *fa, *fb;
> + int ret = -1;
> +
> + fa = calloc(nr_members, sizeof(*fa));
> + fb = calloc(nr_members, sizeof(*fb));
> +
> + if (!fa || !fb)
> + goto out;
> +
> + list_for_each_entry(pair, &a->pairs.head, pairs.node) {
> + evsel = hists_to_evsel(pair->hists);
> + fa[perf_evsel__group_idx(evsel)] = get_field(pair);
> + }
> +
> + list_for_each_entry(pair, &b->pairs.head, pairs.node) {
> + evsel = hists_to_evsel(pair->hists);
> + fb[perf_evsel__group_idx(evsel)] = get_field(pair);
> + }
> +
> + *fields_a = fa;
> + *fields_b = fb;
> + ret = 0;
> +
> +out:
> + if (ret != 0) {
> + free(fa);
> + free(fb);
> + *fields_a = NULL;
> + *fields_b = NULL;
> + }
> +
> + return ret;
> +}
> +
> +static int __hpp__group_sort_idx(struct hist_entry *a, struct hist_entry *b,
> + hpp_field_fn get_field, int idx)
> +{
> + struct evsel *evsel = hists_to_evsel(a->hists);
> + u64 *fields_a, *fields_b;
> + int cmp, nr_members, ret, i;
> +
> + cmp = field_cmp(get_field(a), get_field(b));
> + if (!perf_evsel__is_group_event(evsel))
> + return cmp;
> +
> + nr_members = evsel->core.nr_members;
> + if (idx < 1 || idx >= nr_members)
> + return cmp;
> +
> + ret = pair_fields_alloc(a, b, get_field, nr_members,
> + &fields_a, &fields_b);
> + if (ret) {
> + ret = cmp;
> + goto out;
> + }
> +
> + ret = field_cmp(fields_a[idx], fields_b[idx]);
> + if (ret)
> + goto out;
> +
> + for (i = 1; i < nr_members; i++) {
> + if (i != idx) {
> + ret = field_cmp(fields_a[i], fields_b[i]);
> + if (ret)
> + goto out;
> + }
> + }
> +
> +out:
> + free(fields_a);
> + free(fields_b);
> +
> + return ret;
> +}
> +
> static int __hpp__sort(struct hist_entry *a, struct hist_entry *b,
> hpp_field_fn get_field)
> {
> s64 ret;
> int i, nr_members;
> struct evsel *evsel;
> - struct hist_entry *pair;
> u64 *fields_a, *fields_b;
>
> + if (symbol_conf.group_sort_idx && symbol_conf.event_group) {
> + return __hpp__group_sort_idx(a, b, get_field,
> + symbol_conf.group_sort_idx);
> + }
> +
> ret = field_cmp(get_field(a), get_field(b));
> if (ret || !symbol_conf.event_group)
> return ret;
> @@ -169,22 +254,11 @@ static int __hpp__sort(struct hist_entry *a, struct hist_entry *b,
> return ret;
>
> nr_members = evsel->core.nr_members;
> - fields_a = calloc(nr_members, sizeof(*fields_a));
> - fields_b = calloc(nr_members, sizeof(*fields_b));
> -
> - if (!fields_a || !fields_b)
> + i = pair_fields_alloc(a, b, get_field, nr_members,
> + &fields_a, &fields_b);
> + if (i)
> goto out;
>
> - list_for_each_entry(pair, &a->pairs.head, pairs.node) {
> - evsel = hists_to_evsel(pair->hists);
> - fields_a[perf_evsel__group_idx(evsel)] = get_field(pair);
> - }
> -
> - list_for_each_entry(pair, &b->pairs.head, pairs.node) {
> - evsel = hists_to_evsel(pair->hists);
> - fields_b[perf_evsel__group_idx(evsel)] = get_field(pair);
> - }
> -
> for (i = 1; i < nr_members; i++) {
> ret = field_cmp(fields_a[i], fields_b[i]);
> if (ret)
> diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
> index 10f1ec3e0349..b916afb95ec5 100644
> --- a/tools/perf/util/symbol_conf.h
> +++ b/tools/perf/util/symbol_conf.h
> @@ -73,6 +73,7 @@ struct symbol_conf {
> const char *symfs;
> int res_sample;
> int pad_output_len_dso;
> + int group_sort_idx;
> };
>
> extern struct symbol_conf symbol_conf;
>
Powered by blists - more mailing lists