diff mbox

[2/2] drm/i915/selftests: Flush GPU activity before completing live_contexts

Message ID 20180505091014.26126-2-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson May 5, 2018, 9:10 a.m. UTC
igt_ctx_exec() expects that we retire all active requests/objects before
completing, so that when we clean up the files afterwards they are ready
to be freed. Before we do so, it is then prudent to ensure that we have
indeed retired the GPU activity, raising an error if it fails. If we do
not, we run the risk of triggering an assertion when freeing the object:

  __i915_gem_free_objects:4793 GEM_BUG_ON(i915_gem_object_is_active(obj))

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/selftests/i915_gem_context.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Chris Wilson May 8, 2018, 10:46 a.m. UTC | #1
Quoting Chris Wilson (2018-05-05 10:10:14)
> igt_ctx_exec() expects that we retire all active requests/objects before
> completing, so that when we clean up the files afterwards they are ready
> to be freed. Before we do so, it is then prudent to ensure that we have
> indeed retired the GPU activity, raising an error if it fails. If we do
> not, we run the risk of triggering an assertion when freeing the object:
> 
>   __i915_gem_free_objects:4793 GEM_BUG_ON(i915_gem_object_is_active(obj))
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Ping?

>  drivers/gpu/drm/i915/selftests/i915_gem_context.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index 7ecaed50d0b9..ddb03f009232 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -23,6 +23,7 @@
>   */
>  
>  #include "../i915_selftest.h"
> +#include "igt_flush_test.h"
>  
>  #include "mock_drm.h"
>  #include "huge_gem_object.h"
> @@ -411,6 +412,8 @@ static int igt_ctx_exec(void *arg)
>         }
>  
>  out_unlock:
> +       if (igt_flush_test(i915, I915_WAIT_LOCKED))
> +               err = -EIO;
>         mutex_unlock(&i915->drm.struct_mutex);
>  
>         mock_file_free(i915, file);
> -- 
> 2.17.0
>
Mika Kuoppala May 8, 2018, 11:38 a.m. UTC | #2
Chris Wilson <chris@chris-wilson.co.uk> writes:

> igt_ctx_exec() expects that we retire all active requests/objects before
> completing, so that when we clean up the files afterwards they are ready
> to be freed. Before we do so, it is then prudent to ensure that we have
> indeed retired the GPU activity, raising an error if it fails. If we do
> not, we run the risk of triggering an assertion when freeing the object:
>
>   __i915_gem_free_objects:4793 GEM_BUG_ON(i915_gem_object_is_active(obj))
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/selftests/i915_gem_context.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index 7ecaed50d0b9..ddb03f009232 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -23,6 +23,7 @@
>   */
>  
>  #include "../i915_selftest.h"
> +#include "igt_flush_test.h"
>  
>  #include "mock_drm.h"
>  #include "huge_gem_object.h"
> @@ -411,6 +412,8 @@ static int igt_ctx_exec(void *arg)
>  	}
>  
>  out_unlock:
> +	if (igt_flush_test(i915, I915_WAIT_LOCKED))
> +		err = -EIO;
>  	mutex_unlock(&i915->drm.struct_mutex);
>  
>  	mock_file_free(i915, file);
> -- 
> 2.17.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Chris Wilson May 8, 2018, 11:45 a.m. UTC | #3
Quoting Mika Kuoppala (2018-05-08 12:38:40)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > igt_ctx_exec() expects that we retire all active requests/objects before
> > completing, so that when we clean up the files afterwards they are ready
> > to be freed. Before we do so, it is then prudent to ensure that we have
> > indeed retired the GPU activity, raising an error if it fails. If we do
> > not, we run the risk of triggering an assertion when freeing the object:
> >
> >   __i915_gem_free_objects:4793 GEM_BUG_ON(i915_gem_object_is_active(obj))
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Much appreciated; those purple CI blobs kept worrying me. "I thought I
fixed this already!" ;)

Pushed,
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 7ecaed50d0b9..ddb03f009232 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -23,6 +23,7 @@ 
  */
 
 #include "../i915_selftest.h"
+#include "igt_flush_test.h"
 
 #include "mock_drm.h"
 #include "huge_gem_object.h"
@@ -411,6 +412,8 @@  static int igt_ctx_exec(void *arg)
 	}
 
 out_unlock:
+	if (igt_flush_test(i915, I915_WAIT_LOCKED))
+		err = -EIO;
 	mutex_unlock(&i915->drm.struct_mutex);
 
 	mock_file_free(i915, file);