drm/i915/execlists: Set priority hint prior to submission

Submitted by Chris Wilson on Aug. 21, 2019, 2:23 p.m.

Details

Message ID 20190821142336.21609-1-chris@chris-wilson.co.uk
State Accepted
Commit a20ab592d1a87218229109d109b8e2feae6f598d
Headers show
Series "drm/i915/execlists: Set priority hint prior to submission" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Chris Wilson Aug. 21, 2019, 2:23 p.m.
Since we now run process_csb() outside of the engine->active.lock, we
can process a CS-event immediately upon our ELSP write. As we currently
inspect the pending queue *after* the ELSP write, there is an
opportunity for a CS-event to update the pending queue before we can
read it, making ourselves chases an invalid pointer.

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>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 44780e7fafec..d42584439f51 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1335,9 +1335,9 @@  static void execlists_dequeue(struct intel_engine_cs *engine)
 	if (submit) {
 		*port = execlists_schedule_in(last, port - execlists->pending);
 		memset(port + 1, 0, (last_port - port) * sizeof(*port));
-		execlists_submit_ports(engine);
 		execlists->switch_priority_hint =
 			switch_prio(engine, *execlists->pending);
+		execlists_submit_ports(engine);
 	} else {
 		ring_set_paused(engine, 0);
 	}

Comments

Quoting Chris Wilson (2019-08-21 15:23:36)
> Since we now run process_csb() outside of the engine->active.lock, we
> can process a CS-event immediately upon our ELSP write. As we currently
> inspect the pending queue *after* the ELSP write, there is an
> opportunity for a CS-event to update the pending queue before we can
> read it, making ourselves chases an invalid pointer.
> 

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111427
> 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>
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 44780e7fafec..d42584439f51 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1335,9 +1335,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>         if (submit) {
>                 *port = execlists_schedule_in(last, port - execlists->pending);
>                 memset(port + 1, 0, (last_port - port) * sizeof(*port));
> -               execlists_submit_ports(engine);
>                 execlists->switch_priority_hint =
>                         switch_prio(engine, *execlists->pending);
> +               execlists_submit_ports(engine);
>         } else {
>                 ring_set_paused(engine, 0);
>         }
> -- 
> 2.23.0.rc1
>
Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Chris Wilson (2019-08-21 15:23:36)
>> Since we now run process_csb() outside of the engine->active.lock, we
>> can process a CS-event immediately upon our ELSP write. As we currently
>> inspect the pending queue *after* the ELSP write, there is an
>> opportunity for a CS-event to update the pending queue before we can
>> read it, making ourselves chases an invalid pointer.
>> 
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111427

Enough to make me convinced,
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

>> 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>
>> ---
>>  drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> index 44780e7fafec..d42584439f51 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> @@ -1335,9 +1335,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>>         if (submit) {
>>                 *port = execlists_schedule_in(last, port - execlists->pending);
>>                 memset(port + 1, 0, (last_port - port) * sizeof(*port));
>> -               execlists_submit_ports(engine);
>>                 execlists->switch_priority_hint =
>>                         switch_prio(engine, *execlists->pending);
>> +               execlists_submit_ports(engine);
>>         } else {
>>                 ring_set_paused(engine, 0);
>>         }
>> -- 
>> 2.23.0.rc1
>>