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.
v5 - fix a -Wunused-result
v4 - fix bug in width computation.
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 | 378 ++++++++++--------
tools/perf/tests/shell/lib/perf_has_symbol.sh | 2 +-
tools/perf/tests/tests-scripts.c | 256 ++++++++++++
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, 508 insertions(+), 410 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.44.0.rc0.258.g7320e95886-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.44.0.rc0.258.g7320e95886-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 9e47712507cc..bf79dd366e2e 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.44.0.rc0.258.g7320e95886-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.
Acked-by: Adrian Hunter <[email protected]>
Signed-off-by: Ian Rogers <[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.44.0.rc0.258.g7320e95886-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.44.0.rc0.258.g7320e95886-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.44.0.rc0.258.g7320e95886-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 | 20 ++---
tools/perf/tests/tests-scripts.c | 144 ++++++++++++++++++-------------
tools/perf/tests/tests-scripts.h | 1 -
3 files changed, 94 insertions(+), 71 deletions(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index eff3c62e9b47..162f9eb090ac 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -300,22 +300,19 @@ 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 = NULL;
- path__join(script, sizeof(script) - 3, st->dir, st->file);
-
- if (verbose > 0)
- strncat(script, " -v", sizeof(script) - strlen(script) - 1);
-
- err = system(script);
+ if (asprintf(&cmd, "%s%s", st->file, verbose ? " -v" : "") < 0)
+ return TEST_FAIL;
+ err = system(cmd);
+ free(cmd);
if (!err)
return TEST_OK;
@@ -331,7 +328,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 +342,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 +451,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.44.0.rc0.258.g7320e95886-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 | 90 +--------------------
tools/perf/tests/tests-scripts.c | 129 ++++++++++++++++++-------------
tools/perf/tests/tests-scripts.h | 10 +--
3 files changed, 80 insertions(+), 149 deletions(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 162f9eb090ac..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,73 +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 = NULL;
-
- if (asprintf(&cmd, "%s%s", st->file, verbose ? " -v" : "") < 0)
- 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));
@@ -440,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;
}
@@ -488,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;
}
@@ -550,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..1b99af5c353e 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,30 @@ 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 = NULL;
+
+ if (asprintf(&cmd, "%s%s", file, verbose ? " -v" : "") < 0)
+ 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 +154,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 +209,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 +222,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.44.0.rc0.258.g7320e95886-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]>
---
tools/perf/tests/builtin-test.c | 314 ++++++++++++++++++++++----------
1 file changed, 215 insertions(+), 99 deletions(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index c42cb40fc242..d13ee7683d9d 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;
+ if (has_subtests(t)) {
+ int subw = width > 2 ? width - 2 : width;
- pr_debug("\n--- start ---\n");
- err = run_test(t, subtest);
- pr_debug("---- end ----\n");
+ 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));
- if (!has_subtests(t))
- pr_debug("%s:", t->desc);
- else
- pr_debug("%s subtest %d:", t->desc, subtest + 1);
-
- switch (err) {
+ switch (result) {
case TEST_OK:
pr_info(" Ok\n");
break;
@@ -297,34 +266,186 @@ 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 subi = 0, subn = num_subtests(t); subi < subn; subi++) {
+ len = strlen(test_description(t, subi));
+ 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++;
- int subi;
if (!perf_test__matches(test_description(t, -1), curr, argc, argv)) {
bool skip = true;
- int subn;
-
- subn = num_subtests(t);
- for (subi = 0; subi < subn; subi++) {
+ for (int subi = 0, subn = num_subtests(t); subi < subn; subi++) {
if (perf_test__matches(test_description(t, subi),
curr, argc, argv))
skip = false;
@@ -334,52 +455,45 @@ 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);
- } 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");
+ int err = start_test(t, curr, -1, &child_tests[child_test_num++], width);
- for (subi = 0; subi < subn; subi++) {
- int len = strlen(test_description(t, subi));
-
- if (subw < len)
- subw = len;
+ if (err) {
+ /* TODO: if parallel waitpid the already forked children. */
+ free(child_tests);
+ return err;
}
+ } else {
+ for (int subi = 0, subn = num_subtests(t); subi < subn; subi++) {
+ int err;
- 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 +561,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.44.0.rc0.258.g7320e95886-goog
Hi Ian,
On Fri, Feb 16, 2024 at 3:55 PM Ian Rogers <[email protected]> wrote:
>
> 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]>
This fails to build in alpine:
util/print-events.c: In function 'print_tracepoint_events':
util/print-events.c:92:29: error: implicit declaration of function
'scandirat'; did you mean 'scandir'?
[-Werror=implicit-function-declaration]
92 | evt_items = scandirat(events_fd,
sys_dirent->d_name, &evt_namelist, NULL, alphasort);
| ^~~~~~~~~
| scandir
Thanks,
Namhyung
> ---
> 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 9e47712507cc..bf79dd366e2e 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.44.0.rc0.258.g7320e95886-goog
>
On Fri, Feb 16, 2024 at 3:55 PM Ian Rogers <[email protected]> wrote:
>
> 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]>
Ditto.
tests/tests-scripts.c: In function 'append_scripts_in_dir':
tests/tests-scripts.c:200:18: error: implicit declaration of
function 'scandirat'; did you mean 'scandir'?
[-Werror=implicit-function-declaration]
200 | n_dirs = scandirat(dir_fd, ".", &entlist, NULL,
alphasort);
| ^~~~~~~~~
| scandir
Thanks,
Namhyung
> ---
> tools/perf/tests/builtin-test.c | 20 ++---
> tools/perf/tests/tests-scripts.c | 144 ++++++++++++++++++-------------
> tools/perf/tests/tests-scripts.h | 1 -
> 3 files changed, 94 insertions(+), 71 deletions(-)
>
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index eff3c62e9b47..162f9eb090ac 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -300,22 +300,19 @@ 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 = NULL;
>
> - path__join(script, sizeof(script) - 3, st->dir, st->file);
> -
> - if (verbose > 0)
> - strncat(script, " -v", sizeof(script) - strlen(script) - 1);
> -
> - err = system(script);
> + if (asprintf(&cmd, "%s%s", st->file, verbose ? " -v" : "") < 0)
> + return TEST_FAIL;
> + err = system(cmd);
> + free(cmd);
> if (!err)
> return TEST_OK;
>
> @@ -331,7 +328,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 +342,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 +451,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.44.0.rc0.258.g7320e95886-goog
>
On Tue, Feb 20, 2024 at 5:54 PM Namhyung Kim <[email protected]> wrote:
>
> On Fri, Feb 16, 2024 at 3:55 PM Ian Rogers <[email protected]> wrote:
> >
> > 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]>
>
> Ditto.
>
> tests/tests-scripts.c: In function 'append_scripts_in_dir':
> tests/tests-scripts.c:200:18: error: implicit declaration of
> function 'scandirat'; did you mean 'scandir'?
> [-Werror=implicit-function-declaration]
> 200 | n_dirs = scandirat(dir_fd, ".", &entlist, NULL,
> alphasort);
> | ^~~~~~~~~
> | scandir
Sorry, missed the workaround definition in util.h. Will fix in v6.
Thanks,
Ian
> Thanks,
> Namhyung
>
>
> > ---
> > tools/perf/tests/builtin-test.c | 20 ++---
> > tools/perf/tests/tests-scripts.c | 144 ++++++++++++++++++-------------
> > tools/perf/tests/tests-scripts.h | 1 -
> > 3 files changed, 94 insertions(+), 71 deletions(-)
> >
> > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > index eff3c62e9b47..162f9eb090ac 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -300,22 +300,19 @@ 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 = NULL;
> >
> > - path__join(script, sizeof(script) - 3, st->dir, st->file);
> > -
> > - if (verbose > 0)
> > - strncat(script, " -v", sizeof(script) - strlen(script) - 1);
> > -
> > - err = system(script);
> > + if (asprintf(&cmd, "%s%s", st->file, verbose ? " -v" : "") < 0)
> > + return TEST_FAIL;
> > + err = system(cmd);
> > + free(cmd);
> > if (!err)
> > return TEST_OK;
> >
> > @@ -331,7 +328,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 +342,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 +451,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.44.0.rc0.258.g7320e95886-goog
> >