[PATCH igt] igt/pm_rpm: Use libc 'ftw' rather than opencoding our own filetree walk

* [PATCH igt] igt/pm_rpm: Use libc 'ftw' rather than opencoding our own filetree walk
@ 2017-08-22 12:52 Chris Wilson
  2017-08-22 14:01 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Chris Wilson @ 2017-08-22 12:52 UTC (permalink / raw)
  To: intel-gfx

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.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_debugfs.c | 50 +++++++++++++++++++++----------
 lib/igt_debugfs.h |  1 +
 lib/igt_sysfs.c   | 41 ++++++++++++++++++-------
 lib/igt_sysfs.h   |  1 +
 tests/pm_rpm.c    | 90 ++++++++++++++++++++++++-------------------------------
 5 files changed, 107 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..d96759dc 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,39 @@ static void i2c_subtest(void)
 	enable_one_screen(&ms_data);
 }
 
-static void read_full_file(int fd, const char *name)
+static int read_entry(const char *filepath,
+		      const struct stat *info,
+		      const int typeflag,
+		      struct FTW *pathinfo)
 {
+	char buf[4096];
+	int fd;
 	int rc;
-	char buf[128];
 
-	igt_assert_f(wait_for_suspended(), "File: %s\n", name);
+	fd = open(filepath, O_RDONLY);
+	if (fd < 0) {
+		igt_debug("Failed to open '%s': %m\n", filepath);
+		return 0;
+	}
+
+	igt_assert_f(wait_for_suspended(), "File: %s (%s)\n",
+		     filepath + pathinfo->base, filepath);
 
 	do {
-		rc = read(fd, buf, ARRAY_SIZE(buf));
-	} while (rc == ARRAY_SIZE(buf));
+		rc = read(fd, buf, sizeof(buf));
+	} while (rc == sizeof(buf));
 
-	igt_assert_f(wait_for_suspended(), "File: %s\n", name);
+	igt_assert_f(wait_for_suspended(), "File: %s (%s)\n",
+		     filepath + pathinfo->base, filepath);
+
+	close(fd);
+	return 0;
 }
 
-static void read_files_from_dir(int path, int level)
+static void walk_fs(char *path)
 {
-	DIR *dir;
-	struct dirent *dirent;
-	int rc;
-
-	dir = fdopendir(path);
-	igt_assert(dir);
-
-	igt_assert_lt(level, 128);
-
-	while ((dirent = readdir(dir))) {
-		struct stat stat_buf;
-		int de;
-
-		if (strcmp(dirent->d_name, ".") == 0)
-			continue;
-		if (strcmp(dirent->d_name, "..") == 0)
-			continue;
-
-		de = openat(path, dirent->d_name, O_RDONLY);
-
-		rc = fstat(de, &stat_buf);
-		igt_assert_eq(rc, 0);
-
-		if (S_ISDIR(stat_buf.st_mode))
-			read_files_from_dir(de, level + 1);
-
-		if (S_ISREG(stat_buf.st_mode))
-			read_full_file(de, dirent->d_name);
-
-		close(de);
-	}
-
-	closedir(dir);
+	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 +874,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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread