[igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings

[Daniel Vetter]

On Thu, Nov 22, 2018 at 10:41:26AM +0100, Daniel Vetter wrote:
> On Wed, Nov 21, 2018 at 12:05:45PM +0200, Joonas Lahtinen wrote:
> > Quoting Chris Wilson (2018-11-21 11:18:43)
> > > Quoting Joonas Lahtinen (2018-11-21 08:07:30)
> > > > Quoting Ville Syrjälä (2018-11-14 15:53:26)
> > > > > On Wed, Nov 14, 2018 at 01:34:10PM +0000, Chris Wilson wrote:
> > > > > > Quoting Ville Syrjälä (2018-11-14 13:25:39)
> > > > > > > On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> > > > > > > > On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > > > > > > > > I have whinged on for ages about the dmesg-warnings being an expected
> > > > > > > > > part of kernel testing (where else is the kernel meant to log its
> > > > > > > > > errors?) and should be treated the same as our stderr for the test. That
> > > > > > > > > is if a test fails, it fails and does not need to be conflated with
> > > > > > > > > whether or not there was a dmesg warning (just as the test saying why it
> > > > > > > > > failed on stderr does not need flagging), and that a passing test with a
> > > > > > > > > dmesg warning is simply a warn.
> > > > > > > > > 
> > > > > > > > > The effect is that we simply remove the "dmesg-" flagging from results
> > > > > > > > > names, as the err/dmesg output is simply collated for the error report
> > > > > > > > > already.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > > > > > > Cc: Petri Latvala <petri.latvala at intel.com>
> > > > > > > > > Cc: Martin Peres <martin.peres at linux.intel.com>
> > > > > > > > 
> > > > > > > > 
> > > > > > > > Yep, now's a good a time as any to remove this cargo cult if ever.
> > > > > > > > 
> > > > > > > > Some acks from kernel people in general would be nice.
> > > > > > > > 
> > > > > > > > TODO:
> > > > > > > >  Martin: cibuglog filters ready for this
> > > > > > > >  Tomi et al: Visualization ready for this (tooltips etc)
> > > > > > > 
> > > > > > > Can I still see from the color/etc. whether there was some dmesg spew or
> > > > > > > not? I often glance at those and would prefer to not have to go through
> > > > > > > all the fails to find them.
> > > > > > 
> > > > > > It was never shown in the colour:
> > > > > > 
> > > > > >       warn/dmesg-warn == orange
> > > > > >       fail/dmesg-fail == red
> > > > > 
> > > > > Indeed. Which makes me wonder whether I've been overlooking some
> > > > > dmesg noise. Either that or it's just my memory playing tricks
> > > > > and telling me that I've mostly been looking at the orange stuff.
> > > > > 
> > > > > So let's consider this a feature request then. I would like to see
> > > > > from the overview which tests produced dmesg noise, regardless of
> > > > > whether the test otherwise succeeded or failed.
> > > > 
> > > > I think this is a fair change.
> > > > 
> > > > And for documentation purposes, what I proposed in IRC:
> > > > 
> > > > My suggestion was to add letter "D" to the matrix when there is dmesg
> > > > noise in the specific subtest (Similarly as there's letter T in
> > > > timeout).
> > > 
> > > I just don't understand what is "dmesg noise". dmesg is where I log my
> > > errors, there is no other suitable log.
> > 
> > My take on this is that it provides a quick differentation of:
> > 
> > "The IGT test fails, functionality lost"
> > 
> > "The IGT test passes, functionality is there, but makes kernel spew
> > error (recoverably)"
> > 
> > Both are errors, but there's informational value in seeing the
> > difference, at least to me.
> 
> There's lots of igt tests that entirely rely on the in-kernel checks to
> validate stuff. dmesg output in that case means "test failed,
> functionality lost".
> 
> I'm kinda leaning towards just marking anything with dmesg warn-and-worse
> as "fail". And restrict warn/orange to igt_warn and friends. If that's too
> much we can think about a DRM_WARN and using warning level in syslog to
> result in "warn", and error and worse levels in "fail".
> 
> Kernel on fire, but hey look the test passed is not a conclusion we should
> ever support imo. Given that Joonas has it, I'd say we need to be more
> clear here that dmesg = stuff failed hard.

This would also match what we do in userspace testing in mesa, where debug
builds with assert() enabled also fail hard. And warning is reserved only
for stderr output, i.e. a conscious decision that this failure should only
be reported where no one ever really looks. At least not users :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch