linux-kernel - Re: [PATCH v2] tools/perf/metricgroup: Fix printing event names of metric group with multiple events incase of overlapping events

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] [day] [month] [year] [list]

Date:   Wed, 29 Jan 2020 09:00:31 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     kajoljain <kjain@...ux.ibm.com>
Cc:     acme@...nel.org, linux-kernel@...r.kernel.org,
        linux-perf-users@...r.kernel.org,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Andi Kleen <ak@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Jin Yao <yao.jin@...ux.intel.com>,
        Madhavan Srinivasan <maddy@...ux.vnet.ibm.com>,
        Anju T Sudhakar <anju@...ux.vnet.ibm.com>,
        Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
Subject: Re: [PATCH v2] tools/perf/metricgroup: Fix printing event names of
 metric group with multiple events incase of overlapping events

On Wed, Jan 29, 2020 at 12:10:22PM +0530, kajoljain wrote:

SNIP

> > -				i = 0;
> > -				memset(metric_events, 0,
> > -				       sizeof(struct evsel *) * idnum);
> > -				continue;
> >   			}
> 
> Incase we have match miss, we need to restart the match comparison again.
> 
> But I think we can't totally remove this check as, we also need to make sure
> we take current event in match logic. Maybe something like this:
> 
>                 } else {
> 
> -                       if (i + 1 == idnum) {
> -                               /* Discard the whole match and start again
> */
> -                               i = 0;
> -                               memset(metric_events, 0,
> -                                      sizeof(struct evsel *) * idnum);
> -                               continue;
> -                       }
> -
> -                       if (!strcmp(ev->name, ids[i]))
> -                               metric_events[i] = ev;
> -                       else {
> -                               /* Discard the whole match and start again
> */
> -                               i = 0;
> -                               memset(metric_events, 0,
> -                                      sizeof(struct evsel *) * idnum);
> -                               continue;
> 
> +                       /* Discard the whole match and start again */
> +                       i = 0;
> +                       memset(metric_events, 0,
> +                               sizeof(struct evsel *) * idnum);
> +
> +                       if (!strcmp(ev->name, ids[i])) {
> +                               if (!metric_events[i])
> +                                       metric_events[i] = ev;
> +                               i++;
> +                               if (i == idnum)
> +                                       break;
>                         }
> 
> Please let me know if it sounds fine.

ah right.. we need to compare the event with the first id, sounds good

thanks,
jirka