linux-kernel - Re: RFC: callchain showing same entry as hist

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160819022053.GA5253@sejong>
Date:   Fri, 19 Aug 2016 11:20:53 +0900
From:   Namhyung Kim <namhyung@...nel.org>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
CC:     Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: RFC: callchain showing same entry as hist_entry

Hi Arnaldo,

On Tue, Aug 16, 2016 at 04:21:31PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Aug 16, 2016 at 12:46:57PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Aug 17, 2016 at 12:41:18AM +0900, Namhyung Kim escreveu:
> > > Hmm.. if so, wouldn't it be better skipping the first callchain entry
> > > when the user-given sort key contains "sym" too (not only when it
> > > starts with 'sym')?
> > 
> > Probably, I think, whatever causes the mentioned duplication. And do
> > that at all the UIs, hence the suggestion for a callchain__first_node()
> > or more suitably named routine where such logic would live, to be
> > used for all the callchain rendering interfaces.
> 
> So, it is not possible to have that callchain__first_node() one, since
> we're dealing with one of the "list" entries, that are in each "node",
> etc, we need to do that test when printing the first entry, how about
> this one instead?

Sorry for delay.  If we don't care about the order of 'symbol' sort
key, we need to consider output fields (-F option) too since they'll
be used as sort keys eventually.  Once it sets the perf_hpp_list
fields correctly like below, the condition will be more simplified:

Thanks,
Namhyung


---------------8<-----------------
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 9b65f4a6b35a..0ec907433dc0 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -206,8 +206,7 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 			 * the symbol. No need to print it otherwise it appears as
 			 * displayed twice.
 			 */
-			if (!i++ && field_order == NULL &&
-			    sort_order && !prefixcmp(sort_order, "sym"))
+			if (!i++ && perf_hpp_list.sym)
 				continue;
 			if (!printed) {
 				ret += callchain__fprintf_left_margin(fp, left_margin);
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 1884d7f9b9d2..073e108d83f9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1720,6 +1720,19 @@ static int __sort_dimension__add_hpp_output(struct sort_dimension *sd,
 	if (hse == NULL)
 		return -1;
 
+	if (sd->entry == &sort_parent)
+		list->parent = 1;
+	else if (sd->entry == &sort_sym)
+		list->sym = 1;
+	else if (sd->entry == &sort_dso)
+		list->dso = 1;
+	else if (sd->entry == &sort_socket)
+		list->socket = 1;
+	else if (sd->entry == &sort_thread)
+		list->thread = 1;
+	else if (sd->entry == &sort_comm)
+		list->comm = 1;
+
 	perf_hpp_list__column_register(list, &hse->hpp);
 	return 0;
 }