[igt-dev] [PATCH i-g-t v4 1/1] runner: Implement --abort-on-monitored-error
Petri Latvala
petri.latvala at intel.com
Thu Nov 8 12:39:34 UTC 2018
On Tue, Nov 06, 2018 at 04:53:02PM +0000, Chris Wilson wrote:
> Quoting Petri Latvala (2018-11-06 14:26:29)
> > 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
> >
> > 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 | 189 ++++++++++++++++--
> > 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, 556 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..ed444a9a 100644
> > --- a/runner/executor.c
> > +++ b/runner/executor.c
> > @@ -108,6 +108,90 @@ static void ping_watchdogs(void)
> > }
> > }
> >
> > +static char *handle_lockdep(void)
> > +{
> > + FILE *f = fopen("/proc/lockdep_stats", "r");
> > + const char *debug_locks_line = " debug_locks:";
> > + char *buf = NULL;
> > + size_t bufsize = 0;
> > + int val;
> > + char *reason = NULL;
> > +
> > + if (!f)
> > + return reason;
> > +
> > + 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)
> > + asprintf(&reason, "Lockdep triggered\n");
> > +
> > + break;
> > + }
> > + }
> > +
> > + fclose(f);
> > + free(buf);
> > + return reason;
> > +}
> > +
> > +static char *handle_taint(void)
> > +{
> > + 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 */
>
> Please be considerate to the reader.
> > +
> > + if (!f)
> > + return NULL;
> > +
> > + s = fscanf(f, "%lu", &taint);
> > + fclose(f);
> > +
> > + if (s != 1)
> > + return NULL;
> > +
> > + if (taint & bad_taints) {
> > + char *reason;
> > +
> > + asprintf(&reason, "Kernel tainted (0x%lx)\n", taint);
> > +
> > + return reason;
> > + }
> > +
> > + return NULL;
>
> 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();
> if (f) {
> fscanf(f, "%lu", &taints);
> fclose(f);
> }
>
> if (taints & bad_taints)
> asprintf(&reason,
> "Kernel tainted (%#lx -- %lx)\n",
> taints, taints & bad_taints);
>
> return reason;
>
> > +}
> > +
> > +static struct {
>
> 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(struct settings* settings)
>
> const struct settings *settings
>
> > +{
> > + typeof(*abort_handlers) *it;
> > + char *ret = NULL;
> > +
> > + for (it = abort_handlers; it->condition; it++) {
> > + if (settings->abort_mask & it->condition &&
> > + (ret = it->handler()) != NULL) {
> > + if (settings->log_level >= LOG_LEVEL_NORMAL)
> > + fprintf(stderr, "%s", ret);
> > + }
> > + break;
>
> break? looks incongruous.
I don't know how that slipped there, it was supposed to be in the if...
--
Petri Latvala
More information about the igt-dev
mailing list