Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1203411 > unrolled thread

[PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account

Started byGreg Kroah-Hartman <gregkh@linuxfoundation.org>
First post2015-08-09 00:20 +0200
Last post2015-08-11 18:40 +0200
Articles 5 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-08-09 00:20 +0200
    Re: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos,  etc filters into account "Andre Tomt (LKML)" <lkml@tomt.net> - 2015-08-09 20:30 +0200
      Re: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos,  etc filters into account Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-08-10 21:20 +0200
      Re: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos,  etc filters into account Luis Henriques <luis.henriques@canonical.com> - 2015-08-11 11:00 +0200
        Re: [PATCH 4.1 099/123] perf hists browser: Take the --comm,  --dsos, etc filters into account Kamal Mostafa <kamal@canonical.com> - 2015-08-11 18:40 +0200

#1203411 — [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account

FromGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Date2015-08-09 00:20 +0200
Subject[PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account
Message-ID<pVkD8-3vG-39@gated-at.bofh.it>
4.1-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Arnaldo Carvalho de Melo <acme@redhat.com>

commit 9c0fa8dd3d58de8b688fda758eea1719949c7f0a upstream.

At some point:

  commit 2c86c7ca7606
  Author: Namhyung Kim <namhyung@kernel.org>
  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 <namhyung@kernel.org>
  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.

Reported-by: Michael Petlan <mpetlan@redhat.com>
Fixes: 268397cb2a47 ("perf top/tui: Update nr_entries properly after a filter is applied")
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-6w01d5q97qk0d64kuojme5in@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 tools/perf/ui/browsers/hists.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -45,7 +45,7 @@ static struct rb_node *hists__filter_ent
 
 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)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1203678 — Re: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account

From"Andre Tomt (LKML)" <lkml@tomt.net>
Date2015-08-09 20:30 +0200
SubjectRe: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account
Message-ID<pVDw5-5Jn-3@gated-at.bofh.it>
In reply to#1203411
On 09. aug. 2015 00:09, Greg Kroah-Hartman wrote:
> 4.1-stable review patch.  If anyone has any objections, please let me know.
>
> ------------------
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> commit 9c0fa8dd3d58de8b688fda758eea1719949c7f0a upstream.

This commit needs upstream commit 
0bc2f2f7d080561cc484d2d0a162a9396bed3383 as well to fix the following 
build error


   CC       ui/browsers/hists.o
ui/browsers/hists.c: In function 'hist_browser__has_filter':
ui/browsers/hists.c:48:68: error: 'struct symbol_conf' has no member 
named 'has_filter'
   return hists__has_filter(hb->hists) || hb->min_pcnt || 
symbol_conf.has_filter;
                                                                     ^
ui/browsers/hists.c:49:1: error: control reaches end of non-void 
function [-Werror=return-type]
  }
  ^
cc1: all warnings being treated as errors
/disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:68: recipe 
for target 'ui/browsers/hists.o' failed
make[4]: *** [ui/browsers/hists.o] Error 1
/disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:107: recipe 
for target 'browsers' failed
make[3]: *** [browsers] Error 2
/disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:107: recipe 
for target 'ui' failed
make[2]: *** [ui] Error 2
Makefile.perf:366: recipe for target 'libperf-in.o' failed
make[1]: *** [libperf-in.o] Error 2
Makefile:68: recipe for target 'all' failed
make: *** [all] Error 2


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1204503 — Re: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account

FromGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Date2015-08-10 21:20 +0200
SubjectRe: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account
Message-ID<pW0M2-73c-37@gated-at.bofh.it>
In reply to#1203678
On Sun, Aug 09, 2015 at 08:12:41PM +0200, Andre Tomt (LKML) wrote:
> On 09. aug. 2015 00:09, Greg Kroah-Hartman wrote:
> >4.1-stable review patch.  If anyone has any objections, please let me know.
> >
> >------------------
> >
> >From: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> >commit 9c0fa8dd3d58de8b688fda758eea1719949c7f0a upstream.
> 
> This commit needs upstream commit 0bc2f2f7d080561cc484d2d0a162a9396bed3383
> as well to fix the following build error
> 
> 
>   CC       ui/browsers/hists.o
> ui/browsers/hists.c: In function 'hist_browser__has_filter':
> ui/browsers/hists.c:48:68: error: 'struct symbol_conf' has no member named
> 'has_filter'
>   return hists__has_filter(hb->hists) || hb->min_pcnt ||
> symbol_conf.has_filter;
>                                                                     ^
> ui/browsers/hists.c:49:1: error: control reaches end of non-void function
> [-Werror=return-type]
>  }
>  ^
> cc1: all warnings being treated as errors
> /disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:68: recipe for
> target 'ui/browsers/hists.o' failed
> make[4]: *** [ui/browsers/hists.o] Error 1
> /disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:107: recipe for
> target 'browsers' failed
> make[3]: *** [browsers] Error 2
> /disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:107: recipe for
> target 'ui' failed
> make[2]: *** [ui] Error 2
> Makefile.perf:366: recipe for target 'libperf-in.o' failed
> make[1]: *** [libperf-in.o] Error 2
> Makefile:68: recipe for target 'all' failed
> make: *** [all] Error 2
> 

Now applied, thanks.


greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1204895 — Re: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account

FromLuis Henriques <luis.henriques@canonical.com>
Date2015-08-11 11:00 +0200
SubjectRe: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account
Message-ID<pWdzB-8H-39@gated-at.bofh.it>
In reply to#1203678
On Sun, Aug 09, 2015 at 08:12:41PM +0200, Andre Tomt (LKML) wrote:
> On 09. aug. 2015 00:09, Greg Kroah-Hartman wrote:
> >4.1-stable review patch.  If anyone has any objections, please let me know.
> >
> >------------------
> >
> >From: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> >commit 9c0fa8dd3d58de8b688fda758eea1719949c7f0a upstream.
> 
> This commit needs upstream commit 0bc2f2f7d080561cc484d2d0a162a9396bed3383
> as well to fix the following build error
>

Thanks, I'm queuing this commit for the 3.16 kernel as well.  Also
CC'ing Kamal as it looks like 3.19 needs it too.

Cheers,
--
Luís


> 
>   CC       ui/browsers/hists.o
> ui/browsers/hists.c: In function 'hist_browser__has_filter':
> ui/browsers/hists.c:48:68: error: 'struct symbol_conf' has no member named
> 'has_filter'
>   return hists__has_filter(hb->hists) || hb->min_pcnt ||
> symbol_conf.has_filter;
>                                                                     ^
> ui/browsers/hists.c:49:1: error: control reaches end of non-void function
> [-Werror=return-type]
>  }
>  ^
> cc1: all warnings being treated as errors
> /disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:68: recipe for
> target 'ui/browsers/hists.o' failed
> make[4]: *** [ui/browsers/hists.o] Error 1
> /disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:107: recipe for
> target 'browsers' failed
> make[3]: *** [browsers] Error 2
> /disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:107: recipe for
> target 'ui' failed
> make[2]: *** [ui] Error 2
> Makefile.perf:366: recipe for target 'libperf-in.o' failed
> make[1]: *** [libperf-in.o] Error 2
> Makefile:68: recipe for target 'all' failed
> make: *** [all] Error 2
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1205267 — Re: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account

FromKamal Mostafa <kamal@canonical.com>
Date2015-08-11 18:40 +0200
SubjectRe: [PATCH 4.1 099/123] perf hists browser: Take the --comm, --dsos, etc filters into account
Message-ID<pWkKK-2d6-11@gated-at.bofh.it>
In reply to#1204895
On Tue, 2015-08-11 at 09:52 +0100, Luis Henriques wrote:
> On Sun, Aug 09, 2015 at 08:12:41PM +0200, Andre Tomt (LKML) wrote:
> > On 09. aug. 2015 00:09, Greg Kroah-Hartman wrote:
> > >4.1-stable review patch.  If anyone has any objections, please let me know.
> > >
> > >------------------
> > >
> > >From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > >
> > >commit 9c0fa8dd3d58de8b688fda758eea1719949c7f0a upstream.
> > 
> > This commit needs upstream commit 0bc2f2f7d080561cc484d2d0a162a9396bed3383
> > as well to fix the following build error
> >
> 
> Thanks, I'm queuing this commit for the 3.16 kernel as well.  Also
> CC'ing Kamal as it looks like 3.19 needs it too.

Indeed it does.  Queued for 3.19-stable also:

        0bc2f2f perf symbols: Store if there is a filter in place

Thanks very much Andre.

 -Kamal


> > 
> >   CC       ui/browsers/hists.o
> > ui/browsers/hists.c: In function 'hist_browser__has_filter':
> > ui/browsers/hists.c:48:68: error: 'struct symbol_conf' has no member named
> > 'has_filter'
> >   return hists__has_filter(hb->hists) || hb->min_pcnt ||
> > symbol_conf.has_filter;
> >                                                                     ^
> > ui/browsers/hists.c:49:1: error: control reaches end of non-void function
> > [-Werror=return-type]
> >  }
> >  ^
> > cc1: all warnings being treated as errors
> > /disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:68: recipe for
> > target 'ui/browsers/hists.o' failed
> > make[4]: *** [ui/browsers/hists.o] Error 1
> > /disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:107: recipe for
> > target 'browsers' failed
> > make[3]: *** [browsers] Error 2
> > /disks/build/data/linux-4.1.5-rc1/tools/build/Makefile.build:107: recipe for
> > target 'ui' failed
> > make[2]: *** [ui] Error 2
> > Makefile.perf:366: recipe for target 'libperf-in.o' failed
> > make[1]: *** [libperf-in.o] Error 2
> > Makefile:68: recipe for target 'all' failed
> > make: *** [all] Error 2
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe stable" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web