From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933225AbcASUjh (ORCPT ); Tue, 19 Jan 2016 15:39:37 -0500 Received: from mail.kernel.org ([198.145.29.136]:34862 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933072AbcASUjP (ORCPT ); Tue, 19 Jan 2016 15:39:15 -0500 Date: Tue, 19 Jan 2016 17:39:10 -0300 From: Arnaldo Carvalho de Melo To: Namhyung Kim Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , David Ahern , Stephane Eranian , Andi Kleen , Wang Nan Subject: Re: [PATCH 04/17] perf hists: Cleanup filtering functions Message-ID: <20160119203910.GE27085@kernel.org> References: <1452960197-5323-1-git-send-email-namhyung@kernel.org> <1452960197-5323-5-git-send-email-namhyung@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1452960197-5323-5-git-send-email-namhyung@kernel.org> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Sun, Jan 17, 2016 at 01:03:04AM +0900, Namhyung Kim escreveu: > The hists__filter_by_xxx functions share same logic with different > filters. Factor out the common code into the hists__filter_by_type. > > The hists__filter_by_dso() code contained a check for parent, but I > think it should not be there. The PARENT filter bit was set by > symbol__parent_filter() which is related to symbol instead of dso. Also Ok, so break the patch in two, one removing the check for parent in hists__filter_by_dso(), then the patch introducing the function that receives the filter callback + type, Thanks, - Arnaldo > it didn't change the filter state anyway so end result will be same. > > Signed-off-by: Namhyung Kim > --- > tools/perf/util/hist.c | 92 ++++++++++++++++---------------------------------- > 1 file changed, 29 insertions(+), 63 deletions(-) > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > index 9354455aec5b..0790c053f65c 100644 > --- a/tools/perf/util/hist.c > +++ b/tools/perf/util/hist.c > @@ -1471,28 +1471,6 @@ static bool hists__filter_entry_by_dso(struct hists *hists, > return false; > } > > -void hists__filter_by_dso(struct hists *hists) > -{ > - struct rb_node *nd; > - > - hists->stats.nr_non_filtered_samples = 0; > - > - hists__reset_filter_stats(hists); > - hists__reset_col_len(hists); > - > - for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) { > - struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node); > - > - if (symbol_conf.exclude_other && !h->parent) > - continue; > - > - if (hists__filter_entry_by_dso(hists, h)) > - continue; > - > - hists__remove_entry_filter(hists, h, HIST_FILTER__DSO); > - } > -} > - > static bool hists__filter_entry_by_thread(struct hists *hists, > struct hist_entry *he) > { > @@ -1505,25 +1483,6 @@ static bool hists__filter_entry_by_thread(struct hists *hists, > return false; > } > > -void hists__filter_by_thread(struct hists *hists) > -{ > - struct rb_node *nd; > - > - hists->stats.nr_non_filtered_samples = 0; > - > - hists__reset_filter_stats(hists); > - hists__reset_col_len(hists); > - > - for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) { > - struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node); > - > - if (hists__filter_entry_by_thread(hists, h)) > - continue; > - > - hists__remove_entry_filter(hists, h, HIST_FILTER__THREAD); > - } > -} > - > static bool hists__filter_entry_by_symbol(struct hists *hists, > struct hist_entry *he) > { > @@ -1537,25 +1496,6 @@ static bool hists__filter_entry_by_symbol(struct hists *hists, > return false; > } > > -void hists__filter_by_symbol(struct hists *hists) > -{ > - struct rb_node *nd; > - > - hists->stats.nr_non_filtered_samples = 0; > - > - hists__reset_filter_stats(hists); > - hists__reset_col_len(hists); > - > - for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) { > - struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node); > - > - if (hists__filter_entry_by_symbol(hists, h)) > - continue; > - > - hists__remove_entry_filter(hists, h, HIST_FILTER__SYMBOL); > - } > -} > - > static bool hists__filter_entry_by_socket(struct hists *hists, > struct hist_entry *he) > { > @@ -1568,7 +1508,9 @@ static bool hists__filter_entry_by_socket(struct hists *hists, > return false; > } > > -void hists__filter_by_socket(struct hists *hists) > +typedef bool (*filter_fn_t)(struct hists *hists, struct hist_entry *he); > + > +static void hists__filter_by_type(struct hists *hists, int type, filter_fn_t filter) > { > struct rb_node *nd; > > @@ -1580,13 +1522,37 @@ void hists__filter_by_socket(struct hists *hists) > for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) { > struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node); > > - if (hists__filter_entry_by_socket(hists, h)) > + if (filter(hists, h)) > continue; > > - hists__remove_entry_filter(hists, h, HIST_FILTER__SOCKET); > + hists__remove_entry_filter(hists, h, type); > } > } > > +void hists__filter_by_thread(struct hists *hists) > +{ > + hists__filter_by_type(hists, HIST_FILTER__THREAD, > + hists__filter_entry_by_thread); > +} > + > +void hists__filter_by_dso(struct hists *hists) > +{ > + hists__filter_by_type(hists, HIST_FILTER__DSO, > + hists__filter_entry_by_dso); > +} > + > +void hists__filter_by_symbol(struct hists *hists) > +{ > + hists__filter_by_type(hists, HIST_FILTER__SYMBOL, > + hists__filter_entry_by_symbol); > +} > + > +void hists__filter_by_socket(struct hists *hists) > +{ > + hists__filter_by_type(hists, HIST_FILTER__SOCKET, > + hists__filter_entry_by_socket); > +} > + > void events_stats__inc(struct events_stats *stats, u32 type) > { > ++stats->nr_events[0]; > -- > 2.6.4