Re: [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 
> >> Cc: Michal Winiarski 
> >> Signed-off-by: Antonio Argenziano 
> >> ---
> >>   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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Antonio Argenziano]

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 
Cc: Michal Winiarski 
Signed-off-by: Antonio Argenziano 
---
  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?





+
+   gem_context_destroy(fd, ctx[LO]);
+   gem_context_destroy(fd, ctx[HI]);
+}
+
  static void deep(int fd, unsigned ring)
  {
  #define XS 8
@@ -1033,6 +1066,12 @@ igt_main
 preempt(fd, e->exec_id | 
e->flags, NEW_CTX | HANG_LP);
 igt_fork_hang_detector(fd);
 }
+
+   igt_subtest_f("bad-preemptive-%s", 
e->name) {
+   igt_stop_hang_detector();
+   bad_preemptive(fd, e->exec_id | 
e->flags);
+   igt_fork_hang_detector(fd);
+   }


Split into non-hang/hang portions and use igt_subtest_group +
igt_fixture.


OK, will do.

-Antonio


-Chris


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

Re: [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 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 
> Cc: Michal Winiarski 
> Signed-off-by: Antonio Argenziano 
> ---
>  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.

> +
> +   gem_context_destroy(fd, ctx[LO]);
> +   gem_context_destroy(fd, ctx[HI]);
> +}
> +
>  static void deep(int fd, unsigned ring)
>  {
>  #define XS 8
> @@ -1033,6 +1066,12 @@ igt_main
> preempt(fd, e->exec_id | 
> e->flags, NEW_CTX | HANG_LP);
> igt_fork_hang_detector(fd);
> }
> +
> +   igt_subtest_f("bad-preemptive-%s", 
> e->name) {
> +   igt_stop_hang_detector();
> +   bad_preemptive(fd, e->exec_id 
> | e->flags);
> +   igt_fork_hang_detector(fd);
> +   }

Split into non-hang/hang portions and use igt_subtest_group +
igt_fixture.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx