Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 1/2] i915/gem_eio: Check average reset times

[Mika Kuoppala via igt-dev <igt-dev@lists.freedesktop.org>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-02-15 14:40:30)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > As we have moved to rcu/srcu to serialise the resets, individual resets
>> > are subject to small variations in system grace periods. Allow for this
>> > by only expecting the median reset time to be within our target, thereby
>> > excluding noisy outliers from perturbing our results (but keep the
>> > maximum capped to prevent horrid failures!)
>> >
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  tests/i915/gem_eio.c | 53 ++++++++++++++++++++++++++++++++++----------
>> >  1 file changed, 41 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
>> > index 61054a07e..bd5266104 100644
>> > --- a/tests/i915/gem_eio.c
>> > +++ b/tests/i915/gem_eio.c
>> > @@ -42,6 +42,7 @@
>> >  
>> >  #include "igt.h"
>> >  #include "igt_device.h"
>> > +#include "igt_stats.h"
>> >  #include "igt_sysfs.h"
>> >  #include "sw_sync.h"
>> >  #include "i915/gem_ring.h"
>> > @@ -239,10 +240,9 @@ static void hang_after(int fd, unsigned int us, struct timespec *ts)
>> >       igt_assert_eq(timer_settime(ctx->timer, 0, &its, NULL), 0);
>> >  }
>> >  
>> > -static void check_wait(int fd, uint32_t bo, unsigned int wait)
>> > +static void check_wait(int fd, uint32_t bo, unsigned int wait, igt_stats_t *st)
>> >  {
>> >       struct timespec ts = {};
>> > -     uint64_t elapsed;
>> >  
>> >       if (wait) {
>> >               hang_after(fd, wait, &ts);
>> > @@ -253,10 +253,34 @@ static void check_wait(int fd, uint32_t bo, unsigned int wait)
>> >  
>> >       gem_sync(fd, bo);
>> >  
>> > -     elapsed = igt_nsec_elapsed(&ts);
>> > -     igt_assert_f(elapsed < 250e6,
>> > -                  "Wake up following reset+wedge took %.3fms\n",
>> > -                  elapsed*1e-6);
>> > +     if (st)
>> > +             igt_stats_push(st, igt_nsec_elapsed(&ts));
>> 
>> You prolly want igt_nsec_elapsed() primed regardless
>> of wait, otherwise I see -1 pushed polluting the stats.
>
> It is...
>
> if (wait)
> 	hang_after() sets ts start point
> else
> 	we set ts start point.

Indeed. It would have helped to read the
parameters for hang_after.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev