[PATCH 1/2] drm/i915/selftests: Flush live_evict

[PATCH 1/2] drm/i915/selftests: Flush live_evict

[Chris Wilson]

Be sure to cleanup after live_evict by flushing any residual state off
the GPU using igt_flush_test.

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

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 8c69198c7e4e..a3cb0aade6f1 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -28,6 +28,7 @@
 
 #include "i915_selftest.h"
 
+#include "igt_flush_test.h"
 #include "lib_sw_fence.h"
 #include "mock_drm.h"
 #include "mock_gem_device.h"
@@ -505,6 +506,8 @@ static int igt_evict_contexts(void *arg)
 
 	mutex_lock(&i915->drm.struct_mutex);
 out_locked:
+	if (igt_flush_test(i915, I915_WAIT_LOCKED))
+		err = -EIO;
 	while (reserved) {
 		struct reserved *next = reserved->next;
 
-- 
2.20.1

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

[PATCH 2/2] drm/i915: Don't dereference request if it may have been retired

[Chris Wilson]

This has count me out on countless occasions, when we retrieve a pointer
from the submission/execlists backend, it does not carry a reference to
the context or ring. Those are only pinned while the rquest is active,
so if we see the request is completed, it may be in the process of being
retired and those pointers defunct.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110938
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 898692989313..7fd33e81c2d9 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1311,12 +1311,13 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len)
 	}
 }
 
-static void intel_engine_print_registers(const struct intel_engine_cs *engine,
+static void intel_engine_print_registers(struct intel_engine_cs *engine,
 					 struct drm_printer *m)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
 	const struct intel_engine_execlists * const execlists =
 		&engine->execlists;
+	unsigned long flags;
 	u64 addr;
 
 	if (engine->id == RCS0 && IS_GEN_RANGE(dev_priv, 4, 7))
@@ -1397,15 +1398,16 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine,
 				   idx, hws[idx * 2], hws[idx * 2 + 1]);
 		}
 
-		rcu_read_lock();
+		spin_lock_irqsave(&engine->active.lock, flags);
 		for (idx = 0; idx < execlists_num_ports(execlists); idx++) {
 			struct i915_request *rq;
 			unsigned int count;
+			char hdr[80];
 
 			rq = port_unpack(&execlists->port[idx], &count);
-			if (rq) {
-				char hdr[80];
-
+			if (!rq) {
+				drm_printf(m, "\t\tELSP[%d] idle\n", idx);
+			} else if (!i915_request_signaled(rq)) {
 				snprintf(hdr, sizeof(hdr),
 					 "\t\tELSP[%d] count=%d, ring:{start:%08x, hwsp:%08x, seqno:%08x}, rq: ",
 					 idx, count,
@@ -1414,11 +1416,11 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine,
 					 hwsp_seqno(rq));
 				print_request(m, rq, hdr);
 			} else {
-				drm_printf(m, "\t\tELSP[%d] idle\n", idx);
+				print_request(m, rq, "\t\tELSP[%d] rq: ");
 			}
 		}
 		drm_printf(m, "\t\tHW active? 0x%x\n", execlists->active);
-		rcu_read_unlock();
+		spin_unlock_irqrestore(&engine->active.lock, flags);
 	} else if (INTEL_GEN(dev_priv) > 6) {
 		drm_printf(m, "\tPP_DIR_BASE: 0x%08x\n",
 			   ENGINE_READ(engine, RING_PP_DIR_BASE));
-- 
2.20.1

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

Re: [PATCH 2/2] drm/i915: Don't dereference request if it may have been retired

[Chris Wilson]

Quoting Chris Wilson (2019-06-18 17:19:51)
> This has count me out on countless occasions, when we retrieve a pointer
> from the submission/execlists backend, it does not carry a reference to
> the context or ring. Those are only pinned while the rquest is active,
> so if we see the request is completed, it may be in the process of being
> retired and those pointers defunct.
> 

I guess
Fixes: 3a068721a973 ("drm/i915: Show ring->start for the ELSP context/request queue")
v4.18!

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110938
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/selftests: Flush live_evict

[Tvrtko Ursulin]

On 18/06/2019 17:19, Chris Wilson wrote:
> Be sure to cleanup after live_evict by flushing any residual state off
> the GPU using igt_flush_test.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/selftests/i915_gem_evict.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> index 8c69198c7e4e..a3cb0aade6f1 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> @@ -28,6 +28,7 @@
>   
>   #include "i915_selftest.h"
>   
> +#include "igt_flush_test.h"
>   #include "lib_sw_fence.h"
>   #include "mock_drm.h"
>   #include "mock_gem_device.h"
> @@ -505,6 +506,8 @@ static int igt_evict_contexts(void *arg)
>   
>   	mutex_lock(&i915->drm.struct_mutex);
>   out_locked:
> +	if (igt_flush_test(i915, I915_WAIT_LOCKED))
> +		err = -EIO;
>   	while (reserved) {
>   		struct reserved *next = reserved->next;
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH 2/2] drm/i915: Don't dereference request if it may have been retired

[Tvrtko Ursulin]

On 18/06/2019 17:19, Chris Wilson wrote:
> This has count me out on countless occasions, when we retrieve a pointer
> from the submission/execlists backend, it does not carry a reference to
> the context or ring. Those are only pinned while the rquest is active,
> so if we see the request is completed, it may be in the process of being
> retired and those pointers defunct.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110938
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 898692989313..7fd33e81c2d9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -1311,12 +1311,13 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len)
>   	}
>   }
>   
> -static void intel_engine_print_registers(const struct intel_engine_cs *engine,
> +static void intel_engine_print_registers(struct intel_engine_cs *engine,
>   					 struct drm_printer *m)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
>   	const struct intel_engine_execlists * const execlists =
>   		&engine->execlists;
> +	unsigned long flags;
>   	u64 addr;
>   
>   	if (engine->id == RCS0 && IS_GEN_RANGE(dev_priv, 4, 7))
> @@ -1397,15 +1398,16 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine,
>   				   idx, hws[idx * 2], hws[idx * 2 + 1]);
>   		}
>   
> -		rcu_read_lock();
> +		spin_lock_irqsave(&engine->active.lock, flags);
>   		for (idx = 0; idx < execlists_num_ports(execlists); idx++) {
>   			struct i915_request *rq;
>   			unsigned int count;
> +			char hdr[80];
>   
>   			rq = port_unpack(&execlists->port[idx], &count);
> -			if (rq) {
> -				char hdr[80];
> -
> +			if (!rq) {
> +				drm_printf(m, "\t\tELSP[%d] idle\n", idx);
> +			} else if (!i915_request_signaled(rq)) {
>   				snprintf(hdr, sizeof(hdr),
>   					 "\t\tELSP[%d] count=%d, ring:{start:%08x, hwsp:%08x, seqno:%08x}, rq: ",
>   					 idx, count,
> @@ -1414,11 +1416,11 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine,
>   					 hwsp_seqno(rq));
>   				print_request(m, rq, hdr);
>   			} else {
> -				drm_printf(m, "\t\tELSP[%d] idle\n", idx);
> +				print_request(m, rq, "\t\tELSP[%d] rq: ");
>   			}
>   		}
>   		drm_printf(m, "\t\tHW active? 0x%x\n", execlists->active);
> -		rcu_read_unlock();
> +		spin_unlock_irqrestore(&engine->active.lock, flags);
>   	} else if (INTEL_GEN(dev_priv) > 6) {
>   		drm_printf(m, "\tPP_DIR_BASE: 0x%08x\n",
>   			   ENGINE_READ(engine, RING_PP_DIR_BASE));
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH 2/2] drm/i915: Don't dereference request if it may have been retired

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-06-18 17:58:02)
> 
> On 18/06/2019 17:19, Chris Wilson wrote:
> > This has count me out on countless occasions, when we retrieve a pointer
> > from the submission/execlists backend, it does not carry a reference to
> > the context or ring. Those are only pinned while the rquest is active,
> > so if we see the request is completed, it may be in the process of being
> > retired and those pointers defunct.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110938
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
[snip]
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks for the quick review, pushed to see if CI wakes up happy in the
morning.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/selftests: Flush live_evict

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/selftests: Flush live_evict
URL   : https://patchwork.freedesktop.org/series/62325/
State : failure

== Summary ==

Applying: drm/i915/selftests: Flush live_evict
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/selftests/i915_gem_evict.c
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: drm/i915: Don't dereference request if it may have been retired
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/gt/intel_engine_cs.c
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.

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