[igt-dev] [PATCH i-g-t v3 1/1] runner: Implement --abort-on-monitored-error
Petri Latvala
petri.latvala at intel.com
Fri Nov 2 13:26:58 UTC 2018
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
Signed-off-by: Petri Latvala <petri.latvala at intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
Cc: Tomi Sarvela <tomi.p.sarvela at intel.com>
Cc: Martin Peres <martin.peres at linux.intel.com>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
---
runner/executor.c | 103 +++++++++++++++++++++++++++++++++++++++---
runner/runner_tests.c | 49 ++++++++++++++++++--
runner/settings.c | 75 +++++++++++++++++++++++++++---
runner/settings.h | 8 +++-
4 files changed, 217 insertions(+), 18 deletions(-)
diff --git a/runner/executor.c b/runner/executor.c
index fc79f772..57e94b26 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -108,6 +108,90 @@ 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 = NULL;
+ size_t bufsize = 0;
+ int val;
+ bool ret = false;
+
+ if (!f)
+ return false;
+
+ while (getline(&buf, &bufsize, f) > 0) {
+ if (!strncmp(buf, debug_locks_line, strlen(debug_locks_line))) {
+ char *p = buf + strlen(debug_locks_line);
+ if (sscanf(p, "%d", &val) == 1 &&
+ val != 1) {
+ if (settings->log_level >= LOG_LEVEL_NORMAL) {
+ fprintf(stderr, "Lockdep triggered, aborting\n");
+ }
+ ret = true;
+ }
+
+ break;
+ }
+ }
+
+ fclose(f);
+ free(buf);
+ return ret;
+}
+
+static bool handle_taint(struct settings *settings)
+{
+ FILE *f = fopen("/proc/sys/kernel/tainted", "r");
+ int s;
+ unsigned long taint;
+ unsigned long bad_taints =
+ 0x20 | /* TAINT_PAGE */
+ 0x80 | /* TAINT_DIE */
+ 0x200; /* TAINT_OOPS */
+
+ if (!f)
+ return false;
+
+ s = fscanf(f, "%lu", &taint);
+ fclose(f);
+
+ if (s != 1)
+ return false;
+
+ if (taint & bad_taints) {
+ if (settings->log_level >= LOG_LEVEL_NORMAL) {
+ fprintf(stderr, "Kernel tainted (0x%lx), 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 +1186,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
More information about the igt-dev
mailing list