[igt-dev] [PATCH i-g-t] runner: Make taint aborts more verbose

[Chris Wilson]

Quoting Arkadiusz Hiler (2018-11-26 08:52:22)
> Provide the reader with the taint names and a short explanation, as well
> as point in the direction of the dmesg for more details.
> 
> Cc: Petri Latvala <petri.latvala at intel.com>
> Cc: Martin Peres <martin.peres at linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> ---
>  runner/executor.c | 42 ++++++++++++++++++++++++++++++++----------
>  1 file changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/runner/executor.c b/runner/executor.c
> index 2038c3fd..954cd9a7 100644
> --- a/runner/executor.c
> +++ b/runner/executor.c
> @@ -139,24 +139,46 @@ static char *handle_lockdep(void)
>  
>  static char *handle_taint(void)
>  {
> -       const unsigned long bad_taints =
> -               0x20  | /* TAINT_PAGE */
> -               0x80  | /* TAINT_DIE */
> -               0x200; /* TAINT_OOPS */
> -       unsigned long taints = 0;
> +       /* see Linux's include/linux/kernel.h */
> +       static const struct {
> +               unsigned long bit;
> +               const char *explanation;
> +       } taints[] = {
> +         {(1 << 5), "TAINT_BAD_PAGE: Bad page reference or an unexpected page flags."},
> +         {(1 << 7), "TAINT_DIE: Kernel has died - BUG/OOPS."},
> +         {(1 << 9), "TAINT_WARN: WARN_ON has happened."},

Pure noise, these don't add any useful information, so why? The only
information is the explanation in dmesg. Checking tainted is just
shorthand for us to determine a significant event.

If you want to add the kmsg output filtered by WARN+, that would be more
useful than hinting to the user they need to do it themselves. But that
should be redundant as that is already part of the runner -- so is not
the problem the actual integration, that this appears as a separate test
and not flagging the igt itself, with exceptions to handle tainted
before a test?
-Chris