diff mbox series

[1/2] drm/i915/execlists: Protect peeking at execlists->active

Message ID 20191009100955.21477-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915/execlists: Protect peeking at execlists->active | expand

Commit Message

Chris Wilson Oct. 9, 2019, 10:09 a.m. UTC
Now that we dropped the engine->active.lock serialisation from around
process_csb(), direct submission can run concurrently to the interrupt
handler. As such execlists->active may be advanced as we dequeue,
dropping the reference to the request. We need to employ our RCU request
protection to ensure that the request is not freed too early.

Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Tvrtko Ursulin Oct. 9, 2019, 3:38 p.m. UTC | #1
On 09/10/2019 11:09, Chris Wilson wrote:
> Now that we dropped the engine->active.lock serialisation from around
> process_csb(), direct submission can run concurrently to the interrupt
> handler. As such execlists->active may be advanced as we dequeue,
> dropping the reference to the request. We need to employ our RCU request
> protection to ensure that the request is not freed too early.
> 
> Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_lrc.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 6db762c509b8..7ea58335f04c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1289,7 +1289,7 @@ static void virtual_xfer_breadcrumbs(struct virtual_engine *ve,
>   static struct i915_request *
>   last_active(const struct intel_engine_execlists *execlists)
>   {
> -	struct i915_request * const *last = execlists->active;
> +	struct i915_request * const *last = READ_ONCE(execlists->active);
>   
>   	while (*last && i915_request_completed(*last))
>   		last++;
> @@ -1981,8 +1981,11 @@ static void process_csb(struct intel_engine_cs *engine)
>   static void __execlists_submission_tasklet(struct intel_engine_cs *const engine)
>   {
>   	lockdep_assert_held(&engine->active.lock);
> -	if (!engine->execlists.pending[0])
> +	if (!engine->execlists.pending[0]) {
> +		rcu_read_lock(); /* protect peeking at execlists->active */
>   		execlists_dequeue(engine);
> +		rcu_read_unlock();
> +	}
>   }
>   
>   /*
> 

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

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 6db762c509b8..7ea58335f04c 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1289,7 +1289,7 @@  static void virtual_xfer_breadcrumbs(struct virtual_engine *ve,
 static struct i915_request *
 last_active(const struct intel_engine_execlists *execlists)
 {
-	struct i915_request * const *last = execlists->active;
+	struct i915_request * const *last = READ_ONCE(execlists->active);
 
 	while (*last && i915_request_completed(*last))
 		last++;
@@ -1981,8 +1981,11 @@  static void process_csb(struct intel_engine_cs *engine)
 static void __execlists_submission_tasklet(struct intel_engine_cs *const engine)
 {
 	lockdep_assert_held(&engine->active.lock);
-	if (!engine->execlists.pending[0])
+	if (!engine->execlists.pending[0]) {
+		rcu_read_lock(); /* protect peeking at execlists->active */
 		execlists_dequeue(engine);
+		rcu_read_unlock();
+	}
 }
 
 /*