From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752237AbcCBNZe (ORCPT ); Wed, 2 Mar 2016 08:25:34 -0500 Received: from mail-pf0-f169.google.com ([209.85.192.169]:34195 "EHLO mail-pf0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750992AbcCBNZd (ORCPT ); Wed, 2 Mar 2016 08:25:33 -0500 Date: Wed, 2 Mar 2016 22:24:13 +0900 From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Jiri Olsa , David Ahern , Wang Nan , Linux Kernel Mailing List Subject: Re: [PATCH] Fix hists related 'perf test' entries Message-ID: <20160302132413.GA6040@danjae.kornet> References: <20160302125721.GM3604@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160302125721.GM3604@kernel.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Arnaldo, On Wed, Mar 02, 2016 at 09:57:21AM -0300, Arnaldo Carvalho de Melo wrote: > That got broken by d3a72fd8187b ("perf report: Fix indentation of > dynamic entries in hierarchy"), by using the evlist in setup_sorting() > without checking if it is NULL, as done in some 'perf test' entries: > > $ find tools/ -name "*.c" | xargs grep 'setup_sorting(NULL);' > tools/perf/tests/hists_output.c: setup_sorting(NULL); > tools/perf/tests/hists_output.c: setup_sorting(NULL); > tools/perf/tests/hists_output.c: setup_sorting(NULL); > tools/perf/tests/hists_output.c: setup_sorting(NULL); > tools/perf/tests/hists_output.c: setup_sorting(NULL); > tools/perf/tests/hists_cumulate.c: setup_sorting(NULL); > tools/perf/tests/hists_cumulate.c: setup_sorting(NULL); > tools/perf/tests/hists_cumulate.c: setup_sorting(NULL); > tools/perf/tests/hists_cumulate.c: setup_sorting(NULL); > $ > > Fix it. > > Before: > > [root@jouet ~]# perf test > > 15: Test matching and linking multiple hists : FAILED! > 16: Try 'import perf' in python, checking link problems : Ok > 17: Test breakpoint overflow signal handler : Ok > 18: Test breakpoint overflow sampling : Ok > 19: Test number of exit event of a simple workload : Ok > 20: Test software clock events have valid period values : Ok > 21: Test object code reading : Ok > 22: Test sample parsing : Ok > 23: Test using a dummy software event to keep tracking : Ok > 24: Test parsing with no sample_id_all bit set : Ok > 25: Test filtering hist entries : FAILED! > 26: Test mmap thread lookup : Ok > 27: Test thread mg sharing : Ok > 28: Test output sorting of hist entries : FAILED! > 29: Test cumulation of child hist entries : FAILED! > > > After the patch the above failed tests complete successfully. Ooops, sorry about that. I'll run 'perf test' more often before sending patches. Acked-by: Namhyung Kim Thanks, Namhyung > > --- > > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c > index 5888bfe9a193..4380a2858802 100644 > --- a/tools/perf/util/sort.c > +++ b/tools/perf/util/sort.c > @@ -2635,25 +2635,14 @@ out: > return ret; > } > > -int setup_sorting(struct perf_evlist *evlist) > +static void evlist__set_hists_nr_sort_keys(struct perf_evlist *evlist) > { > - int err; > - struct hists *hists; > struct perf_evsel *evsel; > - struct perf_hpp_fmt *fmt; > - > - err = __setup_sorting(evlist); > - if (err < 0) > - return err; > - > - if (parent_pattern != default_parent_pattern) { > - err = sort_dimension__add("parent", evlist); > - if (err < 0) > - return err; > - } > > evlist__for_each(evlist, evsel) { > - hists = evsel__hists(evsel); > + struct perf_hpp_fmt *fmt; > + struct hists *hists = evsel__hists(evsel); > + > hists->nr_sort_keys = perf_hpp_list.nr_sort_keys; > > /* > @@ -2667,6 +2656,24 @@ int setup_sorting(struct perf_evlist *evlist) > hists->nr_sort_keys--; > } > } > +} > + > +int setup_sorting(struct perf_evlist *evlist) > +{ > + int err; > + > + err = __setup_sorting(evlist); > + if (err < 0) > + return err; > + > + if (parent_pattern != default_parent_pattern) { > + err = sort_dimension__add("parent", evlist); > + if (err < 0) > + return err; > + } > + > + if (evlist != NULL) > + evlist__set_hists_nr_sort_keys(evlist); > > reset_dimensions(); >