[Intel-gfx] [PATCH i-g-t] lib: Cleanup __igt_params_open()

[Intel-gfx] [PATCH i-g-t] lib: Cleanup __igt_params_open()

[Chris Wilson]

The device always exist, so use it to derive the module name required to
lookup either the debugfs params directory or the sysfs module parameters.

Fixes: 2f5cee33ce55 ("igt/params: use igt_params_set_save for igt_set_module_param*")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
 lib/igt_params.c | 116 +++++++----------------------------------------
 1 file changed, 17 insertions(+), 99 deletions(-)

diff --git a/lib/igt_params.c b/lib/igt_params.c
index d8649dfd9..9d9a11c0b 100644
--- a/lib/igt_params.c
+++ b/lib/igt_params.c
@@ -131,117 +131,35 @@ static void igt_params_save(int dir, const char *path, const char *name)
  */
 static int __igt_params_open(int device, char **outpath, const char *param)
 {
+	drm_version_t version;
 	int dir, params = -1;
-	struct stat buffer;
-	char searchname[64];
-	char searchpath[PATH_MAX];
-	char *foundname, *ctx;
+	char path[PATH_MAX];
+	char name[32];
+
+	memset(&version, 0, sizeof(version));
+	version.name_len = sizeof(name);
+	version.name = name;
+	if (ioctl(device, DRM_IOCTL_VERSION, &version))
+		return -1;
 
 	dir = igt_debugfs_dir(device);
 	if (dir >= 0) {
-		int devname;
-
-		devname = openat(dir, "name", O_RDONLY);
-		igt_require_f(devname >= 0,
-		              "Driver need to name itself in debugfs!");
-
-		read(devname, searchname, sizeof(searchname));
-		close(devname);
-
-		foundname = strtok_r(searchname, " ", &ctx);
-		igt_require_f(foundname,
-		              "Driver need to name itself in debugfs!");
-
-		snprintf(searchpath, PATH_MAX, "%s_params", foundname);
-		params = openat(dir, searchpath, O_RDONLY);
-
-		if (params >= 0) {
-			char *debugfspath = malloc(PATH_MAX);
-
-			igt_debugfs_path(device, debugfspath, PATH_MAX);
-			if (param != NULL) {
-				char filepath[PATH_MAX];
-
-				snprintf(filepath, PATH_MAX, "%s/%s",
-					 debugfspath, param);
-
-				if (stat(filepath, &buffer) == 0) {
-					if (outpath != NULL)
-						*outpath = debugfspath;
-					else
-						free(debugfspath);
-				} else {
-					free(debugfspath);
-					close(params);
-					params = -1;
-				}
-			} else if (outpath != NULL) {
-				/*
-				 * Caller is responsible to free this.
-				 */
-				*outpath = debugfspath;
-			} else {
-				free(debugfspath);
-			}
-		}
+		snprintf(path, PATH_MAX, "%s_params", name);
+		params = openat(dir, path, O_RDONLY);
 		close(dir);
 	}
 
 	if (params < 0) { /* builtin? */
-		drm_version_t version;
-		char name[32] = "";
-		char path[PATH_MAX];
-
-		if (device == -1) {
-			/*
-			 * find default device
-			 */
-			int file, i;
-			const char *debugfs_root = igt_debugfs_mount();
-
-			igt_assert(debugfs_root);
-
-			for (i = 0; i < 63; i++) {
-				char testpath[PATH_MAX];
-
-				snprintf(searchpath, PATH_MAX,
-					 "%s/dri/%d/name", debugfs_root, i);
-
-				file = open(searchpath, O_RDONLY);
-
-				if (file < 0)
-					continue;
-
-				read(file, searchname, sizeof(searchname));
-				close(file);
-
-				foundname = strtok_r(searchname, " ", &ctx);
-				if (!foundname)
-					continue;
-
-				snprintf(testpath, PATH_MAX,
-					 "/sys/module/%s/parameters",
-					 foundname);
-
-				if (stat(testpath, &buffer) == 0 &&
-				    S_ISDIR(buffer.st_mode)) {
-					snprintf(name, sizeof(name), "%s",
-						 foundname);
-					break;
-				}
-			}
-		} else {
-			memset(&version, 0, sizeof(version));
-			version.name_len = sizeof(name);
-			version.name = name;
-			ioctl(device, DRM_IOCTL_VERSION, &version);
-		}
 		snprintf(path, sizeof(path), "/sys/module/%s/parameters", name);
 		params = open(path, O_RDONLY);
-		if (params >= 0 && outpath)
-			*outpath = strdup(path);
 	}
 
+	/*
+	 * Caller is responsible to free this.
+	 */
+	if (params >= 0 && outpath)
+		*outpath = strdup(path);
+
 	return params;
 }
 
-- 
2.26.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] perf: Hide any leak in gen8-unprivileged-single-ctx-counters

[Chris Wilson]

gen8-unprivileged-single-counters appears to leak contexts, and does not
appear to be doing so intentionally.. Let's assume it's a bug in the
test and see if the contexts are released along with the device fd.
(If they are not released, that is more clearly an issue.)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_exec_fence.c | 2 +-
 tests/perf.c                | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index 5b504e5c8..b88472c3a 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -1479,7 +1479,7 @@ static void test_syncobj_future_submit(int i915, unsigned int engine)
 	/*
 	 * Here we submit client A waiting on client B, but internally client
 	 * B has a semaphore that waits on client A. This relies on timeslicing
-	 * to reorder B before A, even though userspace has asked to submit
+	 * to reorder A before B, even though userspace has asked to submit
 	 * A & B simultaneously (and due to the sequence we will submit B
 	 * then A).
 	 */
diff --git a/tests/perf.c b/tests/perf.c
index d4ebae30d..c78f02a8d 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -3428,6 +3428,8 @@ gen8_test_single_ctx_render_target_writes_a_counter(void)
 				.format = test_set->perf_oa_format
 			};
 
+			drm_fd = gem_reopen_driver(drm_fd);
+
 			bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096);
 			drm_intel_bufmgr_gem_enable_reuse(bufmgr);
 
-- 
2.26.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] lib: Cleanup __igt_params_open()

[Chris Wilson]

The device always exist, so use it to derive the module name required to
lookup either the debugfs params directory or the sysfs module parameters.

Fixes: 2f5cee33ce55 ("igt/params: use igt_params_set_save for igt_set_module_param*")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
 lib/igt_params.c | 165 +++++++++--------------------------------------
 1 file changed, 29 insertions(+), 136 deletions(-)

diff --git a/lib/igt_params.c b/lib/igt_params.c
index d8649dfd9..23660f445 100644
--- a/lib/igt_params.c
+++ b/lib/igt_params.c
@@ -80,9 +80,18 @@ static void igt_params_exit_handler(int sig)
  * Notice that this function is called by igt_set_module_param(), so that one -
  * or one of its wrappers - is the only function the test programs need to call.
  */
-static void igt_params_save(int dir, const char *path, const char *name)
+static void igt_params_save(int dir, const char *name)
 {
 	struct module_param_data *data;
+	char path[PATH_MAX];
+	char buf[80];
+	int len;
+
+	snprintf(buf, sizeof(buf), "/proc/self/fd/%d", dir);
+	len = readlink(buf, path, sizeof(path) - 1);
+	if (len < 0)
+		return;
+	path[len] = '\0';
 
 	/* Check if this parameter is already saved. */
 	for (data = module_params; data != NULL; data = data->next)
@@ -110,175 +119,59 @@ static void igt_params_save(int dir, const char *path, const char *name)
 }
 
 /**
- * __igt_params_open:
- * @device: fd of the device or -1 for default
- * @outpath: full path to the sysfs directory if not NULL
- * @param: name of parameter of interest
- *
- * Find parameter of interest and return parameter directory fd, parameter
- * is first searched at debugfs/dri/N/<device>_params and if not found will
- * look for parameter at /sys/module/<device>/parameters.
- *
- * Giving -1 here for default device will search for matching device from
- * debugfs/dri/N where N go from 0 to 63. First device found from debugfs
- * which exist also at /sys/module/<device> will be 'default'.
- * Default device will only be used for sysfs, not for debugfs.
+ * igt_params_open:
+ * @device: fd of the device
  *
- * If outpath is not NULL caller is responsible to free given pointer.
+ * This opens the module parameters directory (under sysfs) corresponding
+ * to the device for use with igt_sysfs_set() and igt_sysfs_get().
  *
  * Returns:
- * Directory fd, or -1 on failure.
+ * The directory fd, or -1 on failure.
  */
-static int __igt_params_open(int device, char **outpath, const char *param)
+int igt_params_open(int device)
 {
+	drm_version_t version;
 	int dir, params = -1;
-	struct stat buffer;
-	char searchname[64];
-	char searchpath[PATH_MAX];
-	char *foundname, *ctx;
+	char path[PATH_MAX];
+	char name[32] = "";
+
+	memset(&version, 0, sizeof(version));
+	version.name_len = sizeof(name);
+	version.name = name;
+	if (ioctl(device, DRM_IOCTL_VERSION, &version))
+		return -1;
 
 	dir = igt_debugfs_dir(device);
 	if (dir >= 0) {
-		int devname;
-
-		devname = openat(dir, "name", O_RDONLY);
-		igt_require_f(devname >= 0,
-		              "Driver need to name itself in debugfs!");
-
-		read(devname, searchname, sizeof(searchname));
-		close(devname);
-
-		foundname = strtok_r(searchname, " ", &ctx);
-		igt_require_f(foundname,
-		              "Driver need to name itself in debugfs!");
-
-		snprintf(searchpath, PATH_MAX, "%s_params", foundname);
-		params = openat(dir, searchpath, O_RDONLY);
-
-		if (params >= 0) {
-			char *debugfspath = malloc(PATH_MAX);
-
-			igt_debugfs_path(device, debugfspath, PATH_MAX);
-			if (param != NULL) {
-				char filepath[PATH_MAX];
-
-				snprintf(filepath, PATH_MAX, "%s/%s",
-					 debugfspath, param);
-
-				if (stat(filepath, &buffer) == 0) {
-					if (outpath != NULL)
-						*outpath = debugfspath;
-					else
-						free(debugfspath);
-				} else {
-					free(debugfspath);
-					close(params);
-					params = -1;
-				}
-			} else if (outpath != NULL) {
-				/*
-				 * Caller is responsible to free this.
-				 */
-				*outpath = debugfspath;
-			} else {
-				free(debugfspath);
-			}
-		}
+		snprintf(path, PATH_MAX, "%s_params", name);
+		params = openat(dir, path, O_RDONLY);
 		close(dir);
 	}
 
 	if (params < 0) { /* builtin? */
-		drm_version_t version;
-		char name[32] = "";
-		char path[PATH_MAX];
-
-		if (device == -1) {
-			/*
-			 * find default device
-			 */
-			int file, i;
-			const char *debugfs_root = igt_debugfs_mount();
-
-			igt_assert(debugfs_root);
-
-			for (i = 0; i < 63; i++) {
-				char testpath[PATH_MAX];
-
-				snprintf(searchpath, PATH_MAX,
-					 "%s/dri/%d/name", debugfs_root, i);
-
-				file = open(searchpath, O_RDONLY);
-
-				if (file < 0)
-					continue;
-
-				read(file, searchname, sizeof(searchname));
-				close(file);
-
-				foundname = strtok_r(searchname, " ", &ctx);
-				if (!foundname)
-					continue;
-
-				snprintf(testpath, PATH_MAX,
-					 "/sys/module/%s/parameters",
-					 foundname);
-
-				if (stat(testpath, &buffer) == 0 &&
-				    S_ISDIR(buffer.st_mode)) {
-					snprintf(name, sizeof(name), "%s",
-						 foundname);
-					break;
-				}
-			}
-		} else {
-			memset(&version, 0, sizeof(version));
-			version.name_len = sizeof(name);
-			version.name = name;
-			ioctl(device, DRM_IOCTL_VERSION, &version);
-		}
 		snprintf(path, sizeof(path), "/sys/module/%s/parameters", name);
 		params = open(path, O_RDONLY);
-		if (params >= 0 && outpath)
-			*outpath = strdup(path);
 	}
 
 	return params;
 }
 
-/**
- * igt_params_open:
- * @device: fd of the device
- *
- * This opens the module parameters directory (under sysfs) corresponding
- * to the device for use with igt_sysfs_set() and igt_sysfs_get().
- *
- * Returns:
- * The directory fd, or -1 on failure.
- */
-int igt_params_open(int device)
-{
-	return __igt_params_open(device, NULL, NULL);
-}
-
 __attribute__((format(printf, 3, 0)))
 static bool __igt_params_set(int device, const char *parameter,
 			     const char *fmt, va_list ap, bool save)
 {
-	char *path = NULL;
 	int dir;
 	int ret;
 
-	dir = __igt_params_open(device, save ? &path : NULL, parameter);
+	dir = igt_params_open(device);
 	if (dir < 0)
 		return false;
 
 	if (save)
-		igt_params_save(dir, path, parameter);
+		igt_params_save(dir, parameter);
 
 	ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
-
 	close(dir);
-	free(path);
 
 	return ret > 0;
 }
-- 
2.26.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx