[PATCHSET 0/5] perf tools: Update on filtered entries' percentage output (v3)

[PATCHSET 0/5] perf tools: Update on filtered entries' percentage output (v3)

[Namhyung Kim]

Hello,

I added --percentage option to perf report to control display of
percentage of filtered entries.

 usage: perf report [<options>]

        --percentage <relative|absolute>
                          how to display percentage of filtered entries

"relative" means it's relative to filtered entries only so that the
sum of shown entries will be always 100%.  "absolute" means it retains
original value before and after the filter applied.  In patch 5, I
made the "absolute" as default since it makes more sense IMHO.
    
      $ perf report -s comm
      # Overhead       Command
      # ........  ............
      #
          74.19%           cc1
           7.61%           gcc
           6.11%            as
           4.35%            sh
           4.14%          make
           1.13%        fixdep
      ...
    
      $ perf report -s comm -c cc1,gcc --percentage absolute
      # Overhead       Command
      # ........  ............
      #
          74.19%           cc1
           7.61%           gcc
    
      $ perf report -s comm -c cc1,gcc --percentage relative
      # Overhead       Command
      # ........  ............
      #
          90.69%           cc1
           9.31%           gcc
    
Note that it has zero effect if no filter was applied.

I only added the option to perf report for now.  If it looks good to
you I'll add it to perf top too.

Any comments are welcome, thanks
Namhyung


Namhyung Kim (5):
  perf tools: Count filtered entries to total period also
  perf ui/tui: Add support for showing relative percentage
  perf report: Add --percentage option
  perf report: Add report.percentage config option
  perf tools: Show absolute percentage by default

 tools/perf/Documentation/perf-report.txt | 24 ++++++++++++++++++------
 tools/perf/builtin-report.c              | 28 +++++++++++++++++++++++++++-
 tools/perf/util/event.c                  | 18 ++++++++----------
 tools/perf/util/hist.c                   | 27 ++++++++++++++-------------
 tools/perf/util/hist.h                   |  7 +++++++
 tools/perf/util/symbol.h                 |  5 +++--
 6 files changed, 77 insertions(+), 32 deletions(-)

-- 
1.7.11.7

[PATCH 1/5] perf tools: Count filtered entries to total period also

[Namhyung Kim]

Currently if a sample was filtered by command line option, it just
dropped.  But this affects final output in that the percentage can be
different since the filtered entries were not included to the total.

But user might want to see the original percentages when filter
applied so change the behavior depends on a new symbol_conf.filter_
relative value in order to be controlled by user later.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c |  2 +-
 tools/perf/util/event.c     | 18 ++++++++----------
 tools/perf/util/hist.c      | 12 +++---------
 tools/perf/util/hist.h      |  7 +++++++
 tools/perf/util/symbol.c    |  1 +
 tools/perf/util/symbol.h    |  5 +++--
 6 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3c53ec268fbc..dddacb0056e3 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -229,7 +229,7 @@ static int process_sample_event(struct perf_tool *tool,
 		return -1;
 	}
 
-	if (al.filtered || (rep->hide_unresolved && al.sym == NULL))
+	if (rep->hide_unresolved && al.sym == NULL)
 		return 0;
 
 	if (rep->cpu_list && !test_bit(sample->cpu, rep->cpu_bitmap))
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 1fc1c2f04772..f5a0bcd6be64 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -3,6 +3,7 @@
 #include "debug.h"
 #include "machine.h"
 #include "sort.h"
+#include "hist.h"
 #include "string.h"
 #include "strlist.h"
 #include "thread.h"
@@ -663,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
 	al->thread = thread;
 	al->addr = addr;
 	al->cpumode = cpumode;
-	al->filtered = false;
+	al->filtered = 0;
 
 	if (machine == NULL) {
 		al->map = NULL;
@@ -750,9 +751,6 @@ int perf_event__preprocess_sample(const union perf_event *event,
 	if (thread == NULL)
 		return -1;
 
-	if (thread__is_filtered(thread))
-		goto out_filtered;
-
 	dump_printf(" ... thread: %s:%d\n", thread__comm_str(thread), thread->tid);
 	/*
 	 * Have we already created the kernel maps for this machine?
@@ -767,6 +765,10 @@ int perf_event__preprocess_sample(const union perf_event *event,
 
 	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
 			      sample->ip, al);
+
+	if (thread__is_filtered(thread))
+		al->filtered |= (1 << HIST_FILTER__THREAD);
+
 	dump_printf(" ...... dso: %s\n",
 		    al->map ? al->map->dso->long_name :
 			al->level == 'H' ? "[hypervisor]" : "<not found>");
@@ -782,7 +784,7 @@ int perf_event__preprocess_sample(const union perf_event *event,
 			       (dso->short_name != dso->long_name &&
 				strlist__has_entry(symbol_conf.dso_list,
 						   dso->long_name)))))
-			goto out_filtered;
+			al->filtered |= (1 << HIST_FILTER__DSO);
 
 		al->sym = map__find_symbol(al->map, al->addr,
 					   machine->symbol_filter);
@@ -791,11 +793,7 @@ int perf_event__preprocess_sample(const union perf_event *event,
 	if (symbol_conf.sym_list &&
 		(!al->sym || !strlist__has_entry(symbol_conf.sym_list,
 						al->sym->name)))
-		goto out_filtered;
-
-	return 0;
+		al->filtered |= (1 << HIST_FILTER__SYMBOL);
 
-out_filtered:
-	al->filtered = true;
 	return 0;
 }
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index e4e6249b87d4..73b999448932 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -13,13 +13,6 @@ static bool hists__filter_entry_by_thread(struct hists *hists,
 static bool hists__filter_entry_by_symbol(struct hists *hists,
 					  struct hist_entry *he);
 
-enum hist_filter {
-	HIST_FILTER__DSO,
-	HIST_FILTER__THREAD,
-	HIST_FILTER__PARENT,
-	HIST_FILTER__SYMBOL,
-};
-
 struct callchain_param	callchain_param = {
 	.mode	= CHAIN_GRAPH_REL,
 	.min_percent = 0.5,
@@ -329,8 +322,9 @@ void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h)
 	if (!h->filtered) {
 		hists__calc_col_len(hists, h);
 		++hists->nr_entries;
-		hists->stats.total_period += h->stat.period;
 	}
+	if (!h->filtered || !symbol_conf.filter_relative)
+		hists->stats.total_period += h->stat.period;
 }
 
 static u8 symbol__parent_filter(const struct symbol *parent)
@@ -429,7 +423,7 @@ struct hist_entry *__hists__add_entry(struct hists *hists,
 			.weight = weight,
 		},
 		.parent = sym_parent,
-		.filtered = symbol__parent_filter(sym_parent),
+		.filtered = symbol__parent_filter(sym_parent) | al->filtered,
 		.hists	= hists,
 		.branch_info = bi,
 		.mem_info = mi,
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index a59743fa3ef7..7d1d973d9a39 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -14,6 +14,13 @@ struct hist_entry;
 struct addr_location;
 struct symbol;
 
+enum hist_filter {
+	HIST_FILTER__DSO,
+	HIST_FILTER__THREAD,
+	HIST_FILTER__PARENT,
+	HIST_FILTER__SYMBOL,
+};
+
 /*
  * The kernel collects the number of events it couldn't send in a stretch and
  * when possible sends this number in a PERF_RECORD_LOST event. The number of
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 39ce9adbaaf0..ac3d9748993c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -33,6 +33,7 @@ struct symbol_conf symbol_conf = {
 	.try_vmlinux_path = true,
 	.annotate_src	  = true,
 	.demangle	  = true,
+	.filter_relative  = true,
 	.symfs            = "",
 };
 
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index fffe2888a1c7..da779b23469b 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -104,7 +104,8 @@ struct symbol_conf {
 			annotate_asm_raw,
 			annotate_src,
 			event_group,
-			demangle;
+			demangle,
+			filter_relative;
 	const char	*vmlinux_name,
 			*kallsyms_name,
 			*source_prefix,
@@ -175,7 +176,7 @@ struct addr_location {
 	struct symbol *sym;
 	u64	      addr;
 	char	      level;
-	bool	      filtered;
+	u8	      filtered;
 	u8	      cpumode;
 	s32	      cpu;
 };
-- 
1.7.11.7

[PATCH 2/5] perf ui/tui: Add support for showing relative percentage

[Namhyung Kim]

When filtering by thread, dso or symbol on TUI it also update total
period so that the output shows different result than no filter - the
percentage changed to relative to filtered entries only.  Sometimes
(always?)  this is not desired since users might expect same results
with filter.

So change this behavior depends on symbol_conf.filter_relative value.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 73b999448932..61131ce9234b 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -696,10 +696,11 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
 		return;
 
 	++hists->nr_entries;
+	if (symbol_conf.filter_relative)
+		hists->stats.total_period += h->stat.period;
 	if (h->ms.unfolded)
 		hists->nr_entries += h->nr_rows;
 	h->row_offset = 0;
-	hists->stats.total_period += h->stat.period;
 	hists->stats.nr_events[PERF_RECORD_SAMPLE] += h->stat.nr_events;
 
 	hists__calc_col_len(hists, h);
@@ -722,7 +723,9 @@ void hists__filter_by_dso(struct hists *hists)
 {
 	struct rb_node *nd;
 
-	hists->nr_entries = hists->stats.total_period = 0;
+	hists->nr_entries = 0;
+	if (symbol_conf.filter_relative)
+		hists->stats.total_period = 0;
 	hists->stats.nr_events[PERF_RECORD_SAMPLE] = 0;
 	hists__reset_col_len(hists);
 
@@ -755,7 +758,9 @@ void hists__filter_by_thread(struct hists *hists)
 {
 	struct rb_node *nd;
 
-	hists->nr_entries = hists->stats.total_period = 0;
+	hists->nr_entries = 0;
+	if (symbol_conf.filter_relative)
+		hists->stats.total_period = 0;
 	hists->stats.nr_events[PERF_RECORD_SAMPLE] = 0;
 	hists__reset_col_len(hists);
 
@@ -786,7 +791,9 @@ void hists__filter_by_symbol(struct hists *hists)
 {
 	struct rb_node *nd;
 
-	hists->nr_entries = hists->stats.total_period = 0;
+	hists->nr_entries = 0;
+	if (symbol_conf.filter_relative)
+		hists->stats.total_period = 0;
 	hists->stats.nr_events[PERF_RECORD_SAMPLE] = 0;
 	hists__reset_col_len(hists);
 
-- 
1.7.11.7

[PATCH 3/5] perf report: Add --percentage option

[Namhyung Kim]

The --percentage option is for controlling overhead percentage
displayed.  It can only receive either of "relative" or "absolute".

"relative" means it's relative to filtered entries only so that the
sum of shown entries will be always 100%.  "absolute" means it retains
the original value before and after the filter is applied.

  $ perf report -s comm
  # Overhead       Command
  # ........  ............
  #
      74.19%           cc1
       7.61%           gcc
       6.11%            as
       4.35%            sh
       4.14%          make
       1.13%        fixdep
  ...

  $ perf report -s comm -c cc1,gcc --percentage absolute
  # Overhead       Command
  # ........  ............
  #
      74.19%           cc1
       7.61%           gcc

  $ perf report -s comm -c cc1,gcc --percentage relative
  # Overhead       Command
  # ........  ............
  #
      90.69%           cc1
       9.31%           gcc

Note that it has zero effect if no filter was applied.

Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-report.txt | 24 ++++++++++++++++++------
 tools/perf/builtin-report.c              | 16 ++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 8eab8a4bdeb8..09af66298564 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -25,10 +25,6 @@ OPTIONS
 --verbose::
         Be more verbose. (show symbol address, etc)
 
--d::
---dsos=::
-	Only consider symbols in these dsos. CSV that understands
-	file://filename entries.
 -n::
 --show-nr-samples::
 	Show the number of samples for each symbol
@@ -42,11 +38,18 @@ OPTIONS
 -c::
 --comms=::
 	Only consider symbols in these comms. CSV that understands
-	file://filename entries.
+	file://filename entries.  This option will affect the percentage of
+	the overhead column.  See --percentage for more info.
+-d::
+--dsos=::
+	Only consider symbols in these dsos. CSV that understands
+	file://filename entries.  This option will affect the percentage of
+	the overhead column.  See --percentage for more info.
 -S::
 --symbols=::
 	Only consider these symbols. CSV that understands
-	file://filename entries.
+	file://filename entries.  This option will affect the percentage of
+	the overhead column.  See --percentage for more info.
 
 --symbol-filter=::
 	Only show symbols that match (partially) with this filter.
@@ -237,6 +240,15 @@ OPTIONS
 	Do not show entries which have an overhead under that percent.
 	(Default: 0).
 
+--percentage::
+	Determine how to display the overhead percentage of filtered entries.
+	Filters can be applied by --comms, --dsos and/or --symbols options and
+	Zoom operations on the TUI (thread, dso, etc).
+
+	"relative" means it's relative to filtered entries only so that the
+	sum of shown entries will be always 100%.  "absolute" means it retains
+	the original value before and after the filter is applied.
+
 --header::
 	Show header information in the perf.data file.  This includes
 	various information like hostname, OS and perf version, cpu/mem
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index dddacb0056e3..cb81d45e514c 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -699,6 +699,20 @@ parse_percent_limit(const struct option *opt, const char *str,
 	return 0;
 }
 
+static int
+parse_percentage(const struct option *opt __maybe_unused, const char *str,
+		 int unset __maybe_unused)
+{
+	if (!strcmp(str, "relative"))
+		symbol_conf.filter_relative = true;
+	else if (!strcmp(str, "absolute"))
+		symbol_conf.filter_relative = false;
+	else
+		return -1;
+
+	return 0;
+}
+
 int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	struct perf_session *session;
@@ -821,6 +835,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_BOOLEAN(0, "mem-mode", &report.mem_mode, "mem access profile"),
 	OPT_CALLBACK(0, "percent-limit", &report, "percent",
 		     "Don't show entries under that percent", parse_percent_limit),
+	OPT_CALLBACK(0, "percentage", NULL, "relative|absolute",
+		     "how to display percentage of filtered entries", parse_percentage),
 	OPT_END()
 	};
 	struct perf_data_file file = {
-- 
1.7.11.7

[PATCH 4/5] perf report: Add report.percentage config option

[Namhyung Kim]

Add report.percentage option for setting default value of the
symbol_conf.filter_relative.  It affects the report output only if a
filter applied.

An user can write .perfconfig file like below to show absolute
percentage of filtered entries by default:

  $ cat ~/.perfconfig
  [report]
  percentage = absolute

And it can be changed through command line:

  $ perf report --percentage relative

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index cb81d45e514c..7ddea46594ae 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -71,6 +71,16 @@ static int report__config(const char *var, const char *value, void *cb)
 		rep->min_percent = strtof(value, NULL);
 		return 0;
 	}
+	if (!strcmp(var, "report.percentage")) {
+		if (!strcmp(value, "relative"))
+			symbol_conf.filter_relative = true;
+		else if (!strcmp(value, "absolute"))
+			symbol_conf.filter_relative = false;
+		else
+			return -1;
+
+		return 0;
+	}
 
 	return perf_default_config(var, value, cb);
 }
-- 
1.7.11.7

[PATCH 5/5] perf tools: Show absolute percentage by default

[Namhyung Kim]

Now perf report will show absolute percentage on filter entries by
default.

Suggested-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/symbol.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index ac3d9748993c..39ce9adbaaf0 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -33,7 +33,6 @@ struct symbol_conf symbol_conf = {
 	.try_vmlinux_path = true,
 	.annotate_src	  = true,
 	.demangle	  = true,
-	.filter_relative  = true,
 	.symfs            = "",
 };
 
-- 
1.7.11.7

Re: [PATCH 1/5] perf tools: Count filtered entries to total period also

[Jiri Olsa]

On Thu, Jan 23, 2014 at 08:28:49AM +0900, Namhyung Kim wrote:
> Currently if a sample was filtered by command line option, it just
> dropped.  But this affects final output in that the percentage can be
> different since the filtered entries were not included to the total.
> 
> But user might want to see the original percentages when filter
> applied so change the behavior depends on a new symbol_conf.filter_
> relative value in order to be controlled by user later.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

SNIP

> +++ b/tools/perf/util/hist.h
> @@ -14,6 +14,13 @@ struct hist_entry;
>  struct addr_location;
>  struct symbol;
>  
> +enum hist_filter {
> +	HIST_FILTER__DSO,
> +	HIST_FILTER__THREAD,
> +	HIST_FILTER__PARENT,
> +	HIST_FILTER__SYMBOL,
> +};

in thread__find_addr_map we still have:
		...
                if ((cpumode == PERF_RECORD_MISC_GUEST_USER ||
                        cpumode == PERF_RECORD_MISC_GUEST_KERNEL) &&
                        !perf_guest)
                        al->filtered = true;
                if ((cpumode == PERF_RECORD_MISC_USER ||
                        cpumode == PERF_RECORD_MISC_KERNEL) &&
                        !perf_host)
                        al->filtered = true;
		...

seems like we need HIST_FILTER__GUEST|HOST

also in machine__resolve_callchain_sample there's old (and superfluos) init:
		...
                al.filtered = false;
                thread__find_addr_location(thread, machine, cpumode,
                                           MAP__FUNCTION, ip, &al);
		...


jirka

Re: [PATCH 1/5] perf tools: Count filtered entries to total period also

[Jiri Olsa]

On Thu, Jan 23, 2014 at 08:28:49AM +0900, Namhyung Kim wrote:
> Currently if a sample was filtered by command line option, it just
> dropped.  But this affects final output in that the percentage can be
> different since the filtered entries were not included to the total.
> 
> But user might want to see the original percentages when filter
> applied so change the behavior depends on a new symbol_conf.filter_
> relative value in order to be controlled by user later.

SNIP

> +		al->filtered |= (1 << HIST_FILTER__SYMBOL);
>  
> -out_filtered:
> -	al->filtered = true;
>  	return 0;
>  }
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index e4e6249b87d4..73b999448932 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -13,13 +13,6 @@ static bool hists__filter_entry_by_thread(struct hists *hists,
>  static bool hists__filter_entry_by_symbol(struct hists *hists,
>  					  struct hist_entry *he);
>  
> -enum hist_filter {
> -	HIST_FILTER__DSO,
> -	HIST_FILTER__THREAD,
> -	HIST_FILTER__PARENT,
> -	HIST_FILTER__SYMBOL,
> -};

it striked me that al.filtered is duplicating he.filtered, so I thought
we could leave just the one in hist_entry.. now I see annotate, diff
and others use al.filtered to skip hist_entry adding/processing

I think there's use for relative/total switch in other places,
and we could actually get rid of the al.filtered and make
the relative switch global (also in config?) .. in the future ;-)

> -
>  struct callchain_param	callchain_param = {
>  	.mode	= CHAIN_GRAPH_REL,
>  	.min_percent = 0.5,
> @@ -329,8 +322,9 @@ void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h)
>  	if (!h->filtered) {
>  		hists__calc_col_len(hists, h);
>  		++hists->nr_entries;
> -		hists->stats.total_period += h->stat.period;
>  	}
> +	if (!h->filtered || !symbol_conf.filter_relative)
> +		hists->stats.total_period += h->stat.period;

also we could keep both counts and easily switch on in tui/gui
.. in the future ;-)

jirka

Re: [PATCH 5/5] perf tools: Show absolute percentage by default

[Jiri Olsa]

On Thu, Jan 23, 2014 at 08:28:53AM +0900, Namhyung Kim wrote:
> Now perf report will show absolute percentage on filter entries by
> default.
> 
> Suggested-by: Jiri Olsa <jolsa@redhat.com>

Acked-by: Jiri Olsa <jolsa@redhat.com> ;-)

once the 1st patch issues are solved, I'm ok with this

thanks,
jirka

Re: [PATCH 1/5] perf tools: Count filtered entries to total period also

[Namhyung Kim]

On Thu, 23 Jan 2014 14:21:00 +0100, Jiri Olsa wrote:
> On Thu, Jan 23, 2014 at 08:28:49AM +0900, Namhyung Kim wrote:
>> Currently if a sample was filtered by command line option, it just
>> dropped.  But this affects final output in that the percentage can be
>> different since the filtered entries were not included to the total.
>> 
>> But user might want to see the original percentages when filter
>> applied so change the behavior depends on a new symbol_conf.filter_
>> relative value in order to be controlled by user later.
>> 
>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> SNIP
>
>> +++ b/tools/perf/util/hist.h
>> @@ -14,6 +14,13 @@ struct hist_entry;
>>  struct addr_location;
>>  struct symbol;
>>  
>> +enum hist_filter {
>> +	HIST_FILTER__DSO,
>> +	HIST_FILTER__THREAD,
>> +	HIST_FILTER__PARENT,
>> +	HIST_FILTER__SYMBOL,
>> +};
>
> in thread__find_addr_map we still have:
> 		...
>                 if ((cpumode == PERF_RECORD_MISC_GUEST_USER ||
>                         cpumode == PERF_RECORD_MISC_GUEST_KERNEL) &&
>                         !perf_guest)
>                         al->filtered = true;
>                 if ((cpumode == PERF_RECORD_MISC_USER ||
>                         cpumode == PERF_RECORD_MISC_KERNEL) &&
>                         !perf_host)
>                         al->filtered = true;
> 		...
>
> seems like we need HIST_FILTER__GUEST|HOST
>
> also in machine__resolve_callchain_sample there's old (and superfluos) init:
> 		...
>                 al.filtered = false;
>                 thread__find_addr_location(thread, machine, cpumode,
>                                            MAP__FUNCTION, ip, &al);
> 		...

Both seems to need to be changed - will fix.

Thanks,
Namhyung

Re: [PATCH 1/5] perf tools: Count filtered entries to total period also

[Namhyung Kim]

Hi,

On Thu, 23 Jan 2014 14:38:48 +0100, Jiri Olsa wrote:
> On Thu, Jan 23, 2014 at 08:28:49AM +0900, Namhyung Kim wrote:
>> Currently if a sample was filtered by command line option, it just
>> dropped.  But this affects final output in that the percentage can be
>> different since the filtered entries were not included to the total.
>> 
>> But user might want to see the original percentages when filter
>> applied so change the behavior depends on a new symbol_conf.filter_
>> relative value in order to be controlled by user later.
>
> SNIP
>
>> +		al->filtered |= (1 << HIST_FILTER__SYMBOL);
>>  
>> -out_filtered:
>> -	al->filtered = true;
>>  	return 0;
>>  }
>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>> index e4e6249b87d4..73b999448932 100644
>> --- a/tools/perf/util/hist.c
>> +++ b/tools/perf/util/hist.c
>> @@ -13,13 +13,6 @@ static bool hists__filter_entry_by_thread(struct hists *hists,
>>  static bool hists__filter_entry_by_symbol(struct hists *hists,
>>  					  struct hist_entry *he);
>>  
>> -enum hist_filter {
>> -	HIST_FILTER__DSO,
>> -	HIST_FILTER__THREAD,
>> -	HIST_FILTER__PARENT,
>> -	HIST_FILTER__SYMBOL,
>> -};
>
> it striked me that al.filtered is duplicating he.filtered, so I thought
> we could leave just the one in hist_entry.. now I see annotate, diff
> and others use al.filtered to skip hist_entry adding/processing
>
> I think there's use for relative/total switch in other places,
> and we could actually get rid of the al.filtered and make
> the relative switch global (also in config?) .. in the future ;-)

Okay, as I wrote in the series description, it only changed perf report
part.  I'll make the necessary change to other parts in the next version.

>
>> -
>>  struct callchain_param	callchain_param = {
>>  	.mode	= CHAIN_GRAPH_REL,
>>  	.min_percent = 0.5,
>> @@ -329,8 +322,9 @@ void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h)
>>  	if (!h->filtered) {
>>  		hists__calc_col_len(hists, h);
>>  		++hists->nr_entries;
>> -		hists->stats.total_period += h->stat.period;
>>  	}
>> +	if (!h->filtered || !symbol_conf.filter_relative)
>> +		hists->stats.total_period += h->stat.period;
>
> also we could keep both counts and easily switch on in tui/gui
> .. in the future ;-)

Will do.

Thanks,
Namhyung