[PATCH] tests/igt: dmesg noise is a kernel failure

[PATCH] tests/igt: dmesg noise is a kernel failure

[Daniel Vetter]

At least when testing the kernel. In normal programs pretty much all
the dmesg noise would simply be replaced by debug asserts, but in the
kernel we try rely hard to not fall over minor inconsistencies.

Still for CI purposes there's not really a difference, hence don't
treat it as such.

Motivated since once again I've seen a statistics where this was split
up, and then a reduction of "failures" (but in reality just trading
them in for more "warnings") praised as success.

Cc: jari.tahvanainen@intel.com
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/igt.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/igt.py b/tests/igt.py
index 7ebb03646b50..473027b76936 100644
--- a/tests/igt.py
+++ b/tests/igt.py
@@ -53,6 +53,7 @@ def check_environment():
 
     """
     debugfs_path = "/sys/kernel/debug/dri"
+
     if os.getuid() != 0:
         raise exceptions.PiglitInternalError(
             "Test Environment check: not root!")
@@ -123,6 +124,10 @@ class IGTTest(Test):
         else:
             self.result.result = 'fail'
 
+        # all dmesg noise is considered a test failure when testing the kernel
+        if self.result.dmesg
+            self.result.result = 'fail'
+
 
 def list_tests(listname):
     """Parse igt test list and return them as a list."""
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/igt: dmesg noise is a kernel failure

[Chris Wilson]

On Thu, Oct 06, 2016 at 11:07:17AM +0200, Daniel Vetter wrote:
> At least when testing the kernel. In normal programs pretty much all
> the dmesg noise would simply be replaced by debug asserts, but in the
> kernel we try rely hard to not fall over minor inconsistencies.
> 
> Still for CI purposes there's not really a difference, hence don't
> treat it as such.
> 
> Motivated since once again I've seen a statistics where this was split
> up, and then a reduction of "failures" (but in reality just trading
> them in for more "warnings") praised as success.

Hear, hear! dfail == fail, and dwarn == warn.

Calling a failure, a dfail just softens that it failed.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/igt: dmesg noise is a kernel failure

[Ville Syrjälä]

On Thu, Oct 06, 2016 at 11:07:17AM +0200, Daniel Vetter wrote:
> At least when testing the kernel. In normal programs pretty much all
> the dmesg noise would simply be replaced by debug asserts, but in the
> kernel we try rely hard to not fall over minor inconsistencies.
> 
> Still for CI purposes there's not really a difference, hence don't
> treat it as such.
> 
> Motivated since once again I've seen a statistics where this was split
> up, and then a reduction of "failures" (but in reality just trading
> them in for more "warnings") praised as success.
> 
> Cc: jari.tahvanainen@intel.com
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  tests/igt.py | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tests/igt.py b/tests/igt.py
> index 7ebb03646b50..473027b76936 100644
> --- a/tests/igt.py
> +++ b/tests/igt.py
> @@ -53,6 +53,7 @@ def check_environment():
>  
>      """
>      debugfs_path = "/sys/kernel/debug/dri"
> +
>      if os.getuid() != 0:
>          raise exceptions.PiglitInternalError(
>              "Test Environment check: not root!")
> @@ -123,6 +124,10 @@ class IGTTest(Test):
>          else:
>              self.result.result = 'fail'
>  
> +        # all dmesg noise is considered a test failure when testing the kernel
> +        if self.result.dmesg
> +            self.result.result = 'fail'

Hmm. So now all errors will look the same? I think I'm usually more
interested in the dmesg warns, so this would perhaps make it harder for
me to spot them. Not sure I like that.

> +
>  
>  def list_tests(listname):
>      """Parse igt test list and return them as a list."""
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/igt: dmesg noise is a kernel failure

[Dylan Baker]

[-- Attachment #1.1: Type: text/plain, Size: 1915 bytes --]

Quoting Daniel Vetter (2016-10-06 02:07:17)
> At least when testing the kernel. In normal programs pretty much all
> the dmesg noise would simply be replaced by debug asserts, but in the
> kernel we try rely hard to not fall over minor inconsistencies.
> 
> Still for CI purposes there's not really a difference, hence don't
> treat it as such.
> 
> Motivated since once again I've seen a statistics where this was split
> up, and then a reduction of "failures" (but in reality just trading
> them in for more "warnings") praised as success.
> 
> Cc: jari.tahvanainen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
> Cc: Petri Latvala <petri.latvala-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Daniel Vetter <daniel.vetter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  tests/igt.py | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tests/igt.py b/tests/igt.py
> index 7ebb03646b50..473027b76936 100644
> --- a/tests/igt.py
> +++ b/tests/igt.py
> @@ -53,6 +53,7 @@ def check_environment():
>  
>      """
>      debugfs_path = "/sys/kernel/debug/dri"
> +
>      if os.getuid() != 0:
>          raise exceptions.PiglitInternalError(
>              "Test Environment check: not root!")
> @@ -123,6 +124,10 @@ class IGTTest(Test):
>          else:
>              self.result.result = 'fail'
>  
> +        # all dmesg noise is considered a test failure when testing the kernel
> +        if self.result.dmesg
> +            self.result.result = 'fail'
> +
>  
>  def list_tests(listname):
>      """Parse igt test list and return them as a list."""
> -- 
> 2.9.3
> 
> _______________________________________________
> Piglit mailing list
> Piglit-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/piglit

makes sense to me,
Acked-by: Dylan Baker <dylan-V4Th83Wof2GakBO8gow8eQ@public.gmane.org>

[-- Attachment #1.2: signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

-----BEGIN PGP SIGNATURE-----

iQEcBAABCAAGBQJX9pcOAAoJEAieFpYUBojvZqUH/AoODDqlyG9AQbOZ0QqFyhE2
A3v+D4nSlyrRSmS3tHUUX0zdScIOcA4DIpZKSGkwS9/lttnn4Fe3dMYME5Q2t1G7
jwLDJjyPAILUC0JRgcuT3b6I+zzjsoHRGh+I5R1hnIZfvp80/hNYlS1yzltTqkHu
/Igrw8gth2G0LO3eJ/wg4GAfTOwDPKmIjn/sBCYY/OQ7v5ZC4h419+URcSQaKGf8
B9X1w8X1zz/P02qFOhpx8YbA2hqvajlRU9voTr5bhapVXnipTpHCd4ImF4Cl0I53
avPt/BjZor5IHIUiHJ5O4KN7gBo1Q95Qo1JWEY10VDPl8suHr3TCuQ4DUe9KsfY=
=aVfM
-----END PGP SIGNATURE-----

[-- Attachment #2: Type: text/plain, Size: 151 bytes --]

_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Intel-gfx] [PATCH] tests/igt: dmesg noise is a kernel failure

[Daniel Vetter]

On Thu, Oct 6, 2016 at 11:16 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, Oct 06, 2016 at 11:07:17AM +0200, Daniel Vetter wrote:
>> At least when testing the kernel. In normal programs pretty much all
>> the dmesg noise would simply be replaced by debug asserts, but in the
>> kernel we try rely hard to not fall over minor inconsistencies.
>>
>> Still for CI purposes there's not really a difference, hence don't
>> treat it as such.
>>
>> Motivated since once again I've seen a statistics where this was split
>> up, and then a reduction of "failures" (but in reality just trading
>> them in for more "warnings") praised as success.
>
> Hear, hear! dfail == fail, and dwarn == warn.
>
> Calling a failure, a dfail just softens that it failed.

Maybe a bit more motiviation why I think all dmesg noise should be
tracked as a full-on failure. In userspace there's tons of consistency
checks, and those are done using assert(). Debug builds have them,
production ones dont, and if you hit it your app dies, and it's
counted as a failure under CI.

In the kernel it's not great if we just die, so we try very hard to
limp along in all these cases instead of dying. But it's still a
consistency check firing, and imo should be treated as such. "warn"
would then just be for for igt self-checks in the test
library/infrastructure itself.

The other reasons is CI: "warn" essentially means "something might be
wrong, a human needs to check". That's not all that useful for
automating stuff, hence why I think we really shouldn't have that
status, at least as a usual one. If we go with relabelling, then a
consequence is also that we might need to remove some more
questionable in-kernel checks.

Re Ville's concern: I don't mind whether we call it dmesg-fail or fail
in the end. If it helps folks to scan through results I'm fine with
changing it to dmesg-fail. But dmesg-warn imo needs to go (for igt). I
think dmesg-warn is the right choice for everything else, where some
kernel noise might not always mean that there's an issue with your
userspace. So again needs a human to check what's going on.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[PATCH] tests/igt: dmesg noise is a kernel failure

[Daniel Vetter]

At least when testing the kernel. In normal programs pretty much all
the dmesg noise would simply be replaced by debug asserts, but in the
kernel we try rely hard to not fall over minor inconsistencies.

Still for CI purposes there's not really a difference, hence don't
treat it as such.

Motivated since once again I've seen a statistics where this was split
up, and then a reduction of "failures" (but in reality just trading
them in for more "warnings") praised as success.

v2: Clamp to "dmesg-fail" to keep dmesg noise easily identifiable
(Ville).

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Dylan Baker <dylan@pnwbakers.com>
Cc: jari.tahvanainen@intel.com
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/igt.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/igt.py b/tests/igt.py
index 7ebb03646b50..21e55e115654 100644
--- a/tests/igt.py
+++ b/tests/igt.py
@@ -123,6 +123,10 @@ class IGTTest(Test):
         else:
             self.result.result = 'fail'
 
+        # all dmesg noise is considered a test failure when testing the kernel
+        if self.result.dmesg
+            self.result.result = 'dmesg-fail'
+
 
 def list_tests(listname):
     """Parse igt test list and return them as a list."""
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/igt: dmesg noise is a kernel failure

[Chris Wilson]

On Fri, Oct 07, 2016 at 09:06:31AM +0200, Daniel Vetter wrote:
> At least when testing the kernel. In normal programs pretty much all
> the dmesg noise would simply be replaced by debug asserts, but in the
> kernel we try rely hard to not fall over minor inconsistencies.
> 
> Still for CI purposes there's not really a difference, hence don't
> treat it as such.
> 
> Motivated since once again I've seen a statistics where this was split
> up, and then a reduction of "failures" (but in reality just trading
> them in for more "warnings") praised as success.
> 
> v2: Clamp to "dmesg-fail" to keep dmesg noise easily identifiable
> (Ville).
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Acked-by: Dylan Baker <dylan@pnwbakers.com>
> Cc: jari.tahvanainen@intel.com
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  tests/igt.py | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/igt.py b/tests/igt.py
> index 7ebb03646b50..21e55e115654 100644
> --- a/tests/igt.py
> +++ b/tests/igt.py
> @@ -123,6 +123,10 @@ class IGTTest(Test):
>          else:
>              self.result.result = 'fail'
>  
> +        # all dmesg noise is considered a test failure when testing the kernel
> +        if self.result.dmesg
> +            self.result.result = 'dmesg-fail'

This is changing a fail to dmesg-fail. I hate that.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/igt: dmesg noise is a kernel failure

[Tahvanainen, Jari]

Double checking the full picture of the impact: 
- Current - test result value list for igt (=default piglit): pass, warn, dmesg-warn, fail, dmesg-fail, timeout, crash, incomplete
- Intent with proposal below - test result value list for igt (kernel testing): pass, warn, fail, dmesg-fail, timeout, crash, incomplete
Right or wrong?
Or will fail vanish from this list too like Chris said? Or will there be scenario where one can have fail without dmesg?

BR, Jari

-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk] 
Sent: Friday, October 7, 2016 10:12 AM
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>; piglit discussion list <piglit@lists.freedesktop.org>; Ville Syrjälä <ville.syrjala@linux.intel.com>; Tahvanainen, Jari <jari.tahvanainen@intel.com>; Latvala, Petri <petri.latvala@intel.com>; Vetter, Daniel <daniel.vetter@intel.com>
Subject: Re: [PATCH] tests/igt: dmesg noise is a kernel failure

On Fri, Oct 07, 2016 at 09:06:31AM +0200, Daniel Vetter wrote:
> At least when testing the kernel. In normal programs pretty much all 
> the dmesg noise would simply be replaced by debug asserts, but in the 
> kernel we try rely hard to not fall over minor inconsistencies.
> 
> Still for CI purposes there's not really a difference, hence don't 
> treat it as such.
> 
> Motivated since once again I've seen a statistics where this was split 
> up, and then a reduction of "failures" (but in reality just trading 
> them in for more "warnings") praised as success.
> 
> v2: Clamp to "dmesg-fail" to keep dmesg noise easily identifiable 
> (Ville).
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Acked-by: Dylan Baker <dylan@pnwbakers.com>
> Cc: jari.tahvanainen@intel.com
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  tests/igt.py | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/igt.py b/tests/igt.py index 
> 7ebb03646b50..21e55e115654 100644
> --- a/tests/igt.py
> +++ b/tests/igt.py
> @@ -123,6 +123,10 @@ class IGTTest(Test):
>          else:
>              self.result.result = 'fail'
>  
> +        # all dmesg noise is considered a test failure when testing the kernel
> +        if self.result.dmesg
> +            self.result.result = 'dmesg-fail'

This is changing a fail to dmesg-fail. I hate that.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/igt: dmesg noise is a kernel failure

[Ville Syrjälä]

On Fri, Oct 07, 2016 at 08:12:02AM +0100, Chris Wilson wrote:
> On Fri, Oct 07, 2016 at 09:06:31AM +0200, Daniel Vetter wrote:
> > At least when testing the kernel. In normal programs pretty much all
> > the dmesg noise would simply be replaced by debug asserts, but in the
> > kernel we try rely hard to not fall over minor inconsistencies.
> > 
> > Still for CI purposes there's not really a difference, hence don't
> > treat it as such.
> > 
> > Motivated since once again I've seen a statistics where this was split
> > up, and then a reduction of "failures" (but in reality just trading
> > them in for more "warnings") praised as success.
> > 
> > v2: Clamp to "dmesg-fail" to keep dmesg noise easily identifiable
> > (Ville).
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Acked-by: Dylan Baker <dylan@pnwbakers.com>
> > Cc: jari.tahvanainen@intel.com
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  tests/igt.py | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/tests/igt.py b/tests/igt.py
> > index 7ebb03646b50..21e55e115654 100644
> > --- a/tests/igt.py
> > +++ b/tests/igt.py
> > @@ -123,6 +123,10 @@ class IGTTest(Test):
> >          else:
> >              self.result.result = 'fail'
> >  
> > +        # all dmesg noise is considered a test failure when testing the kernel
> > +        if self.result.dmesg
> > +            self.result.result = 'dmesg-fail'
> 
> This is changing a fail to dmesg-fail. I hate that.

I don't know if there's a way to win here. We're trying to reduce 4
states to 3 states for whatever reason, so we're going to lose
information :(

Doing it this way you'll have to go through every dmesg-fail as well as
the fails if you want to see all test failures. Doing it the other way
means going through all the fails to find the dmesg warns.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx