All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Defer application of request banning to submission
@ 2019-02-13 18:24 Chris Wilson
  2019-02-13 18:27 ` Chris Wilson
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Chris Wilson @ 2019-02-13 18:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

As we currently do not check on submission whether the context is banned
in a timely manner it is possible for some requests to escape
cancellation after their parent context is banned. By moving the ban
into the request submission under the engine->timeline.lock, we
serialise it with the reset and setting of the context ban.

References: eb8d0f5af4ec ("drm/i915: Remove GPU reset dependence on struct_mutex")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_request.c |  3 +++
 drivers/gpu/drm/i915/i915_reset.c   | 19 ++++---------------
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 0acd6baa3c88..5ab4e1c01618 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -366,6 +366,9 @@ void __i915_request_submit(struct i915_request *request)
 	GEM_BUG_ON(!irqs_disabled());
 	lockdep_assert_held(&engine->timeline.lock);
 
+	if (i915_gem_context_is_banned(request->gem_context))
+		i915_request_skip(request, -EIO);
+
 	GEM_BUG_ON(request->global_seqno);
 
 	seqno = next_global_seqno(&engine->timeline);
diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
index 12e74decd7a2..c69b520ce1cf 100644
--- a/drivers/gpu/drm/i915/i915_reset.c
+++ b/drivers/gpu/drm/i915/i915_reset.c
@@ -22,24 +22,13 @@ static void engine_skip_context(struct i915_request *rq)
 {
 	struct intel_engine_cs *engine = rq->engine;
 	struct i915_gem_context *hung_ctx = rq->gem_context;
-	struct i915_timeline *timeline = rq->timeline;
 
 	lockdep_assert_held(&engine->timeline.lock);
-	GEM_BUG_ON(timeline == &engine->timeline);
+	GEM_BUG_ON(!i915_request_is_active(rq));
 
-	spin_lock(&timeline->lock);
-
-	if (i915_request_is_active(rq)) {
-		list_for_each_entry_continue(rq,
-					     &engine->timeline.requests, link)
-			if (rq->gem_context == hung_ctx)
-				i915_request_skip(rq, -EIO);
-	}
-
-	list_for_each_entry(rq, &timeline->requests, link)
-		i915_request_skip(rq, -EIO);
-
-	spin_unlock(&timeline->lock);
+	list_for_each_entry_continue(rq, &engine->timeline.requests, link)
+		if (rq->gem_context == hung_ctx)
+			i915_request_skip(rq, -EIO);
 }
 
 static void client_mark_guilty(struct drm_i915_file_private *file_priv,
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] drm/i915: Defer application of request banning to submission
  2019-02-13 18:24 [PATCH] drm/i915: Defer application of request banning to submission Chris Wilson
@ 2019-02-13 18:27 ` Chris Wilson
  2019-02-15 12:52   ` Mika Kuoppala
  2019-02-14 13:10 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Defer application of request banning to submission (rev2) Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2019-02-13 18:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

As we currently do not check on submission whether the context is banned
in a timely manner it is possible for some requests to escape
cancellation after their parent context is banned. By moving the ban
into the request submission under the engine->timeline.lock, we
serialise it with the reset and setting of the context ban.

References: eb8d0f5af4ec ("drm/i915: Remove GPU reset dependence on struct_mutex")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_request.c |  3 +++
 drivers/gpu/drm/i915/i915_reset.c   | 19 +++++--------------
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 0acd6baa3c88..5ab4e1c01618 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -366,6 +366,9 @@ void __i915_request_submit(struct i915_request *request)
 	GEM_BUG_ON(!irqs_disabled());
 	lockdep_assert_held(&engine->timeline.lock);
 
+	if (i915_gem_context_is_banned(request->gem_context))
+		i915_request_skip(request, -EIO);
+
 	GEM_BUG_ON(request->global_seqno);
 
 	seqno = next_global_seqno(&engine->timeline);
diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
index 12e74decd7a2..7fa97ce19bfd 100644
--- a/drivers/gpu/drm/i915/i915_reset.c
+++ b/drivers/gpu/drm/i915/i915_reset.c
@@ -22,24 +22,15 @@ static void engine_skip_context(struct i915_request *rq)
 {
 	struct intel_engine_cs *engine = rq->engine;
 	struct i915_gem_context *hung_ctx = rq->gem_context;
-	struct i915_timeline *timeline = rq->timeline;
 
 	lockdep_assert_held(&engine->timeline.lock);
-	GEM_BUG_ON(timeline == &engine->timeline);
 
-	spin_lock(&timeline->lock);
-
-	if (i915_request_is_active(rq)) {
-		list_for_each_entry_continue(rq,
-					     &engine->timeline.requests, link)
-			if (rq->gem_context == hung_ctx)
-				i915_request_skip(rq, -EIO);
-	}
-
-	list_for_each_entry(rq, &timeline->requests, link)
-		i915_request_skip(rq, -EIO);
+	if (!i915_request_is_active(rq))
+		return;
 
-	spin_unlock(&timeline->lock);
+	list_for_each_entry_continue(rq, &engine->timeline.requests, link)
+		if (rq->gem_context == hung_ctx)
+			i915_request_skip(rq, -EIO);
 }
 
 static void client_mark_guilty(struct drm_i915_file_private *file_priv,
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Defer application of request banning to submission (rev2)
  2019-02-13 18:24 [PATCH] drm/i915: Defer application of request banning to submission Chris Wilson
  2019-02-13 18:27 ` Chris Wilson
@ 2019-02-14 13:10 ` Patchwork
  2019-02-14 13:34 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-02-14 17:46 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-02-14 13:10 UTC (permalink / raw)
  To: intel-gfx

== Series Details ==

Series: drm/i915: Defer application of request banning to submission (rev2)
URL   : https://patchwork.freedesktop.org/series/56626/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
3e599ebe5565 drm/i915: Defer application of request banning to submission
-:12: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#12: 
References: eb8d0f5af4ec ("drm/i915: Remove GPU reset dependence on struct_mutex")

-:12: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit eb8d0f5af4ec ("drm/i915: Remove GPU reset dependence on struct_mutex")'
#12: 
References: eb8d0f5af4ec ("drm/i915: Remove GPU reset dependence on struct_mutex")

total: 1 errors, 1 warnings, 0 checks, 38 lines checked

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Defer application of request banning to submission (rev2)
  2019-02-13 18:24 [PATCH] drm/i915: Defer application of request banning to submission Chris Wilson
  2019-02-13 18:27 ` Chris Wilson
  2019-02-14 13:10 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Defer application of request banning to submission (rev2) Patchwork
@ 2019-02-14 13:34 ` Patchwork
  2019-02-14 17:46 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-02-14 13:34 UTC (permalink / raw)
  To: intel-gfx

== Series Details ==

Series: drm/i915: Defer application of request banning to submission (rev2)
URL   : https://patchwork.freedesktop.org/series/56626/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5599 -> Patchwork_12217
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56626/revisions/2/mbox/

Possible new issues
-------------------

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_sync@basic-each:
    - {fi-icl-y}:         NOTRUN -> INCOMPLETE

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#105602] / [fdo#108529] +1

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7500u:       NOTRUN -> DMESG-WARN [fdo#102505] / [fdo#103558] / [fdo#105079] / [fdo#105602]
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#103558] / [fdo#105079] / [fdo#105602]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-kbl-7567u:       PASS -> DMESG-FAIL [fdo#105079]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@pm_rpm@module-reload:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#108529]

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         PASS -> FAIL [fdo#104008]

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] -> PASS

  * igt@i915_selftest@live_workarounds:
    - {fi-icl-u2}:        INCOMPLETE [fdo#109626] -> PASS

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       DMESG-FAIL [fdo#109627] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS +2

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104008]: https://bugs.freedesktop.org/show_bug.cgi?id=104008
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109226]: https://bugs.freedesktop.org/show_bug.cgi?id=109226
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109527]: https://bugs.freedesktop.org/show_bug.cgi?id=109527
  [fdo#109530]: https://bugs.freedesktop.org/show_bug.cgi?id=109530
  [fdo#109626]: https://bugs.freedesktop.org/show_bug.cgi?id=109626
  [fdo#109627]: https://bugs.freedesktop.org/show_bug.cgi?id=109627


Participating hosts (43 -> 43)
------------------------------

  Additional (7): fi-skl-6260u fi-whl-u fi-ivb-3770 fi-icl-y fi-kbl-7560u fi-bsw-kefka fi-snb-2600 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-bdw-5557u fi-byt-squawks fi-bsw-cyan fi-icl-u3 fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5599 -> Patchwork_12217

  CI_DRM_5599: 39119c9b385742d49446c25d6902864a60eda6b6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4824: e55d439a9ba744227fb4c9d727338276b78871d4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12217: 3e599ebe55654a1dcaca54e2f5d50b32d853e1c3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3e599ebe5565 drm/i915: Defer application of request banning to submission

== Logs ==

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Defer application of request banning to submission (rev2)
  2019-02-13 18:24 [PATCH] drm/i915: Defer application of request banning to submission Chris Wilson
                   ` (2 preceding siblings ...)
  2019-02-14 13:34 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-02-14 17:46 ` Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-02-14 17:46 UTC (permalink / raw)
  To: intel-gfx

== Series Details ==

Series: drm/i915: Defer application of request banning to submission (rev2)
URL   : https://patchwork.freedesktop.org/series/56626/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5599_full -> Patchwork_12217_full
====================================================

Summary
-------

  **WARNING**

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

  

Possible new issues
-------------------

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

### IGT changes ###

#### Warnings ####

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-iclb:         FAIL [fdo#108948] -> INCOMPLETE

  * igt@prime_nv_api@i915_self_import_to_different_fd:
    - shard-iclb:         {SKIP} [fdo#109291] -> INCOMPLETE

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_linear_blits@interruptible:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-glk:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_cursor_crc@cursor-128x128-sliding:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-apl:          PASS -> FAIL [fdo#109350]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-iclb:         PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-wf_vblank-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#100368]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +1

  * igt@kms_pipe_crc_basic@read-crc-pipe-c:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-glk:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparant-fb:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-apl:          PASS -> FAIL [fdo#103166] +5

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-none:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107724]

  * igt@kms_sysfs_edid_timing:
    - shard-apl:          NOTRUN -> FAIL [fdo#100047]
    - shard-kbl:          NOTRUN -> FAIL [fdo#100047]

  * igt@pm_rpm@gem-pread:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +1

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-snb:          FAIL [fdo#107799] -> PASS

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         DMESG-FAIL [fdo#108954] -> PASS

  * igt@kms_color@pipe-a-ctm-max:
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-glk:          FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
    - shard-iclb:         FAIL [fdo#103167] -> PASS

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-iclb:         FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-none:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          DMESG-FAIL [fdo#105763] -> PASS

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS
    - shard-hsw:          FAIL [fdo#99912] -> PASS

  * igt@pm_rpm@gem-evict-pwrite:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +3

  * igt@pm_rpm@legacy-planes:
    - shard-iclb:         DMESG-WARN [fdo#107732] -> PASS

  * igt@tools_test@tools_test:
    - shard-iclb:         {SKIP} [fdo#109352] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - shard-iclb:         DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108569] / [fdo#109226]

  * igt@i915_suspend@shrink:
    - shard-apl:          INCOMPLETE [fdo#103927] / [fdo#106886] -> DMESG-WARN [fdo#107886] / [fdo#109244]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [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#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107732]: https://bugs.freedesktop.org/show_bug.cgi?id=107732
  [fdo#107799]: https://bugs.freedesktop.org/show_bug.cgi?id=107799
  [fdo#107886]: https://bugs.freedesktop.org/show_bug.cgi?id=107886
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108756]: https://bugs.freedesktop.org/show_bug.cgi?id=108756
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [fdo#109226]: https://bugs.freedesktop.org/show_bug.cgi?id=109226
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109281]: https://bugs.freedesktop.org/show_bug.cgi?id=109281
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109287]: https://bugs.freedesktop.org/show_bug.cgi?id=109287
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#109352]: https://bugs.freedesktop.org/show_bug.cgi?id=109352
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109624]: https://bugs.freedesktop.org/show_bug.cgi?id=109624
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 6)
------------------------------

  Missing    (1): shard-skl 


Build changes
-------------

    * Linux: CI_DRM_5599 -> Patchwork_12217

  CI_DRM_5599: 39119c9b385742d49446c25d6902864a60eda6b6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4824: e55d439a9ba744227fb4c9d727338276b78871d4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12217: 3e599ebe55654a1dcaca54e2f5d50b32d853e1c3 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Defer application of request banning to submission
  2019-02-13 18:27 ` Chris Wilson
@ 2019-02-15 12:52   ` Mika Kuoppala
  2019-02-15 12:57     ` Chris Wilson
  0 siblings, 1 reply; 7+ messages in thread
From: Mika Kuoppala @ 2019-02-15 12:52 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> As we currently do not check on submission whether the context is banned
> in a timely manner it is possible for some requests to escape
> cancellation after their parent context is banned. By moving the ban
> into the request submission under the engine->timeline.lock, we
> serialise it with the reset and setting of the context ban.
>
> References: eb8d0f5af4ec ("drm/i915: Remove GPU reset dependence on struct_mutex")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_request.c |  3 +++
>  drivers/gpu/drm/i915/i915_reset.c   | 19 +++++--------------
>  2 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 0acd6baa3c88..5ab4e1c01618 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -366,6 +366,9 @@ void __i915_request_submit(struct i915_request *request)
>  	GEM_BUG_ON(!irqs_disabled());
>  	lockdep_assert_held(&engine->timeline.lock);
>  
> +	if (i915_gem_context_is_banned(request->gem_context))
> +		i915_request_skip(request, -EIO);
> +
>  	GEM_BUG_ON(request->global_seqno);
>  
>  	seqno = next_global_seqno(&engine->timeline);
> diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> index 12e74decd7a2..7fa97ce19bfd 100644
> --- a/drivers/gpu/drm/i915/i915_reset.c
> +++ b/drivers/gpu/drm/i915/i915_reset.c
> @@ -22,24 +22,15 @@ static void engine_skip_context(struct i915_request *rq)
>  {
>  	struct intel_engine_cs *engine = rq->engine;
>  	struct i915_gem_context *hung_ctx = rq->gem_context;
> -	struct i915_timeline *timeline = rq->timeline;
>  
>  	lockdep_assert_held(&engine->timeline.lock);

This was golden.

> -	GEM_BUG_ON(timeline == &engine->timeline);
>  
> -	spin_lock(&timeline->lock);
> -
> -	if (i915_request_is_active(rq)) {
> -		list_for_each_entry_continue(rq,
> -					     &engine->timeline.requests, link)
> -			if (rq->gem_context == hung_ctx)
> -				i915_request_skip(rq, -EIO);
> -	}
> -
> -	list_for_each_entry(rq, &timeline->requests, link)
> -		i915_request_skip(rq, -EIO);
> +	if (!i915_request_is_active(rq))
> +		return;

Only thing that got me actually pondering was that
we don't flush anything after we have modify the ring.

But that is not about this patch

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>


>  
> -	spin_unlock(&timeline->lock);
> +	list_for_each_entry_continue(rq, &engine->timeline.requests, link)
> +		if (rq->gem_context == hung_ctx)
> +			i915_request_skip(rq, -EIO);
>  }
>  
>  static void client_mark_guilty(struct drm_i915_file_private *file_priv,
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Defer application of request banning to submission
  2019-02-15 12:52   ` Mika Kuoppala
@ 2019-02-15 12:57     ` Chris Wilson
  0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2019-02-15 12:57 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-02-15 12:52:06)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > As we currently do not check on submission whether the context is banned
> > in a timely manner it is possible for some requests to escape
> > cancellation after their parent context is banned. By moving the ban
> > into the request submission under the engine->timeline.lock, we
> > serialise it with the reset and setting of the context ban.
> >
> > References: eb8d0f5af4ec ("drm/i915: Remove GPU reset dependence on struct_mutex")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_request.c |  3 +++
> >  drivers/gpu/drm/i915/i915_reset.c   | 19 +++++--------------
> >  2 files changed, 8 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> > index 0acd6baa3c88..5ab4e1c01618 100644
> > --- a/drivers/gpu/drm/i915/i915_request.c
> > +++ b/drivers/gpu/drm/i915/i915_request.c
> > @@ -366,6 +366,9 @@ void __i915_request_submit(struct i915_request *request)
> >       GEM_BUG_ON(!irqs_disabled());
> >       lockdep_assert_held(&engine->timeline.lock);
> >  
> > +     if (i915_gem_context_is_banned(request->gem_context))
> > +             i915_request_skip(request, -EIO);
> > +
> >       GEM_BUG_ON(request->global_seqno);
> >  
> >       seqno = next_global_seqno(&engine->timeline);
> > diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> > index 12e74decd7a2..7fa97ce19bfd 100644
> > --- a/drivers/gpu/drm/i915/i915_reset.c
> > +++ b/drivers/gpu/drm/i915/i915_reset.c
> > @@ -22,24 +22,15 @@ static void engine_skip_context(struct i915_request *rq)
> >  {
> >       struct intel_engine_cs *engine = rq->engine;
> >       struct i915_gem_context *hung_ctx = rq->gem_context;
> > -     struct i915_timeline *timeline = rq->timeline;
> >  
> >       lockdep_assert_held(&engine->timeline.lock);
> 
> This was golden.
> 
> > -     GEM_BUG_ON(timeline == &engine->timeline);
> >  
> > -     spin_lock(&timeline->lock);
> > -
> > -     if (i915_request_is_active(rq)) {
> > -             list_for_each_entry_continue(rq,
> > -                                          &engine->timeline.requests, link)
> > -                     if (rq->gem_context == hung_ctx)
> > -                             i915_request_skip(rq, -EIO);
> > -     }
> > -
> > -     list_for_each_entry(rq, &timeline->requests, link)
> > -             i915_request_skip(rq, -EIO);
> > +     if (!i915_request_is_active(rq))
> > +             return;
> 
> Only thing that got me actually pondering was that
> we don't flush anything after we have modify the ring.

The magic is that we don't need to flush, the requests are all on their
way to HW and we don't care when, until somebody actually cares at a
later date (e.g. i915_request_wait(), or calling unwedge before
restarting). Flushing is hard since the requests have their own webs of
dependencies which must be maintained even if they die.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-02-15 12:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-13 18:24 [PATCH] drm/i915: Defer application of request banning to submission Chris Wilson
2019-02-13 18:27 ` Chris Wilson
2019-02-15 12:52   ` Mika Kuoppala
2019-02-15 12:57     ` Chris Wilson
2019-02-14 13:10 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Defer application of request banning to submission (rev2) Patchwork
2019-02-14 13:34 ` ✓ Fi.CI.BAT: success " Patchwork
2019-02-14 17:46 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.