| 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: <37D7C6CF3E00A74B8858931C1DB2F077016FA219@SHSMSX103.ccr.corp.intel.com>
Date: Wed, 25 Feb 2015 15:14:06 +0000
From: "Liang, Kan" <kan.liang@...el.com>
To: "acme@...nel.org" <acme@...nel.org>
CC: "jolsa@...hat.com" <jolsa@...hat.com>,
"namhyung@...nel.org" <namhyung@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"ak@...ux.intel.com" <ak@...ux.intel.com>
Subject: RE: [PATCH V7 1/1] perf tool:perf diff support for different
binaries
Hi Arnaldo,
Could you please review the patch?
I've already updated the patch description to try to address your concern.
Please let me know if you have any questions.
Thanks,
Kan
>
> From: Kan Liang <kan.liang@...el.com>
>
> Currently, the perf diff only works with same binaries. That's because it
> compares the symbol start address. It doesn't work if the perf.data comes
> from different binaries. This patch matches the symbol names.
>
> Actually, perf diff once intended to compare the symbol names.
> The commit as below can look for a pair by name.
> 604c5c92972d (perf diff: Change the default sort order to "dso,symbol")
> However, at that time, perf diff used a global list of dsos. That means the
> binaries which has same name can only be loaded once. That's a problem
> for different binaries compare. For example, we have an old binary and an
> updated binary. They very likely have same name and most of the function,
> so only dsos from old binary will be loaded. When processing the data from
> updated binary, perf still use the symbol information from old binary.
> That's wrong.
>
> Then the commit as below used IP to replace symbol name.
> 9c443dfdd31e ("perf diff: Fix support for all --sort combinations") From that
> time, perf diff starts to compare the symbol address.
>
> The global dsos is discarded from a patch in 2010.
> a1645ce12adb ("perf: 'perf kvm' tool for monitoring guest performance
> from host") However, at that time, perf diff already compared by address.
> So perf diff cannot work for different binaries as well.
>
> This patch actually rolls back the perf diff to original design. The document
> is also changed, so everybody knows the original design is to compare the
> symbol names.
>
> Here is an examples.
> The only difference between example_v1.c and example_v2.c is the
> location of f2 and f3. There is no change in behavior, but the previous perf
> diff display the wrong differential profile.
>
> example_v1.c
> noinline void f3(void)
> {
> volatile int i;
> for (i = 0; i < 10000;) {
>
> if(i%2)
> i++;
> else
> i++;
> }
> }
>
> noinline void f2(void)
> {
> volatile int a = 100, b, c;
> for (b = 0; b < 10000; b++)
> c = a * b;
>
> }
>
> noinline void f1(void)
> {
> f2();
> f3();
> }
>
> int main()
> {
> int i;
> for (i = 0; i < 100000; i++)
> f1();
> }
>
> example_v2.c
> noinline void f2(void)
> {
> volatile int a = 100, b, c;
> for (b = 0; b < 10000; b++)
> c = a * b;
> }
>
> noinline void f3(void)
> {
> volatile int i;
> for (i = 0; i < 10000;) {
> if(i%2)
> i++;
> else
> i++;
> }
> }
>
> noinline void f1(void)
> {
> f2();
> f3();
> }
>
> int main()
> {
> int i;
> for (i = 0; i < 100000; i++)
> f1();
> }
>
> [lk@...alhost perf_diff]$ gcc example_v1.c -o example [lk@...alhost
> perf_diff]$ perf record -o example_v1.data ./example [ perf record:
> Woken up 4 times to write data ] [ perf record: Captured and wrote 0.813
> MB example_v1.data (~35522
> samples) ]
>
> [lk@...alhost perf_diff]$ gcc example_v2.c -o example [lk@...alhost
> perf_diff]$ perf record -o example_v2.data ./example [ perf record:
> Woken up 4 times to write data ] [ perf record: Captured and wrote 0.824
> MB example_v2.data (~36015
> samples) ]
>
> Old perf diff result.
> [lk@...alhost perf_diff]$ perf diff example_v1.data example_v2.data
> Event 'cycles'
> Baseline Delta Shared Object Symbol
> ........ ....... ................ ...............................
>
> [kernel.vmlinux] [k] __perf_event_task_sched_out
> 0.00% [kernel.vmlinux] [k] apic_timer_interrupt
> [kernel.vmlinux] [k] idle_cpu
> [kernel.vmlinux] [k] intel_pstate_timer_func
> [kernel.vmlinux] [k] native_read_msr_safe
> 0.00% [kernel.vmlinux] [k] native_read_tsc
> 0.00% [kernel.vmlinux] [k] native_write_msr_safe
> [kernel.vmlinux] [k] ntp_tick_length
> 0.00% [kernel.vmlinux] [k] rb_erase
> 0.00% [kernel.vmlinux] [k] tick_sched_timer
> 0.00% [kernel.vmlinux] [k] unmap_single_vma
> 0.00% [kernel.vmlinux] [k] update_wall_time
> 0.00% example [.] f1
> 46.24% example [.] f2
> 53.71% -7.55% example [.] f3
> +53.81% example [.] f3
> 0.02% example [.] main
>
> New perf diff result.
> [lk@...alhost perf_diff]$ perf diff example_v1.data example_v2.data
> [kernel.vmlinux] [k] __perf_event_task_sched_out
> 0.00% [kernel.vmlinux] [k] apic_timer_interrupt
> [kernel.vmlinux] [k] idle_cpu
> [kernel.vmlinux] [k] intel_pstate_timer_func
> [kernel.vmlinux] [k] native_read_msr_safe
> 0.00% [kernel.vmlinux] [k] native_read_tsc
> 0.00% [kernel.vmlinux] [k] native_write_msr_safe
> [kernel.vmlinux] [k] ntp_tick_length
> 0.00% [kernel.vmlinux] [k] rb_erase
> 0.00% [kernel.vmlinux] [k] tick_sched_timer
> 0.00% [kernel.vmlinux] [k] unmap_single_vma
> 0.00% [kernel.vmlinux] [k] update_wall_time
> 0.00% example [.] f1
> 46.24% -0.08% example [.] f2
> 53.71% +0.11% example [.] f3
> 0.02% example [.] main
>
> Signed-off-by: Kan Liang <kan.liang@...el.com>
> Acked-by: Namhyung Kim <namhyung@...nel.org>
> Acked-by: Jiri Olsa <jolsa@...nel.org>
>
> ---
>
> Changes since V4:
> - Seperate from old patch set
> - Add an example in changelog
> - Update man page
>
> Changes since V5:
> - Correct patch style
>
> Changes since V6:
> - Update description
>
> tools/perf/Documentation/perf-diff.txt | 5 +++++
> tools/perf/util/sort.c | 9 +++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/tools/perf/Documentation/perf-diff.txt
> b/tools/perf/Documentation/perf-diff.txt
> index e463caa..5182661 100644
> --- a/tools/perf/Documentation/perf-diff.txt
> +++ b/tools/perf/Documentation/perf-diff.txt
> @@ -20,6 +20,11 @@ If no parameters are passed it will assume
> perf.data.old and perf.data.
> The differential profile is displayed only for events matching both
> specified perf.data files.
>
> +If no parameters are passed the samples will be sorted by dso and symbol.
> +As the perf.data files could come from different binaries, the symbols
> +addresses could vary. So perf diff is based on the comparison of the
> +files and symbols name.
> +
> OPTIONS
> -------
> -D::
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index
> 7a39c1e..4593f36 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1463,6 +1463,15 @@ int sort_dimension__add(const char *tok)
> sort__has_parent = 1;
> } else if (sd->entry == &sort_sym) {
> sort__has_sym = 1;
> + /*
> + * perf diff displays the performance difference
> amongst
> + * two or more perf.data files. Those files could
> come
> + * from different binaries. So we should not
> compare
> + * their ips, but the name of symbol.
> + */
> + if (sort__mode == SORT_MODE__DIFF)
> + sd->entry->se_collapse = sort__sym_sort;
> +
> } else if (sd->entry == &sort_dso) {
> sort__has_dso = 1;
> }
> --
> 1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists