[Intel-gfx] [PATCH i-g-t 3/3] tests/gem_exec_schedule: Add test for resetting preemptive batch

[Chris Wilson]

Quoting Antonio Argenziano (2017-12-04 18:25:16)
> 
> 
> On 04/12/17 09:42, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2017-12-04 17:23:15)
> >> This patch adds a test that will trigger a preemption of a low priority
> >> batch by a 'bad' batch buffer which will hang. The test aims at making
> >> sure that a hanging high priority batch will not disrupt the submission
> >> flow of low priority contexts.
> >>
> >> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >> Cc: Michal Winiarski <michal.winiarski at intel.com>
> >> Signed-off-by: Antonio Argenziano <antonio.argenziano at intel.com>
> >> ---
> >>   tests/gem_exec_schedule.c | 39 +++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 39 insertions(+)
> >>
> >> diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c
> >> index ae44a6c0..5a4d63f9 100644
> >> --- a/tests/gem_exec_schedule.c
> >> +++ b/tests/gem_exec_schedule.c
> >> @@ -511,6 +511,39 @@ static void preempt_self(int fd, unsigned ring)
> >>          gem_close(fd, result);
> >>   }
> >>   
> >> +static void bad_preemptive(int fd, unsigned ring)
> >> +{
> >> +       igt_spin_t *spin[16];
> >> +       igt_spin_t *bad;
> >> +       uint32_t ctx[2];
> >> +
> >> +       ctx[LO] = gem_context_create(fd);
> >> +       gem_context_set_priority(fd, ctx[LO], MIN_PRIO);
> >> +
> >> +       ctx[HI] = gem_context_create(fd);
> >> +       gem_context_set_priority(fd, ctx[HI], MAX_PRIO);
> >> +
> >> +       for (int n = 0; n < 16; n++) {
> >> +               gem_context_destroy(fd, ctx[LO]);
> >> +               ctx[LO] = gem_context_create(fd);
> >> +               gem_context_set_priority(fd, ctx[LO], MIN_PRIO);
> >> +
> >> +               spin[n] = __igt_spin_batch_new(fd, ctx[LO], ring, 0, true);
> >> +               igt_debug("spin[%d].handle=%d\n", n, spin[n]->handle);
> >> +       }
> >> +
> >> +       bad = igt_spin_batch_new(fd, ctx[HI], ring, 0, false);
> >> +       gem_wait(fd, bad->handle, NULL);
> >> +
> >> +       for (int n = 0; n < 16; n++) {
> >> +               igt_assert(gem_bo_busy(fd, spin[n]->handle));
> >> +               igt_spin_batch_free(fd, spin[n]);
> >> +       }
> > 
> > Is this really policy you want to enforce? I certainly don't intend to
> > mandate that the kernel isn't allowed to use RT throttling.
> 
> Yeah, I wasn't sure about that. Is a gem_wait on each of them enough or 
> should I put a write at the end of each low priority batch to make sure 
> they actually executed?

Just a comment that this is what happens right now, but we may
enact a different policy when we have a real scheduler. Part of the
trick is remembering which tests are allowed to be broken, because on
the whole igt define the behaviour which is expected/relied on by
clients. (The converse is also true in that we may scope out the limits
of user behaviour in igt and then fix the kernel to suite.)

As regards gem_wait(), this here is gem_sync(), and you can neaten up
ctx[LO] by scoping the context to the loop.

And call this something like preemptive_hang; bad* brings bad memories
of tests that were deliberately broken as opposed to demonstrating that
the code survives abuse.
-Chris