Commit fa831fbb authored by Ian Rogers's avatar Ian Rogers Committed by Arnaldo Carvalho de Melo
Browse files

perf metric: Move runtime value to the expr context 



The runtime value is needed when recursively parsing metrics, currently
a value of 1 is passed which is incorrect.

Rather than add more arguments to the bison parser, add runtime to the
context.

Fix call sites not to pass a value. The runtime value is defaulted to 0,
which is arbitrary. In some places this replaces a value of 1, which was
also arbitrary.

This shouldn't affect anything other than PPC.

The use of 0 or 1 shouldn't matter as a proper runtime value would be
needed in a case that it did matter.

Signed-off-by: default avatarIan Rogers <irogers@google.com>
Acked-by: default avatarAndi Kleen <ak@linux.intel.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Antonov <alexander.antonov@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andrew Kilroy <andrew.kilroy@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Changbin Du <changbin.du@intel.com>
Cc: Denys Zagorui <dzagorui@cisco.com>
Cc: Fabian Hemmer <copy@copy.sh>
Cc: Felix Fietkau <nbd@nbd.name>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Cc: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Joakim Zhang <qiangqing.zhang@nxp.com>
Cc: John Garry <john.garry@huawei.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Kees Kook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Nicholas Fraser <nfraser@codeweavers.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Paul Clarke <pc@us.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Riccardo Mancini <rickyman7@gmail.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: ShihCheng Tu <mrtoastcheng@gmail.com>
Cc: Song Liu <songliubraving@fb.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sumanth Korikkar <sumanthk@linux.ibm.com>
Cc: Thomas Richter <tmricht@linux.ibm.com>
Cc: Wan Jiabing <wanjiabing@vivo.com>
Cc: Zhen Lei <thunder.leizhen@huawei.com>
Link: https://lore.kernel.org/r/20211015172132.1162559-6-irogers@google.com


Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent 47f572aa

tools/perf/tests/expr.c

+8 −7
Original line number Diff line number Diff line
@@ -56,7 +56,7 @@ static int test(struct expr_parse_ctx *ctx, const char *e, double val2) 
{

	double val;



	if (expr__parse(&val, ctx, e, 1))

	if (expr__parse(&val, ctx, e))

		TEST_ASSERT_VAL("parse test failed", 0);

	TEST_ASSERT_VAL("unexpected value", val == val2);

	return 0;
@@ -104,17 +104,17 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) 
	}



	p = "FOO/0";

	ret = expr__parse(&val, ctx, p, 1);

	ret = expr__parse(&val, ctx, p);

	TEST_ASSERT_VAL("division by zero", ret == -1);



	p = "BAR/";

	ret = expr__parse(&val, ctx, p, 1);

	ret = expr__parse(&val, ctx, p);

	TEST_ASSERT_VAL("missing operand", ret == -1);



	expr__ctx_clear(ctx);

	TEST_ASSERT_VAL("find ids",

			expr__find_ids("FOO + BAR + BAZ + BOZO", "FOO",

					ctx, 1) == 0);

					ctx) == 0);

	TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 3);

	TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "BAR",

						    (void **)&val_ptr));
@@ -124,9 +124,10 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) 
						    (void **)&val_ptr));



	expr__ctx_clear(ctx);

	ctx->runtime = 3;

	TEST_ASSERT_VAL("find ids",

			expr__find_ids("EVENT1\\,param\\=?@ + EVENT2\\,param\\=?@",

					NULL, ctx, 3) == 0);

					NULL, ctx) == 0);

	TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 2);

	TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1,param=3/",

						    (void **)&val_ptr));
@@ -137,7 +138,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) 
	expr__ctx_clear(ctx);

	TEST_ASSERT_VAL("find ids",

			expr__find_ids("EVENT1 if #smt_on else EVENT2",

				NULL, ctx, 0) == 0);

				NULL, ctx) == 0);

	TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);

	TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids,

						  smt_on() ? "EVENT1" : "EVENT2",
@@ -147,7 +148,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) 
	expr__ctx_clear(ctx);

	TEST_ASSERT_VAL("find ids",

			expr__find_ids("1.0 if EVENT1 > 100.0 else 1.0",

			NULL, ctx, 0) == 0);

			NULL, ctx) == 0);

	TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);



	expr__ctx_free(ctx);

tools/perf/tests/pmu-events.c

+5 −5
Original line number Diff line number Diff line
@@ -866,7 +866,7 @@ static int resolve_metric_simple(struct expr_parse_ctx *pctx, 
			ref->metric_expr = pe->metric_expr;

			list_add_tail(&metric->list, compound_list);



			rc = expr__find_ids(pe->metric_expr, NULL, pctx, 0);

			rc = expr__find_ids(pe->metric_expr, NULL, pctx);

			if (rc)

				goto out_err;

			break; /* The hashmap has been modified, so restart */
@@ -916,7 +916,7 @@ static int test_parsing(void) 
			if (!pe->metric_expr)

				continue;

			expr__ctx_clear(ctx);

			if (expr__find_ids(pe->metric_expr, NULL, ctx, 0) < 0) {

			if (expr__find_ids(pe->metric_expr, NULL, ctx) < 0) {

				expr_failure("Parse find ids failed", map, pe);

				ret++;

				continue;
@@ -949,7 +949,7 @@ static int test_parsing(void) 
				free(metric);

			}



			if (expr__parse(&result, ctx, pe->metric_expr, 0)) {

			if (expr__parse(&result, ctx, pe->metric_expr)) {

				expr_failure("Parse failed", map, pe);

				ret++;

			}
@@ -989,7 +989,7 @@ static int metric_parse_fake(const char *str) 
		pr_debug("expr__ctx_new failed");

		return TEST_FAIL;

	}

	if (expr__find_ids(str, NULL, ctx, 0) < 0) {

	if (expr__find_ids(str, NULL, ctx) < 0) {

		pr_err("expr__find_ids failed\n");

		return -1;

	}
@@ -1010,7 +1010,7 @@ static int metric_parse_fake(const char *str) 
		}

	}



	if (expr__parse(&result, ctx, str, 0))

	if (expr__parse(&result, ctx, str))

		pr_err("expr__parse failed\n");

	else

		ret = 0;

tools/perf/util/expr.c

+8 −7
Original line number Diff line number Diff line
@@ -246,7 +246,7 @@ int expr__resolve_id(struct expr_parse_ctx *ctx, const char *id, 
			data->ref.metric_name);

		pr_debug("processing metric: %s ENTRY\n", id);

		data->kind = EXPR_ID_DATA__REF_VALUE;

		if (expr__parse(&data->ref.val, ctx, data->ref.metric_expr, 1)) {

		if (expr__parse(&data->ref.val, ctx, data->ref.metric_expr)) {

			pr_debug("%s failed to count\n", id);

			return -1;

		}
@@ -284,6 +284,7 @@ struct expr_parse_ctx *expr__ctx_new(void) 


	ctx->ids = hashmap__new(key_hash, key_equal, NULL);

	ctx->parent = NULL;

	ctx->runtime = 0;

	return ctx;

}
@@ -314,10 +315,10 @@ void expr__ctx_free(struct expr_parse_ctx *ctx) 


static int

__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,

	      bool compute_ids, int runtime)

	      bool compute_ids)

{

	struct expr_scanner_ctx scanner_ctx = {

		.runtime = runtime,

		.runtime = ctx->runtime,

	};

	YY_BUFFER_STATE buffer;

	void *scanner;
@@ -345,15 +346,15 @@ __expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr, 
}



int expr__parse(double *final_val, struct expr_parse_ctx *ctx,

		const char *expr, int runtime)

		const char *expr)

{

	return __expr__parse(final_val, ctx, expr, /*compute_ids=*/false, runtime) ? -1 : 0;

	return __expr__parse(final_val, ctx, expr, /*compute_ids=*/false) ? -1 : 0;

}



int expr__find_ids(const char *expr, const char *one,

		   struct expr_parse_ctx *ctx, int runtime)

		   struct expr_parse_ctx *ctx)

{

	int ret = __expr__parse(NULL, ctx, expr, /*compute_ids=*/true, runtime);

	int ret = __expr__parse(NULL, ctx, expr, /*compute_ids=*/true);



	if (one)

		expr__del_id(ctx, one);

tools/perf/util/expr.h

+3 −2
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ struct expr_id { 
struct expr_parse_ctx {

	struct hashmap	*ids;

	struct expr_id	*parent;

	int runtime;

};



struct expr_id_data;
@@ -52,10 +53,10 @@ int expr__resolve_id(struct expr_parse_ctx *ctx, const char *id, 
		     struct expr_id_data **datap);



int expr__parse(double *final_val, struct expr_parse_ctx *ctx,

		const char *expr, int runtime);

		const char *expr);



int expr__find_ids(const char *expr, const char *one,

		struct expr_parse_ctx *ids, int runtime);

		   struct expr_parse_ctx *ids);



double expr_id_data__value(const struct expr_id_data *data);

struct expr_id *expr_id_data__parent(struct expr_id_data *data);

tools/perf/util/metricgroup.c

+3 −4
Original line number Diff line number Diff line
@@ -124,7 +124,6 @@ struct metric { 
	const char *metric_unit;

	struct list_head metric_refs;

	int metric_refs_cnt;

	int runtime;

	bool has_constraint;

};
@@ -391,7 +390,7 @@ static int metricgroup__setup_events(struct list_head *groups, 
		expr->metric_name = m->metric_name;

		expr->metric_unit = m->metric_unit;

		expr->metric_events = metric_events;

		expr->runtime = m->runtime;

		expr->runtime = m->pctx->runtime;

		list_add(&expr->nd, &me->head);

	}
@@ -812,7 +811,7 @@ static int __add_metric(struct list_head *metric_list, 
		m->metric_name = pe->metric_name;

		m->metric_expr = pe->metric_expr;

		m->metric_unit = pe->unit;

		m->runtime = runtime;

		m->pctx->runtime = runtime;

		m->has_constraint = metric_no_group || metricgroup__has_constraint(pe);

		INIT_LIST_HEAD(&m->metric_refs);

		m->metric_refs_cnt = 0;
@@ -862,7 +861,7 @@ static int __add_metric(struct list_head *metric_list, 
	 * For both the parent and referenced metrics, we parse

	 * all the metric's IDs and add it to the parent context.

	 */

	if (expr__find_ids(pe->metric_expr, NULL, m->pctx, runtime) < 0) {

	if (expr__find_ids(pe->metric_expr, NULL, m->pctx) < 0) {

		if (m->metric_refs_cnt == 0) {

			expr__ctx_free(m->pctx);

			free(m);