Skip to contentSkip to conversation

drm/i915: avoid false fence signaling when enabling preemption

PATCH In review Min He 3 participants 9 messages Mar 12, 2018 – Mar 13, 2018

Subsystems (2)
DRM drivers Intel i915
lore
Share & tools
lore

Min He

author
#0 lore raw

PATCH 1/1 drm/i915: avoid false fence signaling when enabling preemption

In i915_request_wait(), it adds a local intel_wait variable into rb tree of waiters. However, when the corresponding request is preempted, the seqno of this wait will not be updated, which will lead to a false signaling to the request and cause the i915_request_wait() to return early before the request is really completed.

In this patch, we check if the seqno in wait is same as the request before we signal the fence, and thus fix the above issue.

Signed-off-by Min He <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_irq.c +1 -1
index 828f310..1ef961d 100644
@@ -1121,7 +1121,7 @@ static void notify_ring(struct intel_engine_cs *engine)
 	}
 	spin_unlock(&engine->breadcrumbs.irq_lock);
 
-	if (rq) {
+	if (rq && intel_wait_check_request(wait, rq)) {
 		dma_fence_signal(&rq->fence);
 		GEM_BUG_ON(!i915_request_completed(rq));
 		i915_request_put(rq);
Signature · 6 lines
2.7.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
CI
#1 lore raw

✗ Fi.CI.BAT: failure for drm/i915: avoid false fence signaling when enabling preemption

== Series Details ==

== Series Details ==

Series: drm/i915: avoid false fence signaling when enabling preemption
URL : https://patchwork.freedesktop.org/series/39768/
State : failure

== Summary ==

Series 39768v1 drm/i915: avoid false fence signaling when enabling preemption
https://patchwork.freedesktop.org/api/1.0/series/39768/revisions/1/mbox/

---- Possible new issues:

Test kms_psr_sink_crc:

        Subgroup psr_basic:
                skip       -> INCOMPLETE (fi-bxt-dsi)

---- Known issues:

Test gem_mmap_gtt:

        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:422s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:369s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:495s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:277s
fi-bxt-dsi       total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:26 
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:488s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:477s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:469s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:407s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:575s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:578s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:414s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:288s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:518s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:396s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:416s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:463s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:420s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:472s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:462s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:507s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:585s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:432s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:524s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:533s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:497s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:480s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:422s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:431s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:395s

Blacklisted hosts:
fi-cnl-drrs total:288 pass:257 dwarn:3 dfail:0 fail:0 skip:19 time:516s

7e5ff4244a75361f62abdb9e484e31a125131920 drm-tip: 2018y-03m-09d-22h-21m-43s UTC integration manifest e27052de242c drm/i915: avoid false fence signaling when enabling preemption

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8299/issues.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#2 lore raw
In reply to Min He

Quoting Min He (2018-03-12 06:17:10)

Quoted 5 lines

In i915_request_wait(), it adds a local intel_wait variable into rb tree of waiters. However, when the corresponding request is preempted, the seqno of this wait will not be updated, which will lead to a false signaling to the request and cause the i915_request_wait() to return early before the request is really completed.

Incorrect. We confirm that the request wasn't preempted before signaling. Once i915_seqno_passed + intel_wait_check_request, we cannot unsubmit the request.
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#3 lore raw

Quoting Chris Wilson (2018-03-12 10:42:56)

Quoted 10 lines

Quoting Min He (2018-03-12 06:17:10)

Quoted 5 lines

In i915_request_wait(), it adds a local intel_wait variable into rb tree of waiters. However, when the corresponding request is preempted, the seqno of this wait will not be updated, which will lead to a false signaling to the request and cause the i915_request_wait() to return early before the request is really completed.

Incorrect. We confirm that the request wasn't preempted before signaling. Once i915_seqno_passed + intel_wait_check_request, we cannot unsubmit the request.

Or to put it another way, there is an assertion there that would fire if we make a mistake; you should cite that oops in the commit message. -Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#4 lore raw
In reply to Chris Wilson

Quoting Chris Wilson (2018-03-12 10:42:56)

Quoted 10 lines

Quoting Min He (2018-03-12 06:17:10)

Quoted 5 lines

In i915_request_wait(), it adds a local intel_wait variable into rb tree of waiters. However, when the corresponding request is preempted, the seqno of this wait will not be updated, which will lead to a false signaling to the request and cause the i915_request_wait() to return early before the request is really completed.

Incorrect. We confirm that the request wasn't preempted before signaling. Once i915_seqno_passed + intel_wait_check_request, we cannot unsubmit the request.

I should also point out that the locking around accessing the wait is absent...
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#5 lore raw

Chris,
I just found that I was based on an older code branch, and your commit " drm/i915: Check waiter->seqno carefully in case of preemption" already fixed this issue I mentioned.

I should be more careful and thanks a lot for your time and patience.

Quoted 15 lines

-----Original Message-----
From: Chris Wilson [mailto:[email protected]]
Sent: Monday, March 12, 2018 6:47 PM
To: He, Min <[email protected]>; [email protected]
Subject: Re: [Intel-gfx] [PATCH] drm/i915: avoid false fence signaling when enabling preemption

Quoting Chris Wilson (2018-03-12 10:42:56)

Quoted 7 lines

Quoting Min He (2018-03-12 06:17:10)

Quoted 5 lines

In i915_request_wait(), it adds a local intel_wait variable into rb tree of waiters. However, when the corresponding request is preempted, the seqno of this wait will not be updated, which will lead to a false signaling to the request and cause the i915_request_wait() to return early before the request is really completed.

Quoted 7 lines
Quoted 3 lines

Incorrect. We confirm that the request wasn't preempted before signaling. Once i915_seqno_passed + intel_wait_check_request, we cannot unsubmit the request.

I should also point out that the locking around accessing the wait is absent...
-Chris

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#6 lore raw

✓ Fi.CI.BAT: success for drm/i915: avoid false fence signaling when enabling preemption

In reply to Min He
== Series Details ==

== Series Details ==

Series: drm/i915: avoid false fence signaling when enabling preemption
URL : https://patchwork.freedesktop.org/series/39768/
State : success

== Summary ==

Series 39768v1 drm/i915: avoid false fence signaling when enabling preemption
https://patchwork.freedesktop.org/api/1.0/series/39768/revisions/1/mbox/

---- Known issues:

Test gem_exec_suspend:

        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7500u) fdo#105128
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-hsw-4770) fdo#104944
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (fi-bxt-dsi) fdo#103927

fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
fdo#104944 https://bugs.freedesktop.org/show_bug.cgi?id=104944
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:429s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:433s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:383s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:532s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:300s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:504s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:507s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:508s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:496s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:411s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:588s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:584s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:422s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:314s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:533s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:403s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:421s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:463s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:426s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:475s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:463s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:515s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:645s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:441s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:530s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:541s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:505s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:497s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:429s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:436s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:534s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:404s

Blacklisted hosts:
fi-cnl-drrs total:288 pass:0 dwarn:0 dfail:0 fail:0 skip:0 time:3s
fi-cfl-u failed to collect. IGT log at Patchwork_8300/fi-cfl-u/run0.log

73f9dfa99f1bd865b6376f7c13b110037c684af4 drm-tip: 2018y-03m-12d-09h-43m-16s UTC integration manifest e69d02ef47ff drm/i915: avoid false fence signaling when enabling preemption

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8300/issues.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#7 lore raw

✗ Fi.CI.IGT: failure for drm/i915: avoid false fence signaling when enabling preemption

In reply to Min He
== Series Details ==

== Series Details ==

Series: drm/i915: avoid false fence signaling when enabling preemption
URL : https://patchwork.freedesktop.org/series/39768/
State : failure

== Summary ==

---- Possible new issues:

Test kms_cursor_crc:

        Subgroup cursor-64x64-suspend:
                fail       -> PASS       (shard-apl)
Test kms_plane_multiple:
        Subgroup legacy-pipe-a-tiling-x:
                pass       -> FAIL       (shard-apl)
Test kms_vblank:
        Subgroup pipe-a-wait-busy-hang:
                skip       -> PASS       (shard-snb)

---- Known issues:

Test drv_hangman:

        Subgroup error-state-capture-blt:
                pass       -> INCOMPLETE (shard-snb) fdo#104058
Test drv_selftest:
        Subgroup live_gtt:
                incomplete -> PASS       (shard-apl) fdo#103927
Test gem_eio:
        Subgroup in-flight-contexts:
                pass       -> INCOMPLETE (shard-apl) fdo#105341
Test kms_chv_cursor_fail:
        Subgroup pipe-a-64x64-top-edge:
                skip       -> PASS       (shard-snb) fdo#105185
Test kms_flip:
        Subgroup dpms-vs-vblank-race-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#103060
Test pm_lpsp:
        Subgroup screens-disabled:
                pass       -> FAIL       (shard-hsw) fdo#104941
Test pm_rpm:
        Subgroup system-suspend:
                incomplete -> PASS       (shard-hsw) fdo#103375

fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341
fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#104941 https://bugs.freedesktop.org/show_bug.cgi?id=104941
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

shard-apl        total:3385 pass:1785 dwarn:1   dfail:0   fail:9   skip:1589 time:12604s
shard-hsw        total:3467 pass:1771 dwarn:1   dfail:0   fail:3   skip:1691 time:11930s
shard-snb        total:3432 pass:1346 dwarn:1   dfail:0   fail:3   skip:2081 time:7031s

Blacklisted hosts:
shard-kbl total:3348 pass:1880 dwarn:1 dfail:1 fail:7 skip:1457 time:9059s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8300/shards.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#8 lore raw

Re: ✗ Fi.CI.IGT: failure for drm/i915: avoid false fence signaling when enabling preemption

Quoting Patchwork (2018-03-12 12:40:11)

Quoted 35 lines

== Series Details ==

Series: drm/i915: avoid false fence signaling when enabling preemption
URL : https://patchwork.freedesktop.org/series/39768/
State : failure

== Summary ==

---- Possible new issues:

Test kms_cursor_crc:

        Subgroup cursor-64x64-suspend:
                fail       -> PASS       (shard-apl)
Test kms_plane_multiple:
        Subgroup legacy-pipe-a-tiling-x:
                pass       -> FAIL       (shard-apl)
Test kms_vblank:
        Subgroup pipe-a-wait-busy-hang:
                skip       -> PASS       (shard-snb)

---- Known issues:

Test drv_hangman:

        Subgroup error-state-capture-blt:
                pass       -> INCOMPLETE (shard-snb) fdo#104058
Test drv_selftest:
        Subgroup live_gtt:
                incomplete -> PASS       (shard-apl) fdo#103927
Test gem_eio:
        Subgroup in-flight-contexts:
                pass       -> INCOMPLETE (shard-apl) fdo#105341
Test kms_chv_cursor_fail:
        Subgroup pipe-a-64x64-top-edge:
                skip       -> PASS       (shard-snb) fdo#105185

Test kms_flip:

Quoted 8 lines
        Subgroup dpms-vs-vblank-race-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#103060
Test pm_lpsp:
        Subgroup screens-disabled:
                pass       -> FAIL       (shard-hsw) fdo#104941
Test pm_rpm:
        Subgroup system-suspend:
                incomplete -> PASS       (shard-hsw) fdo#103375

There's a challenge then. Write a small enough test to demonstrate this patch has issues. gem_exec_whisper would be my goto.
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx