linux-kernel - Re: [PATCH 4/7] perf hist: Initialize hierachy tree explicitly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160913000932.GB3641@sejong>
Date:   Tue, 13 Sep 2016 09:09:32 +0900
From:   Namhyung Kim <namhyung@...nel.org>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Jiri Olsa <jolsa@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH 4/7] perf hist: Initialize hierachy tree explicitly

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 <namhyung@...nel.org>
> > ---
> >  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