If command line option '-p' is provided, start/fork all tests in the
main thread and then gather them in order at the end. On a laptop test
time was reduced from 5 minutes 21 seconds to 1 minute 50 seconds. The
option isn't default as the test shows up perf and test flakes.
The '-v' option is modified so that 'perf test -v' will give verbose
output only from failing tests.
v3 - fix a crash if shell test directory isn't found, remove merged patch.
v2 - fix parallel test output/verbose issue
v1 - initial PoC
Ian Rogers (8):
perf thread_map: Skip exited threads when scanning /proc
perf list: Add scandirat compatibility function
perf tests: Avoid fork in perf_has_symbol test
tools subcmd: Add a no exec function call option
perf test: Rename builtin-test-list and add missed header guard
perf tests: Use scandirat for shell script finding
perf tests: Run time generate shell test suites
perf tests: Add option to run tests in parallel
tools/lib/subcmd/run-command.c | 2 +
tools/lib/subcmd/run-command.h | 2 +
tools/perf/tests/Build | 2 +-
tools/perf/tests/builtin-test-list.c | 207 ----------
tools/perf/tests/builtin-test-list.h | 12 -
tools/perf/tests/builtin-test.c | 369 ++++++++++--------
tools/perf/tests/shell/lib/perf_has_symbol.sh | 2 +-
tools/perf/tests/tests-scripts.c | 257 ++++++++++++
tools/perf/tests/tests-scripts.h | 9 +
tools/perf/util/print-events.c | 12 +-
tools/perf/util/thread_map.c | 9 +-
tools/perf/util/util.c | 19 +
tools/perf/util/util.h | 8 +
13 files changed, 507 insertions(+), 403 deletions(-)
delete mode 100644 tools/perf/tests/builtin-test-list.c
delete mode 100644 tools/perf/tests/builtin-test-list.h
create mode 100644 tools/perf/tests/tests-scripts.c
create mode 100644 tools/perf/tests/tests-scripts.h
--
2.43.0.687.g38aa6559b0-goog
Scanning /proc is inherently racy. Scanning /proc/pid/task within that
is also racy as the pid can terminate. Rather than failing in
__thread_map__new_all_cpus, skip pids for such failures.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/thread_map.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index ea3b431b9783..b5f12390c355 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -109,9 +109,10 @@ static struct perf_thread_map *__thread_map__new_all_cpus(uid_t uid)
snprintf(path, sizeof(path), "/proc/%d/task", pid);
items = scandir(path, &namelist, filter, NULL);
- if (items <= 0)
- goto out_free_closedir;
-
+ if (items <= 0) {
+ pr_debug("scandir for %d returned empty, skipping\n", pid);
+ continue;
+ }
while (threads->nr + items >= max_threads) {
max_threads *= 2;
grow = true;
@@ -152,8 +153,6 @@ static struct perf_thread_map *__thread_map__new_all_cpus(uid_t uid)
for (i = 0; i < items; i++)
zfree(&namelist[i]);
free(namelist);
-
-out_free_closedir:
zfree(&threads);
goto out_closedir;
}
--
2.43.0.687.g38aa6559b0-goog
perf test -vv Symbols is used to indentify symbols within the perf
binary. Add the -F flag so that the test command doesn't fork the test
before running. This removes a little overhead.
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
---
tools/perf/tests/shell/lib/perf_has_symbol.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/tests/shell/lib/perf_has_symbol.sh b/tools/perf/tests/shell/lib/perf_has_symbol.sh
index 5d59c32ae3e7..561c93b75d77 100644
--- a/tools/perf/tests/shell/lib/perf_has_symbol.sh
+++ b/tools/perf/tests/shell/lib/perf_has_symbol.sh
@@ -3,7 +3,7 @@
perf_has_symbol()
{
- if perf test -vv "Symbols" 2>&1 | grep "[[:space:]]$1$"; then
+ if perf test -vv -F "Symbols" 2>&1 | grep "[[:space:]]$1$"; then
echo "perf does have symbol '$1'"
return 0
fi
--
2.43.0.687.g38aa6559b0-goog
Tools like perf fork tests in case they crash, but they don't want to
exec a full binary. Add an option to call a function rather than do an
exec. The child process exits with the result of the function call and
is passed the struct of the run_command, things like container_of can
then allow the child process function to determine additional
arguments.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/lib/subcmd/run-command.c | 2 ++
tools/lib/subcmd/run-command.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c
index 5cdac2162532..d435eb42354b 100644
--- a/tools/lib/subcmd/run-command.c
+++ b/tools/lib/subcmd/run-command.c
@@ -122,6 +122,8 @@ int start_command(struct child_process *cmd)
}
if (cmd->preexec_cb)
cmd->preexec_cb();
+ if (cmd->no_exec_cmd)
+ exit(cmd->no_exec_cmd(cmd));
if (cmd->exec_cmd) {
execv_cmd(cmd->argv);
} else {
diff --git a/tools/lib/subcmd/run-command.h b/tools/lib/subcmd/run-command.h
index 17d969c6add3..d794138a797f 100644
--- a/tools/lib/subcmd/run-command.h
+++ b/tools/lib/subcmd/run-command.h
@@ -47,6 +47,8 @@ struct child_process {
unsigned exec_cmd:1; /* if this is to be external sub-command */
unsigned stdout_to_stderr:1;
void (*preexec_cb)(void);
+ /* If set, call function in child rather than doing an exec. */
+ int (*no_exec_cmd)(struct child_process *process);
};
int start_command(struct child_process *);
--
2.43.0.687.g38aa6559b0-goog
scandirat is used during the printing of tracepoint events but may be
missing from certain libcs. Add a compatibility implementation that
uses the symlink of an fd in /proc as a path for the reliably present
scandir.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/print-events.c | 12 +++---------
tools/perf/util/util.c | 19 +++++++++++++++++++
tools/perf/util/util.h | 8 ++++++++
3 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
index b0fc48be623f..15ec55b07bfd 100644
--- a/tools/perf/util/print-events.c
+++ b/tools/perf/util/print-events.c
@@ -63,6 +63,8 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
{
char *events_path = get_tracing_file("events");
int events_fd = open(events_path, O_PATH);
+ struct dirent **sys_namelist = NULL;
+ int sys_items;
put_tracing_file(events_path);
if (events_fd < 0) {
@@ -70,10 +72,7 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
return;
}
-#ifdef HAVE_SCANDIRAT_SUPPORT
-{
- struct dirent **sys_namelist = NULL;
- int sys_items = tracing_events__scandir_alphasort(&sys_namelist);
+ sys_items = tracing_events__scandir_alphasort(&sys_namelist);
for (int i = 0; i < sys_items; i++) {
struct dirent *sys_dirent = sys_namelist[i];
@@ -130,11 +129,6 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
}
free(sys_namelist);
-}
-#else
- printf("\nWARNING: Your libc doesn't have the scandirat function, please ask its maintainers to implement it.\n"
- " As a rough fallback, please do 'ls %s' to see the available tracepoint events.\n", events_path);
-#endif
close(events_fd);
}
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index c1fd9ba6d697..4f561e5e4162 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -552,3 +552,22 @@ int sched_getcpu(void)
return -1;
}
#endif
+
+#ifndef HAVE_SCANDIRAT_SUPPORT
+int scandirat(int dirfd, const char *dirp,
+ struct dirent ***namelist,
+ int (*filter)(const struct dirent *),
+ int (*compar)(const struct dirent **, const struct dirent **))
+{
+ char path[PATH_MAX];
+ int err, fd = openat(dirfd, dirp, O_PATH);
+
+ if (fd < 0)
+ return fd;
+
+ snprintf(path, sizeof(path), "/proc/%d/fd/%d", getpid(), fd);
+ err = scandir(path, namelist, filter, compar);
+ close(fd);
+ return err;
+}
+#endif
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 7c8915d92dca..9966c21aaf04 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -6,6 +6,7 @@
/* glibc 2.20 deprecates _BSD_SOURCE in favour of _DEFAULT_SOURCE */
#define _DEFAULT_SOURCE 1
+#include <dirent.h>
#include <fcntl.h>
#include <stdbool.h>
#include <stddef.h>
@@ -56,6 +57,13 @@ int perf_tip(char **strp, const char *dirpath);
int sched_getcpu(void);
#endif
+#ifndef HAVE_SCANDIRAT_SUPPORT
+int scandirat(int dirfd, const char *dirp,
+ struct dirent ***namelist,
+ int (*filter)(const struct dirent *),
+ int (*compar)(const struct dirent **, const struct dirent **));
+#endif
+
extern bool perf_singlethreaded;
void perf_set_singlethreaded(void);
--
2.43.0.687.g38aa6559b0-goog
builtin-test-list is primarily concerned with shell script
tests. Rename the file to better reflect this and add a missed header
guard.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/tests/Build | 2 +-
tools/perf/tests/builtin-test.c | 2 +-
tools/perf/tests/{builtin-test-list.c => tests-scripts.c} | 2 +-
tools/perf/tests/{builtin-test-list.h => tests-scripts.h} | 4 ++++
4 files changed, 7 insertions(+), 3 deletions(-)
rename tools/perf/tests/{builtin-test-list.c => tests-scripts.c} (99%)
rename tools/perf/tests/{builtin-test-list.h => tests-scripts.h} (79%)
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 53ba9c3e20e0..c7f9d9676095 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
perf-y += builtin-test.o
-perf-y += builtin-test-list.o
+perf-y += tests-scripts.o
perf-y += parse-events.o
perf-y += dso-data.o
perf-y += attr.o
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 4a5973f9bb9b..eff3c62e9b47 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -29,7 +29,7 @@
#include <subcmd/exec-cmd.h>
#include <linux/zalloc.h>
-#include "builtin-test-list.h"
+#include "tests-scripts.h"
static bool dont_fork;
const char *dso_to_test;
diff --git a/tools/perf/tests/builtin-test-list.c b/tools/perf/tests/tests-scripts.c
similarity index 99%
rename from tools/perf/tests/builtin-test-list.c
rename to tools/perf/tests/tests-scripts.c
index a65b9e547d82..4ebd841da05b 100644
--- a/tools/perf/tests/builtin-test-list.c
+++ b/tools/perf/tests/tests-scripts.c
@@ -15,7 +15,7 @@
#include <sys/wait.h>
#include <sys/stat.h>
#include "builtin.h"
-#include "builtin-test-list.h"
+#include "tests-scripts.h"
#include "color.h"
#include "debug.h"
#include "hist.h"
diff --git a/tools/perf/tests/builtin-test-list.h b/tools/perf/tests/tests-scripts.h
similarity index 79%
rename from tools/perf/tests/builtin-test-list.h
rename to tools/perf/tests/tests-scripts.h
index eb81f3aa6683..3a3ec6191848 100644
--- a/tools/perf/tests/builtin-test-list.h
+++ b/tools/perf/tests/tests-scripts.h
@@ -1,4 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef TESTS_SCRIPTS_H
+#define TESTS_SCRIPTS_H
struct script_file {
char *dir;
@@ -10,3 +12,5 @@ struct script_file {
const struct script_file *list_script_files(void);
/* Get maximum width of description string */
int list_script_max_width(void);
+
+#endif /* TESTS_SCRIPTS_H */
--
2.43.0.687.g38aa6559b0-goog
By default tests are forked, add an option (-p or --parallel) so that
the forked tests are all started in parallel and then their output
gathered serially. This is opt-in as running in parallel can cause
test flakes.
Rather than fork within the code, the start_command/finish_command
from libsubcmd are used. This changes how stderr and stdout are
handled. The child stderr and stdout are always read to avoid the
child blocking. If verbose is 1 (-v) then if the test fails the child
stdout and stderr are displayed. If the verbose is >1 (e.g. -vv) then
the stdout and stderr from the child are immediately displayed.
An unscientific test on my laptop shows the wall clock time for perf
test without parallel being 5 minutes 21 seconds and with parallel
(-p) being 1 minute 50 seconds.
Signed-off-by: Ian Rogers <[email protected]>
---
v1 of this code had a bug where stdout/stderr weren't read fully. This
and additional issues/improvements are dealt with in v2.
---
tools/perf/tests/builtin-test.c | 305 ++++++++++++++++++++++----------
1 file changed, 213 insertions(+), 92 deletions(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index c42cb40fc242..b815db8ebf79 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -6,6 +6,7 @@
*/
#include <fcntl.h>
#include <errno.h>
+#include <poll.h>
#include <unistd.h>
#include <string.h>
#include <stdlib.h>
@@ -21,9 +22,11 @@
#include "debug.h"
#include "color.h"
#include <subcmd/parse-options.h>
+#include <subcmd/run-command.h>
#include "string2.h"
#include "symbol.h"
#include "util/rlimit.h"
+#include "util/strbuf.h"
#include <linux/kernel.h>
#include <linux/string.h>
#include <subcmd/exec-cmd.h>
@@ -31,7 +34,13 @@
#include "tests-scripts.h"
+/*
+ * Command line option to not fork the test running in the same process and
+ * making them easier to debug.
+ */
static bool dont_fork;
+/* Fork the tests in parallel and then wait for their completion. */
+static bool parallel;
const char *dso_to_test;
const char *test_objdump_path = "objdump";
@@ -209,76 +218,36 @@ static bool perf_test__matches(const char *desc, int curr, int argc, const char
return false;
}
-static int run_test(struct test_suite *test, int subtest)
-{
- int status, err = -1, child = dont_fork ? 0 : fork();
- char sbuf[STRERR_BUFSIZE];
-
- if (child < 0) {
- pr_err("failed to fork test: %s\n",
- str_error_r(errno, sbuf, sizeof(sbuf)));
- return -1;
- }
-
- if (!child) {
- if (!dont_fork) {
- pr_debug("test child forked, pid %d\n", getpid());
-
- if (verbose <= 0) {
- int nullfd = open("/dev/null", O_WRONLY);
-
- if (nullfd >= 0) {
- close(STDERR_FILENO);
- close(STDOUT_FILENO);
-
- dup2(nullfd, STDOUT_FILENO);
- dup2(STDOUT_FILENO, STDERR_FILENO);
- close(nullfd);
- }
- } else {
- signal(SIGSEGV, sighandler_dump_stack);
- signal(SIGFPE, sighandler_dump_stack);
- }
- }
-
- err = test_function(test, subtest)(test, subtest);
- if (!dont_fork)
- exit(err);
- }
-
- if (!dont_fork) {
- wait(&status);
+struct child_test {
+ struct child_process process;
+ struct test_suite *test;
+ int test_num;
+ int subtest;
+};
- if (WIFEXITED(status)) {
- err = (signed char)WEXITSTATUS(status);
- pr_debug("test child finished with %d\n", err);
- } else if (WIFSIGNALED(status)) {
- err = -1;
- pr_debug("test child interrupted\n");
- }
- }
+static int run_test_child(struct child_process *process)
+{
+ struct child_test *child = container_of(process, struct child_test, process);
+ int err;
- return err;
+ pr_debug("--- start ---\n");
+ pr_debug("test child forked, pid %d\n", getpid());
+ err = test_function(child->test, child->subtest)(child->test, child->subtest);
+ pr_debug("---- end(%d) ----\n", err);
+ fflush(NULL);
+ return -err;
}
-#define for_each_test(j, k, t) \
- for (j = 0, k = 0; j < ARRAY_SIZE(tests); j++, k = 0) \
- while ((t = tests[j][k++]) != NULL)
-
-static int test_and_print(struct test_suite *t, int subtest)
+static int print_test_result(struct test_suite *t, int i, int subtest, int result, int width)
{
- int err;
-
- pr_debug("\n--- start ---\n");
- err = run_test(t, subtest);
- pr_debug("---- end ----\n");
+ if (has_subtests(t)) {
+ int subw = width > 2 ? width - 2 : width;
- if (!has_subtests(t))
- pr_debug("%s:", t->desc);
- else
- pr_debug("%s subtest %d:", t->desc, subtest + 1);
+ pr_info("%3d.%1d: %-*s:", i + 1, subtest + 1, subw, test_description(t, subtest));
+ } else
+ pr_info("%3d: %-*s:", i + 1, width, test_description(t, subtest));
- switch (err) {
+ switch (result) {
case TEST_OK:
pr_info(" Ok\n");
break;
@@ -297,22 +266,177 @@ static int test_and_print(struct test_suite *t, int subtest)
break;
}
- return err;
+ return 0;
+}
+
+static int finish_test(struct child_test *child_test, int width)
+{
+ struct test_suite *t = child_test->test;
+ int i = child_test->test_num;
+ int subi = child_test->subtest;
+ int out = child_test->process.out;
+ int err = child_test->process.err;
+ bool out_done = out <= 0;
+ bool err_done = err <= 0;
+ struct strbuf out_output = STRBUF_INIT;
+ struct strbuf err_output = STRBUF_INIT;
+ int ret;
+
+ /*
+ * 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));
+
+ /*
+ * Busy loop reading from the child's stdout and stderr that are set to
+ * be non-blocking until EOF.
+ */
+ if (!out_done)
+ fcntl(out, F_SETFL, O_NONBLOCK);
+ if (!err_done)
+ 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));
+ else
+ pr_info("%3d: %s:\n", i + 1, test_description(t, -1));
+ }
+ while (!out_done || !err_done) {
+ struct pollfd pfds[2] = {
+ { .fd = out,
+ .events = POLLIN | POLLERR | POLLHUP | POLLNVAL,
+ },
+ { .fd = err,
+ .events = POLLIN | POLLERR | POLLHUP | POLLNVAL,
+ },
+ };
+ char buf[512];
+ ssize_t len;
+
+ /* Poll to avoid excessive spinning, timeout set for 1000ms. */
+ poll(pfds, ARRAY_SIZE(pfds), /*timeout=*/1000);
+ if (!out_done && pfds[0].revents) {
+ errno = 0;
+ len = read(out, buf, sizeof(buf) - 1);
+
+ if (len <= 0) {
+ out_done = errno != EAGAIN;
+ } else {
+ buf[len] = '\0';
+ if (verbose > 1)
+ fprintf(stdout, "%s", buf);
+ else
+ strbuf_addstr(&out_output, buf);
+ }
+ }
+ if (!err_done && pfds[1].revents) {
+ errno = 0;
+ len = read(err, buf, sizeof(buf) - 1);
+
+ if (len <= 0) {
+ err_done = errno != EAGAIN;
+ } else {
+ buf[len] = '\0';
+ if (verbose > 1)
+ fprintf(stdout, "%s", buf);
+ else
+ strbuf_addstr(&err_output, buf);
+ }
+ }
+ }
+ /* Clean up child process. */
+ ret = finish_command(&child_test->process);
+ if (verbose == 1 && ret == TEST_FAIL) {
+ /* Add header for test that was skipped above. */
+ if (has_subtests(t))
+ pr_info("%3d.%1d: %s:\n", i + 1, subi + 1, test_description(t, subi));
+ else
+ pr_info("%3d: %s:\n", i + 1, test_description(t, -1));
+ fprintf(stdout, "%s", out_output.buf);
+ fprintf(stderr, "%s", err_output.buf);
+ }
+ strbuf_release(&out_output);
+ strbuf_release(&err_output);
+ print_test_result(t, i, subi, ret, width);
+ if (out > 0)
+ close(out);
+ if (err > 0)
+ close(err);
+ return 0;
+}
+
+static int start_test(struct test_suite *test, int i, int subi, struct child_test **child,
+ int width)
+{
+ int err;
+
+ *child = NULL;
+ if (dont_fork) {
+ pr_debug("--- start ---\n");
+ err = test_function(test, subi)(test, subi);
+ pr_debug("---- end ----\n");
+ print_test_result(test, i, subi, err, width);
+ return 0;
+ }
+
+ *child = zalloc(sizeof(**child));
+ if (!*child)
+ return -ENOMEM;
+
+ (*child)->test = test;
+ (*child)->test_num = i;
+ (*child)->subtest = subi;
+ (*child)->process.pid = -1;
+ (*child)->process.no_stdin = 1;
+ if (verbose <= 0) {
+ (*child)->process.no_stdout = 1;
+ (*child)->process.no_stderr = 1;
+ } else {
+ (*child)->process.out = -1;
+ (*child)->process.err = -1;
+ }
+ (*child)->process.no_exec_cmd = run_test_child;
+ err = start_command(&(*child)->process);
+ if (err || parallel)
+ return err;
+ return finish_test(*child, width);
}
+#define for_each_test(j, k, t) \
+ for (j = 0, k = 0; j < ARRAY_SIZE(tests); j++, k = 0) \
+ while ((t = tests[j][k++]) != NULL)
+
static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
{
struct test_suite *t;
unsigned int j, k;
int i = 0;
int width = 0;
+ size_t num_tests = 0;
+ struct child_test **child_tests;
+ int child_test_num = 0;
for_each_test(j, k, t) {
int len = strlen(test_description(t, -1));
if (width < len)
width = len;
+
+ if (has_subtests(t)) {
+ for (int l = 0, subn = num_subtests(t); l < subn; l++) {
+ len = strlen(test_description(t, -1));
+ if (width < len)
+ width = len;
+ num_tests++;
+ }
+ } else
+ num_tests++;
}
+ child_tests = calloc(num_tests, sizeof(*child_tests));
+ if (!child_tests)
+ return -ENOMEM;
for_each_test(j, k, t) {
int curr = i++;
@@ -334,52 +458,47 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
continue;
}
- pr_info("%3d: %-*s:", i, width, test_description(t, -1));
-
if (intlist__find(skiplist, i)) {
+ pr_info("%3d: %-*s:", curr + 1, width, test_description(t, -1));
color_fprintf(stderr, PERF_COLOR_YELLOW, " Skip (user override)\n");
continue;
}
if (!has_subtests(t)) {
- test_and_print(t, -1);
+ int err = start_test(t, curr, -1, &child_tests[child_test_num++], width);
+
+ if (err) {
+ /* TODO: if parallel waitpid the already forked children. */
+ free(child_tests);
+ return err;
+ }
} else {
int subn = num_subtests(t);
- /*
- * minus 2 to align with normal testcases.
- * For subtest we print additional '.x' in number.
- * for example:
- *
- * 35: Test LLVM searching and compiling :
- * 35.1: Basic BPF llvm compiling test : Ok
- */
- int subw = width > 2 ? width - 2 : width;
-
- if (subn <= 0) {
- color_fprintf(stderr, PERF_COLOR_YELLOW,
- " Skip (not compiled in)\n");
- continue;
- }
- pr_info("\n");
for (subi = 0; subi < subn; subi++) {
- int len = strlen(test_description(t, subi));
+ int err;
- if (subw < len)
- subw = len;
- }
-
- for (subi = 0; subi < subn; subi++) {
if (!perf_test__matches(test_description(t, subi),
curr, argc, argv))
continue;
- pr_info("%3d.%1d: %-*s:", i, subi + 1, subw,
- test_description(t, subi));
- test_and_print(t, subi);
+ err = start_test(t, curr, subi, &child_tests[child_test_num++],
+ width);
+ if (err)
+ return err;
}
}
}
+ for (i = 0; i < child_test_num; i++) {
+ if (parallel) {
+ int ret = finish_test(child_tests[i], width);
+
+ if (ret)
+ return ret;
+ }
+ free(child_tests[i]);
+ }
+ free(child_tests);
return 0;
}
@@ -447,6 +566,8 @@ int cmd_test(int argc, const char **argv)
"be more verbose (show symbol address, etc)"),
OPT_BOOLEAN('F', "dont-fork", &dont_fork,
"Do not fork for testcase"),
+ OPT_BOOLEAN('p', "parallel", ¶llel,
+ "Run the tests altogether in parallel"),
OPT_STRING('w', "workload", &workload, "work", "workload to run for testing"),
OPT_STRING(0, "dso", &dso_to_test, "dso", "dso to test"),
OPT_STRING(0, "objdump", &test_objdump_path, "path",
--
2.43.0.687.g38aa6559b0-goog
Rather than special shell test logic, do a single pass to create an
array of test suites. Hold the shell test file name in the test suite
priv field. This makes the special shell test logic in builtin-test.c
redundant so remove it.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/tests/builtin-test.c | 91 +---------------------
tools/perf/tests/tests-scripts.c | 130 ++++++++++++++++++-------------
tools/perf/tests/tests-scripts.h | 10 +--
3 files changed, 81 insertions(+), 150 deletions(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 6d5001daaf63..c42cb40fc242 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -130,6 +130,7 @@ static struct test_suite *generic_tests[] = {
static struct test_suite **tests[] = {
generic_tests,
arch_tests,
+ NULL, /* shell tests created at runtime. */
};
static struct test_workload *workloads[] = {
@@ -299,74 +300,12 @@ static int test_and_print(struct test_suite *t, int subtest)
return err;
}
-struct shell_test {
- const char *file;
-};
-
-static int shell_test__run(struct test_suite *test, int subdir __maybe_unused)
-{
- int err;
- struct shell_test *st = test->priv;
- char *cmd;
-
- asprintf(&cmd, "%s%s", st->file, verbose ? " -v" : "");
- if (!cmd)
- return TEST_FAIL;
- err = system(cmd);
- free(cmd);
- if (!err)
- return TEST_OK;
-
- return WEXITSTATUS(err) == 2 ? TEST_SKIP : TEST_FAIL;
-}
-
-static int run_shell_tests(int argc, const char *argv[], int i, int width,
- struct intlist *skiplist)
-{
- struct shell_test st;
- const struct script_file *files, *file;
-
- files = list_script_files();
- if (!files)
- return 0;
- for (file = files; file->file; file++) {
- int curr = i++;
- struct test_case test_cases[] = {
- {
- .desc = file->desc,
- .run_case = shell_test__run,
- },
- { .name = NULL, }
- };
- struct test_suite test_suite = {
- .desc = test_cases[0].desc,
- .test_cases = test_cases,
- .priv = &st,
- };
- st.file = file->file;
-
- if (test_suite.desc == NULL ||
- !perf_test__matches(test_suite.desc, curr, argc, argv))
- continue;
-
- pr_info("%3d: %-*s:", i, width, test_suite.desc);
-
- if (intlist__find(skiplist, i)) {
- color_fprintf(stderr, PERF_COLOR_YELLOW, " Skip (user override)\n");
- continue;
- }
-
- test_and_print(&test_suite, 0);
- }
- return 0;
-}
-
static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
{
struct test_suite *t;
unsigned int j, k;
int i = 0;
- int width = list_script_max_width();
+ int width = 0;
for_each_test(j, k, t) {
int len = strlen(test_description(t, -1));
@@ -441,28 +380,6 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
}
}
}
-
- return run_shell_tests(argc, argv, i, width, skiplist);
-}
-
-static int perf_test__list_shell(int argc, const char **argv, int i)
-{
- const struct script_file *files, *file;
-
- files = list_script_files();
- if (!files)
- return 0;
- for (file = files; file->file; file++) {
- int curr = i++;
- struct test_suite t = {
- .desc = file->desc
- };
-
- if (!perf_test__matches(t.desc, curr, argc, argv))
- continue;
-
- pr_info("%3d: %s\n", i, t.desc);
- }
return 0;
}
@@ -489,9 +406,6 @@ static int perf_test__list(int argc, const char **argv)
test_description(t, subi));
}
}
-
- perf_test__list_shell(argc, argv, i);
-
return 0;
}
@@ -551,6 +465,7 @@ int cmd_test(int argc, const char **argv)
/* Unbuffered output */
setvbuf(stdout, NULL, _IONBF, 0);
+ tests[2] = create_script_test_suites();
argc = parse_options_subcommand(argc, argv, test_options, test_subcommands, test_usage, 0);
if (argc >= 1 && !strcmp(argv[0], "list"))
return perf_test__list(argc - 1, argv + 1);
diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
index 9b3b66dd5508..e60867b1e5ce 100644
--- a/tools/perf/tests/tests-scripts.c
+++ b/tools/perf/tests/tests-scripts.c
@@ -26,16 +26,6 @@
#include "tests.h"
#include "util/rlimit.h"
-
-/*
- * As this is a singleton built once for the run of the process, there is
- * no value in trying to free it and just let it stay around until process
- * exits when it's cleaned up.
- */
-static size_t files_num = 0;
-static struct script_file *files = NULL;
-static int files_max_width = 0;
-
static int shell_tests__dir_fd(void)
{
char path[PATH_MAX], *exec_path;
@@ -131,12 +121,31 @@ static char *strdup_check(const char *str)
return newstr;
}
-static void append_script(int dir_fd, const char *name, char *desc)
+static int shell_test__run(struct test_suite *test, int subtest __maybe_unused)
+{
+ const char *file = test->priv;
+ int err;
+ char *cmd;
+
+ asprintf(&cmd, "%s%s", file, verbose ? " -v" : "");
+ if (!cmd)
+ return TEST_FAIL;
+ err = system(cmd);
+ free(cmd);
+ if (!err)
+ return TEST_OK;
+
+ return WEXITSTATUS(err) == 2 ? TEST_SKIP : TEST_FAIL;
+}
+
+static void append_script(int dir_fd, const char *name, char *desc,
+ struct test_suite ***result,
+ size_t *result_sz)
{
char filename[PATH_MAX], link[128];
- struct script_file *files_tmp;
- size_t files_num_tmp, len;
- int width;
+ struct test_suite *test_suite, **result_tmp;
+ struct test_case *tests;
+ size_t len;
snprintf(link, sizeof(link), "/proc/%d/fd/%d", getpid(), dir_fd);
len = readlink(link, filename, sizeof(filename));
@@ -146,33 +155,43 @@ static void append_script(int dir_fd, const char *name, char *desc)
}
filename[len++] = '/';
strcpy(&filename[len], name);
- files_num_tmp = files_num + 1;
- if (files_num_tmp >= SIZE_MAX) {
- pr_err("Too many script files\n");
- abort();
+
+ tests = calloc(2, sizeof(*tests));
+ if (!tests) {
+ pr_err("Out of memory while building script test suite list\n");
+ return;
}
+ tests[0].name = strdup_check(name);
+ tests[0].desc = strdup_check(desc);
+ tests[0].run_case = shell_test__run;
+
+ test_suite = zalloc(sizeof(*test_suite));
+ if (!test_suite) {
+ pr_err("Out of memory while building script test suite list\n");
+ free(tests);
+ return;
+ }
+ test_suite->desc = desc;
+ test_suite->test_cases = tests;
+ test_suite->priv = strdup_check(filename);
/* Realloc is good enough, though we could realloc by chunks, not that
* anyone will ever measure performance here */
- files_tmp = realloc(files,
- (files_num_tmp + 1) * sizeof(struct script_file));
- if (files_tmp == NULL) {
- pr_err("Out of memory while building test list\n");
- abort();
+ result_tmp = realloc(*result, (*result_sz + 1) * sizeof(*result_tmp));
+ if (result_tmp == NULL) {
+ pr_err("Out of memory while building script test suite list\n");
+ free(tests);
+ free(test_suite);
+ return;
}
/* Add file to end and NULL terminate the struct array */
- files = files_tmp;
- files_num = files_num_tmp;
- files[files_num - 1].file = strdup_check(filename);
- files[files_num - 1].desc = desc;
- files[files_num].file = NULL;
- files[files_num].desc = NULL;
-
- width = strlen(desc); /* Track max width of desc */
- if (width > files_max_width)
- files_max_width = width;
+ *result = result_tmp;
+ (*result)[*result_sz] = test_suite;
+ (*result_sz)++;
}
-static void append_scripts_in_dir(int dir_fd)
+static void append_scripts_in_dir(int dir_fd,
+ struct test_suite ***result,
+ size_t *result_sz)
{
struct dirent **entlist;
struct dirent *ent;
@@ -191,7 +210,7 @@ static void append_scripts_in_dir(int dir_fd)
char *desc = shell_test__description(dir_fd, ent->d_name);
if (desc) /* It has a desc line - valid script */
- append_script(dir_fd, ent->d_name, desc);
+ append_script(dir_fd, ent->d_name, desc, result, result_sz);
continue;
}
if (ent->d_type != DT_DIR) {
@@ -204,32 +223,35 @@ static void append_scripts_in_dir(int dir_fd)
continue;
}
fd = openat(dir_fd, ent->d_name, O_PATH);
- append_scripts_in_dir(fd);
+ append_scripts_in_dir(fd, result, result_sz);
}
for (i = 0; i < n_dirs; i++) /* Clean up */
zfree(&entlist[i]);
free(entlist);
}
-const struct script_file *list_script_files(void)
+struct test_suite **create_script_test_suites(void)
{
- int dir_fd;
-
- if (files)
- return files; /* Singleton - we already know our list */
-
- dir_fd = shell_tests__dir_fd(); /* Walk dir */
- if (dir_fd < 0)
- return NULL;
+ struct test_suite **result = NULL, **result_tmp;
+ size_t result_sz = 0;
+ int dir_fd = shell_tests__dir_fd(); /* Walk dir */
- append_scripts_in_dir(dir_fd);
- close(dir_fd);
+ /*
+ * Append scripts if fd is good, otherwise return a NULL terminated zero
+ * length array.
+ */
+ if (dir_fd >= 0)
+ append_scripts_in_dir(dir_fd, &result, &result_sz);
- return files;
-}
-
-int list_script_max_width(void)
-{
- list_script_files(); /* Ensure we have scanned all scripts */
- return files_max_width;
+ result_tmp = realloc(result, (result_sz + 1) * sizeof(*result_tmp));
+ if (result_tmp == NULL) {
+ pr_err("Out of memory while building script test suite list\n");
+ abort();
+ }
+ /* NULL terminate the test suite array. */
+ result = result_tmp;
+ result[result_sz] = NULL;
+ if (dir_fd >= 0)
+ close(dir_fd);
+ return result;
}
diff --git a/tools/perf/tests/tests-scripts.h b/tools/perf/tests/tests-scripts.h
index 3508a293aaf9..b553ad26ea17 100644
--- a/tools/perf/tests/tests-scripts.h
+++ b/tools/perf/tests/tests-scripts.h
@@ -2,14 +2,8 @@
#ifndef TESTS_SCRIPTS_H
#define TESTS_SCRIPTS_H
-struct script_file {
- char *file;
- char *desc;
-};
+#include "tests.h"
-/* List available script tests to run - singleton - never freed */
-const struct script_file *list_script_files(void);
-/* Get maximum width of description string */
-int list_script_max_width(void);
+struct test_suite **create_script_test_suites(void);
#endif /* TESTS_SCRIPTS_H */
--
2.43.0.687.g38aa6559b0-goog
Avoid filename appending buffers by using openat, faccessat and
scandirat more widely. Turn the script's path back to a file name
using readlink from /proc/<pid>/fd/<fd>.
Read the script's description using api/io.h to avoid fdopen
conversions. Whilst reading perform additional sanity checks on the
script's contents.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/tests/builtin-test.c | 21 ++---
tools/perf/tests/tests-scripts.c | 144 ++++++++++++++++++-------------
tools/perf/tests/tests-scripts.h | 1 -
3 files changed, 95 insertions(+), 71 deletions(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index eff3c62e9b47..6d5001daaf63 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -300,22 +300,20 @@ static int test_and_print(struct test_suite *t, int subtest)
}
struct shell_test {
- const char *dir;
const char *file;
};
static int shell_test__run(struct test_suite *test, int subdir __maybe_unused)
{
int err;
- char script[PATH_MAX];
struct shell_test *st = test->priv;
+ char *cmd;
- path__join(script, sizeof(script) - 3, st->dir, st->file);
-
- if (verbose > 0)
- strncat(script, " -v", sizeof(script) - strlen(script) - 1);
-
- err = system(script);
+ asprintf(&cmd, "%s%s", st->file, verbose ? " -v" : "");
+ if (!cmd)
+ return TEST_FAIL;
+ err = system(cmd);
+ free(cmd);
if (!err)
return TEST_OK;
@@ -331,7 +329,7 @@ static int run_shell_tests(int argc, const char *argv[], int i, int width,
files = list_script_files();
if (!files)
return 0;
- for (file = files; file->dir; file++) {
+ for (file = files; file->file; file++) {
int curr = i++;
struct test_case test_cases[] = {
{
@@ -345,13 +343,12 @@ static int run_shell_tests(int argc, const char *argv[], int i, int width,
.test_cases = test_cases,
.priv = &st,
};
- st.dir = file->dir;
+ st.file = file->file;
if (test_suite.desc == NULL ||
!perf_test__matches(test_suite.desc, curr, argc, argv))
continue;
- st.file = file->file;
pr_info("%3d: %-*s:", i, width, test_suite.desc);
if (intlist__find(skiplist, i)) {
@@ -455,7 +452,7 @@ static int perf_test__list_shell(int argc, const char **argv, int i)
files = list_script_files();
if (!files)
return 0;
- for (file = files; file->dir; file++) {
+ for (file = files; file->file; file++) {
int curr = i++;
struct test_suite t = {
.desc = file->desc
diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
index 4ebd841da05b..9b3b66dd5508 100644
--- a/tools/perf/tests/tests-scripts.c
+++ b/tools/perf/tests/tests-scripts.c
@@ -14,6 +14,7 @@
#include <subcmd/parse-options.h>
#include <sys/wait.h>
#include <sys/stat.h>
+#include <api/io.h>
#include "builtin.h"
#include "tests-scripts.h"
#include "color.h"
@@ -35,55 +36,69 @@ static size_t files_num = 0;
static struct script_file *files = NULL;
static int files_max_width = 0;
-static const char *shell_tests__dir(char *path, size_t size)
+static int shell_tests__dir_fd(void)
{
- const char *devel_dirs[] = { "./tools/perf/tests", "./tests", };
- char *exec_path;
- unsigned int i;
+ char path[PATH_MAX], *exec_path;
+ static const char * const devel_dirs[] = { "./tools/perf/tests/shell", "./tests/shell", };
- for (i = 0; i < ARRAY_SIZE(devel_dirs); ++i) {
- struct stat st;
+ for (size_t i = 0; i < ARRAY_SIZE(devel_dirs); ++i) {
+ int fd = open(devel_dirs[i], O_PATH);
- if (!lstat(devel_dirs[i], &st)) {
- scnprintf(path, size, "%s/shell", devel_dirs[i]);
- if (!lstat(devel_dirs[i], &st))
- return path;
- }
+ if (fd >= 0)
+ return fd;
}
/* Then installed path. */
exec_path = get_argv_exec_path();
- scnprintf(path, size, "%s/tests/shell", exec_path);
+ scnprintf(path, sizeof(path), "%s/tests/shell", exec_path);
free(exec_path);
- return path;
+ return open(path, O_PATH);
}
-static const char *shell_test__description(char *description, size_t size,
- const char *path, const char *name)
+static char *shell_test__description(int dir_fd, const char *name)
{
- FILE *fp;
- char filename[PATH_MAX];
- int ch;
+ struct io io;
+ char buf[128], desc[256];
+ int ch, pos = 0;
- path__join(filename, sizeof(filename), path, name);
- fp = fopen(filename, "r");
- if (!fp)
+ io__init(&io, openat(dir_fd, name, O_RDONLY), buf, sizeof(buf));
+ if (io.fd < 0)
return NULL;
/* Skip first line - should be #!/bin/sh Shebang */
+ if (io__get_char(&io) != '#')
+ goto err_out;
+ if (io__get_char(&io) != '!')
+ goto err_out;
do {
- ch = fgetc(fp);
- } while (ch != EOF && ch != '\n');
-
- description = fgets(description, size, fp);
- fclose(fp);
+ ch = io__get_char(&io);
+ if (ch < 0)
+ goto err_out;
+ } while (ch != '\n');
- /* Assume first char on line is omment everything after that desc */
- return description ? strim(description + 1) : NULL;
+ do {
+ ch = io__get_char(&io);
+ if (ch < 0)
+ goto err_out;
+ } while (ch == '#' || isspace(ch));
+ while (ch > 0 && ch != '\n') {
+ desc[pos++] = ch;
+ if (pos >= (int)sizeof(desc) - 1)
+ break;
+ ch = io__get_char(&io);
+ }
+ while (pos > 0 && isspace(desc[--pos]))
+ ;
+ desc[++pos] = '\0';
+ close(io.fd);
+ return strdup(desc);
+err_out:
+ close(io.fd);
+ return NULL;
}
/* Is this full file path a shell script */
-static bool is_shell_script(const char *path)
+static bool is_shell_script(int dir_fd, const char *path)
{
const char *ext;
@@ -91,20 +106,16 @@ static bool is_shell_script(const char *path)
if (!ext)
return false;
if (!strcmp(ext, ".sh")) { /* Has .sh extension */
- if (access(path, R_OK | X_OK) == 0) /* Is executable */
+ if (faccessat(dir_fd, path, R_OK | X_OK, 0) == 0) /* Is executable */
return true;
}
return false;
}
/* Is this file in this dir a shell script (for test purposes) */
-static bool is_test_script(const char *path, const char *name)
+static bool is_test_script(int dir_fd, const char *name)
{
- char filename[PATH_MAX];
-
- path__join(filename, sizeof(filename), path, name);
- if (!is_shell_script(filename)) return false;
- return true;
+ return is_shell_script(dir_fd, name);
}
/* Duplicate a string and fall over and die if we run out of memory */
@@ -120,12 +131,21 @@ static char *strdup_check(const char *str)
return newstr;
}
-static void append_script(const char *dir, const char *file, const char *desc)
+static void append_script(int dir_fd, const char *name, char *desc)
{
+ char filename[PATH_MAX], link[128];
struct script_file *files_tmp;
- size_t files_num_tmp;
+ size_t files_num_tmp, len;
int width;
+ snprintf(link, sizeof(link), "/proc/%d/fd/%d", getpid(), dir_fd);
+ len = readlink(link, filename, sizeof(filename));
+ if (len < 0) {
+ pr_err("Failed to readlink %s", link);
+ return;
+ }
+ filename[len++] = '/';
+ strcpy(&filename[len], name);
files_num_tmp = files_num + 1;
if (files_num_tmp >= SIZE_MAX) {
pr_err("Too many script files\n");
@@ -142,10 +162,8 @@ static void append_script(const char *dir, const char *file, const char *desc)
/* Add file to end and NULL terminate the struct array */
files = files_tmp;
files_num = files_num_tmp;
- files[files_num - 1].dir = strdup_check(dir);
- files[files_num - 1].file = strdup_check(file);
- files[files_num - 1].desc = strdup_check(desc);
- files[files_num].dir = NULL;
+ files[files_num - 1].file = strdup_check(filename);
+ files[files_num - 1].desc = desc;
files[files_num].file = NULL;
files[files_num].desc = NULL;
@@ -154,32 +172,39 @@ static void append_script(const char *dir, const char *file, const char *desc)
files_max_width = width;
}
-static void append_scripts_in_dir(const char *path)
+static void append_scripts_in_dir(int dir_fd)
{
struct dirent **entlist;
struct dirent *ent;
int n_dirs, i;
- char filename[PATH_MAX];
/* List files, sorted by alpha */
- n_dirs = scandir(path, &entlist, NULL, alphasort);
+ n_dirs = scandirat(dir_fd, ".", &entlist, NULL, alphasort);
if (n_dirs == -1)
return;
for (i = 0; i < n_dirs && (ent = entlist[i]); i++) {
+ int fd;
+
if (ent->d_name[0] == '.')
continue; /* Skip hidden files */
- if (is_test_script(path, ent->d_name)) { /* It's a test */
- char bf[256];
- const char *desc = shell_test__description
- (bf, sizeof(bf), path, ent->d_name);
+ if (is_test_script(dir_fd, ent->d_name)) { /* It's a test */
+ char *desc = shell_test__description(dir_fd, ent->d_name);
if (desc) /* It has a desc line - valid script */
- append_script(path, ent->d_name, desc);
- } else if (is_directory(path, ent)) { /* Scan the subdir */
- path__join(filename, sizeof(filename),
- path, ent->d_name);
- append_scripts_in_dir(filename);
+ append_script(dir_fd, ent->d_name, desc);
+ continue;
+ }
+ if (ent->d_type != DT_DIR) {
+ struct stat st;
+
+ if (ent->d_type != DT_UNKNOWN)
+ continue;
+ fstatat(dir_fd, ent->d_name, &st, 0);
+ if (!S_ISDIR(st.st_mode))
+ continue;
}
+ fd = openat(dir_fd, ent->d_name, O_PATH);
+ append_scripts_in_dir(fd);
}
for (i = 0; i < n_dirs; i++) /* Clean up */
zfree(&entlist[i]);
@@ -188,14 +213,17 @@ static void append_scripts_in_dir(const char *path)
const struct script_file *list_script_files(void)
{
- char path_dir[PATH_MAX];
- const char *path;
+ int dir_fd;
if (files)
return files; /* Singleton - we already know our list */
- path = shell_tests__dir(path_dir, sizeof(path_dir)); /* Walk dir */
- append_scripts_in_dir(path);
+ dir_fd = shell_tests__dir_fd(); /* Walk dir */
+ if (dir_fd < 0)
+ return NULL;
+
+ append_scripts_in_dir(dir_fd);
+ close(dir_fd);
return files;
}
diff --git a/tools/perf/tests/tests-scripts.h b/tools/perf/tests/tests-scripts.h
index 3a3ec6191848..3508a293aaf9 100644
--- a/tools/perf/tests/tests-scripts.h
+++ b/tools/perf/tests/tests-scripts.h
@@ -3,7 +3,6 @@
#define TESTS_SCRIPTS_H
struct script_file {
- char *dir;
char *file;
char *desc;
};
--
2.43.0.687.g38aa6559b0-goog
On Mon, Feb 12, 2024 at 10:59 AM Ian Rogers <[email protected]> wrote:
>
> By default tests are forked, add an option (-p or --parallel) so that
> the forked tests are all started in parallel and then their output
> gathered serially. This is opt-in as running in parallel can cause
> test flakes.
>
> Rather than fork within the code, the start_command/finish_command
> from libsubcmd are used. This changes how stderr and stdout are
> handled. The child stderr and stdout are always read to avoid the
> child blocking. If verbose is 1 (-v) then if the test fails the child
> stdout and stderr are displayed. If the verbose is >1 (e.g. -vv) then
> the stdout and stderr from the child are immediately displayed.
>
> An unscientific test on my laptop shows the wall clock time for perf
> test without parallel being 5 minutes 21 seconds and with parallel
> (-p) being 1 minute 50 seconds.
>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> v1 of this code had a bug where stdout/stderr weren't read fully. This
> and additional issues/improvements are dealt with in v2.
> ---
[SNIP]
> static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
> {
> struct test_suite *t;
> unsigned int j, k;
> int i = 0;
> int width = 0;
> + size_t num_tests = 0;
> + struct child_test **child_tests;
> + int child_test_num = 0;
>
> for_each_test(j, k, t) {
> int len = strlen(test_description(t, -1));
>
> if (width < len)
> width = len;
> +
> + if (has_subtests(t)) {
> + for (int l = 0, subn = num_subtests(t); l < subn; l++) {
> + len = strlen(test_description(t, -1));
Shouldn't it be strlen(test_description(t, i)) ? Looks like it has len
of parent test already.
Thanks,
Namhyung
> + if (width < len)
> + width = len;
> + num_tests++;
> + }
> + } else
> + num_tests++;
> }
> + child_tests = calloc(num_tests, sizeof(*child_tests));
> + if (!child_tests)
> + return -ENOMEM;
>
> for_each_test(j, k, t) {
> int curr = i++;
> @@ -334,52 +458,47 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
> continue;
> }
>
> - pr_info("%3d: %-*s:", i, width, test_description(t, -1));
> -
> if (intlist__find(skiplist, i)) {
> + pr_info("%3d: %-*s:", curr + 1, width, test_description(t, -1));
> color_fprintf(stderr, PERF_COLOR_YELLOW, " Skip (user override)\n");
> continue;
> }
>
> if (!has_subtests(t)) {
> - test_and_print(t, -1);
> + int err = start_test(t, curr, -1, &child_tests[child_test_num++], width);
> +
> + if (err) {
> + /* TODO: if parallel waitpid the already forked children. */
> + free(child_tests);
> + return err;
> + }
> } else {
> int subn = num_subtests(t);
> - /*
> - * minus 2 to align with normal testcases.
> - * For subtest we print additional '.x' in number.
> - * for example:
> - *
> - * 35: Test LLVM searching and compiling :
> - * 35.1: Basic BPF llvm compiling test : Ok
> - */
> - int subw = width > 2 ? width - 2 : width;
> -
> - if (subn <= 0) {
> - color_fprintf(stderr, PERF_COLOR_YELLOW,
> - " Skip (not compiled in)\n");
> - continue;
> - }
> - pr_info("\n");
>
> for (subi = 0; subi < subn; subi++) {
> - int len = strlen(test_description(t, subi));
> + int err;
>
> - if (subw < len)
> - subw = len;
> - }
> -
> - for (subi = 0; subi < subn; subi++) {
> if (!perf_test__matches(test_description(t, subi),
> curr, argc, argv))
> continue;
>
> - pr_info("%3d.%1d: %-*s:", i, subi + 1, subw,
> - test_description(t, subi));
> - test_and_print(t, subi);
> + err = start_test(t, curr, subi, &child_tests[child_test_num++],
> + width);
> + if (err)
> + return err;
> }
> }
> }
> + for (i = 0; i < child_test_num; i++) {
> + if (parallel) {
> + int ret = finish_test(child_tests[i], width);
> +
> + if (ret)
> + return ret;
> + }
> + free(child_tests[i]);
> + }
> + free(child_tests);
> return 0;
> }
>
> @@ -447,6 +566,8 @@ int cmd_test(int argc, const char **argv)
> "be more verbose (show symbol address, etc)"),
> OPT_BOOLEAN('F', "dont-fork", &dont_fork,
> "Do not fork for testcase"),
> + OPT_BOOLEAN('p', "parallel", ¶llel,
> + "Run the tests altogether in parallel"),
> OPT_STRING('w', "workload", &workload, "work", "workload to run for testing"),
> OPT_STRING(0, "dso", &dso_to_test, "dso", "dso to test"),
> OPT_STRING(0, "objdump", &test_objdump_path, "path",
> --
> 2.43.0.687.g38aa6559b0-goog
>
On Tue, Feb 13, 2024 at 5:14 PM Namhyung Kim <[email protected]> wrote:
>
> On Mon, Feb 12, 2024 at 10:59 AM Ian Rogers <[email protected]> wrote:
> >
> > By default tests are forked, add an option (-p or --parallel) so that
> > the forked tests are all started in parallel and then their output
> > gathered serially. This is opt-in as running in parallel can cause
> > test flakes.
> >
> > Rather than fork within the code, the start_command/finish_command
> > from libsubcmd are used. This changes how stderr and stdout are
> > handled. The child stderr and stdout are always read to avoid the
> > child blocking. If verbose is 1 (-v) then if the test fails the child
> > stdout and stderr are displayed. If the verbose is >1 (e.g. -vv) then
> > the stdout and stderr from the child are immediately displayed.
> >
> > An unscientific test on my laptop shows the wall clock time for perf
> > test without parallel being 5 minutes 21 seconds and with parallel
> > (-p) being 1 minute 50 seconds.
> >
> > Signed-off-by: Ian Rogers <[email protected]>
> > ---
> > v1 of this code had a bug where stdout/stderr weren't read fully. This
> > and additional issues/improvements are dealt with in v2.
> > ---
> [SNIP]
> > static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
> > {
> > struct test_suite *t;
> > unsigned int j, k;
> > int i = 0;
> > int width = 0;
> > + size_t num_tests = 0;
> > + struct child_test **child_tests;
> > + int child_test_num = 0;
> >
> > for_each_test(j, k, t) {
> > int len = strlen(test_description(t, -1));
> >
> > if (width < len)
> > width = len;
> > +
> > + if (has_subtests(t)) {
> > + for (int l = 0, subn = num_subtests(t); l < subn; l++) {
> > + len = strlen(test_description(t, -1));
>
> Shouldn't it be strlen(test_description(t, i)) ? Looks like it has len
> of parent test already.
>
> Thanks,
> Namhyung
Thanks Namhyung, will fix in v4.
Ian
> > + if (width < len)
> > + width = len;
> > + num_tests++;
> > + }
> > + } else
> > + num_tests++;
> > }
> > + child_tests = calloc(num_tests, sizeof(*child_tests));
> > + if (!child_tests)
> > + return -ENOMEM;
> >
> > for_each_test(j, k, t) {
> > int curr = i++;
> > @@ -334,52 +458,47 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
> > continue;
> > }
> >
> > - pr_info("%3d: %-*s:", i, width, test_description(t, -1));
> > -
> > if (intlist__find(skiplist, i)) {
> > + pr_info("%3d: %-*s:", curr + 1, width, test_description(t, -1));
> > color_fprintf(stderr, PERF_COLOR_YELLOW, " Skip (user override)\n");
> > continue;
> > }
> >
> > if (!has_subtests(t)) {
> > - test_and_print(t, -1);
> > + int err = start_test(t, curr, -1, &child_tests[child_test_num++], width);
> > +
> > + if (err) {
> > + /* TODO: if parallel waitpid the already forked children. */
> > + free(child_tests);
> > + return err;
> > + }
> > } else {
> > int subn = num_subtests(t);
> > - /*
> > - * minus 2 to align with normal testcases.
> > - * For subtest we print additional '.x' in number.
> > - * for example:
> > - *
> > - * 35: Test LLVM searching and compiling :
> > - * 35.1: Basic BPF llvm compiling test : Ok
> > - */
> > - int subw = width > 2 ? width - 2 : width;
> > -
> > - if (subn <= 0) {
> > - color_fprintf(stderr, PERF_COLOR_YELLOW,
> > - " Skip (not compiled in)\n");
> > - continue;
> > - }
> > - pr_info("\n");
> >
> > for (subi = 0; subi < subn; subi++) {
> > - int len = strlen(test_description(t, subi));
> > + int err;
> >
> > - if (subw < len)
> > - subw = len;
> > - }
> > -
> > - for (subi = 0; subi < subn; subi++) {
> > if (!perf_test__matches(test_description(t, subi),
> > curr, argc, argv))
> > continue;
> >
> > - pr_info("%3d.%1d: %-*s:", i, subi + 1, subw,
> > - test_description(t, subi));
> > - test_and_print(t, subi);
> > + err = start_test(t, curr, subi, &child_tests[child_test_num++],
> > + width);
> > + if (err)
> > + return err;
> > }
> > }
> > }
> > + for (i = 0; i < child_test_num; i++) {
> > + if (parallel) {
> > + int ret = finish_test(child_tests[i], width);
> > +
> > + if (ret)
> > + return ret;
> > + }
> > + free(child_tests[i]);
> > + }
> > + free(child_tests);
> > return 0;
> > }
> >
> > @@ -447,6 +566,8 @@ int cmd_test(int argc, const char **argv)
> > "be more verbose (show symbol address, etc)"),
> > OPT_BOOLEAN('F', "dont-fork", &dont_fork,
> > "Do not fork for testcase"),
> > + OPT_BOOLEAN('p', "parallel", ¶llel,
> > + "Run the tests altogether in parallel"),
> > OPT_STRING('w', "workload", &workload, "work", "workload to run for testing"),
> > OPT_STRING(0, "dso", &dso_to_test, "dso", "dso to test"),
> > OPT_STRING(0, "objdump", &test_objdump_path, "path",
> > --
> > 2.43.0.687.g38aa6559b0-goog
> >