* [PATCH v5 0/5] perf test: Add a runs-per-test option
@ 2025-01-10 4:57 Ian Rogers
2025-01-10 4:57 ` [PATCH v5 1/5] perf test: Rename functions and variables for better clarity Ian Rogers
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: Ian Rogers @ 2025-01-10 4:57 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark,
linux-perf-users, linux-kernel
Add a runs-per-test option to perf test inorder to better diagnose
flakes. Each runs executes as if it were a different test, the tests
appear together in the output:
```
$ perf test -r 3 lbr -v
122: perf record LBR tests : Ok
122: perf record LBR tests : Ok
122: perf record LBR tests : Ok
```
v5: Fix the patch that updates the documentation for the sequential
option, as spotted by Namhyung.
v4: Fix crash in dont_fork mode caused by cleanup work.
v3: Improve perf test documentation, including for the new option, and
add some example output to the commit messages as requested by
Namhyung.
v2: In the v1 patch Kan and I noted cleanup that would make the code
cleaner and less indented:
https://lore.kernel.org/lkml/20241109160219.49976-1-irogers@google.com/
Add patches doing some builtin-test.c cleanup and then add the
runs-per-test flag.
Ian Rogers (5):
perf test: Rename functions and variables for better clarity
perf test: Send list output to stdout rather than stderr
perf test: Fix parallel/sequential option documentation
perf test: Add a runs-per-test flag
perf test: Improve verbose documentation
tools/perf/Documentation/perf-test.txt | 20 ++-
tools/perf/tests/builtin-test.c | 223 ++++++++++++-------------
2 files changed, 119 insertions(+), 124 deletions(-)
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/5] perf test: Rename functions and variables for better clarity
2025-01-10 4:57 [PATCH v5 0/5] perf test: Add a runs-per-test option Ian Rogers
@ 2025-01-10 4:57 ` Ian Rogers
2025-01-10 4:57 ` [PATCH v5 2/5] perf test: Send list output to stdout rather than stderr Ian Rogers
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Ian Rogers @ 2025-01-10 4:57 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark,
linux-perf-users, linux-kernel
The relationship between subtests and test cases is somewhat
confusing, so let's do away with the notion of sub-tests and switch to
just working with some number of test cases. Add a
test_suite__for_each_test_case as in many cases, except the special
one test case situation, the iteration can just be on all test
cases. Switch variable names to be more intention revealing of what
their value is.
This work was motivated by discussion with Kan where it was noted the
code is becoming overly indented:
https://lore.kernel.org/lkml/20241109160219.49976-1-irogers@google.com/
Unifying more of the sub-test/no-sub-tests avoids one level of
indentation in a number of places.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/builtin-test.c | 201 +++++++++++++++-----------------
1 file changed, 93 insertions(+), 108 deletions(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index a5b9ccd0033a..daf52c83b0f9 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -149,58 +149,51 @@ static struct test_workload *workloads[] = {
#define workloads__for_each(workload) \
for (unsigned i = 0; i < ARRAY_SIZE(workloads) && ({ workload = workloads[i]; 1; }); i++)
-static int num_subtests(const struct test_suite *t)
+#define test_suite__for_each_test_case(suite, idx) \
+ for (idx = 0; (suite)->test_cases && (suite)->test_cases[idx].name != NULL; idx++)
+
+static int test_suite__num_test_cases(const struct test_suite *t)
{
int num;
- if (!t->test_cases)
- return 0;
-
- num = 0;
- while (t->test_cases[num].name)
- num++;
+ test_suite__for_each_test_case(t, num);
return num;
}
-static bool has_subtests(const struct test_suite *t)
-{
- return num_subtests(t) > 1;
-}
-
-static const char *skip_reason(const struct test_suite *t, int subtest)
+static const char *skip_reason(const struct test_suite *t, int test_case)
{
if (!t->test_cases)
return NULL;
- return t->test_cases[subtest >= 0 ? subtest : 0].skip_reason;
+ return t->test_cases[test_case >= 0 ? test_case : 0].skip_reason;
}
-static const char *test_description(const struct test_suite *t, int subtest)
+static const char *test_description(const struct test_suite *t, int test_case)
{
- if (t->test_cases && subtest >= 0)
- return t->test_cases[subtest].desc;
+ if (t->test_cases && test_case >= 0)
+ return t->test_cases[test_case].desc;
return t->desc;
}
-static test_fnptr test_function(const struct test_suite *t, int subtest)
+static test_fnptr test_function(const struct test_suite *t, int test_case)
{
- if (subtest <= 0)
+ if (test_case <= 0)
return t->test_cases[0].run_case;
- return t->test_cases[subtest].run_case;
+ return t->test_cases[test_case].run_case;
}
-static bool test_exclusive(const struct test_suite *t, int subtest)
+static bool test_exclusive(const struct test_suite *t, int test_case)
{
- if (subtest <= 0)
+ if (test_case <= 0)
return t->test_cases[0].exclusive;
- return t->test_cases[subtest].exclusive;
+ return t->test_cases[test_case].exclusive;
}
-static bool perf_test__matches(const char *desc, int curr, int argc, const char *argv[])
+static bool perf_test__matches(const char *desc, int suite_num, int argc, const char *argv[])
{
int i;
@@ -212,7 +205,7 @@ static bool perf_test__matches(const char *desc, int curr, int argc, const char
long nr = strtoul(argv[i], &end, 10);
if (*end == '\0') {
- if (nr == curr + 1)
+ if (nr == suite_num + 1)
return true;
continue;
}
@@ -227,8 +220,8 @@ static bool perf_test__matches(const char *desc, int curr, int argc, const char
struct child_test {
struct child_process process;
struct test_suite *test;
- int test_num;
- int subtest;
+ int suite_num;
+ int test_case_num;
};
static jmp_buf run_test_jmp_buf;
@@ -258,7 +251,7 @@ static int run_test_child(struct child_process *process)
pr_debug("--- start ---\n");
pr_debug("test child forked, pid %d\n", getpid());
- err = test_function(child->test, child->subtest)(child->test, child->subtest);
+ err = test_function(child->test, child->test_case_num)(child->test, child->test_case_num);
pr_debug("---- end(%d) ----\n", err);
err_out:
@@ -270,15 +263,16 @@ static int run_test_child(struct child_process *process)
#define TEST_RUNNING -3
-static int print_test_result(struct test_suite *t, int i, int subtest, int result, int width,
- int running)
+static int print_test_result(struct test_suite *t, int curr_suite, int curr_test_case,
+ int result, int width, int running)
{
- if (has_subtests(t)) {
+ if (test_suite__num_test_cases(t) > 1) {
int subw = width > 2 ? width - 2 : width;
- pr_info("%3d.%1d: %-*s:", i + 1, subtest + 1, subw, test_description(t, subtest));
+ pr_info("%3d.%1d: %-*s:", curr_suite + 1, curr_test_case + 1, subw,
+ test_description(t, curr_test_case));
} else
- pr_info("%3d: %-*s:", i + 1, width, test_description(t, subtest));
+ pr_info("%3d: %-*s:", curr_suite + 1, width, test_description(t, curr_test_case));
switch (result) {
case TEST_RUNNING:
@@ -288,7 +282,7 @@ static int print_test_result(struct test_suite *t, int i, int subtest, int resul
pr_info(" Ok\n");
break;
case TEST_SKIP: {
- const char *reason = skip_reason(t, subtest);
+ const char *reason = skip_reason(t, curr_test_case);
if (reason)
color_fprintf(stderr, PERF_COLOR_YELLOW, " Skip (%s)\n", reason);
@@ -310,7 +304,7 @@ static void finish_test(struct child_test **child_tests, int running_test, int c
{
struct child_test *child_test = child_tests[running_test];
struct test_suite *t;
- int i, subi, err;
+ int curr_suite, curr_test_case, err;
bool err_done = false;
struct strbuf err_output = STRBUF_INIT;
int last_running = -1;
@@ -321,15 +315,15 @@ static void finish_test(struct child_test **child_tests, int running_test, int c
return;
}
t = child_test->test;
- i = child_test->test_num;
- subi = child_test->subtest;
+ curr_suite = child_test->suite_num;
+ curr_test_case = child_test->test_case_num;
err = child_test->process.err;
/*
* For test suites with subtests, display the suite name ahead of the
* sub test names.
*/
- if (has_subtests(t) && subi == 0)
- pr_info("%3d: %-*s:\n", i + 1, width, test_description(t, -1));
+ if (test_suite__num_test_cases(t) > 1 && curr_test_case == 0)
+ pr_info("%3d: %-*s:\n", curr_suite + 1, width, test_description(t, -1));
/*
* Busy loop reading from the child's stdout/stderr that are set to be
@@ -338,10 +332,11 @@ static void finish_test(struct child_test **child_tests, int running_test, int c
if (err > 0)
fcntl(err, F_SETFL, O_NONBLOCK);
if (verbose > 1) {
- if (has_subtests(t))
- pr_info("%3d.%1d: %s:\n", i + 1, subi + 1, test_description(t, subi));
+ if (test_suite__num_test_cases(t) > 1)
+ pr_info("%3d.%1d: %s:\n", curr_suite + 1, curr_test_case + 1,
+ test_description(t, curr_test_case));
else
- pr_info("%3d: %s:\n", i + 1, test_description(t, -1));
+ pr_info("%3d: %s:\n", curr_suite + 1, test_description(t, -1));
}
while (!err_done) {
struct pollfd pfds[1] = {
@@ -366,7 +361,8 @@ static void finish_test(struct child_test **child_tests, int running_test, int c
*/
fprintf(debug_file(), PERF_COLOR_DELETE_LINE);
}
- print_test_result(t, i, subi, TEST_RUNNING, width, running);
+ print_test_result(t, curr_suite, curr_test_case, TEST_RUNNING,
+ width, running);
last_running = running;
}
}
@@ -404,14 +400,14 @@ static void finish_test(struct child_test **child_tests, int running_test, int c
fprintf(stderr, "%s", err_output.buf);
strbuf_release(&err_output);
- print_test_result(t, i, subi, ret, width, /*running=*/0);
+ print_test_result(t, curr_suite, curr_test_case, ret, width, /*running=*/0);
if (err > 0)
close(err);
zfree(&child_tests[running_test]);
}
-static int start_test(struct test_suite *test, int i, int subi, struct child_test **child,
- int width, int pass)
+static int start_test(struct test_suite *test, int curr_suite, int curr_test_case,
+ struct child_test **child, int width, int pass)
{
int err;
@@ -419,17 +415,18 @@ static int start_test(struct test_suite *test, int i, int subi, struct child_tes
if (dont_fork) {
if (pass == 1) {
pr_debug("--- start ---\n");
- err = test_function(test, subi)(test, subi);
+ err = test_function(test, curr_test_case)(test, curr_test_case);
pr_debug("---- end ----\n");
- print_test_result(test, i, subi, err, width, /*running=*/0);
+ print_test_result(test, curr_suite, curr_test_case, err, width,
+ /*running=*/0);
}
return 0;
}
- if (pass == 1 && !sequential && test_exclusive(test, subi)) {
+ if (pass == 1 && !sequential && test_exclusive(test, curr_test_case)) {
/* When parallel, skip exclusive tests on the first pass. */
return 0;
}
- if (pass != 1 && (sequential || !test_exclusive(test, subi))) {
+ if (pass != 1 && (sequential || !test_exclusive(test, curr_test_case))) {
/* Sequential and non-exclusive tests were run on the first pass. */
return 0;
}
@@ -438,8 +435,8 @@ static int start_test(struct test_suite *test, int i, int subi, struct child_tes
return -ENOMEM;
(*child)->test = test;
- (*child)->test_num = i;
- (*child)->subtest = subi;
+ (*child)->suite_num = curr_suite;
+ (*child)->test_case_num = curr_test_case;
(*child)->process.pid = -1;
(*child)->process.no_stdin = 1;
if (verbose <= 0) {
@@ -479,19 +476,15 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
int err = 0;
for (struct test_suite **t = suites; *t; t++) {
- int len = strlen(test_description(*t, -1));
+ int i, len = strlen(test_description(*t, -1));
if (width < len)
width = len;
- if (has_subtests(*t)) {
- for (int subi = 0, subn = num_subtests(*t); subi < subn; subi++) {
- len = strlen(test_description(*t, subi));
- if (width < len)
- width = len;
- num_tests++;
- }
- } else {
+ test_suite__for_each_test_case(*t, i) {
+ len = strlen(test_description(*t, i));
+ if (width < len)
+ width = len;
num_tests++;
}
}
@@ -510,7 +503,7 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
continue;
pr_debug3("Killing %d pid %d\n",
- child_test->test_num + 1,
+ child_test->suite_num + 1,
child_test->process.pid);
kill(child_test->process.pid, err);
}
@@ -526,47 +519,43 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
*/
for (int pass = 1; pass <= 2; pass++) {
int child_test_num = 0;
- int i = 0;
+ int curr_suite = 0;
- for (struct test_suite **t = suites; *t; t++) {
- int curr = i++;
+ for (struct test_suite **t = suites; *t; t++, curr_suite++) {
+ int curr_test_case;
- if (!perf_test__matches(test_description(*t, -1), curr, argc, argv)) {
+ if (!perf_test__matches(test_description(*t, -1), curr_suite, argc, argv)) {
/*
* Test suite shouldn't be run based on
- * description. See if subtest should.
+ * description. See if any test case should.
*/
bool skip = true;
- for (int subi = 0, subn = num_subtests(*t); subi < subn; subi++) {
- if (perf_test__matches(test_description(*t, subi),
- curr, argc, argv))
+ test_suite__for_each_test_case(*t, curr_test_case) {
+ if (perf_test__matches(test_description(*t, curr_test_case),
+ curr_suite, argc, argv)) {
skip = false;
+ break;
+ }
}
-
if (skip)
continue;
}
- if (intlist__find(skiplist, i)) {
- pr_info("%3d: %-*s:", curr + 1, width, test_description(*t, -1));
+ if (intlist__find(skiplist, curr_suite + 1)) {
+ pr_info("%3d: %-*s:", curr_suite + 1, width,
+ test_description(*t, -1));
color_fprintf(stderr, PERF_COLOR_YELLOW, " Skip (user override)\n");
continue;
}
- if (!has_subtests(*t)) {
- err = start_test(*t, curr, -1, &child_tests[child_test_num++],
- width, pass);
- if (err)
- goto err_out;
- continue;
- }
- for (int subi = 0, subn = num_subtests(*t); subi < subn; subi++) {
- if (!perf_test__matches(test_description(*t, subi),
- curr, argc, argv))
+ test_suite__for_each_test_case(*t, curr_test_case) {
+ if (!perf_test__matches(test_description(*t, curr_test_case),
+ curr_suite, argc, argv))
continue;
- err = start_test(*t, curr, subi, &child_tests[child_test_num++],
+ err = start_test(*t, curr_suite, curr_test_case,
+ &child_tests[child_test_num++],
width, pass);
if (err)
goto err_out;
@@ -592,23 +581,22 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
static int perf_test__list(struct test_suite **suites, int argc, const char **argv)
{
- int i = 0;
+ int curr_suite = 0;
- for (struct test_suite **t = suites; *t; t++) {
- int curr = i++;
+ for (struct test_suite **t = suites; *t; t++, curr_suite++) {
+ int curr_test_case;
- if (!perf_test__matches(test_description(*t, -1), curr, argc, argv))
+ if (!perf_test__matches(test_description(*t, -1), curr_suite, argc, argv))
continue;
- pr_info("%3d: %s\n", i, test_description(*t, -1));
+ pr_info("%3d: %s\n", curr_suite + 1, test_description(*t, -1));
- if (has_subtests(*t)) {
- int subn = num_subtests(*t);
- int subi;
+ if (test_suite__num_test_cases(*t) <= 1)
+ continue;
- for (subi = 0; subi < subn; subi++)
- pr_info("%3d:%1d: %s\n", i, subi + 1,
- test_description(*t, subi));
+ test_suite__for_each_test_case(*t, curr_test_case) {
+ pr_info("%3d:%1d: %s\n", curr_suite + 1, curr_test_case + 1,
+ test_description(*t, curr_test_case));
}
}
return 0;
@@ -665,27 +653,24 @@ static struct test_suite **build_suites(void)
if (suites[2] == NULL)
suites[2] = create_script_test_suites();
-#define for_each_test(t) \
+#define for_each_suite(suite) \
for (size_t i = 0, j = 0; i < ARRAY_SIZE(suites); i++, j = 0) \
- while ((t = suites[i][j++]) != NULL)
+ while ((suite = suites[i][j++]) != NULL)
- for_each_test(t)
+ for_each_suite(t)
num_suites++;
result = calloc(num_suites + 1, sizeof(struct test_suite *));
for (int pass = 1; pass <= 2; pass++) {
- for_each_test(t) {
+ for_each_suite(t) {
bool exclusive = false;
+ int curr_test_case;
- if (!has_subtests(t)) {
- exclusive = test_exclusive(t, -1);
- } else {
- for (int subi = 0, subn = num_subtests(t); subi < subn; subi++) {
- if (test_exclusive(t, subi)) {
- exclusive = true;
- break;
- }
+ test_suite__for_each_test_case(t, curr_test_case) {
+ if (test_exclusive(t, curr_test_case)) {
+ exclusive = true;
+ break;
}
}
if ((!exclusive && pass == 1) || (exclusive && pass == 2))
@@ -693,7 +678,7 @@ static struct test_suite **build_suites(void)
}
}
return result;
-#undef for_each_test
+#undef for_each_suite
}
int cmd_test(int argc, const char **argv)
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 2/5] perf test: Send list output to stdout rather than stderr
2025-01-10 4:57 [PATCH v5 0/5] perf test: Add a runs-per-test option Ian Rogers
2025-01-10 4:57 ` [PATCH v5 1/5] perf test: Rename functions and variables for better clarity Ian Rogers
@ 2025-01-10 4:57 ` Ian Rogers
2025-01-10 4:57 ` [PATCH v5 3/5] perf test: Fix parallel/sequential option documentation Ian Rogers
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Ian Rogers @ 2025-01-10 4:57 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark,
linux-perf-users, linux-kernel
Follow the workload listing in using stdout rather than
stderr. Correct the numbering of sub-tests to be 1.1 rather than 1:1.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/builtin-test.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index daf52c83b0f9..c6071c4db741 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -579,7 +579,7 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
return err;
}
-static int perf_test__list(struct test_suite **suites, int argc, const char **argv)
+static int perf_test__list(FILE *fp, struct test_suite **suites, int argc, const char **argv)
{
int curr_suite = 0;
@@ -589,13 +589,13 @@ static int perf_test__list(struct test_suite **suites, int argc, const char **ar
if (!perf_test__matches(test_description(*t, -1), curr_suite, argc, argv))
continue;
- pr_info("%3d: %s\n", curr_suite + 1, test_description(*t, -1));
+ fprintf(fp, "%3d: %s\n", curr_suite + 1, test_description(*t, -1));
if (test_suite__num_test_cases(*t) <= 1)
continue;
test_suite__for_each_test_case(*t, curr_test_case) {
- pr_info("%3d:%1d: %s\n", curr_suite + 1, curr_test_case + 1,
+ fprintf(fp, "%3d.%1d: %s\n", curr_suite + 1, curr_test_case + 1,
test_description(*t, curr_test_case));
}
}
@@ -721,7 +721,7 @@ int cmd_test(int argc, const char **argv)
argc = parse_options_subcommand(argc, argv, test_options, test_subcommands, test_usage, 0);
if (argc >= 1 && !strcmp(argv[0], "list")) {
suites = build_suites();
- ret = perf_test__list(suites, argc - 1, argv + 1);
+ ret = perf_test__list(stdout, suites, argc - 1, argv + 1);
free(suites);
return ret;
}
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 3/5] perf test: Fix parallel/sequential option documentation
2025-01-10 4:57 [PATCH v5 0/5] perf test: Add a runs-per-test option Ian Rogers
2025-01-10 4:57 ` [PATCH v5 1/5] perf test: Rename functions and variables for better clarity Ian Rogers
2025-01-10 4:57 ` [PATCH v5 2/5] perf test: Send list output to stdout rather than stderr Ian Rogers
@ 2025-01-10 4:57 ` Ian Rogers
2025-01-10 4:57 ` [PATCH v5 4/5] perf test: Add a runs-per-test flag Ian Rogers
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Ian Rogers @ 2025-01-10 4:57 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark,
linux-perf-users, linux-kernel
The parallel option was removed in commit 94d1a913bdc4 ("perf test:
Make parallel testing the default"). Update the sequential
documentation to reflect it isn't the default except for "exclusive"
tests.
Fixes: 94d1a913bdc4 ("perf test: Make parallel testing the default")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/Documentation/perf-test.txt | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/tools/perf/Documentation/perf-test.txt b/tools/perf/Documentation/perf-test.txt
index efcdec528a8f..2e40869b64de 100644
--- a/tools/perf/Documentation/perf-test.txt
+++ b/tools/perf/Documentation/perf-test.txt
@@ -33,13 +33,9 @@ OPTIONS
-S::
--sequential::
- Run tests one after the other, this is the default mode.
-
--p::
---parallel::
- Run tests in parallel, speeds up the whole process but is not safe with
- the current infrastructure, where some tests that compete for some resources,
- for instance, 'perf probe' tests that add/remove probes or clean all probes, etc.
+ Run all tests one after the other. By default "exclusive"
+ tests are run sequentially, but other tests are run in
+ parallel to speed execution.
-F::
--dont-fork::
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 4/5] perf test: Add a runs-per-test flag
2025-01-10 4:57 [PATCH v5 0/5] perf test: Add a runs-per-test option Ian Rogers
` (2 preceding siblings ...)
2025-01-10 4:57 ` [PATCH v5 3/5] perf test: Fix parallel/sequential option documentation Ian Rogers
@ 2025-01-10 4:57 ` Ian Rogers
2025-01-10 4:57 ` [PATCH v5 5/5] perf test: Improve verbose documentation Ian Rogers
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Ian Rogers @ 2025-01-10 4:57 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark,
linux-perf-users, linux-kernel
To detect flakes it is useful to run tests more than once. Add a
runs-per-test flag that will run each test multiple times. Example
output:
```
$ perf test -r 3 lbr -v
122: perf record LBR tests : Ok
122: perf record LBR tests : Ok
122: perf record LBR tests : Ok
```
Update the documentation for the runs-per-test option.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/Documentation/perf-test.txt | 5 +++++
tools/perf/tests/builtin-test.c | 28 ++++++++++++++++----------
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/tools/perf/Documentation/perf-test.txt b/tools/perf/Documentation/perf-test.txt
index 2e40869b64de..85f868c324ff 100644
--- a/tools/perf/Documentation/perf-test.txt
+++ b/tools/perf/Documentation/perf-test.txt
@@ -37,6 +37,11 @@ OPTIONS
tests are run sequentially, but other tests are run in
parallel to speed execution.
+-r::
+--runs-per-test::
+ Run each test the given number of times, by default once. This
+ option can be useful to determine if a test is flaky.
+
-F::
--dont-fork::
Do not fork child for each test, run all tests within single process, this
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index c6071c4db741..14d30a5053be 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -42,6 +42,8 @@
static bool dont_fork;
/* Fork the tests in parallel and wait for their completion. */
static bool sequential;
+/* Number of times each test is run. */
+static unsigned int runs_per_test = 1;
const char *dso_to_test;
const char *test_objdump_path = "objdump";
@@ -485,7 +487,7 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
len = strlen(test_description(*t, i));
if (width < len)
width = len;
- num_tests++;
+ num_tests += runs_per_test;
}
}
child_tests = calloc(num_tests, sizeof(*child_tests));
@@ -549,16 +551,18 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
continue;
}
- test_suite__for_each_test_case(*t, curr_test_case) {
- if (!perf_test__matches(test_description(*t, curr_test_case),
- curr_suite, argc, argv))
- continue;
-
- err = start_test(*t, curr_suite, curr_test_case,
- &child_tests[child_test_num++],
- width, pass);
- if (err)
- goto err_out;
+ for (unsigned int run = 0; run < runs_per_test; run++) {
+ test_suite__for_each_test_case(*t, curr_test_case) {
+ if (!perf_test__matches(test_description(*t, curr_test_case),
+ curr_suite, argc, argv))
+ continue;
+
+ err = start_test(*t, curr_suite, curr_test_case,
+ &child_tests[child_test_num++],
+ width, pass);
+ if (err)
+ goto err_out;
+ }
}
}
if (!sequential) {
@@ -698,6 +702,8 @@ int cmd_test(int argc, const char **argv)
"Do not fork for testcase"),
OPT_BOOLEAN('S', "sequential", &sequential,
"Run the tests one after another rather than in parallel"),
+ OPT_UINTEGER('r', "runs-per-test", &runs_per_test,
+ "Run each test the given number of times, default 1"),
OPT_STRING('w', "workload", &workload, "work", "workload to run for testing, use '--list-workloads' to list the available ones."),
OPT_BOOLEAN(0, "list-workloads", &list_workloads, "List the available builtin workloads to use with -w/--workload"),
OPT_STRING(0, "dso", &dso_to_test, "dso", "dso to test"),
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 5/5] perf test: Improve verbose documentation
2025-01-10 4:57 [PATCH v5 0/5] perf test: Add a runs-per-test option Ian Rogers
` (3 preceding siblings ...)
2025-01-10 4:57 ` [PATCH v5 4/5] perf test: Add a runs-per-test flag Ian Rogers
@ 2025-01-10 4:57 ` Ian Rogers
2025-01-13 22:29 ` [PATCH v5 0/5] perf test: Add a runs-per-test option Namhyung Kim
2025-01-17 17:54 ` Namhyung Kim
6 siblings, 0 replies; 8+ messages in thread
From: Ian Rogers @ 2025-01-10 4:57 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark,
linux-perf-users, linux-kernel
Add a little more detail on the output expectations for each verbose
level.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/Documentation/perf-test.txt | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf-test.txt b/tools/perf/Documentation/perf-test.txt
index 85f868c324ff..32da0d1fa86a 100644
--- a/tools/perf/Documentation/perf-test.txt
+++ b/tools/perf/Documentation/perf-test.txt
@@ -28,8 +28,11 @@ OPTIONS
Tests to skip (comma separated numeric list).
-v::
+-vv::
+-vvv::
--verbose::
- Be more verbose.
+ With a single '-v', verbose level 1, only failing test output
+ is displayed. With '-vv' and higher all test output is shown.
-S::
--sequential::
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5 0/5] perf test: Add a runs-per-test option
2025-01-10 4:57 [PATCH v5 0/5] perf test: Add a runs-per-test option Ian Rogers
` (4 preceding siblings ...)
2025-01-10 4:57 ` [PATCH v5 5/5] perf test: Improve verbose documentation Ian Rogers
@ 2025-01-13 22:29 ` Namhyung Kim
2025-01-17 17:54 ` Namhyung Kim
6 siblings, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2025-01-13 22:29 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, James Clark, linux-perf-users, linux-kernel
On Thu, Jan 09, 2025 at 08:57:31PM -0800, Ian Rogers wrote:
> Add a runs-per-test option to perf test inorder to better diagnose
> flakes. Each runs executes as if it were a different test, the tests
> appear together in the output:
>
> ```
> $ perf test -r 3 lbr -v
> 122: perf record LBR tests : Ok
> 122: perf record LBR tests : Ok
> 122: perf record LBR tests : Ok
> ```
>
> v5: Fix the patch that updates the documentation for the sequential
> option, as spotted by Namhyung.
> v4: Fix crash in dont_fork mode caused by cleanup work.
> v3: Improve perf test documentation, including for the new option, and
> add some example output to the commit messages as requested by
> Namhyung.
> v2: In the v1 patch Kan and I noted cleanup that would make the code
> cleaner and less indented:
> https://lore.kernel.org/lkml/20241109160219.49976-1-irogers@google.com/
> Add patches doing some builtin-test.c cleanup and then add the
> runs-per-test flag.
>
> Ian Rogers (5):
> perf test: Rename functions and variables for better clarity
> perf test: Send list output to stdout rather than stderr
> perf test: Fix parallel/sequential option documentation
> perf test: Add a runs-per-test flag
> perf test: Improve verbose documentation
Reviewed-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
>
> tools/perf/Documentation/perf-test.txt | 20 ++-
> tools/perf/tests/builtin-test.c | 223 ++++++++++++-------------
> 2 files changed, 119 insertions(+), 124 deletions(-)
>
> --
> 2.47.1.613.gc27f4b7a9f-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 0/5] perf test: Add a runs-per-test option
2025-01-10 4:57 [PATCH v5 0/5] perf test: Add a runs-per-test option Ian Rogers
` (5 preceding siblings ...)
2025-01-13 22:29 ` [PATCH v5 0/5] perf test: Add a runs-per-test option Namhyung Kim
@ 2025-01-17 17:54 ` Namhyung Kim
6 siblings, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2025-01-17 17:54 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, James Clark, linux-perf-users, linux-kernel,
Ian Rogers
On Thu, 09 Jan 2025 20:57:31 -0800, Ian Rogers wrote:
> Add a runs-per-test option to perf test inorder to better diagnose
> flakes. Each runs executes as if it were a different test, the tests
> appear together in the output:
>
> ```
> $ perf test -r 3 lbr -v
> 122: perf record LBR tests : Ok
> 122: perf record LBR tests : Ok
> 122: perf record LBR tests : Ok
> ```
>
> [...]
Applied to perf-tools-next, thanks!
Best regards,
Namhyung
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-01-17 17:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-10 4:57 [PATCH v5 0/5] perf test: Add a runs-per-test option Ian Rogers
2025-01-10 4:57 ` [PATCH v5 1/5] perf test: Rename functions and variables for better clarity Ian Rogers
2025-01-10 4:57 ` [PATCH v5 2/5] perf test: Send list output to stdout rather than stderr Ian Rogers
2025-01-10 4:57 ` [PATCH v5 3/5] perf test: Fix parallel/sequential option documentation Ian Rogers
2025-01-10 4:57 ` [PATCH v5 4/5] perf test: Add a runs-per-test flag Ian Rogers
2025-01-10 4:57 ` [PATCH v5 5/5] perf test: Improve verbose documentation Ian Rogers
2025-01-13 22:29 ` [PATCH v5 0/5] perf test: Add a runs-per-test option Namhyung Kim
2025-01-17 17:54 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).