linux-kernel - Re: [PATCH v6 1/4] tools/perf: support parsing parameterized 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141222193045.GA19784@us.ibm.com>
Date:	Mon, 22 Dec 2014 11:30:45 -0800
From:	Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
To:	Jiri Olsa <jolsa@...hat.com>
Cc:	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Michael Ellerman <mpe@...erman.id.au>,
	Paul Mackerras <paulus@...ba.org>, dev@...yps.com,
	peterz@...radead.org, hbabu@...ibm.com,
	linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v6 1/4] tools/perf: support parsing parameterized events

Jiri Olsa [jolsa@...hat.com] wrote:
| On Sun, Dec 21, 2014 at 11:49:24PM -0800, Sukadev Bhattiprolu wrote:
| 
| SNIP
| 
| > +	}
| >  
| >  	switch (format->value) {
| >  	case PERF_PMU_FORMAT_VALUE_CONFIG:
| > @@ -592,11 +629,16 @@ static int pmu_config_term(struct list_head *formats,
| >  	}
| >  
| >  	/*
| > -	 * XXX If we ever decide to go with string values for
| > -	 * non-hardcoded terms, here's the place to translate
| > -	 * them into value.
| > +	 * Either directly use a numeric term, or try to translate string terms
| > +	 * using event parameters.
| >  	 */
| > -	pmu_format_value(format->bits, term->val.num, vp, zero);
| > +	if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
| > +		val = term->val.num;
| > +	else
| > +		if (pmu_resolve_param_term(term, head_terms, &val))
| > +			return -EINVAL;
| > +
| 
| I'm ok with the change logic, but I'm missing here check for the 'term'
| string value to be '?', so we force subst terms to have '?' as value..
| I believe thats what we decided in the previous set discussion, right?

The =? is not a user input, so I did not think of validating that.

perf tool expects kernel/sysfs to show entries like 'core=?'. Are you
saying that we should error out if kernel mistakenly displays 'core=$val'
or 'core=?val' ? 

If a required parameter is missing, we catch that in pmu_resolve_param_term().
If a bogus parameter is specified we catch that above in pmu_config_term().


| 
| I guess the it'd be nice to parse it directly in the bison code like
| below (could be done later), but I'd be ok with simple check on this
| place for now.
| 
| thanks,
| jirka
| 
| 
| ---
| diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
| index 93c4c9fbc922..7e021c64d5cc 100644
| --- a/tools/perf/util/parse-events.y
| +++ b/tools/perf/util/parse-events.y
| @@ -484,6 +484,14 @@ PE_TERM '=' PE_VALUE
|  	$$ = term;
|  }
|  |
| +PE_TERM '=' PE_SUBST
| +{
| +	struct parse_events_term *term;
| +
| +	ABORT_ON(parse_events_term__subst(&term, (int)$1, NULL, NULL));
| +	$$ = term;
| +}
| +|
|  PE_TERM
|  {
|  	struct parse_events_term *term;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/