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

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Petri Latvala (2018-11-02 12:08:48)
> 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))) {

There's no guarantee here that you have the complete line within the
buf before parsing. Try getline() instead.

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

The value is (unsigned) long, important if we ever get more than 32b of
taints on 64b machines.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev