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

* [igt-dev] [PATCH i-g-t 1/1] runner: Implement --abort-on-monitored-error
@ 2018-11-02 12:08 Petri Latvala
  2018-11-02 12:24 ` Martin Peres
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Petri Latvala @ 2018-11-02 12:08 UTC (permalink / raw)
  To: igt-dev; +Cc: Tomi Sarvela, Petri Latvala, Martin Peres

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.

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>
---

It should be noted that this is automatically active in CI, for it
gives --abort-on-monitored-error without a list to igt_runner already.


 runner/executor.c     | 92 ++++++++++++++++++++++++++++++++++++++++---
 runner/runner_tests.c | 49 ++++++++++++++++++++---
 runner/settings.c     | 75 ++++++++++++++++++++++++++++++++---
 runner/settings.h     |  8 +++-
 4 files changed, 206 insertions(+), 18 deletions(-)

diff --git a/runner/executor.c b/runner/executor.c
index fc79f772..98c31f36 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -108,6 +108,79 @@ static void ping_watchdogs(void)
 	}
 }
 
+static bool handle_lockdep(struct settings *settings)
+{
+	FILE *f = fopen("/proc/lockdep_stats", "r");
+	const char *debug_locks_line = " debug_locks:";
+	char buf[512];
+	int val;
+
+	if (!f)
+		return false;
+
+	while (fgets(buf, sizeof(buf), f) != NULL) {
+		if (!strncmp(buf, debug_locks_line, strlen(debug_locks_line))) {
+			char *p = buf + strlen(debug_locks_line);
+			if (sscanf(p, "%d", &val) == 1 &&
+			    val != 0) {
+				if (settings->log_level >= LOG_LEVEL_NORMAL) {
+					fprintf(stderr, "Lockdep triggered, aborting\n");
+				}
+				return true;
+			}
+
+			break;
+		}
+	}
+
+	return false;
+}
+
+static bool handle_taint(struct settings *settings)
+{
+	FILE *f = fopen("/proc/sys/kernel/tainted", "r");
+	int taint;
+	int bad_taints =
+		0x20  | /* TAINT_PAGE */
+		0x80  | /* TAINT_DIE */
+		0x200; /* TAINT_OOPS */
+
+	if (!f || fscanf(f, "%d", &taint) != 1)
+		return false;
+
+	if (taint & bad_taints) {
+		if (settings->log_level >= LOG_LEVEL_NORMAL) {
+			fprintf(stderr, "Kernel tainted (0x%x), aborting\n", taint);
+		}
+
+		return true;
+	}
+
+	return false;
+}
+
+static struct {
+	int condition;
+	bool (*handler)(struct settings*);
+} abort_handlers[] = {
+	{ ABORT_LOCKDEP, handle_lockdep },
+	{ ABORT_TAINT, handle_taint },
+	{ 0, 0 },
+};
+
+static bool need_to_abort(struct settings* settings)
+{
+	typeof(*abort_handlers) *it;
+
+	for (it = abort_handlers; it->condition; it++) {
+		if (settings->abort_mask & it->condition &&
+		    it->handler(settings))
+			return true;
+	}
+
+	return false;
+}
+
 static void prune_subtest(struct job_list_entry *entry, char *subtest)
 {
 	char *excl;
@@ -1102,12 +1175,19 @@ bool execute(struct execute_state *state,
 
 	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);
+		int result;
+
+		if (need_to_abort(settings)) {
+			status = false;
+			break;
+		}
+
+		result = execute_next_entry(state,
+					    job_list->size,
+					    &time_spent,
+					    settings,
+					    &job_list->entries[state->next],
+					    testdirfd, resdirfd);
 
 		if (result < 0) {
 			status = false;
diff --git a/runner/runner_tests.c b/runner/runner_tests.c
index 9c0f9eb0..829c60c3 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), 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), 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") {
 		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), 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") {
+		char *argv[] = { "runner",
+				 "--abort-on-monitored-error=taint",
+				 "test-root-dir",
+				 "results-path",
+		};
+
+		igt_assert(parse_options(ARRAY_SIZE(argv), argv, &settings));
+		igt_assert_eq(settings.abort_mask, ABORT_TAINT);
+
+		argv[1] = "--abort-on-monitored-error=lockdep";
+		igt_assert(parse_options(ARRAY_SIZE(argv), argv, &settings));
+		igt_assert_eq(settings.abort_mask, ABORT_LOCKDEP);
+
+		argv[1] = "--abort-on-monitored-error=taint";
+		igt_assert(parse_options(ARRAY_SIZE(argv), 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), 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), 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), argv, &settings));
+		igt_assert_eq(settings.abort_mask, ABORT_ALL);
+
+		argv[1] = "--abort-on-monitored-error=";
+		igt_assert(parse_options(ARRAY_SIZE(argv), argv, &settings));
+		igt_assert_eq(settings.abort_mask, 0);
+
+		argv[1] = "--abort-on-monitored-error=doesnotexist";
+		igt_assert(!parse_options(ARRAY_SIZE(argv), argv, &settings));
+
+	}
+
 	igt_subtest("parse-clears-old-data") {
 		char *argv[] = { "runner",
 				 "-n", "foo",
diff --git a/runner/settings.c b/runner/settings.c
index e2401455..e056493a 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,52 @@ 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, *p;
+	if (!optarg)
+		return set_abort_condition(settings, NULL);
+
+	dup = strdup(optarg);
+	while (dup) {
+		if ((p = strchr(dup, ',')) != NULL) {
+			*p = '\0';
+			p++;
+		}
+
+		if (!set_abort_condition(settings, dup))
+			return false;
+
+		dup = p;
+	}
+
+	return true;
+}
+
 static const char *usage_str =
 	"usage: runner [options] [test_root] results-path\n\n"
 	"Options:\n"
@@ -67,9 +123,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 +255,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 +293,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 +507,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 +564,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] 8+ messages in thread