[Intel-gfx] [PATCH igt 20/24] lib/core: Don't leak dummyloads between subtests

[Chris Wilson]

Quoting Daniel Vetter (2017-08-15 09:01:55)
> On Mon, Aug 14, 2017 at 09:18:44PM +0100, Chris Wilson wrote:
> > If a test fails or skips early, it may not clean up after itself. In
> > lieu of having a framework for test deconstructors, hook
> > igt_terminate_spin_batches() into exit_subtest() itself so that we don't
> > allow a recursive batch from an earlier test to leak into the next and
> > cause an unexpected GPU hang.
> > 
> > Similarly, we also want to terminate the dummyload as the first step in
> > our atexit handlers (currently it is at the start of the last step) as
> > some atexit handlers may be unwittingly exposed to dummyloads and so
> > cause another wait on GPU hang.
> > 
> > We trust that the core already distinguishes correctly between the
> > principal test process and its children.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  lib/igt_core.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > index ce8482a1..e96fd4e6 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -63,6 +63,7 @@
> >  #include "intel_chipset.h"
> >  #include "intel_io.h"
> >  #include "igt_debugfs.h"
> > +#include "igt_dummyload.h"
> >  #include "igt_ftrace.h"
> >  #include "version.h"
> >  #include "config.h"
> > @@ -1039,6 +1040,8 @@ static void exit_subtest(const char *result)
> >              (!__igt_plain_output) ? "\x1b[0m" : "");
> >       fflush(stdout);
> >  
> > +     igt_terminate_spin_batches();
> 
> Sounds like time for a subtest exit handler infrastructure?

It's been time for that for the several years. I keep asking for test
constructors and destructors.

> On that, before I start typing: Can we make a simple assumption like:
> - exit handler registered from fixture -> clean up in exit handler
> - exit handler registered within subtest -> clean up at end of subtest
> - do we need anything special for subtest_group?

subtest group just wrap a fixture. But those fixtures are just
masquerading for constructors and destructors, as well as doing the
pre-requisite tests.

> 
> I guess this also means that we need to properly stack exit handlers.
> 
> Sprinkling this all over manually doesn't really feel good ...

But foolproof. Look at the issue with the atexit handler that spinbatch
installs for itself, or rather uses the existing one. They run foul of
ordering issues where LIFO is not always the best policy.
-Chris