linux-kernel - Re: [PATCH 2/7] perf hist: Introduce hists__link

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: <CAM9d7ciNoDyrqubUQRe_0zSxEEBOS83HOx_QSVg=WZg0w9PW3Q@mail.gmail.com>
Date:   Tue, 20 Sep 2016 10:17:19 +0900
From:   Namhyung Kim <namhyung@...nel.org>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        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 2/7] perf hist: Introduce hists__link_hierarchy()

On Mon, Sep 19, 2016 at 4:59 PM, Jiri Olsa <jolsa@...hat.com> wrote:
> On Tue, Sep 13, 2016 at 04:45:47PM +0900, Namhyung Kim wrote:
>> The hists__link_hierarchy() is to support hierarchy report with event
>> group.  When it matches leader event and other members (with the
>> hists__match_hierarchy), it also needs to link unmatched member entries
>> with a dummy leader event so that it can show up in the output.
>
> SNIP
>
>>
>> +static int hists__link_hierarchy(struct hists *leader_hists,
>> +                              struct hist_entry *parent,
>> +                              struct rb_root *leader_root,
>> +                              struct rb_root *other_root)
>> +{
>> +     struct rb_node *nd;
>> +     struct hist_entry *pos, *leader;
>> +
>> +     for (nd = rb_first(other_root); nd; nd = rb_next(nd)) {
>> +             pos = rb_entry(nd, struct hist_entry, rb_node_in);
>> +
>> +             if (hist_entry__has_pairs(pos)) {
>> +                     bool found = false;
>> +
>> +                     list_for_each_entry(leader, &pos->pairs.head, pairs.node) {
>> +                             if (leader->hists == leader_hists) {
>> +                                     found = true;
>> +                                     break;
>> +                             }
>> +                     }
>> +                     if (!found)
>> +                             return -1;
>> +             } else {
>> +                     leader = add_dummy_hierarchy_entry(leader_hists,
>> +                                                        leader_root, pos);
>
> could we call hists__add_dummy_entry in here? seems like there's only
> difference in function arguments.. we could safe one function ;-)

Maybe, but I don't see much value doing it.  IMHO this hists
match/link thing is already complex.  Combining normal and hierarchy
case in a single function will increase the complexity so I'd like to
keep it separate for hierarchy case.

Thanks,
Namhyung