[igt-dev] [PATCH i-g-t v6 1/1] runner: Implement --abort-on-monitored-error

* [igt-dev] [PATCH i-g-t v6 1/1] runner: Implement --abort-on-monitored-error
@ 2018-11-08 12:38 Petri Latvala
  2018-11-08 13:15 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v6,1/1] " Patchwork
  2018-11-08 21:50 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  0 siblings, 2 replies; 5+ messages in thread
From: Petri Latvala @ 2018-11-08 12:38 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala, Martin Peres, Tomi Sarvela

Deviating a bit from the piglit command line flag, igt_runner takes an
optional comma-separated list as an argument to
--abort-on-monitored-error for the list of conditions to abort
on. Without a list all possible conditions will be checked.

Two conditions implemented:
 - "taint" checks the kernel taint level for TAINT_PAGE, TAINT_DIE and
 TAINT_OOPS
 - "lockdep" checks the kernel lockdep status

Checking is done after every test binary execution, and if an abort
condition is met, the reason is printed to stderr (unless log level is
quiet) and the runner doesn't execute any further tests. Aborting
between subtests (when running in --multiple-mode) is not done.

A TODO item for the future is having aborting appear in the test
results.

v2:
 - Remember to fclose
 - Taints are unsigned long (Chris)
 - Use getline instead of fgets (Chris)
v3:
 - Fix brainfart with lockdep
v4:
 - Rebase
 - Refactor the abort condition checking to pass down strings
 - Present the abort result in results.json as a pseudo test result
 - Unit tests for the pseudo result
v5:
 - Refactors (Chris)
 - Don't claim lockdep was triggered if debug_locks is not on
   anymore. Just say it's not active.
 - Dump lockdep_stats when aborting due to lockdep (Chris)
 - Use igt@runner@aborted instead for the pseudo result (Martin)

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
Cc: Martin Peres <martin.peres@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 runner/executor.c                             | 190 ++++++++++++++++--
 runner/executor.h                             |   1 +
 .../aborted-after-a-test/0/dmesg.txt          |   5 +
 .../aborted-after-a-test/0/err.txt            |   2 +
 .../aborted-after-a-test/0/journal.txt        |   2 +
 .../aborted-after-a-test/0/out.txt            |   3 +
 .../aborted-after-a-test/README.txt           |   1 +
 .../aborted-after-a-test/aborted.txt          |   3 +
 .../aborted-after-a-test/endtime.txt          |   1 +
 .../aborted-after-a-test/joblist.txt          |   5 +
 .../aborted-after-a-test/metadata.txt         |  12 ++
 .../aborted-after-a-test/reference.json       |  90 +++++++++
 .../aborted-after-a-test/starttime.txt        |   1 +
 .../aborted-after-a-test/uname.txt            |   1 +
 .../aborted-on-boot/README.txt                |   1 +
 .../aborted-on-boot/aborted.txt               |   3 +
 .../aborted-on-boot/endtime.txt               |   1 +
 .../aborted-on-boot/joblist.txt               |   5 +
 .../aborted-on-boot/metadata.txt              |  12 ++
 .../aborted-on-boot/reference.json            |  59 ++++++
 .../aborted-on-boot/starttime.txt             |   1 +
 .../json_tests_data/aborted-on-boot/uname.txt |   1 +
 .../dmesg-results/metadata.txt                |   2 +-
 .../metadata.txt                              |   2 +-
 .../json_tests_data/normal-run/metadata.txt   |   2 +-
 .../piglit-style-dmesg/metadata.txt           |   2 +-
 .../warnings-with-dmesg-warns/metadata.txt    |   2 +-
 runner/json_tests_data/warnings/metadata.txt  |   2 +-
 runner/resultgen.c                            |  48 ++++-
 runner/runner_json_tests.c                    |   2 +
 runner/runner_tests.c                         |  49 ++++-
 runner/settings.c                             |  79 +++++++-
 runner/settings.h                             |   8 +-
 33 files changed, 557 insertions(+), 41 deletions(-)
 create mode 100644 runner/json_tests_data/aborted-after-a-test/0/dmesg.txt
 create mode 100644 runner/json_tests_data/aborted-after-a-test/0/err.txt
 create mode 100644 runner/json_tests_data/aborted-after-a-test/0/journal.txt
 create mode 100644 runner/json_tests_data/aborted-after-a-test/0/out.txt
 create mode 100644 runner/json_tests_data/aborted-after-a-test/README.txt
 create mode 100644 runner/json_tests_data/aborted-after-a-test/aborted.txt
 create mode 100644 runner/json_tests_data/aborted-after-a-test/endtime.txt
 create mode 100644 runner/json_tests_data/aborted-after-a-test/joblist.txt
 create mode 100644 runner/json_tests_data/aborted-after-a-test/metadata.txt
 create mode 100644 runner/json_tests_data/aborted-after-a-test/reference.json
 create mode 100644 runner/json_tests_data/aborted-after-a-test/starttime.txt
 create mode 100644 runner/json_tests_data/aborted-after-a-test/uname.txt
 create mode 100644 runner/json_tests_data/aborted-on-boot/README.txt
 create mode 100644 runner/json_tests_data/aborted-on-boot/aborted.txt
 create mode 100644 runner/json_tests_data/aborted-on-boot/endtime.txt
 create mode 100644 runner/json_tests_data/aborted-on-boot/joblist.txt
 create mode 100644 runner/json_tests_data/aborted-on-boot/metadata.txt
 create mode 100644 runner/json_tests_data/aborted-on-boot/reference.json
 create mode 100644 runner/json_tests_data/aborted-on-boot/starttime.txt
 create mode 100644 runner/json_tests_data/aborted-on-boot/uname.txt

diff --git a/runner/executor.c b/runner/executor.c
index 007b72ce..9eba737d 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -108,6 +108,91 @@ static void ping_watchdogs(void)
 	}
 }
 
+static char *handle_lockdep(void)
+{
+	const char *header = "Lockdep not active\n\n/proc/lockdep_stats contents:\n";
+	int fd = open("/proc/lockdep_stats", O_RDONLY);
+	const char *debug_locks_line = " debug_locks:";
+	char buf[4096], *p;
+	ssize_t bufsize = 0;
+	int val;
+
+	if (fd < 0)
+		return NULL;
+
+	strcpy(buf, header);
+
+	if ((bufsize = read(fd, buf + strlen(header), sizeof(buf) - strlen(header) - 1)) < 0)
+		return NULL;
+	bufsize += strlen(header);
+	buf[bufsize] = '\0';
+	close(fd);
+
+	if ((p = strstr(buf, debug_locks_line)) != NULL &&
+	    sscanf(p + strlen(debug_locks_line), "%d", &val) == 1 &&
+	    val != 1) {
+		return strdup(buf);
+	}
+
+	return NULL;
+}
+
+static char *handle_taint(void)
+{
+	const unsigned long bad_taints =
+		0x20  | /* TAINT_PAGE */
+		0x80  | /* TAINT_DIE */
+		0x200; /* TAINT_OOPS */
+	unsigned long taints = 0;
+	char *reason = NULL;
+	FILE *f;
+
+	f = fopen("/proc/sys/kernel/tainted", "r");
+	if (f) {
+		fscanf(f, "%lu", &taints);
+		fclose(f);
+	}
+
+	if (taints & bad_taints)
+		asprintf(&reason,
+			 "Kernel tainted (%#lx -- %lx)",
+			 taints, taints & bad_taints);
+
+	return reason;
+}
+
+static const struct {
+	int condition;
+	char *(*handler)(void);
+} abort_handlers[] = {
+	{ ABORT_LOCKDEP, handle_lockdep },
+	{ ABORT_TAINT, handle_taint },
+	{ 0, 0 },
+};
+
+static char *need_to_abort(const struct settings* settings)
+{
+	typeof(*abort_handlers) *it;
+
+	for (it = abort_handlers; it->condition; it++) {
+		char *abort;
+
+		if (!(settings->abort_mask & it->condition))
+			continue;
+
+		abort = it->handler();
+		if (!abort)
+			continue;
+
+		if (settings->log_level >= LOG_LEVEL_NORMAL)
+			fprintf(stderr, "Aborting: %s\n", abort);
+
+		return abort;
+	}
+
+	return NULL;
+}
+
 static void prune_subtest(struct job_list_entry *entry, char *subtest)
 {
 	char *excl;
@@ -714,6 +799,37 @@ static void print_time_left(struct execute_state *state,
 	printf("(%*.0fs left) ", width, state->time_left);
 }
 
+static char *entry_display_name(struct job_list_entry *entry)
+{
+	size_t size = strlen(entry->binary) + 1;
+	char *ret = malloc(size);
+
+	sprintf(ret, "%s", entry->binary);
+
+	if (entry->subtest_count > 0) {
+		size_t i;
+		const char *delim = "";
+
+		size += 3; /* strlen(" (") + strlen(")") */
+		ret = realloc(ret, size);
+		strcat(ret, " (");
+
+		for (i = 0; i < entry->subtest_count; i++) {
+			size += strlen(delim) + strlen(entry->subtests[i]);
+			ret = realloc(ret, size);
+
+			strcat(ret, delim);
+			strcat(ret, entry->subtests[i]);
+
+			delim = ", ";
+		}
+		/* There's already room for this */
+		strcat(ret, ")");
+	}
+
+	return ret;
+}
+
 /*
  * Returns:
  *  =0 - Success
@@ -797,24 +913,15 @@ static int execute_next_entry(struct execute_state *state,
 	}
 
 	if (settings->log_level >= LOG_LEVEL_NORMAL) {
+		char *displayname;
 		int width = digits(total);
 		printf("[%0*zd/%0*zd] ", width, idx + 1, width, total);
 
 		print_time_left(state, settings);
 
-		printf("%s", entry->binary);
-
-		if (entry->subtest_count > 0) {
-			size_t i;
-			const char *delim = "";
-
-			printf(" (");
-			for (i = 0; i < entry->subtest_count; i++) {
-				printf("%s%s", delim, entry->subtests[i]);
-				delim = ", ";
-			}
-			printf(")");
-		}
+		displayname = entry_display_name(entry);
+		printf("%s", displayname);
+		free(displayname);
 
 		printf("\n");
 	}
@@ -896,7 +1003,8 @@ static bool clear_old_results(char *path)
 
 	if (remove_file(dirfd, "uname.txt") ||
 	    remove_file(dirfd, "starttime.txt") ||
-	    remove_file(dirfd, "endtime.txt")) {
+	    remove_file(dirfd, "endtime.txt") ||
+	    remove_file(dirfd, "aborted.txt")) {
 		close(dirfd);
 		fprintf(stderr, "Error clearing old results: %s\n", strerror(errno));
 		return false;
@@ -957,6 +1065,7 @@ bool initialize_execute_state_from_resume(int dirfd,
 	free_settings(settings);
 	free_job_list(list);
 	memset(state, 0, sizeof(*state));
+	state->resuming = true;
 
 	if (!read_settings(settings, dirfd) ||
 	    !read_job_list(list, dirfd)) {
@@ -1044,6 +1153,22 @@ static bool overall_timeout_exceeded(struct execute_state *state)
 	return state->time_left == 0.0;
 }
 
+static void write_abort_file(int resdirfd, const char *reason, const char *testname)
+{
+	int abortfd;
+
+	if ((abortfd = openat(resdirfd, "aborted.txt", O_CREAT | O_WRONLY | O_EXCL, 0666)) >= 0) {
+		/*
+		 * Ignore failure to open, there's
+		 * already an abort probably (if this
+		 * is a resume)
+		 */
+		dprintf(abortfd, "Aborted after: %s\n\n", testname);
+		write(abortfd, reason, strlen(reason));
+		close(abortfd);
+	}
+}
+
 bool execute(struct execute_state *state,
 	     struct settings *settings,
 	     struct job_list *job_list)
@@ -1100,14 +1225,29 @@ bool execute(struct execute_state *state,
 	}
 	close(unamefd);
 
+	/* Check if we're already in abort-state at bootup */
+	if (!state->resuming) {
+		char *reason;
+
+		if ((reason = need_to_abort(settings)) != NULL) {
+			write_abort_file(resdirfd, reason, "startup");
+			free(reason);
+
+			goto end;
+		}
+	}
+
 	for (; state->next < job_list->size;
 	     state->next++) {
-		int result = execute_next_entry(state,
-						job_list->size,
-						&time_spent,
-						settings,
-						&job_list->entries[state->next],
-						testdirfd, resdirfd);
+		char *reason;
+		int result;
+
+		result = execute_next_entry(state,
+					    job_list->size,
+					    &time_spent,
+					    settings,
+					    &job_list->entries[state->next],
+					    testdirfd, resdirfd);
 
 		if (result < 0) {
 			status = false;
@@ -1124,6 +1264,15 @@ bool execute(struct execute_state *state,
 			break;
 		}
 
+		if ((reason = need_to_abort(settings)) != NULL) {
+			char *displayname = entry_display_name(&job_list->entries[state->next]);
+			write_abort_file(resdirfd, reason, displayname);
+			free(displayname);
+			free(reason);
+			status = false;
+			break;
+		}
+
 		if (result > 0) {
 			double time_left = state->time_left;
 
@@ -1140,6 +1289,7 @@ bool execute(struct execute_state *state,
 		close(timefd);
 	}
 
+ end:
 	close(testdirfd);
 	close(resdirfd);
 	close_watchdogs(settings);
diff --git a/runner/executor.h b/runner/executor.h
index 252339ab..12883f15 100644
--- a/runner/executor.h
+++ b/runner/executor.h
@@ -13,6 +13,7 @@ struct execute_state
 	 * > 0 : Timeout in use, time left.
 	 */
 	double time_left;
+	double resuming;
 };
 
 enum {
diff --git a/runner/json_tests_data/aborted-after-a-test/0/dmesg.txt b/runner/json_tests_data/aborted-after-a-test/0/dmesg.txt
new file mode 100644
index 00000000..a189e704
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/0/dmesg.txt
@@ -0,0 +1,5 @@
+6,951,3216186095083,-;Console: switching to colour dummy device 80x25
+14,952,3216186095097,-;[IGT] successtest: executing
+14,953,3216186101115,-;[IGT] successtest: starting subtest first-subtest
+14,954,3216186101160,-;[IGT] successtest: exiting, ret=0
+6,955,3216186101299,-;Console: switching to colour frame buffer device 240x75
diff --git a/runner/json_tests_data/aborted-after-a-test/0/err.txt b/runner/json_tests_data/aborted-after-a-test/0/err.txt
new file mode 100644
index 00000000..5dc78057
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/0/err.txt
@@ -0,0 +1,2 @@
+Starting subtest: first-subtest
+Subtest first-subtest: SUCCESS (0.000s)
diff --git a/runner/json_tests_data/aborted-after-a-test/0/journal.txt b/runner/json_tests_data/aborted-after-a-test/0/journal.txt
new file mode 100644
index 00000000..86a30e07
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/0/journal.txt
@@ -0,0 +1,2 @@
+first-subtest
+exit:0 (0.014s)
diff --git a/runner/json_tests_data/aborted-after-a-test/0/out.txt b/runner/json_tests_data/aborted-after-a-test/0/out.txt
new file mode 100644
index 00000000..5946bf31
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/0/out.txt
@@ -0,0 +1,3 @@
+IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)
+Starting subtest: first-subtest
+Subtest first-subtest: SUCCESS (0.000s)
diff --git a/runner/json_tests_data/aborted-after-a-test/README.txt b/runner/json_tests_data/aborted-after-a-test/README.txt
new file mode 100644
index 00000000..1ad89053
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/README.txt
@@ -0,0 +1 @@
+A run that aborted after running the first test.
diff --git a/runner/json_tests_data/aborted-after-a-test/aborted.txt b/runner/json_tests_data/aborted-after-a-test/aborted.txt
new file mode 100644
index 00000000..3c8f2707
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/aborted.txt
@@ -0,0 +1,3 @@
+Aborted after: successtest (first-subtest)
+
+Kernel tainted (0x200)
diff --git a/runner/json_tests_data/aborted-after-a-test/endtime.txt b/runner/json_tests_data/aborted-after-a-test/endtime.txt
new file mode 100644
index 00000000..635f6ae9
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/endtime.txt
@@ -0,0 +1 @@
+1539953735.172373
diff --git a/runner/json_tests_data/aborted-after-a-test/joblist.txt b/runner/json_tests_data/aborted-after-a-test/joblist.txt
new file mode 100644
index 00000000..31ef8413
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/joblist.txt
@@ -0,0 +1,5 @@
+successtest first-subtest
+successtest second-subtest
+no-subtests
+skippers skip-one
+skippers skip-two
diff --git a/runner/json_tests_data/aborted-after-a-test/metadata.txt b/runner/json_tests_data/aborted-after-a-test/metadata.txt
new file mode 100644
index 00000000..785076e9
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/metadata.txt
@@ -0,0 +1,12 @@
+abort_mask : 3
+name : normal-run
+dry_run : 0
+sync : 0
+log_level : 0
+overwrite : 0
+multiple_mode : 0
+inactivity_timeout : 0
+use_watchdog : 0
+piglit_style_dmesg : 0
+test_root : /path/does/not/exist
+results_path : /path/does/not/exist
diff --git a/runner/json_tests_data/aborted-after-a-test/reference.json b/runner/json_tests_data/aborted-after-a-test/reference.json
new file mode 100644
index 00000000..1e08deca
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/reference.json
@@ -0,0 +1,90 @@
+{
+  "__type__":"TestrunResult",
+  "results_version":9,
+  "name":"normal-run",
+  "uname":"Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64",
+  "time_elapsed":{
+    "__type__":"TimeAttribute",
+    "start":1539953735.1110389,
+    "end":1539953735.1723731
+  },
+  "tests":{
+    "igt@successtest@first-subtest":{
+      "out":"Starting subtest: first-subtest\nSubtest first-subtest: SUCCESS (0.000s)\n",
+      "igt-version":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)",
+      "result":"pass",
+      "time":{
+        "__type__":"TimeAttribute",
+        "start":0,
+        "end":0
+      },
+      "err":"Starting subtest: first-subtest\nSubtest first-subtest: SUCCESS (0.000s)\n",
+      "dmesg":"<6> [3216186.095083] Console: switching to colour dummy device 80x25\n<6> [3216186.095097] [IGT] successtest: executing\n<6> [3216186.101115] [IGT] successtest: starting subtest first-subtest\n<6> [3216186.101160] [IGT] successtest: exiting, ret=0\n<6> [3216186.101299] Console: switching to colour frame buffer device 240x75\n"
+    },
+    "igt@runner@aborted":{
+	"out":"Aborted after: successtest (first-subtest)\n\nKernel tainted (0x200)\n",
+	"result":"fail",
+	"err":"",
+	"dmesg":"",
+    }
+  },
+  "totals":{
+    "":{
+      "crash":0,
+      "pass":1,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
+      "skip":0,
+      "incomplete":0,
+      "timeout":0,
+      "notrun":0,
+      "fail":1,
+      "warn":0
+    },
+    "root":{
+      "crash":0,
+      "pass":1,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
+      "skip":0,
+      "incomplete":0,
+      "timeout":0,
+      "notrun":0,
+      "fail":1,
+      "warn":0
+    },
+    "igt@successtest":{
+      "crash":0,
+      "pass":1,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
+      "skip":0,
+      "incomplete":0,
+      "timeout":0,
+      "notrun":0,
+      "fail":0,
+      "warn":0
+    },
+    "igt@runner":{
+      "crash":0,
+      "pass":0,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
+      "skip":0,
+      "incomplete":0,
+      "timeout":0,
+      "notrun":0,
+      "fail":1,
+      "warn":0
+    },
+  },
+  "runtimes":{
+    "igt@successtest":{
+      "time":{
+        "__type__":"TimeAttribute",
+        "start":0,
+        "end":0.014
+      }
+    },
+  }
+}
diff --git a/runner/json_tests_data/aborted-after-a-test/starttime.txt b/runner/json_tests_data/aborted-after-a-test/starttime.txt
new file mode 100644
index 00000000..ae038f18
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/starttime.txt
@@ -0,0 +1 @@
+1539953735.111039
diff --git a/runner/json_tests_data/aborted-after-a-test/uname.txt b/runner/json_tests_data/aborted-after-a-test/uname.txt
new file mode 100644
index 00000000..a7aef6f7
--- /dev/null
+++ b/runner/json_tests_data/aborted-after-a-test/uname.txt
@@ -0,0 +1 @@
+Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64
diff --git a/runner/json_tests_data/aborted-on-boot/README.txt b/runner/json_tests_data/aborted-on-boot/README.txt
new file mode 100644
index 00000000..e957c40f
--- /dev/null
+++ b/runner/json_tests_data/aborted-on-boot/README.txt
@@ -0,0 +1 @@
+A run that aborted before running any tests.
diff --git a/runner/json_tests_data/aborted-on-boot/aborted.txt b/runner/json_tests_data/aborted-on-boot/aborted.txt
new file mode 100644
index 00000000..4a1c1a32
--- /dev/null
+++ b/runner/json_tests_data/aborted-on-boot/aborted.txt
@@ -0,0 +1,3 @@
+Aborted after: startup
+
+Kernel tainted (0x200)
diff --git a/runner/json_tests_data/aborted-on-boot/endtime.txt b/runner/json_tests_data/aborted-on-boot/endtime.txt
new file mode 100644
index 00000000..635f6ae9
--- /dev/null
+++ b/runner/json_tests_data/aborted-on-boot/endtime.txt
@@ -0,0 +1 @@
+1539953735.172373
diff --git a/runner/json_tests_data/aborted-on-boot/joblist.txt b/runner/json_tests_data/aborted-on-boot/joblist.txt
new file mode 100644
index 00000000..31ef8413
--- /dev/null
+++ b/runner/json_tests_data/aborted-on-boot/joblist.txt
@@ -0,0 +1,5 @@
+successtest first-subtest
+successtest second-subtest
+no-subtests
+skippers skip-one
+skippers skip-two
diff --git a/runner/json_tests_data/aborted-on-boot/metadata.txt b/runner/json_tests_data/aborted-on-boot/metadata.txt
new file mode 100644
index 00000000..785076e9
--- /dev/null
+++ b/runner/json_tests_data/aborted-on-boot/metadata.txt
@@ -0,0 +1,12 @@
+abort_mask : 3
+name : normal-run
+dry_run : 0
+sync : 0
+log_level : 0
+overwrite : 0
+multiple_mode : 0
+inactivity_timeout : 0
+use_watchdog : 0
+piglit_style_dmesg : 0
+test_root : /path/does/not/exist
+results_path : /path/does/not/exist
diff --git a/runner/json_tests_data/aborted-on-boot/reference.json b/runner/json_tests_data/aborted-on-boot/reference.json
new file mode 100644
index 00000000..bfaee1bb
--- /dev/null
+++ b/runner/json_tests_data/aborted-on-boot/reference.json
@@ -0,0 +1,59 @@
+{
+  "__type__":"TestrunResult",
+  "results_version":9,
+  "name":"normal-run",
+  "uname":"Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64",
+  "time_elapsed":{
+    "__type__":"TimeAttribute",
+    "start":1539953735.1110389,
+    "end":1539953735.1723731
+  },
+  "tests":{
+    "igt@runner@aborted":{
+	"out":"Aborted after: startup\n\nKernel tainted (0x200)\n",
+	"result":"fail",
+	"err":"",
+	"dmesg":"",
+    }
+  },
+  "totals":{
+    "":{
+      "crash":0,
+      "pass":0,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
+      "skip":0,
+      "incomplete":0,
+      "timeout":0,
+      "notrun":0,
+      "fail":1,
+      "warn":0
+    },
+    "root":{
+      "crash":0,
+      "pass":0,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
+      "skip":0,
+      "incomplete":0,
+      "timeout":0,
+      "notrun":0,
+      "fail":1,
+      "warn":0
+    },
+    "igt@runner":{
+      "crash":0,
+      "pass":0,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
+      "skip":0,
+      "incomplete":0,
+      "timeout":0,
+      "notrun":0,
+      "fail":1,
+      "warn":0
+    },
+  },
+  "runtimes":{
+  }
+}
diff --git a/runner/json_tests_data/aborted-on-boot/starttime.txt b/runner/json_tests_data/aborted-on-boot/starttime.txt
new file mode 100644
index 00000000..ae038f18
--- /dev/null
+++ b/runner/json_tests_data/aborted-on-boot/starttime.txt
@@ -0,0 +1 @@
+1539953735.111039
diff --git a/runner/json_tests_data/aborted-on-boot/uname.txt b/runner/json_tests_data/aborted-on-boot/uname.txt
new file mode 100644
index 00000000..a7aef6f7
--- /dev/null
+++ b/runner/json_tests_data/aborted-on-boot/uname.txt
@@ -0,0 +1 @@
+Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64
diff --git a/runner/json_tests_data/dmesg-results/metadata.txt b/runner/json_tests_data/dmesg-results/metadata.txt
index 1316560d..c501ae0e 100644
--- a/runner/json_tests_data/dmesg-results/metadata.txt
+++ b/runner/json_tests_data/dmesg-results/metadata.txt
@@ -1,4 +1,4 @@
-abort_on_error : 0
+abort_mask : 0
 name : normal-run
 dry_run : 0
 sync : 0
diff --git a/runner/json_tests_data/incomplete-before-any-subtests/metadata.txt b/runner/json_tests_data/incomplete-before-any-subtests/metadata.txt
index 1316560d..c501ae0e 100644
--- a/runner/json_tests_data/incomplete-before-any-subtests/metadata.txt
+++ b/runner/json_tests_data/incomplete-before-any-subtests/metadata.txt
@@ -1,4 +1,4 @@
-abort_on_error : 0
+abort_mask : 0
 name : normal-run
 dry_run : 0
 sync : 0
diff --git a/runner/json_tests_data/normal-run/metadata.txt b/runner/json_tests_data/normal-run/metadata.txt
index 1316560d..c501ae0e 100644
--- a/runner/json_tests_data/normal-run/metadata.txt
+++ b/runner/json_tests_data/normal-run/metadata.txt
@@ -1,4 +1,4 @@
-abort_on_error : 0
+abort_mask : 0
 name : normal-run
 dry_run : 0
 sync : 0
diff --git a/runner/json_tests_data/piglit-style-dmesg/metadata.txt b/runner/json_tests_data/piglit-style-dmesg/metadata.txt
index 7f1372c1..3eca78cd 100644
--- a/runner/json_tests_data/piglit-style-dmesg/metadata.txt
+++ b/runner/json_tests_data/piglit-style-dmesg/metadata.txt
@@ -1,4 +1,4 @@
-abort_on_error : 0
+abort_mask : 0
 name : normal-run
 dry_run : 0
 sync : 0
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/metadata.txt b/runner/json_tests_data/warnings-with-dmesg-warns/metadata.txt
index 1316560d..c501ae0e 100644
--- a/runner/json_tests_data/warnings-with-dmesg-warns/metadata.txt
+++ b/runner/json_tests_data/warnings-with-dmesg-warns/metadata.txt
@@ -1,4 +1,4 @@
-abort_on_error : 0
+abort_mask : 0
 name : normal-run
 dry_run : 0
 sync : 0
diff --git a/runner/json_tests_data/warnings/metadata.txt b/runner/json_tests_data/warnings/metadata.txt
index 1316560d..c501ae0e 100644
--- a/runner/json_tests_data/warnings/metadata.txt
+++ b/runner/json_tests_data/warnings/metadata.txt
@@ -1,4 +1,4 @@
-abort_on_error : 0
+abort_mask : 0
 name : normal-run
 dry_run : 0
 sync : 0
diff --git a/runner/resultgen.c b/runner/resultgen.c
index a62e400e..b0933ad7 100644
--- a/runner/resultgen.c
+++ b/runner/resultgen.c
@@ -649,6 +649,7 @@ static bool fill_from_dmesg(int fd,
 		append_line(&dmesg, &dmesglen, formatted);
 		free(formatted);
 	}
+	free(line);
 
 	if (current_test != NULL) {
 		add_dmesg(current_test, dmesg, dmesglen, warnings, warningslen);
@@ -717,6 +718,15 @@ static void add_subtest(struct subtests *subtests, char *subtest)
 	subtests->names[subtests->size - 1] = subtest;
 }
 
+static void free_subtests(struct subtests *subtests)
+{
+	size_t i;
+
+	for (i = 0; i < subtests->size; i++)
+		free(subtests->names[i]);
+	free(subtests->names);
+}
+
 static void fill_from_journal(int fd,
 			      struct job_list_entry *entry,
 			      struct subtests *subtests,
@@ -902,7 +912,7 @@ static void add_result_to_totals(struct json_object *totals,
 	json_object_object_add(totals, result, json_object_new_int(old + 1));
 }
 
-static void add_to_totals(char *binary,
+static void add_to_totals(const char *binary,
 			  struct subtests *subtests,
 			  struct results *results)
 {
@@ -950,6 +960,7 @@ static bool parse_test_directory(int dirfd,
 {
 	int fds[_F_LAST];
 	struct subtests subtests = {};
+	bool status = true;
 
 	if (!open_output_files(dirfd, fds, false)) {
 		fprintf(stderr, "Error opening output files\n");
@@ -966,15 +977,18 @@ static bool parse_test_directory(int dirfd,
 	    !fill_from_output(fds[_F_ERR], entry->binary, "err", &subtests, results->tests) ||
 	    !fill_from_dmesg(fds[_F_DMESG], settings, entry->binary, &subtests, results->tests)) {
 		fprintf(stderr, "Error parsing output files\n");
-		return false;
+		status = false;
+		goto parse_output_end;
 	}
 
 	override_results(entry->binary, &subtests, results->tests);
 	add_to_totals(entry->binary, &subtests, results);
 
+ parse_output_end:
 	close_outputs(fds);
+	free_subtests(&subtests);
 
-	return true;
+	return status;
 }
 
 static void create_result_root_nodes(struct json_object *root,
@@ -1077,6 +1091,34 @@ struct json_object *generate_results_json(int dirfd)
 		close(testdirfd);
 	}
 
+	if ((fd = openat(dirfd, "aborted.txt", O_RDONLY)) >= 0) {
+		char buf[4096];
+		char piglit_name[] = "igt@runner@aborted";
+		struct subtests abortsub = {};
+		struct json_object *aborttest = get_or_create_json_object(results.tests, piglit_name);
+		ssize_t s;
+
+		add_subtest(&abortsub, strdup("aborted"));
+
+		s = read(fd, buf, sizeof(buf));
+
+		json_object_object_add(aborttest, "out",
+				       json_object_new_string_len(buf, s));
+		json_object_object_add(aborttest, "err",
+				       json_object_new_string(""));
+		json_object_object_add(aborttest, "dmesg",
+				       json_object_new_string(""));
+		json_object_object_add(aborttest, "result",
+				       json_object_new_string("fail"));
+
+		add_to_totals("runner", &abortsub, &results);
+
+		free_subtests(&abortsub);
+	}
+
+	free_settings(&settings);
+	free_job_list(&job_list);
+
 	return obj;
 }
 
diff --git a/runner/runner_json_tests.c b/runner/runner_json_tests.c
index 758700d4..75d6237f 100644
--- a/runner/runner_json_tests.c
+++ b/runner/runner_json_tests.c
@@ -154,6 +154,8 @@ static const char *dirnames[] = {
 	"piglit-style-dmesg",
 	"incomplete-before-any-subtests",
 	"dmesg-results",
+	"aborted-on-boot",
+	"aborted-after-a-test",
 };
 
 igt_main
diff --git a/runner/runner_tests.c b/runner/runner_tests.c
index d07e6f37..6213b8e7 100644
--- a/runner/runner_tests.c
+++ b/runner/runner_tests.c
@@ -142,7 +142,7 @@ static void assert_settings_equal(struct settings *one, struct settings *two)
 	 * Regex lists are not serialized, and thus won't be compared
 	 * here.
 	 */
-	igt_assert_eq(one->abort_on_error, two->abort_on_error);
+	igt_assert_eq(one->abort_mask, two->abort_mask);
 	igt_assert_eqstr(one->test_list, two->test_list);
 	igt_assert_eqstr(one->name, two->name);
 	igt_assert_eq(one->dry_run, two->dry_run);
@@ -208,7 +208,7 @@ igt_main
 
 		igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
 
-		igt_assert(!settings.abort_on_error);
+		igt_assert_eq(settings.abort_mask, 0);
 		igt_assert(!settings.test_list);
 		igt_assert_eqstr(settings.name, "path-to-results");
 		igt_assert(!settings.dry_run);
@@ -323,7 +323,7 @@ igt_main
 		setenv("IGT_TEST_ROOT", testdatadir, 1);
 		igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
 
-		igt_assert(!settings.abort_on_error);
+		igt_assert_eq(settings.abort_mask, 0);
 		igt_assert(!settings.test_list);
 		igt_assert_eqstr(settings.name, "path-to-results");
 		igt_assert(!settings.dry_run);
@@ -348,7 +348,7 @@ igt_main
 	igt_subtest("parse-all-settings") {
 		const char *argv[] = { "runner",
 				       "-n", "foo",
-				       "--abort-on-monitored-error",
+				       "--abort-on-monitored-error=taint,lockdep",
 				       "--test-list", "path-to-test-list",
 				       "--ignore-missing",
 				       "--dry-run",
@@ -370,7 +370,7 @@ igt_main
 
 		igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
 
-		igt_assert(settings.abort_on_error);
+		igt_assert_eq(settings.abort_mask, ABORT_TAINT | ABORT_LOCKDEP);
 		igt_assert(strstr(settings.test_list, "path-to-test-list") != NULL);
 		igt_assert_eqstr(settings.name, "foo");
 		igt_assert(settings.dry_run);
@@ -428,6 +428,45 @@ igt_main
 		igt_assert_eq(settings.log_level, LOG_LEVEL_VERBOSE);
 	}
 
+	igt_subtest("abort-conditions") {
+		const char *argv[] = { "runner",
+				       "--abort-on-monitored-error=taint",
+				       "test-root-dir",
+				       "results-path",
+		};
+
+		igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
+		igt_assert_eq(settings.abort_mask, ABORT_TAINT);
+
+		argv[1] = "--abort-on-monitored-error=lockdep";
+		igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
+		igt_assert_eq(settings.abort_mask, ABORT_LOCKDEP);
+
+		argv[1] = "--abort-on-monitored-error=taint";
+		igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
+		igt_assert_eq(settings.abort_mask, ABORT_TAINT);
+
+		argv[1] = "--abort-on-monitored-error=lockdep,taint";
+		igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
+		igt_assert_eq(settings.abort_mask, ABORT_TAINT | ABORT_LOCKDEP);
+
+		argv[1] = "--abort-on-monitored-error=taint,lockdep";
+		igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
+		igt_assert_eq(settings.abort_mask, ABORT_TAINT | ABORT_LOCKDEP);
+
+		argv[1] = "--abort-on-monitored-error=all";
+		igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
+		igt_assert_eq(settings.abort_mask, ABORT_ALL);
+
+		argv[1] = "--abort-on-monitored-error=";
+		igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
+		igt_assert_eq(settings.abort_mask, 0);
+
+		argv[1] = "--abort-on-monitored-error=doesnotexist";
+		igt_assert(!parse_options(ARRAY_SIZE(argv), (char**)argv, &settings));
+
+	}
+
 	igt_subtest("parse-clears-old-data") {
 		const char *argv[] = { "runner",
 				       "-n", "foo",
diff --git a/runner/settings.c b/runner/settings.c
index e2401455..e64244e6 100644
--- a/runner/settings.c
+++ b/runner/settings.c
@@ -41,6 +41,16 @@ static struct {
 	{ 0, 0 },
 };
 
+static struct {
+	int value;
+	const char *name;
+} abort_conditions[] = {
+	{ ABORT_TAINT, "taint" },
+	{ ABORT_LOCKDEP, "lockdep" },
+	{ ABORT_ALL, "all" },
+	{ 0, 0 },
+};
+
 static bool set_log_level(struct settings* settings, const char *level)
 {
 	typeof(*log_levels) *it;
@@ -55,6 +65,56 @@ static bool set_log_level(struct settings* settings, const char *level)
 	return false;
 }
 
+static bool set_abort_condition(struct settings* settings, const char *cond)
+{
+	typeof(*abort_conditions) *it;
+
+	if (!cond) {
+		settings->abort_mask = ABORT_ALL;
+		return true;
+	}
+
+	if (strlen(cond) == 0) {
+		settings->abort_mask = 0;
+		return true;
+	}
+
+	for (it = abort_conditions; it->name; it++) {
+		if (!strcmp(cond, it->name)) {
+			settings->abort_mask |= it->value;
+			return true;
+		}
+	}
+
+	return false;
+}
+
+static bool parse_abort_conditions(struct settings *settings, const char *optarg)
+{
+	char *dup, *origdup, *p;
+	if (!optarg)
+		return set_abort_condition(settings, NULL);
+
+	origdup = dup = strdup(optarg);
+	while (dup) {
+		if ((p = strchr(dup, ',')) != NULL) {
+			*p = '\0';
+			p++;
+		}
+
+		if (!set_abort_condition(settings, dup)) {
+			free(origdup);
+			return false;
+		}
+
+		dup = p;
+	}
+
+	free(origdup);
+
+	return true;
+}
+
 static const char *usage_str =
 	"usage: runner [options] [test_root] results-path\n\n"
 	"Options:\n"
@@ -67,9 +127,15 @@ static const char *usage_str =
 	"                        Run only matching tests (can be used more than once)\n"
 	"  -x <regex>, --exclude-tests <regex>\n"
 	"                        Exclude matching tests (can be used more than once)\n"
-	"  --abort-on-monitored-error\n"
+	"  --abort-on-monitored-error[=list]\n"
 	"                        Abort execution when a fatal condition is detected.\n"
-	"                        <TODO>\n"
+	"                        A comma-separated list of conditions to check can be\n"
+	"                        given. If not given, all conditions are checked. An\n"
+	"                        empty string as a condition disables aborting\n"
+	"                        Possible conditions:\n"
+	"                         lockdep - abort when kernel lockdep has been angered.\n"
+	"                         taint   - abort when kernel becomes fatally tainted.\n"
+	"                         all     - abort for all of the above.\n"
 	"  -s, --sync            Sync results to disk after every test\n"
 	"  -l {quiet,verbose,dummy}, --log-level {quiet,verbose,dummy}\n"
 	"                        Set the logger verbosity level\n"
@@ -193,7 +259,7 @@ bool parse_options(int argc, char **argv,
 		{"dry-run", no_argument, NULL, OPT_DRY_RUN},
 		{"include-tests", required_argument, NULL, OPT_INCLUDE},
 		{"exclude-tests", required_argument, NULL, OPT_EXCLUDE},
-		{"abort-on-monitored-error", no_argument, NULL, OPT_ABORT_ON_ERROR},
+		{"abort-on-monitored-error", optional_argument, NULL, OPT_ABORT_ON_ERROR},
 		{"sync", no_argument, NULL, OPT_SYNC},
 		{"log-level", required_argument, NULL, OPT_LOG_LEVEL},
 		{"test-list", required_argument, NULL, OPT_TEST_LIST},
@@ -231,7 +297,8 @@ bool parse_options(int argc, char **argv,
 				goto error;
 			break;
 		case OPT_ABORT_ON_ERROR:
-			settings->abort_on_error = true;
+			if (!parse_abort_conditions(settings, optarg))
+				goto error;
 			break;
 		case OPT_SYNC:
 			settings->sync = true;
@@ -444,7 +511,7 @@ bool serialize_settings(struct settings *settings)
 		return false;
 	}
 
-	SERIALIZE_LINE(f, settings, abort_on_error, "%d");
+	SERIALIZE_LINE(f, settings, abort_mask, "%d");
 	if (settings->test_list)
 		SERIALIZE_LINE(f, settings, test_list, "%s");
 	if (settings->name)
@@ -501,7 +568,7 @@ bool read_settings(struct settings *settings, int dirfd)
 
 	while (fscanf(f, "%ms : %ms", &name, &val) == 2) {
 		int numval = atoi(val);
-		PARSE_LINE(settings, name, val, abort_on_error, numval);
+		PARSE_LINE(settings, name, val, abort_mask, numval);
 		PARSE_LINE(settings, name, val, test_list, val ? strdup(val) : NULL);
 		PARSE_LINE(settings, name, val, name, val ? strdup(val) : NULL);
 		PARSE_LINE(settings, name, val, dry_run, numval);
diff --git a/runner/settings.h b/runner/settings.h
index b489abc5..267d72cf 100644
--- a/runner/settings.h
+++ b/runner/settings.h
@@ -12,6 +12,12 @@ enum {
 	LOG_LEVEL_VERBOSE = 1,
 };
 
+#define ABORT_TAINT   (1 << 0)
+#define ABORT_LOCKDEP (1 << 1)
+#define ABORT_ALL     (ABORT_TAINT | ABORT_LOCKDEP)
+
+_Static_assert(ABORT_ALL == (ABORT_TAINT | ABORT_LOCKDEP), "ABORT_ALL must be all conditions bitwise or'd");
+
 struct regex_list {
 	char **regex_strings;
 	regex_t** regexes;
@@ -19,7 +25,7 @@ struct regex_list {
 };
 
 struct settings {
-	bool abort_on_error;
+	int abort_mask;
 	char *test_list;
 	char *name;
 	bool dry_run;
-- 
2.18.0

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

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