| 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: <20160226224354.GC8720@kernel.org>
Date: Fri, 26 Feb 2016 19:43:54 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Jiri Olsa <jolsa@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
David Ahern <dsahern@...il.com>,
Andi Kleen <andi@...stfloor.org>,
Stephane Eranian <eranian@...gle.com>,
Wang Nan <wangnan0@...wei.com>
Subject: Re: [PATCH 5/5] perf report: Update column width of dynamic entries
Em Sat, Feb 27, 2016 at 03:52:47AM +0900, Namhyung Kim escreveu:
> The column width of dynamic entries is updated when comparing hist
> entries. However some unique entries can miss the chance to update. So
> move the update to output resort stage to make sure every entry will get
> called before display.
>
> To do that, abuse ->sort callback to update the width when the third
> argument is NULL. When resorting entries in normal path, it never be
> NULL so it should be fine IMHO.
>
> Before:
>
> # Overhead ptr / bytes_req / gfp_flags
> # .............. ..........................................
> #
> 37.50% 0xffff8803f7669400
> 37.50% 448
> 37.50% GFP_ATOMIC|GFP_NOWARN|GFP_NOMEMALLOC
> 10.42% 0xffff8803f766be00
> 8.33% 96
> 8.33% GFP_ATOMIC|GFP_NOWARN|GFP_NOMEMALLOC
> 2.08% 512
> 2.08% GFP_KERNEL|GFP_NOWARN|GFP_REPEAT|GFP <-- here
So, I tested this with:
perf report ... > /tmp/before
apply the patch, and then do > /tmp/after, do the diff, reproduced what
you described, slightly different output:
[root@...icio ~]# diff -u /tmp/before /tmp/after | grep ^[+-]
--- /tmp/before 2016-02-26 19:38:31.321497576 -0300
+++ /tmp/after 2016-02-26 19:39:24.043614934 -0300
-# .............. ....................................
+# .............. ........................................
- 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA
+ 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC
- 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA
+ 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC
- 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA
+ 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC
- 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA
+ 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC
- 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA
+ 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC
- 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA
+ 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC
- 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA
+ 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC
[root@...icio ~]# diff -u /tmp/before /tmp/after | grep ^[+-]
This is something else to add to the regression tests, output diff for
as many command line combos outputs as we can...
Anyway, good work, all applied with Jiri's acks.
- Arnaldo
>
> After:
>
> # Overhead ptr / bytes_req / gfp_flags
> # .............. .....................................................
> #
> 37.50% 0xffff8803f7669400
> 37.50% 448
> 37.50% GFP_ATOMIC|GFP_NOWARN|GFP_NOMEMALLOC
> 10.42% 0xffff8803f766be00
> 8.33% 96
> 8.33% GFP_ATOMIC|GFP_NOWARN|GFP_NOMEMALLOC
> 2.08% 512
> 2.08% GFP_KERNEL|GFP_NOWARN|GFP_REPEAT|GFP_NOMEMALLOC
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
> tools/perf/util/hist.c | 11 +++++++++++
> tools/perf/util/sort.c | 8 +++++---
> 2 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 9b3f582867d6..4b8b67bc0cd8 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -1371,6 +1371,10 @@ static void hierarchy_insert_output_entry(struct rb_root *root,
>
> rb_link_node(&he->rb_node, parent, p);
> rb_insert_color(&he->rb_node, root);
> +
> + /* update column width of dynamic entry */
> + if (perf_hpp__is_dynamic_entry(he->fmt))
> + he->fmt->sort(he->fmt, he, NULL);
> }
>
> static void hists__hierarchy_output_resort(struct hists *hists,
> @@ -1440,6 +1444,7 @@ static void __hists__insert_output_entry(struct rb_root *entries,
> struct rb_node **p = &entries->rb_node;
> struct rb_node *parent = NULL;
> struct hist_entry *iter;
> + struct perf_hpp_fmt *fmt;
>
> if (use_callchain) {
> if (callchain_param.mode == CHAIN_GRAPH_REL) {
> @@ -1466,6 +1471,12 @@ static void __hists__insert_output_entry(struct rb_root *entries,
>
> rb_link_node(&he->rb_node, parent, p);
> rb_insert_color(&he->rb_node, entries);
> +
> + perf_hpp_list__for_each_sort_list(&perf_hpp_list, fmt) {
> + if (perf_hpp__is_dynamic_entry(fmt) &&
> + perf_hpp__defined_dynamic_entry(fmt, he->hists))
> + fmt->sort(fmt, he, NULL); /* update column width */
> + }
> }
>
> static void output_resort(struct hists *hists, struct ui_progress *prog,
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 3b1b4018f111..1457c9d975a0 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1819,6 +1819,11 @@ static int64_t __sort__hde_cmp(struct perf_hpp_fmt *fmt,
>
> hde = container_of(fmt, struct hpp_dynamic_entry, hpp);
>
> + if (b == NULL) {
> + update_dynamic_len(hde, a);
> + return 0;
> + }
> +
> field = hde->field;
> if (field->flags & FIELD_IS_DYNAMIC) {
> unsigned long long dyn;
> @@ -1833,9 +1838,6 @@ static int64_t __sort__hde_cmp(struct perf_hpp_fmt *fmt,
> } else {
> offset = field->offset;
> size = field->size;
> -
> - update_dynamic_len(hde, a);
> - update_dynamic_len(hde, b);
> }
>
> return memcmp(a->raw_data + offset, b->raw_data + offset, size);
> --
> 2.7.1
Powered by blists - more mailing lists