[PATCH i-g-t] lib/i915: Trim ring measurement by one

[PATCH i-g-t] lib/i915: Trim ring measurement by one

[Chris Wilson]

Be a little more conservative in our ring measurement and exclude one
batch to leave room in case our user needs to wrap (where a request will
be expanded to cover the unused space at the end of the ring).

References: https://bugs.freedesktop.org/show_bug.cgi?id=111374
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/i915/gem_ring.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/i915/gem_ring.c b/lib/i915/gem_ring.c
index fdb9fc1b1..bf7f439e1 100644
--- a/lib/i915/gem_ring.c
+++ b/lib/i915/gem_ring.c
@@ -103,7 +103,7 @@ __gem_measure_ring_inflight(int fd, unsigned int engine, enum measure_ring_flags
 	} while (1);
 
 	igt_assert_eq(__execbuf(fd, &execbuf), -EINTR);
-	igt_assert(count);
+	igt_assert(count > 1);
 
 	memset(&itv, 0, sizeof(itv));
 	setitimer(ITIMER_REAL, &itv, NULL);
@@ -118,7 +118,8 @@ __gem_measure_ring_inflight(int fd, unsigned int engine, enum measure_ring_flags
 
 	gem_quiescent_gpu(fd);
 
-	return count;
+	/* Be conservative in case we must wrap later */
+	return count - 1;
 }
 
 /**
-- 
2.23.0.rc1

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

Re: [PATCH i-g-t] lib/i915: Trim ring measurement by one

[Mika Kuoppala]

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

> Be a little more conservative in our ring measurement and exclude one
> batch to leave room in case our user needs to wrap (where a request will
> be expanded to cover the unused space at the end of the ring).
>

did read the wrapping part and that seems to be the case that
we enlarge the wrapping request.

However do we lose some coverage on the actual wrap tests?

-Mika

> References: https://bugs.freedesktop.org/show_bug.cgi?id=111374
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  lib/i915/gem_ring.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/i915/gem_ring.c b/lib/i915/gem_ring.c
> index fdb9fc1b1..bf7f439e1 100644
> --- a/lib/i915/gem_ring.c
> +++ b/lib/i915/gem_ring.c
> @@ -103,7 +103,7 @@ __gem_measure_ring_inflight(int fd, unsigned int engine, enum measure_ring_flags
>  	} while (1);
>  
>  	igt_assert_eq(__execbuf(fd, &execbuf), -EINTR);
> -	igt_assert(count);
> +	igt_assert(count > 1);
>  
>  	memset(&itv, 0, sizeof(itv));
>  	setitimer(ITIMER_REAL, &itv, NULL);
> @@ -118,7 +118,8 @@ __gem_measure_ring_inflight(int fd, unsigned int engine, enum measure_ring_flags
>  
>  	gem_quiescent_gpu(fd);
>  
> -	return count;
> +	/* Be conservative in case we must wrap later */
> +	return count - 1;
>  }
>  
>  /**
> -- 
> 2.23.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] lib/i915: Trim ring measurement by one

[Chris Wilson]

Quoting Mika Kuoppala (2019-08-12 12:52:56)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Be a little more conservative in our ring measurement and exclude one
> > batch to leave room in case our user needs to wrap (where a request will
> > be expanded to cover the unused space at the end of the ring).
> >
> 
> did read the wrapping part and that seems to be the case that
> we enlarge the wrapping request.
> 
> However do we lose some coverage on the actual wrap tests?

There's only one with that idea in mind, gem_ringfill(). Which is
already "broken" because it assumes that all engines have the same ring
size, so we will be underfilling most. Could also do with an ALL_ENGINES
mode.

There's nothing that explicitly tries to hit all the possible ways in
which ring wrapping occurs, it tends to fallout of faux-stress tests
like gem_exec_parallel or gem_ctx_create.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] lib/i915: Trim ring measurement by one

[Chris Wilson]

Quoting Mika Kuoppala (2019-08-12 12:52:56)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Be a little more conservative in our ring measurement and exclude one
> > batch to leave room in case our user needs to wrap (where a request will
> > be expanded to cover the unused space at the end of the ring).
> >
> 
> did read the wrapping part and that seems to be the case that
> we enlarge the wrapping request.
> 
> However do we lose some coverage on the actual wrap tests?

The tests where we call measure_ring_size, are those that we do not want
to block due to running out of space (and wrapping) :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] lib/i915: Trim ring measurement by one

[Mika Kuoppala]

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

> Quoting Mika Kuoppala (2019-08-12 12:52:56)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Be a little more conservative in our ring measurement and exclude one
>> > batch to leave room in case our user needs to wrap (where a request will
>> > be expanded to cover the unused space at the end of the ring).
>> >
>> 
>> did read the wrapping part and that seems to be the case that
>> we enlarge the wrapping request.
>> 
>> However do we lose some coverage on the actual wrap tests?
>
> The tests where we call measure_ring_size, are those that we do not want
> to block due to running out of space (and wrapping) :)

Ok. The bugs on the wrap itself should bubble to surface
indirectly. Agreed that estimating the inflight wrong
is bad.

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