Skip to contentSkip to conversation

perf/urgent fixes

GIT PULL New Arnaldo Carvalho de Melo 2 participants 4 messages Jul 13, 2015 – Jul 15, 2015 perf events

lore
Share & tools
lore

Arnaldo Carvalho de Melo

author maintainer
#0 lore raw

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit a833581e372a4adae2319d8dc379493edbc444e9:

x86, perf: Fix static_key bug in load_mm_cr4() (2015-07-10 10:24:38 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo

for you to fetch changes up to 9c0fa8dd3d58de8b688fda758eea1719949c7f0a:

perf hists browser: Take the --comm, --dsos, etc filters into account (2015-07-13 16:06:09 -0300)

----------------------------------------------------------------
perf/urgent fix:

User visible:

- Fix 'perf report' and 'perf top' handling of the '--dsos DSO-LIST', '--comms COMM-LIST' and '--symbols SYM-LIST' command line options, that were segfaulting due to not considering those lists as filters in the hists browser TUI code. (Arnaldo Carvalho de Melo)

----------------------------------------------------------------
Arnaldo Carvalho de Melo (2):
perf symbols: Store if there is a filter in place
perf hists browser: Take the --comm, --dsos, etc filters into account

 tools/perf/ui/browsers/hists.c | 2 +-
 tools/perf/util/symbol.c       | 2 ++
 tools/perf/util/symbol.h       | 3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)
author maintainer
#1 lore raw

PATCH 1/2 perf symbols: Store if there is a filter in place

From: Arnaldo Carvalho de Melo <[email protected]>

When setting yup the symbols library we setup several filter lists, for dsos, comms, symbols, etc, and there is code that, if there are filters, do certain operations, like recalculate the number of non filtered histogram entries in the top/report TUI.

But they were considering just the "Zoom" filters, when they need to take into account as well the above mentioned filters (perf top --comms, --dsos, etc).

So store in symbol_conf.has_filter true if any of those filters is in place.

Cc Adrian Hunter <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/symbol.c | 2 ++
 tools/perf/util/symbol.h | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)
tools/perf/util/symbol.c +2 -0
index 48b588c6951a..60f11414bb5c 100644
@@ -1911,6 +1911,8 @@ int setup_list(struct strlist **list, const char *list_str,
 		pr_err("problems parsing %s list\n", list_name);
 		return -1;
 	}
+
+	symbol_conf.has_filter = true;
 	return 0;
 }
 
tools/perf/util/symbol.h +2 -1
index bef47ead1d9b..b98ce51af142 100644
@@ -105,7 +105,8 @@ struct symbol_conf {
 			demangle_kernel,
 			filter_relative,
 			show_hist_headers,
-			branch_callstack;
+			branch_callstack,
+			has_filter;
 	const char	*vmlinux_name,
 			*kallsyms_name,
 			*source_prefix,
Signature · 1 line
2.1.0
author maintainer
#2 lore raw

PATCH 2/2 perf hists browser: Take the --comm, --dsos, etc filters into account

In reply to Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <[email protected]>

At some point:

commit 2c86c7ca7606
Author: Namhyung Kim <[email protected]>
Date: Mon Mar 17 18:18:54 2014 -0300

    perf report: Merge al->filtered with hist_entry->filtered

We stopped dropping samples for things filtered via the --comms, --dsos, --symbols, etc, i.e. things marked as filtered in the symbol resolution routines (thread__find_addr_map(), perf_event__preprocess_sample(), etc).

But then, in:

commit 268397cb2a47
Author: Namhyung Kim <[email protected]>
Date: Tue Apr 22 14:49:31 2014 +0900

perf top/tui: Update nr_entries properly after a filter is applied

We don't take into account entries that were filtered in perf_event__preprocess_sample() and friends, which leads to inconsistency in the browser seek routines, that expects the number of hist_entry->filtered entries to match what it thinks is the number of unfiltered, browsable entries.

So, for instance, when we do:

  perf top --symbols ___non_existent_symbol___

the hist_browser__nr_entries() routine thinks there are no filters in place, uses the hists->nr_entries but all entries are filtered, leading to a segfault.

Tested with:

   perf top --symbols malloc,free --percentage=relative

Freezing, by pressing 'f', at any time and doing the math on the percentages ends up with 100%, ditto for:

   perf top --dsos libpthread-2.20.so,libxul.so --percentage=relative

Both were segfaulting, all fixed now.

More work needed to do away with checking if filters are in place, we should just use the nr_non_filtered_samples counter, no need to conditionally use it or hists.nr_filter, as what the browser does is just show unfiltered stuff. An audit of how it is being accounted is needed, this is the minimal fix.

Fixes 268397cb2a47 ("perf top/tui: Update nr_entries properly after a filter is applied")
Cc Adrian Hunter <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/ui/browsers/hists.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/ui/browsers/hists.c +1 -1
index 7629bef2fd79..fa67613976a8 100644
@@ -48,7 +48,7 @@ static struct rb_node *hists__filter_entries(struct rb_node *nd,
 
 static bool hist_browser__has_filter(struct hist_browser *hb)
 {
-	return hists__has_filter(hb->hists) || hb->min_pcnt;
+	return hists__has_filter(hb->hists) || hb->min_pcnt || symbol_conf.has_filter;
 }
 
 static int hist_browser__get_folding(struct hist_browser *browser)
Signature · 1 line
2.1.0
#3 lore raw
In reply to Arnaldo Carvalho de Melo
Quoted 35 lines · Arnaldo Carvalho de Melo

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit a833581e372a4adae2319d8dc379493edbc444e9:

x86, perf: Fix static_key bug in load_mm_cr4() (2015-07-10 10:24:38 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo

for you to fetch changes up to 9c0fa8dd3d58de8b688fda758eea1719949c7f0a:

perf hists browser: Take the --comm, --dsos, etc filters into account (2015-07-13 16:06:09 -0300)

----------------------------------------------------------------
perf/urgent fix:

User visible:

- Fix 'perf report' and 'perf top' handling of the '--dsos DSO-LIST', '--comms COMM-LIST' and '--symbols SYM-LIST' command line options, that were segfaulting due to not considering those lists as filters in the hists browser TUI code. (Arnaldo Carvalho de Melo)

----------------------------------------------------------------
Arnaldo Carvalho de Melo (2):
perf symbols: Store if there is a filter in place
perf hists browser: Take the --comm, --dsos, etc filters into account

Quoted 4 lines
 tools/perf/ui/browsers/hists.c | 2 +-
 tools/perf/util/symbol.c       | 2 ++
 tools/perf/util/symbol.h       | 3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)

Pulled, thanks Arnaldo!

Ingo