[PATCH] drm/i915/execlists: Disable submission tasklets when rescheduling

[PATCH] drm/i915/execlists: Disable submission tasklets when rescheduling

[Chris Wilson]

As we reschedule the requests, we do not want the submission tasklet
running until we finish updating the priority chains. (We start
rewriting priorities from the oldest, but the dequeue looks at the most
recent in-flight, so there is a small race condition where dequeue may
decide that preemption is falsely required.) Combine the tasklet kicking
from adding a new request with the set-wedge protection so that we only
have to adjust the preempt-counter once to achieve both goals.

Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c     | 4 ++--
 drivers/gpu/drm/i915/i915_request.c | 5 +----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 438a2fc5bba0..a278e4dc5e09 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -577,10 +577,10 @@ static void __fence_set_priority(struct dma_fence *fence,
 	rq = to_request(fence);
 	engine = rq->engine;
 
-	rcu_read_lock();
+	local_bh_disable(); /* RCU serialisation for set-wedged protection */
 	if (engine->schedule)
 		engine->schedule(rq, attr);
-	rcu_read_unlock();
+	local_bh_enable(); /* kick the tasklets if queues were reprioritised */
 }
 
 static void fence_set_priority(struct dma_fence *fence,
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 7bb613c00cc3..eb66579db823 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1111,12 +1111,9 @@ void __i915_request_add(struct i915_request *request, bool flush_caches)
 	 * decide whether to preempt the entire chain so that it is ready to
 	 * run at the earliest possible convenience.
 	 */
-	rcu_read_lock();
+	local_bh_disable();
 	if (engine->schedule)
 		engine->schedule(request, &request->ctx->sched);
-	rcu_read_unlock();
-
-	local_bh_disable();
 	i915_sw_fence_commit(&request->submit);
 	local_bh_enable(); /* Kick the execlists tasklet if just scheduled */
 
-- 
2.17.0

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

Re: [PATCH] drm/i915/execlists: Disable submission tasklets when rescheduling

[Chris Wilson]

Quoting Chris Wilson (2018-05-01 10:49:17)
> As we reschedule the requests, we do not want the submission tasklet
> running until we finish updating the priority chains. (We start
> rewriting priorities from the oldest, but the dequeue looks at the most
> recent in-flight, so there is a small race condition where dequeue may
> decide that preemption is falsely required.) Combine the tasklet kicking
> from adding a new request with the set-wedge protection so that we only
> have to adjust the preempt-counter once to achieve both goals.
> 
> Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c     | 4 ++--
>  drivers/gpu/drm/i915/i915_request.c | 5 +----
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 438a2fc5bba0..a278e4dc5e09 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -577,10 +577,10 @@ static void __fence_set_priority(struct dma_fence *fence,
>         rq = to_request(fence);
>         engine = rq->engine;
>  
> -       rcu_read_lock();
> +       local_bh_disable(); /* RCU serialisation for set-wedged protection */
>         if (engine->schedule)
>                 engine->schedule(rq, attr);
> -       rcu_read_unlock();
> +       local_bh_enable(); /* kick the tasklets if queues were reprioritised */
>  }
>  
>  static void fence_set_priority(struct dma_fence *fence,
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 7bb613c00cc3..eb66579db823 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1111,12 +1111,9 @@ void __i915_request_add(struct i915_request *request, bool flush_caches)
>          * decide whether to preempt the entire chain so that it is ready to
>          * run at the earliest possible convenience.
>          */
> -       rcu_read_lock();
> +       local_bh_disable();
>         if (engine->schedule)
>                 engine->schedule(request, &request->ctx->sched);
> -       rcu_read_unlock();
> -
> -       local_bh_disable();
>         i915_sw_fence_commit(&request->submit);
>         local_bh_enable(); /* Kick the execlists tasklet if just scheduled */

It's an improvement in that we don't mix and match rcu/bh anymore,
but it doesn't entirely close the race; as local-bh is local to this cpu
and the tasklet may be running on another. Bother.
-Chris
_______________________________________________
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: Disable submission tasklets when rescheduling

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Disable submission tasklets when rescheduling
URL   : https://patchwork.freedesktop.org/series/42513/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4116 -> Patchwork_8855 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8855 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8855, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42513/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8855:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_8855 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-ivb-3520m:       PASS -> FAIL (fdo#103375) +1

    
    ==== Warnings ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> FAIL (fdo#103375)

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084


== Participating hosts (38 -> 34) ==

  Missing    (4): fi-byt-j1900 fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4116 -> Patchwork_8855

  CI_DRM_4116: 7ff375cb94000d93f7a9d541cb0c8180fbea80f2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4453: 29ae12bd764e3b1876356e7628a32192b4ec9066 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8855: 92a5ada14a20ae92e5e2506ae85c6409918128fe @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4453: d0a0bca2194a673c4d9a70a2256837c59213c64b @ git://anongit.freedesktop.org/piglit


== Linux commits ==

92a5ada14a20 drm/i915/execlists: Disable submission tasklets when rescheduling

== Logs ==

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

✗ Fi.CI.IGT: failure for drm/i915/execlists: Disable submission tasklets when rescheduling

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Disable submission tasklets when rescheduling
URL   : https://patchwork.freedesktop.org/series/42513/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4116_full -> Patchwork_8855_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8855_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8855_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42513/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8855_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-glk:          PASS -> INCOMPLETE

    
    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          PASS -> SKIP +1

    igt@gem_exec_schedule@deep-bsd2:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_8855_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-apl:          PASS -> FAIL (fdo#103375)

    igt@kms_flip@absolute-wf_vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#106087)

    igt@kms_flip@flip-vs-wf_vblank-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#103928) +1
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@plain-flip-fb-recreate:
      shard-hsw:          PASS -> FAIL (fdo#100368)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite:
      shard-snb:          PASS -> FAIL (fdo#103167)

    igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-hsw:          FAIL (fdo#103928) -> PASS

    igt@kms_flip@flip-vs-blocking-wf-vblank:
      shard-kbl:          DMESG-FAIL (fdo#103558) -> PASS +1

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-kbl:          DMESG-WARN (fdo#105602, fdo#103558, fdo#103313) -> PASS

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS +1

    igt@kms_flip_event_leak:
      shard-kbl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +33

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (9 -> 8) ==

  Missing    (1): shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4116 -> Patchwork_8855

  CI_DRM_4116: 7ff375cb94000d93f7a9d541cb0c8180fbea80f2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4453: 29ae12bd764e3b1876356e7628a32192b4ec9066 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8855: 92a5ada14a20ae92e5e2506ae85c6409918128fe @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4453: d0a0bca2194a673c4d9a70a2256837c59213c64b @ git://anongit.freedesktop.org/piglit

== Logs ==

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