From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756617AbcBBQwM (ORCPT ); Tue, 2 Feb 2016 11:52:12 -0500 Received: from mail.kernel.org ([198.145.29.136]:40180 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752594AbcBBQwJ (ORCPT ); Tue, 2 Feb 2016 11:52:09 -0500 Date: Tue, 2 Feb 2016 13:52:05 -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 06/21] perf hists: Return error from hists__collapse_resort() Message-ID: <20160202165205.GF32488@kernel.org> References: <1454424003-19031-1-git-send-email-namhyung@kernel.org> <1454424003-19031-7-git-send-email-namhyung@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1454424003-19031-7-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 Tue, Feb 02, 2016 at 11:39:48PM +0900, Namhyung Kim escreveu: > Currently hists__collapse_resort() and hists__collapse_insert_entry() > don't return error code. Now callchain_merge() can check error case, > abort and pass the error to the user. Later patch can add more work > which can be failed too. > > Acked-by: Jiri Olsa > Signed-off-by: Namhyung Kim > --- > tools/perf/util/hist.c | 27 +++++++++++++++++---------- > tools/perf/util/hist.h | 4 ++-- > 2 files changed, 19 insertions(+), 12 deletions(-) > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > index 098310bc4489..b476b599e415 100644 > --- a/tools/perf/util/hist.c > +++ b/tools/perf/util/hist.c > @@ -1016,8 +1016,8 @@ void hist_entry__delete(struct hist_entry *he) > * collapse the histogram > */ > > -bool hists__collapse_insert_entry(struct hists *hists __maybe_unused, > - struct rb_root *root, struct hist_entry *he) > +int hists__collapse_insert_entry(struct hists *hists, struct rb_root *root, > + struct hist_entry *he) > { > struct rb_node **p = &root->rb_node; > struct rb_node *parent = NULL; > @@ -1037,12 +1037,13 @@ bool hists__collapse_insert_entry(struct hists *hists __maybe_unused, > > if (symbol_conf.use_callchain) { > callchain_cursor_reset(&callchain_cursor); > - callchain_merge(&callchain_cursor, > - iter->callchain, > - he->callchain); > + if (callchain_merge(&callchain_cursor, > + iter->callchain, > + he->callchain) < 0) > + return -1; So now we're exiting on error here, whereas before we would unconditionally call hist_entry__delete() right after this branch... Is that right? Looking at it... > } > hist_entry__delete(he); > - return false; > + return 0; > } > > if (cmp < 0) > @@ -1054,7 +1055,7 @@ bool hists__collapse_insert_entry(struct hists *hists __maybe_unused, > > rb_link_node(&he->rb_node_in, parent, p); > rb_insert_color(&he->rb_node_in, root); > - return true; > + return 1; > } > > struct rb_root *hists__get_rotate_entries_in(struct hists *hists) > @@ -1080,14 +1081,15 @@ static void hists__apply_filters(struct hists *hists, struct hist_entry *he) > hists__filter_entry_by_socket(hists, he); > } > > -void hists__collapse_resort(struct hists *hists, struct ui_progress *prog) > +int hists__collapse_resort(struct hists *hists, struct ui_progress *prog) > { > struct rb_root *root; > struct rb_node *next; > struct hist_entry *n; > + int ret; > > if (!sort__need_collapse) > - return; > + return 0; > > hists->nr_entries = 0; > > @@ -1102,7 +1104,11 @@ void hists__collapse_resort(struct hists *hists, struct ui_progress *prog) > next = rb_next(&n->rb_node_in); > > rb_erase(&n->rb_node_in, root); > - if (hists__collapse_insert_entry(hists, &hists->entries_collapsed, n)) { > + ret = hists__collapse_insert_entry(hists, &hists->entries_collapsed, n); > + if (ret < 0) > + return -1; > + > + if (ret) { > /* > * If it wasn't combined with one of the entries already > * collapsed, we need to apply the filters that may have > @@ -1113,6 +1119,7 @@ void hists__collapse_resort(struct hists *hists, struct ui_progress *prog) > if (prog) > ui_progress__update(prog, 1); > } > + return 0; > } > > static int hist_entry__sort(struct hist_entry *a, struct hist_entry *b) > diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h > index d4ec4822a103..9c72f8331d1c 100644 > --- a/tools/perf/util/hist.h > +++ b/tools/perf/util/hist.h > @@ -129,7 +129,7 @@ int hist_entry__sort_snprintf(struct hist_entry *he, char *bf, size_t size, > void hist_entry__delete(struct hist_entry *he); > > void hists__output_resort(struct hists *hists, struct ui_progress *prog); > -void hists__collapse_resort(struct hists *hists, struct ui_progress *prog); > +int hists__collapse_resort(struct hists *hists, struct ui_progress *prog); > > void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel); > void hists__delete_entries(struct hists *hists); > @@ -188,7 +188,7 @@ int hists__init(void); > int __hists__init(struct hists *hists); > > struct rb_root *hists__get_rotate_entries_in(struct hists *hists); > -bool hists__collapse_insert_entry(struct hists *hists __maybe_unused, > +int hists__collapse_insert_entry(struct hists *hists, > struct rb_root *root, struct hist_entry *he); > > struct perf_hpp { > -- > 2.6.4