[PATCH] drm/i915/execlists: Remove the priority "optimisation"

[PATCH] drm/i915/execlists: Remove the priority "optimisation"

[Chris Wilson]

Originally we set the priority to max upon inserting the request into
the execlists queue (and removing it from the scheduler lists). We could
then use the prio==INT_MAX as a shortcut within execlists_schedule() to
detect the end of the dependency chain. Since commit 1f181225f8ec
("drm/i915/execlists: Keep request->priority for its lifetime") this is
no longer true as we use the request completion as an indicator the
schedule dependency chain is complete instead. (This allows us to then
reschedule requests even when its context is in flight.) However, this
makes the GEM_BUG_ON() inside execlists_schedule() racy as we may change
the rq->prio at the same time. As the assertion is useful, let's keep
the assertion and remove the micro-optimisation.

Fixes: 1f181225f8ec ("drm/i915/execlists: Keep request->priority for its lifetime")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 233c992a886b..c63960fee102 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -734,7 +734,6 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
 
 		list_for_each_entry_safe(rq, rn, &p->requests, priotree.link) {
 			INIT_LIST_HEAD(&rq->priotree.link);
-			rq->priotree.priority = INT_MAX;
 
 			dma_fence_set_error(&rq->fence, -EIO);
 			__i915_gem_request_submit(rq);
@@ -891,7 +890,6 @@ static void intel_lrc_irq_handler(unsigned long data)
 				execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
 
 				trace_i915_gem_request_out(rq);
-				rq->priotree.priority = INT_MAX;
 				i915_gem_request_put(rq);
 
 				execlists_port_complete(execlists, port);
-- 
2.15.0.rc1

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

Re: [PATCH] drm/i915/execlists: Remove the priority "optimisation"

[Michał Winiarski]

On Tue, Oct 24, 2017 at 12:55:01PM +0100, Chris Wilson wrote:
> Originally we set the priority to max upon inserting the request into
> the execlists queue (and removing it from the scheduler lists). We could
> then use the prio==INT_MAX as a shortcut within execlists_schedule() to
> detect the end of the dependency chain. Since commit 1f181225f8ec
> ("drm/i915/execlists: Keep request->priority for its lifetime") this is
> no longer true as we use the request completion as an indicator the
> schedule dependency chain is complete instead. (This allows us to then
> reschedule requests even when its context is in flight.) However, this
> makes the GEM_BUG_ON() inside execlists_schedule() racy as we may change
> the rq->prio at the same time. As the assertion is useful, let's keep
> the assertion and remove the micro-optimisation.
> 
> Fixes: 1f181225f8ec ("drm/i915/execlists: Keep request->priority for its lifetime")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 233c992a886b..c63960fee102 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -734,7 +734,6 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
>  
>  		list_for_each_entry_safe(rq, rn, &p->requests, priotree.link) {
>  			INIT_LIST_HEAD(&rq->priotree.link);
> -			rq->priotree.priority = INT_MAX;
>  
>  			dma_fence_set_error(&rq->fence, -EIO);
>  			__i915_gem_request_submit(rq);
> @@ -891,7 +890,6 @@ static void intel_lrc_irq_handler(unsigned long data)
>  				execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
>  
>  				trace_i915_gem_request_out(rq);
> -				rq->priotree.priority = INT_MAX;
>  				i915_gem_request_put(rq);
>  
>  				execlists_port_complete(execlists, port);
> -- 
> 2.15.0.rc1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/execlists: Remove the priority "optimisation"

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Remove the priority "optimisation"
URL   : https://patchwork.freedesktop.org/series/32533/
State : success

== Summary ==

Series 32533v1 drm/i915/execlists: Remove the priority "optimisation"
https://patchwork.freedesktop.org/api/1.0/series/32533/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test kms_frontbuffer_tracking:
        Subgroup basic:
                fail       -> INCOMPLETE (fi-glk-dsi)

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:442s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:445s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:370s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:526s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:261s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:497s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:494s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:489s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:474s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:556s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:602s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:423s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:250s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:578s
fi-glk-dsi       total:225  pass:198  dwarn:0   dfail:0   fail:0   skip:26 
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:430s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:426s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:439s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:460s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:483s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:575s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:477s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:584s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:552s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:458s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:645s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:522s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:500s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:462s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:559s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:418s

5c82a37eff83ab4e60e760fbaf03db5ba0563497 drm-tip: 2017y-10m-23d-18h-06m-28s UTC integration manifest
ff9cad6080d6 drm/i915/execlists: Remove the priority "optimisation"

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6159/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/execlists: Remove the priority "optimisation"

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Remove the priority "optimisation"
URL   : https://patchwork.freedesktop.org/series/32533/
State : success

== Summary ==

Test perf:
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252
Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-A:
                dmesg-warn -> PASS       (shard-hsw) fdo#102249 +1
Test drv_module_reload:
        Subgroup basic-reload-inject:
                dmesg-warn -> PASS       (shard-hsw) fdo#102707
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2540 pass:1432 dwarn:2   dfail:0   fail:9   skip:1097 time:9163s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6159/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/execlists: Remove the priority "optimisation"

[Chris Wilson]

Quoting Michał Winiarski (2017-10-24 13:26:59)
> On Tue, Oct 24, 2017 at 12:55:01PM +0100, Chris Wilson wrote:
> > Originally we set the priority to max upon inserting the request into
> > the execlists queue (and removing it from the scheduler lists). We could
> > then use the prio==INT_MAX as a shortcut within execlists_schedule() to
> > detect the end of the dependency chain. Since commit 1f181225f8ec
> > ("drm/i915/execlists: Keep request->priority for its lifetime") this is
> > no longer true as we use the request completion as an indicator the
> > schedule dependency chain is complete instead. (This allows us to then
> > reschedule requests even when its context is in flight.) However, this
> > makes the GEM_BUG_ON() inside execlists_schedule() racy as we may change
> > the rq->prio at the same time. As the assertion is useful, let's keep
> > the assertion and remove the micro-optimisation.
> > 
> > Fixes: 1f181225f8ec ("drm/i915/execlists: Keep request->priority for its lifetime")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

And pushed, thanks for the poke.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915/execlists: Remove the priority "optimisation"

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Remove the priority "optimisation"
URL   : https://patchwork.freedesktop.org/series/32533/
State : failure

== Summary ==

Series 32533 revision 1 was fully merged or fully failed: no git log

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6159/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx