linux-kernel - Re: [RFC PATCH v2 10/13] perf metricgroup: Split up metricgroup__add

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: <b793b75e-0c55-a986-0a5e-a7aac269cae5@huawei.com>
Date:   Wed, 22 Apr 2020 13:00:22 +0100
From:   John Garry <john.garry@...wei.com>
To:     Jiri Olsa <jolsa@...hat.com>
CC:     <peterz@...radead.org>, <mingo@...hat.com>, <acme@...nel.org>,
        <mark.rutland@....com>, <alexander.shishkin@...ux.intel.com>,
        <namhyung@...nel.org>, <will@...nel.org>, <ak@...ux.intel.com>,
        <linuxarm@...wei.com>, <linux-kernel@...r.kernel.org>,
        <qiangqing.zhang@....com>, <irogers@...gle.com>,
        <robin.murphy@....com>, <zhangshaokun@...ilicon.com>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH v2 10/13] perf metricgroup: Split up
 metricgroup__add_metric()

On 22/04/2020 12:44, Jiri Olsa wrote:
>>   static int metricgroup__add_metric(const char *metric, struct strbuf *events,
>>   				   struct list_head *group_list)
>>   {
>> @@ -502,37 +542,12 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
>>   			break;
>>   		if (!pe->metric_expr)
>>   			continue;
>> -		if (match_metric(pe->metric_group, metric) ||
>> -		    match_metric(pe->metric_name, metric)) {
>> -			const char **ids;
>> -			int idnum;
>> -			struct egroup *eg;
>> -
>> -			pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
>>   
>> -			if (expr__find_other(pe->metric_expr,
>> -					     NULL, &ids, &idnum) < 0)
>> -				continue;
>> -			if (events->len > 0)
>> -				strbuf_addf(events, ",");
>> -
>> -			if (metricgroup__has_constraint(pe))
>> -				metricgroup__add_metric_non_group(events, ids, idnum);
>> -			else
>> -				metricgroup__add_metric_weak_group(events, ids, idnum);
>> -
>> -			eg = malloc(sizeof(struct egroup));
>> -			if (!eg) {
>> -				ret = -ENOMEM;
>> -				break;
>> -			}
>> -			eg->ids = ids;
>> -			eg->idnum = idnum;
>> -			eg->metric_name = pe->metric_name;
>> -			eg->metric_expr = pe->metric_expr;
>> -			eg->metric_unit = pe->unit;
>> -			list_add_tail(&eg->nd, group_list);
>> -			ret = 0;
> also this place got changed just recently a lot,
> so you might want to rebase to the Arnaldo's latest perf/core

Hi jirka,

Yeah, I saw that. I can check.

TBH, apart from that, I would be welcome to opinion on this latter patch 
of the series, concerned with metrics. I just split (butcher) the 
function and call common parts from 2x places now. Maybe there's a more 
fluid way to do this.

Cheers,
John