All of lore.kernel.org
 help / color / mirror / Atom feed
* [i-g-t PATCH v3 1/3] lib/igt_core: Add igt_system helpers
@ 2017-06-06  8:54 Abdiel Janulgue
  2017-06-06  8:54 ` [i-g-t PATCH 2/3] igt/igt_core: Provide an option to check for the log buffer contents Abdiel Janulgue
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Abdiel Janulgue @ 2017-06-06  8:54 UTC (permalink / raw)
  To: intel-gfx

Support executing external processes with the goal of capturing its
standard streams to the igt logging infrastructure in addition to its
exit status.

v3: Rename igt_exec -> igt_system (Chris).

v2: Fix leaks on fd teardown. Make sure redirected process printout when
    > 64kb still works, like full dmesg. (Petri).

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 lib/igt_core.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_core.h |  10 ++++
 2 files changed, 167 insertions(+)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 9c3b37f..26f7fc5 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2099,3 +2099,160 @@ FILE *__igt_fopen_data(const char* igt_srcdir, const char* igt_datadir,
 
 	return fp;
 }
+
+struct output_pipe {
+	int output_fd;
+	int save_fd;
+	int read_fd;
+	int write_fd;
+	bool redirected;
+	enum igt_log_level log_level;
+};
+
+static bool redirect_output(struct output_pipe *p, int output_fd,
+			    enum igt_log_level level)
+{
+	int fds[2];
+
+	if (pipe(fds) == -1)
+		goto err;
+
+	/* save output */
+	if ((p->save_fd = dup(output_fd)) == -1)
+		goto err;
+
+	/* Redirect output to our buffer */
+	if (dup2(fds[1], output_fd) == -1)
+		goto err;
+
+	p->output_fd = output_fd;
+	p->read_fd = fds[0];
+	p->write_fd = fds[1];
+	p->redirected = true;
+	p->log_level = level;
+
+	return true;
+err:
+	close(fds[0]);
+	close(fds[1]);
+	close(p->save_fd);
+
+	return false;
+}
+
+static void unredirect_output(struct output_pipe *p)
+{
+	if (!p->redirected)
+		return;
+
+	/* read_fd is closed separately. We still need to read its
+	 * buffered contents after un-redirecting the stream.
+	 */
+	close(p->write_fd);
+	dup2(p->save_fd, p->output_fd);
+	close(p->save_fd);
+	p->redirected = false;
+}
+
+/**
+ * igt_system:
+ *
+ * An improved replacement of the system() call.
+ *
+ * Executes the shell command specified in @command with the added feature of
+ * concurrently capturing its stdout and stderr to igt_log and igt_warn
+ * respectively.
+ *
+ * Returns: The exit status of the executed process. -1 for failure.
+ */
+int igt_system(const char *command)
+{
+#define OUT 0
+#define ERR 1
+	struct output_pipe op[2];
+	int i, status;
+	struct igt_helper_process process = {};
+	char buf[PIPE_BUF];
+
+	if (!redirect_output(&op[OUT], STDOUT_FILENO, IGT_LOG_INFO))
+		goto err;
+	if (!redirect_output(&op[ERR], STDERR_FILENO, IGT_LOG_WARN))
+		goto err;
+
+	igt_fork_helper(&process) {
+		igt_assert(execl("/bin/sh", "sh", "-c", command,
+				 (char *) NULL) != -1);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(op); i++) {
+		struct output_pipe *current = &op[i];
+
+		/* Unredirect so igt_log() works */
+		unredirect_output(current);
+		memset(buf, 0, sizeof(buf));
+		while (read(current->read_fd, buf, sizeof(buf)) > 0) {
+			igt_log(IGT_LOG_DOMAIN, current->log_level,
+				"[cmd] %s", buf);
+			memset(buf, 0, sizeof(buf));
+		}
+		close(current->read_fd);
+	}
+	status = igt_wait_helper(&process);
+
+	return WEXITSTATUS(status);
+err:
+	/* Failed to redirect one or both streams. Roll back changes. */
+	for (i = 0; i < ARRAY_SIZE(op); i++) {
+		if (!op[i].redirected)
+			continue;
+		close(op[i].read_fd);
+		unredirect_output(&op[i]);
+	}
+
+	return -1;
+}
+
+/**
+ * igt_system_quiet:
+ * Similar to igt_system(), except redirect output to /dev/null
+ *
+ * Returns: The exit status of the executed process. -1 for failure.
+ */
+int igt_system_quiet(const char *command)
+{
+	int stderr_fd_copy, stdout_fd_copy, status, nullfd;
+
+	/* redirect */
+	if ((nullfd = open("/dev/null", O_WRONLY)) == -1)
+		goto err;
+	if ((stdout_fd_copy = dup(STDOUT_FILENO)) == -1)
+		goto err;
+	if ((stderr_fd_copy = dup(STDERR_FILENO)) == -1)
+		goto err;
+
+	if (dup2(nullfd, STDOUT_FILENO) == -1)
+		goto err;
+	if (dup2(nullfd, STDERR_FILENO) == -1)
+		goto err;
+
+	if ((status = system(command)) == -1)
+		goto err;
+
+	/* restore */
+	if (dup2(stdout_fd_copy, STDOUT_FILENO) == -1)
+		goto err;
+	if (dup2(stderr_fd_copy, STDERR_FILENO) == -1)
+		goto err;
+
+	close(stdout_fd_copy);
+	close(stderr_fd_copy);
+	close(nullfd);
+
+	return WEXITSTATUS(status);
+err:
+	close(stderr_fd_copy);
+	close(stdout_fd_copy);
+	close(nullfd);
+
+	return -1;
+}
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 4a125af..1855c5e 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -919,4 +919,14 @@ FILE *__igt_fopen_data(const char* igt_srcdir, const char* igt_datadir,
 #define igt_fopen_data(filename) \
 	__igt_fopen_data(IGT_SRCDIR, IGT_DATADIR, filename)
 
+int igt_system(const char *command);
+int igt_system_quiet(const char *command);
+#define igt_system_cmd(status, format...) \
+	do { \
+		char *buf = 0; \
+		igt_assert(asprintf(&buf, format) != -1); \
+	        status = igt_system(buf); \
+		free(buf); \
+	} while (0)
+
 #endif /* IGT_CORE_H */
-- 
2.7.4

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

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

* [i-g-t PATCH 2/3] igt/igt_core: Provide an option to check for the log buffer contents
  2017-06-06  8:54 [i-g-t PATCH v3 1/3] lib/igt_core: Add igt_system helpers Abdiel Janulgue
@ 2017-06-06  8:54 ` Abdiel Janulgue
  2017-06-12 11:48   ` Arkadiusz Hiler
  2017-06-06  8:54 ` [i-g-t PATCH v3 3/3] Convert shell script tests to C version Abdiel Janulgue
  2017-06-12 12:37 ` [i-g-t PATCH v3 1/3] lib/igt_core: Add igt_system helpers Arkadiusz Hiler
  2 siblings, 1 reply; 7+ messages in thread
From: Abdiel Janulgue @ 2017-06-06  8:54 UTC (permalink / raw)
  To: intel-gfx

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 lib/igt_core.c | 24 ++++++++++++++++++++++++
 lib/igt_core.h |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 26f7fc5..94f5529 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -329,6 +329,30 @@ static void _igt_log_buffer_dump(void)
 	pthread_mutex_unlock(&log_buffer_mutex);
 }
 
+/**
+ * igt_log_buffer_inspect:
+ *
+ * Provides a way to replay the internal igt log buffer for inspection.
+ * @check: A user-specified handler that gets invoked for each line of
+           the log buffer. The handler should return true to stop
+           inspecting the rest of the buffer.
+ * @data: passed as a user argument to the inspection function.
+ */
+void igt_log_buffer_inspect(igt_buffer_log_handler_t check, void *data)
+{
+	uint8_t i;
+	pthread_mutex_lock(&log_buffer_mutex);
+
+	i = log_buffer.start;
+	do {
+		if (check(log_buffer.entries[i], data))
+			break;
+		i++;
+	} while (i != log_buffer.start && i != log_buffer.end);
+
+	pthread_mutex_unlock(&log_buffer_mutex);
+}
+
 __attribute__((format(printf, 1, 2)))
 static void kmsg(const char *format, ...)
 #define KERN_EMER	"<0>"
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 1855c5e..a2ed972 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -818,6 +818,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
  */
 #define igt_critical(f...) igt_log(IGT_LOG_DOMAIN, IGT_LOG_CRITICAL, f)
 
+typedef bool (*igt_buffer_log_handler_t)(const char *line, void *data);
+void igt_log_buffer_inspect(igt_buffer_log_handler_t check, void *data);
+
 extern enum igt_log_level igt_log_level;
 
 /**
-- 
2.7.4

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

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

* [i-g-t PATCH v3 3/3] Convert shell script tests to C version
  2017-06-06  8:54 [i-g-t PATCH v3 1/3] lib/igt_core: Add igt_system helpers Abdiel Janulgue
  2017-06-06  8:54 ` [i-g-t PATCH 2/3] igt/igt_core: Provide an option to check for the log buffer contents Abdiel Janulgue
@ 2017-06-06  8:54 ` Abdiel Janulgue
  2017-06-12 11:14   ` Arkadiusz Hiler
  2017-06-12 12:37 ` [i-g-t PATCH v3 1/3] lib/igt_core: Add igt_system helpers Arkadiusz Hiler
  2 siblings, 1 reply; 7+ messages in thread
From: Abdiel Janulgue @ 2017-06-06  8:54 UTC (permalink / raw)
  To: intel-gfx

v3: Drop redundant test covered by drv_hangman/basic. Descend thru
    debugfs path when reading sysfs entries (Chris).

v2: Use internal igt_debugfs functions instead of cat and document
    debugfs tests.
    Convert sysfs_l3_parity properly.
    Rename redundant names in tests.

Converted:
 - check_drm_clients (ensures no other clients are running.
   functionality provided by drm_open_driver_master).
 - debugfs_emon_crash
 - debugfs_wedged
 - drv_debugfs_reader
 - sysfs_l3_parity
 - test_rte_check  (same as check_drm_clients)
 - tools_test
 - ZZ_check_dmesg

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 tests/Makefile.sources   |   9 +---
 tests/ZZ_check_dmesg     |  11 -----
 tests/check_drm_clients  |   6 ---
 tests/debugfs.c          |  96 +++++++++++++++++++++++++++++++++++++
 tests/debugfs_emon_crash |  16 -------
 tests/debugfs_wedged     |  10 ----
 tests/drv_debugfs_reader |   9 ----
 tests/sysfs_l3_parity    |  22 ---------
 tests/test_rte_check     |   6 ---
 tests/tools.c            | 122 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/tools_test         |  16 -------
 11 files changed, 220 insertions(+), 103 deletions(-)
 delete mode 100755 tests/ZZ_check_dmesg
 delete mode 100755 tests/check_drm_clients
 create mode 100644 tests/debugfs.c
 delete mode 100755 tests/debugfs_emon_crash
 delete mode 100755 tests/debugfs_wedged
 delete mode 100755 tests/drv_debugfs_reader
 delete mode 100755 tests/sysfs_l3_parity
 delete mode 100755 tests/test_rte_check
 create mode 100644 tests/tools.c
 delete mode 100755 tests/tools_test

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 9553e4d..c4a78a9 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -243,6 +243,8 @@ TESTS_progs = \
 	drv_module_reload \
 	kms_sysfs_edid_timing \
 	perf \
+	debugfs \
+	tools \
 	$(NULL)
 
 # IMPORTANT: The ZZ_ tests need to be run last!
@@ -251,11 +253,6 @@ TESTS_scripts_M = \
 	$(NULL)
 
 TESTS_scripts = \
-	debugfs_emon_crash \
-	drv_debugfs_reader \
-	sysfs_l3_parity \
-	test_rte_check \
-	tools_test \
 	$(NULL)
 
 # This target contains testcases which support automagic subtest enumeration
@@ -317,9 +314,7 @@ HANG = \
 	$(NULL)
 
 scripts = \
-	check_drm_clients \
 	ddx_intel_after_fbdev \
-	debugfs_wedged \
 	drm_lib.sh \
 	drm_getopt.sh \
 	$(NULL)
diff --git a/tests/ZZ_check_dmesg b/tests/ZZ_check_dmesg
deleted file mode 100755
index e28ba35..0000000
--- a/tests/ZZ_check_dmesg
+++ /dev/null
@@ -1,11 +0,0 @@
-#!/bin/sh
-
-if dmesg | grep '\*ERROR\*'  > /dev/null ; then
-	echo "DRM_ERROR dirt in dmesg"
-	exit 1
-fi
-
-if dmesg | grep -- '------\[ cut here \]----' > /dev/null  ; then
-	echo "found a backtrace in dmesg"
-	exit 1
-fi
diff --git a/tests/check_drm_clients b/tests/check_drm_clients
deleted file mode 100755
index 2a891b8..0000000
--- a/tests/check_drm_clients
+++ /dev/null
@@ -1,6 +0,0 @@
-#!/bin/bash
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-exit $IGT_EXIT_SUCCESS
diff --git a/tests/debugfs.c b/tests/debugfs.c
new file mode 100644
index 0000000..b9ae86c
--- /dev/null
+++ b/tests/debugfs.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include "igt.h"
+#include "igt_sysfs.h"
+#include <fcntl.h>
+#include <sys/types.h>
+#include <dirent.h>
+
+static void get_sysfs_entry(int path_fd)
+{
+	struct dirent *dirent;
+	DIR *dir;
+
+	dir = fdopendir(path_fd);
+	if (!dir)
+		return;
+
+	while ((dirent = readdir(dir))) {
+		if (!strcmp(dirent->d_name, ".") ||
+		    !strcmp(dirent->d_name, ".."))
+			continue;
+		if (dirent->d_type == DT_DIR) {
+			int sub_fd = -1;
+			igt_assert((sub_fd =
+				    openat(path_fd, dirent->d_name, O_RDONLY |
+					   O_DIRECTORY)) > 0);
+			get_sysfs_entry(sub_fd);
+			close(sub_fd);
+		} else {
+			char *buf = igt_sysfs_get(path_fd, dirent->d_name);
+			free(buf);
+		}
+	}
+	closedir(dir);
+}
+
+igt_main
+{
+	int fd = -1, debugfs;
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		fd = drm_open_driver_master(DRIVER_INTEL);
+		igt_require_gem(fd);
+		debugfs = igt_debugfs_dir(fd);
+	}
+
+	igt_subtest("drv_reader") {
+		get_sysfs_entry(debugfs);
+	}
+
+	igt_subtest("emon_crash") {
+		int i;
+		/*
+		 * This check if we can crash the kernel with
+		 * segmentation-fault by reading
+		 * /sys/kernel/debug/dri/0/i915_emon_status too quickly
+		 */
+		for (i = 0; i < 1000; i++) {
+			char *buf = igt_sysfs_get(debugfs,
+						  "i915_emon_status");
+			free(buf);
+		}
+
+		/* If we got here, we haven't crashed */
+		igt_success();
+	}
+
+	igt_fixture {
+		close(debugfs);
+		close(fd);
+	}
+}
diff --git a/tests/debugfs_emon_crash b/tests/debugfs_emon_crash
deleted file mode 100755
index 1dbfcb2..0000000
--- a/tests/debugfs_emon_crash
+++ /dev/null
@@ -1,16 +0,0 @@
-#!/bin/bash
-#
-# This check if we can crash the kernel with segmentation-fault
-# by reading /sys/kernel/debug/dri/0/i915_emon_status too quickly
-#
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-for z in $(seq 1 1000); do
-	cat $i915_dfs_path/i915_emon_status > /dev/null 2&>1
-done
-
-# If we got here, we haven't crashed
-
-exit $IGT_EXIT_SUCCESS
diff --git a/tests/debugfs_wedged b/tests/debugfs_wedged
deleted file mode 100755
index f15ac46..0000000
--- a/tests/debugfs_wedged
+++ /dev/null
@@ -1,10 +0,0 @@
-#!/bin/bash
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-# Testcase: wedge the hw to check the error_state reading
-# 
-# Unfortunately wedged is permanent, so this test is not run by default
-echo 1 > ${i915_dfs_path}/i915_wedged
-cat $i915_dfs_path/i915_error_state > /dev/null 2>&1
diff --git a/tests/drv_debugfs_reader b/tests/drv_debugfs_reader
deleted file mode 100755
index 6ea4e64..0000000
--- a/tests/drv_debugfs_reader
+++ /dev/null
@@ -1,9 +0,0 @@
-#!/bin/bash
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-# read everything we can
-cat $i915_dfs_path/* > /dev/null 2>&1
-
-exit $IGT_EXIT_SUCCESS
diff --git a/tests/sysfs_l3_parity b/tests/sysfs_l3_parity
deleted file mode 100755
index d5f3284..0000000
--- a/tests/sysfs_l3_parity
+++ /dev/null
@@ -1,22 +0,0 @@
-#!/bin/bash
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -e || exit $?
-
-#Check that we can remap a row
-$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -d
-disabled=`$SOURCE_DIR/../tools/intel_l3_parity -l | grep -c 'Row 0, Bank 0, Subbank 0 is disabled'`
-if [ "$disabled" != "1" ] ; then
-	echo "Fail"
-	exit $IGT_EXIT_FAILURE
-fi
-
-$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -e
-
-#Check that we can clear remaps
-if [ `$SOURCE_DIR/../tools/intel_l3_parity -l | wc -l` != 1 ] ; then
-	echo "Fail 2"
-	exit $IGT_EXIT_FAILURE
-fi
diff --git a/tests/test_rte_check b/tests/test_rte_check
deleted file mode 100755
index 2a891b8..0000000
--- a/tests/test_rte_check
+++ /dev/null
@@ -1,6 +0,0 @@
-#!/bin/bash
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-exit $IGT_EXIT_SUCCESS
diff --git a/tests/tools.c b/tests/tools.c
new file mode 100644
index 0000000..2ec951a
--- /dev/null
+++ b/tests/tools.c
@@ -0,0 +1,122 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include "igt.h"
+#include <stdio.h>
+
+/**
+ * Parse the r-value of a [cmd] string.
+ */
+static bool check_cmd_return_value(const char *s, void *data)
+{
+	int *val = data;
+	char *cmd, *found;
+	const char *delim = "[cmd]";
+	const int delim_len = strlen(delim);
+
+	if (!(cmd = strstr(s, delim)))
+		return false;
+
+	found = cmd + delim_len + 1;
+	igt_assert(delim_len + strlen(found) < strlen(cmd));
+
+	*val = atoi(found);
+	return true;
+}
+
+igt_main
+{
+	int fd = -1;
+	igt_skip_on_simulation();
+
+	igt_subtest("sysfs_l3_parity") {
+		int exec_return;
+
+		igt_system_cmd(exec_return,
+			       "../tools/intel_l3_parity -r 0 -b 0 "
+			       "-s 0 -e");
+		igt_assert(exec_return == IGT_EXIT_SUCCESS);
+
+		igt_system_cmd(exec_return,
+			       "../tools/intel_l3_parity -l | "
+			       "grep -c 'Row 0, Bank 0, Subbank 0 "
+			       "is disabled'");
+		if (exec_return == IGT_EXIT_SUCCESS) {
+			int val = -1;
+			igt_log_buffer_inspect(check_cmd_return_value,
+					       &val);
+			igt_assert(val == 1);
+		} else {
+			igt_fail(IGT_EXIT_FAILURE);
+		}
+
+		igt_system_cmd(exec_return,
+			       "../tools/intel_l3_parity -r 0 -b 0 "
+			       "-s 0 -e");
+		igt_assert(exec_return == IGT_EXIT_SUCCESS);
+
+		/* Check that we can clear remaps */
+		igt_system_cmd(exec_return,
+			       "../tools/intel_l3_parity -l | "
+			       "wc -l");
+		if (exec_return == IGT_EXIT_SUCCESS) {
+			int val = -1;
+			igt_log_buffer_inspect(check_cmd_return_value,
+					       &val);
+			igt_assert(val == 1);
+		} else {
+			igt_fail(IGT_EXIT_FAILURE);
+		}
+	}
+
+	igt_subtest("tools_test") {
+		char *cmd;
+
+		igt_assert(asprintf(&cmd,
+				    "../tools/intel_reg read 0x4030")
+			   != -1);
+		igt_assert(igt_system_quiet(cmd) == IGT_EXIT_SUCCESS);
+		free(cmd);
+
+		igt_assert(asprintf(&cmd, "../tools/intel_reg dump")
+			   != -1);
+		igt_assert(igt_system_quiet(cmd) == IGT_EXIT_SUCCESS);
+		free(cmd);
+	}
+
+	igt_subtest("check_dmesg") {
+		char *cmd;
+		igt_assert(asprintf(&cmd, "dmesg | grep '\\*ERROR\\*'")
+			   != -1);
+		igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
+		free(cmd);
+
+		igt_assert(asprintf(&cmd, "dmesg | grep "
+				    "-- '------\\[ cut here \\]----'")
+			   != -1);
+		igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
+		free(cmd);
+	}
+}
diff --git a/tests/tools_test b/tests/tools_test
deleted file mode 100755
index a27fb87..0000000
--- a/tests/tools_test
+++ /dev/null
@@ -1,16 +0,0 @@
-#!/bin/bash
-# Test some of the most critical tools we have accidentally broken before.
-# TODO: Possibly make tests parse output
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-# ARB_MODE has existed for many gens
-PATH=$SOURCE_DIR/../tools:$PATH
-do_or_die "intel_reg read 0x4030"
-do_or_die "intel_reg dump"
-
-# TODO: Add more tests
-
-exit $IGT_EXIT_SUCCESS
-
-- 
2.7.4

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

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

* Re: [i-g-t PATCH v3 3/3] Convert shell script tests to C version
  2017-06-06  8:54 ` [i-g-t PATCH v3 3/3] Convert shell script tests to C version Abdiel Janulgue
@ 2017-06-12 11:14   ` Arkadiusz Hiler
  2017-06-13 10:22     ` Abdiel Janulgue
  0 siblings, 1 reply; 7+ messages in thread
From: Arkadiusz Hiler @ 2017-06-12 11:14 UTC (permalink / raw)
  To: Abdiel Janulgue; +Cc: intel-gfx

On Tue, Jun 06, 2017 at 11:54:14AM +0300, Abdiel Janulgue wrote:
> v3: Drop redundant test covered by drv_hangman/basic. Descend thru
>     debugfs path when reading sysfs entries (Chris).
> 
> v2: Use internal igt_debugfs functions instead of cat and document
>     debugfs tests.
>     Convert sysfs_l3_parity properly.
>     Rename redundant names in tests.
> 
> Converted:
>  - check_drm_clients (ensures no other clients are running.
>    functionality provided by drm_open_driver_master).
>  - debugfs_emon_crash
>  - debugfs_wedged
>  - drv_debugfs_reader
>  - sysfs_l3_parity
>  - test_rte_check  (same as check_drm_clients)
>  - tools_test
>  - ZZ_check_dmesg

Hey,

Doing each tool in a separate commit would help with readability.

> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> ---
>  tests/Makefile.sources   |   9 +---
>  tests/ZZ_check_dmesg     |  11 -----
>  tests/check_drm_clients  |   6 ---
>  tests/debugfs.c          |  96 +++++++++++++++++++++++++++++++++++++
>  tests/debugfs_emon_crash |  16 -------
>  tests/debugfs_wedged     |  10 ----
>  tests/drv_debugfs_reader |   9 ----
>  tests/sysfs_l3_parity    |  22 ---------
>  tests/test_rte_check     |   6 ---
>  tests/tools.c            | 122 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/tools_test         |  16 -------
>  11 files changed, 220 insertions(+), 103 deletions(-)
>  delete mode 100755 tests/ZZ_check_dmesg
>  delete mode 100755 tests/check_drm_clients
>  create mode 100644 tests/debugfs.c
>  delete mode 100755 tests/debugfs_emon_crash
>  delete mode 100755 tests/debugfs_wedged
>  delete mode 100755 tests/drv_debugfs_reader
>  delete mode 100755 tests/sysfs_l3_parity
>  delete mode 100755 tests/test_rte_check
>  create mode 100644 tests/tools.c
>  delete mode 100755 tests/tools_test
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 9553e4d..c4a78a9 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -243,6 +243,8 @@ TESTS_progs = \
>  	drv_module_reload \
>  	kms_sysfs_edid_timing \
>  	perf \
> +	debugfs \

The name clashes on AOSP build:
build/core/base_rules.mk:238: error: external/igt/tests: MODULE.TARGET.EXECUTABLES.debugfs already defined by external/e2fsprogs/debugfs.

debugfs is a name of a binary that debugs ext{2,3,4} family of FSes.

I have to look into the build system and our Android.mks more deeply as
IGTs output binaries should be stored elsewhere and the clash is rather
enigmatic.

Also are we changing names of the test? I thought it was only a rewrite.

> +	tools \
>  	$(NULL)
>  
>  # IMPORTANT: The ZZ_ tests need to be run last!
> @@ -251,11 +253,6 @@ TESTS_scripts_M = \
>  	$(NULL)
>  
>  TESTS_scripts = \
> -	debugfs_emon_crash \
> -	drv_debugfs_reader \
> -	sysfs_l3_parity \
> -	test_rte_check \
> -	tools_test \
>  	$(NULL)

TESTS_scripts now is only $(NULL), so you may as well just remove it
from the `kernel_tests` variable and get rid of it completely.

>  
>  # This target contains testcases which support automagic subtest enumeration
> @@ -317,9 +314,7 @@ HANG = \
>  	$(NULL)
>  
>  scripts = \
> -	check_drm_clients \
>  	ddx_intel_after_fbdev \
> -	debugfs_wedged \
>  	drm_lib.sh \
>  	drm_getopt.sh \
>  	$(NULL)

<SNIP>

> diff --git a/tests/debugfs.c b/tests/debugfs.c
> new file mode 100644
> index 0000000..b9ae86c
> --- /dev/null
> +++ b/tests/debugfs.c
> @@ -0,0 +1,96 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +#include "igt.h"
> +#include "igt_sysfs.h"
> +#include <fcntl.h>
> +#include <sys/types.h>
> +#include <dirent.h>
> +
> +static void get_sysfs_entry(int path_fd)

I was a little bit confused by the function - we are not getting
anything from it. Just reading and discarding.

Maybe read_and_discard_sysfs_entry? A comment would also do.

> +{
> +	struct dirent *dirent;
> +	DIR *dir;
> +
> +	dir = fdopendir(path_fd);
> +	if (!dir)
> +		return;
> +
> +	while ((dirent = readdir(dir))) {
> +		if (!strcmp(dirent->d_name, ".") ||
> +		    !strcmp(dirent->d_name, ".."))
> +			continue;
> +		if (dirent->d_type == DT_DIR) {
> +			int sub_fd = -1;
> +			igt_assert((sub_fd =
> +				    openat(path_fd, dirent->d_name, O_RDONLY |
> +					   O_DIRECTORY)) > 0);
> +			get_sysfs_entry(sub_fd);
> +			close(sub_fd);
> +		} else {
> +			char *buf = igt_sysfs_get(path_fd, dirent->d_name);

igt_sysfs_get may get us NULL.
Shouldn't we assert on that? It's an error-worthy.

Also we may end up trying to free it in with the free below.

> +			free(buf);
> +		}
> +	}
> +	closedir(dir);
> +}
> +
> +igt_main
> +{
> +	int fd = -1, debugfs;
> +	igt_skip_on_simulation();
> +
> +	igt_fixture {
> +		fd = drm_open_driver_master(DRIVER_INTEL);
> +		igt_require_gem(fd);
> +		debugfs = igt_debugfs_dir(fd);
> +	}
> +
> +	igt_subtest("drv_reader") {
> +		get_sysfs_entry(debugfs);
> +	}
> +
> +	igt_subtest("emon_crash") {
> +		int i;
> +		/*
> +		 * This check if we can crash the kernel with
> +		 * segmentation-fault by reading
> +		 * /sys/kernel/debug/dri/0/i915_emon_status too quickly
> +		 */
> +		for (i = 0; i < 1000; i++) {
> +			char *buf = igt_sysfs_get(debugfs,
> +						  "i915_emon_status");
> +			free(buf);
> +		}
> +
> +		/* If we got here, we haven't crashed */
> +		igt_success();
> +	}
> +
> +	igt_fixture {
> +		close(debugfs);
> +		close(fd);
> +	}
> +}

<SNIP>

> diff --git a/tests/tools.c b/tests/tools.c
> new file mode 100644
> index 0000000..2ec951a
> --- /dev/null
> +++ b/tests/tools.c
> @@ -0,0 +1,122 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +#include "igt.h"
> +#include <stdio.h>
> +
> +/**
> + * Parse the r-value of a [cmd] string.
> + */
> +static bool check_cmd_return_value(const char *s, void *data)
> +{
> +	int *val = data;
> +	char *cmd, *found;
> +	const char *delim = "[cmd]";
> +	const int delim_len = strlen(delim);
> +
> +	if (!(cmd = strstr(s, delim)))
> +		return false;
> +
> +	found = cmd + delim_len + 1;
> +	igt_assert(delim_len + strlen(found) < strlen(cmd));
> +
> +	*val = atoi(found);
> +	return true;
> +}
> +
> +igt_main
> +{
> +	int fd = -1;
> +	igt_skip_on_simulation();
> +
> +	igt_subtest("sysfs_l3_parity") {
> +		int exec_return;
> +
> +		igt_system_cmd(exec_return,
> +			       "../tools/intel_l3_parity -r 0 -b 0 "
> +			       "-s 0 -e");
> +		igt_assert(exec_return == IGT_EXIT_SUCCESS);
> +
> +		igt_system_cmd(exec_return,
> +			       "../tools/intel_l3_parity -l | "
> +			       "grep -c 'Row 0, Bank 0, Subbank 0 "
> +			       "is disabled'");
> +		if (exec_return == IGT_EXIT_SUCCESS) {
> +			int val = -1;
> +			igt_log_buffer_inspect(check_cmd_return_value,
> +					       &val);
> +			igt_assert(val == 1);
> +		} else {
> +			igt_fail(IGT_EXIT_FAILURE);
> +		}
> +
> +		igt_system_cmd(exec_return,
> +			       "../tools/intel_l3_parity -r 0 -b 0 "
> +			       "-s 0 -e");
> +		igt_assert(exec_return == IGT_EXIT_SUCCESS);
> +
> +		/* Check that we can clear remaps */
> +		igt_system_cmd(exec_return,
> +			       "../tools/intel_l3_parity -l | "
> +			       "wc -l");
> +		if (exec_return == IGT_EXIT_SUCCESS) {
> +			int val = -1;
> +			igt_log_buffer_inspect(check_cmd_return_value,
> +					       &val);
> +			igt_assert(val == 1);
> +		} else {
> +			igt_fail(IGT_EXIT_FAILURE);
> +		}
> +	}
> +
> +	igt_subtest("tools_test") {
> +		char *cmd;
> +
> +		igt_assert(asprintf(&cmd,
> +				    "../tools/intel_reg read 0x4030")
> +			   != -1);
> +		igt_assert(igt_system_quiet(cmd) == IGT_EXIT_SUCCESS);
> +		free(cmd);
> +
> +		igt_assert(asprintf(&cmd, "../tools/intel_reg dump")
> +			   != -1);
> +		igt_assert(igt_system_quiet(cmd) == IGT_EXIT_SUCCESS);
> +		free(cmd);
> +	}
> +
> +	igt_subtest("check_dmesg") {
> +		char *cmd;
> +		igt_assert(asprintf(&cmd, "dmesg | grep '\\*ERROR\\*'")
> +			   != -1);
> +		igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
> +		free(cmd);
> +
> +		igt_assert(asprintf(&cmd, "dmesg | grep "
> +				    "-- '------\\[ cut here \\]----'")
> +			   != -1);
> +		igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
> +		free(cmd);

Are we testing the dmesg here?

If not they why systeming out "dmesg | grep" instead of going through
/dev/kmsg?

> +	}
> +}

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

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

* Re: [i-g-t PATCH 2/3] igt/igt_core: Provide an option to check for the log buffer contents
  2017-06-06  8:54 ` [i-g-t PATCH 2/3] igt/igt_core: Provide an option to check for the log buffer contents Abdiel Janulgue
@ 2017-06-12 11:48   ` Arkadiusz Hiler
  0 siblings, 0 replies; 7+ messages in thread
From: Arkadiusz Hiler @ 2017-06-12 11:48 UTC (permalink / raw)
  To: Abdiel Janulgue; +Cc: intel-gfx

On Tue, Jun 06, 2017 at 11:54:13AM +0300, Abdiel Janulgue wrote:
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> ---
>  lib/igt_core.c | 24 ++++++++++++++++++++++++
>  lib/igt_core.h |  3 +++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 26f7fc5..94f5529 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -329,6 +329,30 @@ static void _igt_log_buffer_dump(void)
>  	pthread_mutex_unlock(&log_buffer_mutex);
>  }
>  
> +/**
> + * igt_log_buffer_inspect:
> + *
> + * Provides a way to replay the internal igt log buffer for inspection.
> + * @check: A user-specified handler that gets invoked for each line of
> +           the log buffer. The handler should return true to stop
> +           inspecting the rest of the buffer.

Looks like we have an asterisk even for the indented lines elsewhere.

With the above nitpick
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>


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

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

* Re: [i-g-t PATCH v3 1/3] lib/igt_core: Add igt_system helpers
  2017-06-06  8:54 [i-g-t PATCH v3 1/3] lib/igt_core: Add igt_system helpers Abdiel Janulgue
  2017-06-06  8:54 ` [i-g-t PATCH 2/3] igt/igt_core: Provide an option to check for the log buffer contents Abdiel Janulgue
  2017-06-06  8:54 ` [i-g-t PATCH v3 3/3] Convert shell script tests to C version Abdiel Janulgue
@ 2017-06-12 12:37 ` Arkadiusz Hiler
  2 siblings, 0 replies; 7+ messages in thread
From: Arkadiusz Hiler @ 2017-06-12 12:37 UTC (permalink / raw)
  To: Abdiel Janulgue; +Cc: intel-gfx

On Tue, Jun 06, 2017 at 11:54:12AM +0300, Abdiel Janulgue wrote:
> Support executing external processes with the goal of capturing its
> standard streams to the igt logging infrastructure in addition to its
> exit status.
> 
> v3: Rename igt_exec -> igt_system (Chris).
> 
> v2: Fix leaks on fd teardown. Make sure redirected process printout when
>     > 64kb still works, like full dmesg. (Petri).
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

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

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

* Re: [i-g-t PATCH v3 3/3] Convert shell script tests to C version
  2017-06-12 11:14   ` Arkadiusz Hiler
@ 2017-06-13 10:22     ` Abdiel Janulgue
  0 siblings, 0 replies; 7+ messages in thread
From: Abdiel Janulgue @ 2017-06-13 10:22 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: intel-gfx



On 12.06.2017 14:14, Arkadiusz Hiler wrote:
> On Tue, Jun 06, 2017 at 11:54:14AM +0300, Abdiel Janulgue wrote:
>> v3: Drop redundant test covered by drv_hangman/basic. Descend thru
>>     debugfs path when reading sysfs entries (Chris).
>>
>> v2: Use internal igt_debugfs functions instead of cat and document
>>     debugfs tests.
>>     Convert sysfs_l3_parity properly.
>>     Rename redundant names in tests.
>>
>> Converted:
>>  - check_drm_clients (ensures no other clients are running.
>>    functionality provided by drm_open_driver_master).
>>  - debugfs_emon_crash
>>  - debugfs_wedged
>>  - drv_debugfs_reader
>>  - sysfs_l3_parity
>>  - test_rte_check  (same as check_drm_clients)
>>  - tools_test
>>  - ZZ_check_dmesg
> 
> Hey,
> 
> Doing each tool in a separate commit would help with readability.

Will do.

> 
>>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
>> ---
>>  tests/Makefile.sources   |   9 +---
>>  tests/ZZ_check_dmesg     |  11 -----
>>  tests/check_drm_clients  |   6 ---
>>  tests/debugfs.c          |  96 +++++++++++++++++++++++++++++++++++++
>>  tests/debugfs_emon_crash |  16 -------
>>  tests/debugfs_wedged     |  10 ----
>>  tests/drv_debugfs_reader |   9 ----
>>  tests/sysfs_l3_parity    |  22 ---------
>>  tests/test_rte_check     |   6 ---
>>  tests/tools.c            | 122 +++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/tools_test         |  16 -------
>>  11 files changed, 220 insertions(+), 103 deletions(-)
>>  delete mode 100755 tests/ZZ_check_dmesg
>>  delete mode 100755 tests/check_drm_clients
>>  create mode 100644 tests/debugfs.c
>>  delete mode 100755 tests/debugfs_emon_crash
>>  delete mode 100755 tests/debugfs_wedged
>>  delete mode 100755 tests/drv_debugfs_reader
>>  delete mode 100755 tests/sysfs_l3_parity
>>  delete mode 100755 tests/test_rte_check
>>  create mode 100644 tests/tools.c
>>  delete mode 100755 tests/tools_test
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 9553e4d..c4a78a9 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -243,6 +243,8 @@ TESTS_progs = \
>>  	drv_module_reload \
>>  	kms_sysfs_edid_timing \
>>  	perf \
>> +	debugfs \
> 
> The name clashes on AOSP build:
> build/core/base_rules.mk:238: error: external/igt/tests: MODULE.TARGET.EXECUTABLES.debugfs already defined by external/e2fsprogs/debugfs.
> 
> debugfs is a name of a binary that debugs ext{2,3,4} family of FSes.
> 
> I have to look into the build system and our Android.mks more deeply as
> IGTs output binaries should be stored elsewhere and the clash is rather
> enigmatic.
> 
> Also are we changing names of the test? I thought it was only a rewrite.

It's just a rewrite. Now that we combine those debugfs_* scripts into
one executable, maybe just the prefix would suffice? How about
debugfs_tests.c?

> 
>> +	tools \
>>  	$(NULL)
>>  
>>  # IMPORTANT: The ZZ_ tests need to be run last!
>> @@ -251,11 +253,6 @@ TESTS_scripts_M = \
>>  	$(NULL)
>>  
>>  TESTS_scripts = \
>> -	debugfs_emon_crash \
>> -	drv_debugfs_reader \
>> -	sysfs_l3_parity \
>> -	test_rte_check \
>> -	tools_test \
>>  	$(NULL)
> 
> TESTS_scripts now is only $(NULL), so you may as well just remove it
> from the `kernel_tests` variable and get rid of it completely.
> 
>>  
>>  # This target contains testcases which support automagic subtest enumeration
>> @@ -317,9 +314,7 @@ HANG = \
>>  	$(NULL)
>>  
>>  scripts = \
>> -	check_drm_clients \
>>  	ddx_intel_after_fbdev \
>> -	debugfs_wedged \
>>  	drm_lib.sh \
>>  	drm_getopt.sh \
>>  	$(NULL)
> 
> <SNIP>
> 
>> diff --git a/tests/debugfs.c b/tests/debugfs.c
>> new file mode 100644
>> index 0000000..b9ae86c
>> --- /dev/null
>> +++ b/tests/debugfs.c
>> @@ -0,0 +1,96 @@
>> +/*
>> + * Copyright © 2017 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +#ifdef HAVE_CONFIG_H
>> +#include "config.h"
>> +#endif
>> +#include "igt.h"
>> +#include "igt_sysfs.h"
>> +#include <fcntl.h>
>> +#include <sys/types.h>
>> +#include <dirent.h>
>> +
>> +static void get_sysfs_entry(int path_fd)
> 
> I was a little bit confused by the function - we are not getting
> anything from it. Just reading and discarding.
> 
> Maybe read_and_discard_sysfs_entry? A comment would also do.

read_and_discard_sysfs_entry() sounds better.

> 
>> +{
>> +	struct dirent *dirent;
>> +	DIR *dir;
>> +
>> +	dir = fdopendir(path_fd);
>> +	if (!dir)
>> +		return;
>> +
>> +	while ((dirent = readdir(dir))) {
>> +		if (!strcmp(dirent->d_name, ".") ||
>> +		    !strcmp(dirent->d_name, ".."))
>> +			continue;
>> +		if (dirent->d_type == DT_DIR) {
>> +			int sub_fd = -1;
>> +			igt_assert((sub_fd =
>> +				    openat(path_fd, dirent->d_name, O_RDONLY |
>> +					   O_DIRECTORY)) > 0);
>> +			get_sysfs_entry(sub_fd);
>> +			close(sub_fd);
>> +		} else {
>> +			char *buf = igt_sysfs_get(path_fd, dirent->d_name);
> 
> igt_sysfs_get may get us NULL.
> Shouldn't we assert on that? It's an error-worthy.

Yep.

> <SNIP>

>> +		igt_assert(asprintf(&cmd, "dmesg | grep "
>> +				    "-- '------\\[ cut here \\]----'")
>> +			   != -1);
>> +		igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
>> +		free(cmd);
> 
> Are we testing the dmesg here?
> 
> If not they why systeming out "dmesg | grep" instead of going through
> /dev/kmsg?

Yes. This could be improved. Thanks for the review!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2017-06-13 10:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-06  8:54 [i-g-t PATCH v3 1/3] lib/igt_core: Add igt_system helpers Abdiel Janulgue
2017-06-06  8:54 ` [i-g-t PATCH 2/3] igt/igt_core: Provide an option to check for the log buffer contents Abdiel Janulgue
2017-06-12 11:48   ` Arkadiusz Hiler
2017-06-06  8:54 ` [i-g-t PATCH v3 3/3] Convert shell script tests to C version Abdiel Janulgue
2017-06-12 11:14   ` Arkadiusz Hiler
2017-06-13 10:22     ` Abdiel Janulgue
2017-06-12 12:37 ` [i-g-t PATCH v3 1/3] lib/igt_core: Add igt_system helpers Arkadiusz Hiler

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.