[igt-dev] [PATCH i-g-t] lib/kmod: Stop producing results at all for kernel selftests on taint

[Petri Latvala]

On Tue, Mar 02, 2021 at 01:29:08PM +0000, Chris Wilson wrote:
> Quoting Petri Latvala (2021-03-02 13:23:08)
> > On Tue, Mar 02, 2021 at 01:04:29PM +0000, Chris Wilson wrote:
> > > Quoting Petri Latvala (2021-03-02 12:39:35)
> > > > On Tue, Mar 02, 2021 at 11:48:02AM +0000, Chris Wilson wrote:
> > > > > Quoting Petri Latvala (2021-02-25 08:35:53)
> > > > > > Instead of producing skips for the rest when one selftest taints the
> > > > > > kernel, stop running them altogether. Having the skips produced yields
> > > > > > no value and just makes future improvements (like correctly tagging
> > > > > > tests that cause taints) harder. In effect, this gets us back to the
> > > > > > old setup when tainting made igt_runner immediately kill the test and
> > > > > > similarly made us not get spurious results for the rest of the
> > > > > > selftests.
> > > > > 
> > > > > Skip isn't spurious here. I don't see how this impacts tagging the
> > > > > earlier tests. So the only question for me is whether SKIP or NOTRUN is
> > > > > more applicable. And skip is far more informative when run by hand...
> > > > 
> > > > When run by hand on an already tainted kernel, the first (specified)
> > > > selftest still gets a skip and others don't get executed.
> > > > 
> > > > The immediate value I'll be getting from this is my ongoing work on
> > > > the ugly igt at runner@aborted pseudoresult to instead mark the last
> > > > executed test as 'ABORT' result when igt_runner decides to abort the
> > > > execution (due to a taint). If we have skips, I'd need to do some
> > > > selftest-specific handling to not mark the last executed dynamic
> > > > subtest, but the last non-skipping dynamic subtest.
> > > 
> > > The problem does not seem to be intrinsic to igt_kselftests, as any
> > > dynamic subtest that triggers a taint will then proceed to run the next
> > > igt_dynamic which may pass, fail or skip. The runner may not have a
> > > chance to check for a taint until the end of the subtest. It sounds like
> > > a job for the igt_runner <-> igt channel.
> > 
> > Would also require checking for taint mid-execution...
> 
> Yes, don't we currently check on a timer, but if there was a channel for
> intermediate test results, we would check then as well.

Ah yes a slight confusion on my part. Aborting, in general, is checked
between exec()s. Tainting check is done on any activity.

> > Anyway best practices for dynamic subtests is to never skip, but
> > instead not exist at all. I suppose I could instead mark every single
> > dynamic subtest as abort when abort happens.
> 
> Why is the test itself marked as abort? The run is aborted, the test
> failed.

Marking the test itself helps handling the aborts better, in the bug
analysis sense. Instead of just seeing the text "hey we stopped
everything after this test", you have that test's logs.

So, not only did the test fail, it caused us to abort, is the
sentiment to send with the abort result.


-- 
Petri Latvala