By using ftw, we avoid the issue of having to handle directory recursion ourselves and can focus on the test of checking the reading a sysfs/debugfs does not break runtime suspend. In the process, disregard errors when opening the individual files as they may fail for other reasons. v2: Bracket the file open/close with the wait_for_suspended() tests. Whilst the fd is open, it may be keeping the device awake, e.g. i915_forcewake_user. Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> --- lib/igt_debugfs.c | 50 +++++++++++++++++++++--------- lib/igt_debugfs.h | 1 + lib/igt_sysfs.c | 41 +++++++++++++++++++------ lib/igt_sysfs.h | 1 + tests/pm_rpm.c | 91 ++++++++++++++++++++++++------------------------------- 5 files changed, 108 insertions(+), 76 deletions(-) diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index ee1f0f54..84066ab8 100644 --- a/lib/igt_debugfs.c +++ b/lib/igt_debugfs.c @@ -127,38 +127,38 @@ const char *igt_debugfs_mount(void) } /** - * igt_debugfs_dir: + * igt_debugfs_path: * @device: fd of the device + * @path: buffer to store path + * @pathlen: len of @path buffer. * - * This opens the debugfs directory corresponding to device for use - * with igt_sysfs_get() and related functions. + * This finds the debugfs directory corresponding to @device. * * Returns: - * The directory fd, or -1 on failure. + * The directory path, or NULL on failure. */ -int igt_debugfs_dir(int device) +char *igt_debugfs_path(int device, char *path, int pathlen) { struct stat st; const char *debugfs_root; - char path[200]; int idx; if (fstat(device, &st)) { igt_debug("Couldn't stat FD for DRM device: %s\n", strerror(errno)); - return -1; + return NULL; } if (!S_ISCHR(st.st_mode)) { igt_debug("FD for DRM device not a char device!\n"); - return -1; + return NULL; } debugfs_root = igt_debugfs_mount(); idx = minor(st.st_rdev); - snprintf(path, sizeof(path), "%s/dri/%d/name", debugfs_root, idx); + snprintf(path, pathlen, "%s/dri/%d/name", debugfs_root, idx); if (stat(path, &st)) - return -1; + return NULL; if (idx >= 64) { int file, name_len, cmp_len; @@ -166,17 +166,17 @@ int igt_debugfs_dir(int device) file = open(path, O_RDONLY); if (file < 0) - return -1; + return NULL; name_len = read(file, name, sizeof(name)); close(file); for (idx = 0; idx < 16; idx++) { - snprintf(path, sizeof(path), "%s/dri/%d/name", + snprintf(path, pathlen, "%s/dri/%d/name", debugfs_root, idx); file = open(path, O_RDONLY); if (file < 0) - return -1; + return NULL; cmp_len = read(file, cmp, sizeof(cmp)); close(file); @@ -186,10 +186,30 @@ int igt_debugfs_dir(int device) } if (idx == 16) - return -1; + return NULL; } - snprintf(path, sizeof(path), "%s/dri/%d", debugfs_root, idx); + snprintf(path, pathlen, "%s/dri/%d", debugfs_root, idx); + return path; +} + +/** + * igt_debugfs_dir: + * @device: fd of the device + * + * This opens the debugfs directory corresponding to device for use + * with igt_sysfs_get() and related functions. + * + * Returns: + * The directory fd, or -1 on failure. + */ +int igt_debugfs_dir(int device) +{ + char path[200]; + + if (!igt_debugfs_path(device, path, sizeof(path))) + return -1; + igt_debug("Opening debugfs directory '%s'\n", path); return open(path, O_RDONLY); } diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h index f1a76406..4fa49d21 100644 --- a/lib/igt_debugfs.h +++ b/lib/igt_debugfs.h @@ -32,6 +32,7 @@ enum pipe; const char *igt_debugfs_mount(void); +char *igt_debugfs_path(int device, char *path, int pathlen); int igt_debugfs_dir(int device); diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c index 15ed34be..d412610d 100644 --- a/lib/igt_sysfs.c +++ b/lib/igt_sysfs.c @@ -86,26 +86,26 @@ static int writeN(int fd, const char *buf, int len) } /** - * igt_sysfs_open: + * igt_sysfs_path: * @device: fd of the device (or -1 to default to Intel) + * @path: buffer to fill with the sysfs path to the device + * @pathlen: length of @path buffer * @idx: optional pointer to store the card index of the opened device * - * This opens the sysfs directory corresponding to device for use - * with igt_sysfs_set() and igt_sysfs_get(). + * This finds the sysfs directory corresponding to @device. * * Returns: - * The directory fd, or -1 on failure. + * The directory path, or NULL on failure. */ -int igt_sysfs_open(int device, int *idx) +char *igt_sysfs_path(int device, char *path, int pathlen, int *idx) { - char path[80]; struct stat st; if (device != -1 && (fstat(device, &st) || !S_ISCHR(st.st_mode))) - return -1; + return NULL; for (int n = 0; n < 16; n++) { - int len = sprintf(path, "/sys/class/drm/card%d", n); + int len = snprintf(path, pathlen, "/sys/class/drm/card%d", n); if (device != -1) { FILE *file; int ret, maj, min; @@ -132,10 +132,31 @@ int igt_sysfs_open(int device, int *idx) path[len] = '\0'; if (idx) *idx = n; - return open(path, O_RDONLY); + return path; } - return -1; + return NULL; +} + +/** + * igt_sysfs_open: + * @device: fd of the device (or -1 to default to Intel) + * @idx: optional pointer to store the card index of the opened device + * + * This opens the sysfs directory corresponding to device for use + * with igt_sysfs_set() and igt_sysfs_get(). + * + * Returns: + * The directory fd, or -1 on failure. + */ +int igt_sysfs_open(int device, int *idx) +{ + char path[80]; + + if (!igt_sysfs_path(device, path, sizeof(path), idx)) + return -1; + + return open(path, O_RDONLY); } /** diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h index d666438a..3ee89b0f 100644 --- a/lib/igt_sysfs.h +++ b/lib/igt_sysfs.h @@ -27,6 +27,7 @@ #include <stdbool.h> +char *igt_sysfs_path(int device, char *path, int pathlen, int *idx); int igt_sysfs_open(int device, int *idx); int igt_sysfs_open_parameters(int device); bool igt_sysfs_set_parameter(int device, diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c index 47c9f114..20e8ca72 100644 --- a/tests/pm_rpm.c +++ b/tests/pm_rpm.c @@ -25,13 +25,13 @@ * */ -#include "igt.h" -#include "igt_kmod.h" -#include "igt_sysfs.h" +#include "config.h" + #include <stdio.h> #include <stdint.h> #include <stdbool.h> #include <string.h> +#include <ftw.h> #include <unistd.h> #include <fcntl.h> @@ -45,6 +45,10 @@ #include <drm.h> +#include "igt.h" +#include "igt_kmod.h" +#include "igt_sysfs.h" +#include "igt_debugfs.h" #define MSR_PC8_RES 0x630 #define MSR_PC9_RES 0x631 @@ -830,55 +834,40 @@ static void i2c_subtest(void) enable_one_screen(&ms_data); } -static void read_full_file(int fd, const char *name) -{ - int rc; - char buf[128]; - - igt_assert_f(wait_for_suspended(), "File: %s\n", name); - - do { - rc = read(fd, buf, ARRAY_SIZE(buf)); - } while (rc == ARRAY_SIZE(buf)); - - igt_assert_f(wait_for_suspended(), "File: %s\n", name); -} - -static void read_files_from_dir(int path, int level) +static int read_entry(const char *filepath, + const struct stat *info, + const int typeflag, + struct FTW *pathinfo) { - DIR *dir; - struct dirent *dirent; + char buf[4096]; + int fd; int rc; - dir = fdopendir(path); - igt_assert(dir); - - igt_assert_lt(level, 128); - - while ((dirent = readdir(dir))) { - struct stat stat_buf; - int de; + igt_assert_f(wait_for_suspended(), "Before opening: %s (%s)\n", + filepath + pathinfo->base, filepath); - if (strcmp(dirent->d_name, ".") == 0) - continue; - if (strcmp(dirent->d_name, "..") == 0) - continue; - - de = openat(path, dirent->d_name, O_RDONLY); + fd = open(filepath, O_RDONLY | O_NONBLOCK); + if (fd < 0) { + igt_debug("Failed to open '%s': %m\n", filepath); + return 0; + } - rc = fstat(de, &stat_buf); - igt_assert_eq(rc, 0); + do { + rc = read(fd, buf, sizeof(buf)); + } while (rc == sizeof(buf)); - if (S_ISDIR(stat_buf.st_mode)) - read_files_from_dir(de, level + 1); + close(fd); - if (S_ISREG(stat_buf.st_mode)) - read_full_file(de, dirent->d_name); + igt_assert_f(wait_for_suspended(), "After closing: %s (%s)\n", + filepath + pathinfo->base, filepath); - close(de); - } + return 0; +} - closedir(dir); +static void walk_fs(char *path) +{ + disable_all_screens_and_wait(&ms_data); + nftw(path, read_entry, 20, FTW_PHYS | FTW_MOUNT); } /* This test will probably pass, with a small chance of hanging the machine in @@ -886,21 +875,21 @@ static void read_files_from_dir(int path, int level) * errors, so a "pass" here should be confirmed by a check on dmesg. */ static void debugfs_read_subtest(void) { - disable_all_screens_and_wait(&ms_data); + char path[256]; - read_files_from_dir(debugfs, 0); + igt_require_f(igt_debugfs_path(drm_fd, path, sizeof(path)), + "Can't find the debugfs directory\n"); + walk_fs(path); } /* Read the comment on debugfs_read_subtest(). */ static void sysfs_read_subtest(void) { - int dir = igt_sysfs_open(drm_fd, NULL); - igt_require_f(dir != -1, "Can't open the sysfs directory\n"); - - disable_all_screens_and_wait(&ms_data); + char path[80]; - read_files_from_dir(dir, 0); - close(dir); + igt_require_f(igt_sysfs_path(drm_fd, path, sizeof(path), NULL), + "Can't find the sysfs directory\n"); + walk_fs(path); } /* Make sure we don't suspend when we have the i915_forcewake_user file open. */ -- 2.14.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx