[igt-dev] [PATCH i-g-t 1/1] runner: Implement --abort-on-monitored-error
Martin Peres
martin.peres at linux.intel.com
Fri Nov 2 12:24:01 UTC 2018
On 02/11/2018 14:08, Petri Latvala wrote:
> 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 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>
\o/
Oh yeah | !
/ \
Acked-by: Martin Peres <martin.peres at linux.intel.com>
> ---
>
> 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;
>
More information about the igt-dev
mailing list