From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752735AbcIMAJg (ORCPT ); Mon, 12 Sep 2016 20:09:36 -0400 Received: from LGEAMRELO13.lge.com ([156.147.23.53]:36152 "EHLO lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750839AbcIMAJf (ORCPT ); Mon, 12 Sep 2016 20:09:35 -0400 X-Original-SENDERIP: 156.147.1.126 X-Original-MAILFROM: namhyung@kernel.org X-Original-SENDERIP: 10.177.227.17 X-Original-MAILFROM: namhyung@kernel.org Date: Tue, 13 Sep 2016 09:09:32 +0900 From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , Andi Kleen Subject: Re: [PATCH 4/7] perf hist: Initialize hierachy tree explicitly Message-ID: <20160913000932.GB3641@sejong> References: <20160912061958.16656-1-namhyung@kernel.org> <20160912061958.16656-5-namhyung@kernel.org> <20160912142228.GA4897@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160912142228.GA4897@kernel.org> User-Agent: Mutt/1.7.0 (2016-08-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Sep 12, 2016 at 11:22:28AM -0300, Arnaldo Carvalho de Melo wrote: > Em Mon, Sep 12, 2016 at 03:19:55PM +0900, Namhyung Kim escreveu: > > The hroot_in and hroot_out are root of hiearchy tree of hist entry. But > > as hist entry is initialized by copying existing template entry, it > > Which makes default_ops->new() pointing to a zalloc based constructor > totally useless, right? I.e. we malloc() + bzero() to then imediatelly > memcpy from the template :-) Right. > > > sometimes has non-empty tree and copied it incorrectly. This is a > > problem especially when event group is used since it creates dummy > > entries from already-processed entries in other event members. > > I guess this needs a "Fixes:" probably for the changeset introducing > those members :-\ This problem can occur only if event group is used with hierarchy which will be enabled by this patchset. In the normal hierarchy path, hist entries are created before processing and have empty hierarchy tree. So current code has no problem. Thanks, Namhyung > > - Arnaldo > > > Signed-off-by: Namhyung Kim > > --- > > tools/perf/util/hist.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > > index 702ba3a8ead6..37a08f20730a 100644 > > --- a/tools/perf/util/hist.c > > +++ b/tools/perf/util/hist.c > > @@ -417,6 +417,8 @@ static int hist_entry__init(struct hist_entry *he, > > } > > INIT_LIST_HEAD(&he->pairs.node); > > thread__get(he->thread); > > + he->hroot_in = RB_ROOT; > > + he->hroot_out = RB_ROOT; > > > > if (!symbol_conf.report_hierarchy) > > he->leaf = true; > > -- > > 2.9.3