* [PATCH v3] perf test: Make metric testing more robust.
@ 2021-12-23 18:56 Ian Rogers
2021-12-24 8:49 ` John Garry
0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2021-12-23 18:56 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
John Garry, Kajol Jain, linux-perf-users, linux-kernel
Cc: eranian, Ian Rogers
When testing metric expressions we fake counter values from 1 going
upward. For some metrics this can yield negative values that are clipped
to zero, and then cause divide by zero failures. Such clipping is
questionable but may be a result of tools automatically generating
metrics. A workaround for this case is to try a second time with counter
values going in the opposite direction.
This case was seen in a metric like:
event1 / max(event2 - event3, 0)
But it may also happen in more sensible metrics like:
event1 / (event2 + event3 - 1 - event4)
v2. Rebase and more detail in commit message.
v3. Is a rebase.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/pmu-events.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index df1c9a3cc05b..b2ddf928d32a 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -962,8 +962,18 @@ static int test__parsing(struct test_suite *test __maybe_unused,
}
if (expr__parse(&result, ctx, pe->metric_expr)) {
- expr_failure("Parse failed", map, pe);
- ret++;
+ /*
+ * Parsing failed, make numbers go from large to
+ * small which can resolve divide by zero
+ * issues.
+ */
+ k = 1024;
+ hashmap__for_each_entry(ctx->ids, cur, bkt)
+ expr__add_id_val(ctx, strdup(cur->key), k--);
+ if (expr__parse(&result, ctx, pe->metric_expr)) {
+ expr_failure("Parse failed", map, pe);
+ ret++;
+ }
}
}
}
@@ -1022,10 +1032,20 @@ static int metric_parse_fake(const char *str)
}
}
- if (expr__parse(&result, ctx, str))
- pr_err("expr__parse failed\n");
- else
- ret = 0;
+ ret = 0;
+ if (expr__parse(&result, ctx, str)) {
+ /*
+ * Parsing failed, make numbers go from large to small which can
+ * resolve divide by zero issues.
+ */
+ i = 1024;
+ hashmap__for_each_entry(ctx->ids, cur, bkt)
+ expr__add_id_val(ctx, strdup(cur->key), i--);
+ if (expr__parse(&result, ctx, str)) {
+ pr_err("expr__parse failed\n");
+ ret = -1;
+ }
+ }
out:
expr__ctx_free(ctx);
--
2.34.1.307.g9b7440fafd-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] perf test: Make metric testing more robust.
2021-12-23 18:56 [PATCH v3] perf test: Make metric testing more robust Ian Rogers
@ 2021-12-24 8:49 ` John Garry
2022-02-15 15:10 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 3+ messages in thread
From: John Garry @ 2021-12-24 8:49 UTC (permalink / raw)
To: Ian Rogers, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Kajol Jain, linux-perf-users,
linux-kernel
Cc: eranian
On 23/12/2021 18:56, Ian Rogers wrote:
> When testing metric expressions we fake counter values from 1 going
> upward. For some metrics this can yield negative values that are clipped
> to zero, and then cause divide by zero failures. Such clipping is
> questionable but may be a result of tools automatically generating
> metrics. A workaround for this case is to try a second time with counter
> values going in the opposite direction.
>
> This case was seen in a metric like:
> event1 / max(event2 - event3, 0)
> But it may also happen in more sensible metrics like:
> event1 / (event2 + event3 - 1 - event4)
>
> v2. Rebase and more detail in commit message.
> v3. Is a rebase.
Incorrect location for this info
>
> Signed-off-by: Ian Rogers <irogers@google.com>
Reviewed-by: John Garry <john.garry@huawei.com>
> ---
> tools/perf/tests/pmu-events.c | 32 ++++++++++++++++++++++++++------
> 1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> index df1c9a3cc05b..b2ddf928d32a 100644
> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c
> @@ -962,8 +962,18 @@ static int test__parsing(struct test_suite *test __maybe_unused,
> }
>
> if (expr__parse(&result, ctx, pe->metric_expr)) {
> - expr_failure("Parse failed", map, pe);
> - ret++;
> + /*
> + * Parsing failed, make numbers go from large to
> + * small which can resolve divide by zero
can or may?
> + * issues.
> + */
> + k = 1024;
> + hashmap__for_each_entry(ctx->ids, cur, bkt)
> + expr__add_id_val(ctx, strdup(cur->key), k--);
> + if (expr__parse(&result, ctx, pe->metric_expr)) {
> + expr_failure("Parse failed", map, pe);
> + ret++;
> + }
> }
> }
> }
> @@ -1022,10 +1032,20 @@ static int metric_parse_fake(const char *str)
> }
> }
>
> - if (expr__parse(&result, ctx, str))
> - pr_err("expr__parse failed\n");
> - else
> - ret = 0;
> + ret = 0;
> + if (expr__parse(&result, ctx, str)) {
> + /*
> + * Parsing failed, make numbers go from large to small which can
> + * resolve divide by zero issues.
> + */
> + i = 1024;
> + hashmap__for_each_entry(ctx->ids, cur, bkt)
> + expr__add_id_val(ctx, strdup(cur->key), i--);
> + if (expr__parse(&result, ctx, str)) {
> + pr_err("expr__parse failed\n");
> + ret = -1;
> + }
> + }
>
> out:
> expr__ctx_free(ctx);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] perf test: Make metric testing more robust.
2021-12-24 8:49 ` John Garry
@ 2022-02-15 15:10 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-15 15:10 UTC (permalink / raw)
To: John Garry
Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, Kajol Jain,
linux-perf-users, linux-kernel, eranian
Em Fri, Dec 24, 2021 at 08:49:35AM +0000, John Garry escreveu:
> On 23/12/2021 18:56, Ian Rogers wrote:
> > When testing metric expressions we fake counter values from 1 going
> > upward. For some metrics this can yield negative values that are clipped
> > to zero, and then cause divide by zero failures. Such clipping is
> > questionable but may be a result of tools automatically generating
> > metrics. A workaround for this case is to try a second time with counter
> > values going in the opposite direction.
> >
> > This case was seen in a metric like:
> > event1 / max(event2 - event3, 0)
> > But it may also happen in more sensible metrics like:
> > event1 / (event2 + event3 - 1 - event4)
> >
> > v2. Rebase and more detail in commit message.
> > v3. Is a rebase.
>
> Incorrect location for this info
>
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> Reviewed-by: John Garry <john.garry@huawei.com>
Found thru the cracks, applying.
- Arnaldo
> > ---
> > tools/perf/tests/pmu-events.c | 32 ++++++++++++++++++++++++++------
> > 1 file changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> > index df1c9a3cc05b..b2ddf928d32a 100644
> > --- a/tools/perf/tests/pmu-events.c
> > +++ b/tools/perf/tests/pmu-events.c
> > @@ -962,8 +962,18 @@ static int test__parsing(struct test_suite *test __maybe_unused,
> > }
> > if (expr__parse(&result, ctx, pe->metric_expr)) {
> > - expr_failure("Parse failed", map, pe);
> > - ret++;
> > + /*
> > + * Parsing failed, make numbers go from large to
> > + * small which can resolve divide by zero
>
> can or may?
>
> > + * issues.
> > + */
> > + k = 1024;
> > + hashmap__for_each_entry(ctx->ids, cur, bkt)
> > + expr__add_id_val(ctx, strdup(cur->key), k--);
> > + if (expr__parse(&result, ctx, pe->metric_expr)) {
> > + expr_failure("Parse failed", map, pe);
> > + ret++;
> > + }
> > }
> > }
> > }
> > @@ -1022,10 +1032,20 @@ static int metric_parse_fake(const char *str)
> > }
> > }
> > - if (expr__parse(&result, ctx, str))
> > - pr_err("expr__parse failed\n");
> > - else
> > - ret = 0;
> > + ret = 0;
> > + if (expr__parse(&result, ctx, str)) {
> > + /*
> > + * Parsing failed, make numbers go from large to small which can
> > + * resolve divide by zero issues.
> > + */
> > + i = 1024;
> > + hashmap__for_each_entry(ctx->ids, cur, bkt)
> > + expr__add_id_val(ctx, strdup(cur->key), i--);
> > + if (expr__parse(&result, ctx, str)) {
> > + pr_err("expr__parse failed\n");
> > + ret = -1;
> > + }
> > + }
> > out:
> > expr__ctx_free(ctx);
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-15 15:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-23 18:56 [PATCH v3] perf test: Make metric testing more robust Ian Rogers
2021-12-24 8:49 ` John Garry
2022-02-15 15:10 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.