All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: igt-dev@lists.freedesktop.org
Cc: Jani Nikula <jani.nikula@intel.com>,
	intel-gfx@lists.freedesktop.org,
	Chris Wilson <chris@chris-wilson.co.uk>
Subject: [Intel-gfx] [PATCH i-g-t 2/2] lib: Cleanup __igt_params_open()
Date: Tue, 19 May 2020 00:32:40 +0100	[thread overview]
Message-ID: <20200518233240.1540362-2-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <20200518233240.1540362-1-chris@chris-wilson.co.uk>

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 3decc5b2a..c06416988 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

WARNING: multiple messages have this Message-ID (diff)
From: Chris Wilson <chris@chris-wilson.co.uk>
To: igt-dev@lists.freedesktop.org
Cc: Jani Nikula <jani.nikula@intel.com>,
	intel-gfx@lists.freedesktop.org,
	Chris Wilson <chris@chris-wilson.co.uk>
Subject: [igt-dev] [PATCH i-g-t 2/2] lib: Cleanup __igt_params_open()
Date: Tue, 19 May 2020 00:32:40 +0100	[thread overview]
Message-ID: <20200518233240.1540362-2-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <20200518233240.1540362-1-chris@chris-wilson.co.uk>

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 3decc5b2a..c06416988 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

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  reply	other threads:[~2020-05-18 23:33 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-18 23:32 [Intel-gfx] [PATCH i-g-t 1/2] Always pass device to igt_params_set Chris Wilson
2020-05-18 23:32 ` [igt-dev] " Chris Wilson
2020-05-18 23:32 ` Chris Wilson [this message]
2020-05-18 23:32   ` [igt-dev] [PATCH i-g-t 2/2] lib: Cleanup __igt_params_open() Chris Wilson
2020-05-19  0:21 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/2] Always pass device to igt_params_set Patchwork
2020-05-19  2:27 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200518233240.1540362-2-chris@chris-wilson.co.uk \
    --to=chris@chris-wilson.co.uk \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.