[PATCH] drm/i915: Clear breadcrumb node when cancelling signaling

[PATCH] drm/i915: Clear breadcrumb node when cancelling signaling

[Chris Wilson]

When we call intel_engine_cancel_signaling() to stop reporting whether
or not a request is completed via an asynchronous signal, we remove that
request from the breadcrumb wait queue. However, we may be concurrently
processing that request in the signaler itself, the actual operations on
the request itself are serialised but we do not actually clear the
waiter after removing it from the tree allowing both parties to attempt
to do so and corrupting the rbtree. (Elsewhere removing from the
breadcrumb wait queue could only be done on behalf of i915_wait_request,
so this race could not happen).

Reported-by: "He, Bo" <bo.he@intel.com>
Fixes: 9eb143bbec7d ("drm/i915: Allow a request to be cancelled")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: "He, Bo" <bo.he@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_breadcrumbs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 4c4fbf5f20f9..5ae2d276f7f3 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -549,6 +549,7 @@ static void __intel_engine_remove_wait(struct intel_engine_cs *engine,
 
 	GEM_BUG_ON(RB_EMPTY_NODE(&wait->node));
 	rb_erase(&wait->node, &b->waiters);
+	RB_CLEAR_NODE(&wait->node);
 
 out:
 	GEM_BUG_ON(b->irq_wait == wait);
-- 
2.15.0

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

✓ Fi.CI.BAT: success for drm/i915: Clear breadcrumb node when cancelling signaling

[Patchwork]

== Series Details ==

Series: drm/i915: Clear breadcrumb node when cancelling signaling
URL   : https://patchwork.freedesktop.org/series/33870/
State : success

== Summary ==

Series 33870v1 drm/i915: Clear breadcrumb node when cancelling signaling
https://patchwork.freedesktop.org/api/1.0/series/33870/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c:
                skip       -> PASS       (fi-hsw-4770r)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:464s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:378s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:546s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:274s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:500s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:504s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:498s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:485s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:428s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:431s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:437s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:422s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:459s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:480s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:531s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:484s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:529s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:576s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:543s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:565s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:506s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:502s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:462s
fi-snb-2520m     total:246  pass:212  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:426s
Blacklisted hosts:
fi-cfl-s         total:289  pass:254  dwarn:3   dfail:0   fail:0   skip:32  time:532s
fi-cnl-y         total:270  pass:243  dwarn:0   dfail:0   fail:0   skip:26 

2cac5c93bc9427509e5164289e810bdcabfc0351 drm-tip: 2017y-11m-15d-09h-10m-41s UTC integration manifest
6293c3c178f1 drm/i915: Clear breadcrumb node when cancelling signaling

== Logs ==

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

✗ Fi.CI.IGT: warning for drm/i915: Clear breadcrumb node when cancelling signaling

[Patchwork]

== Series Details ==

Series: drm/i915: Clear breadcrumb node when cancelling signaling
URL   : https://patchwork.freedesktop.org/series/33870/
State : warning

== Summary ==

Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-c:
                dmesg-warn -> PASS       (shard-hsw) fdo#102249
Test kms_atomic_transition:
        Subgroup plane-all-modeset-transition:
                pass       -> DMESG-WARN (shard-hsw)
Test drv_module_reload:
        Subgroup basic-reload-inject:
                dmesg-warn -> PASS       (shard-hsw) fdo#102707

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

shard-hsw        total:2584 pass:1472 dwarn:3   dfail:1   fail:9   skip:1099 time:9517s
Blacklisted hosts:
shard-apl        total:2565 pass:1602 dwarn:1   dfail:1   fail:24  skip:936 time:12777s
shard-kbl        total:2565 pass:1704 dwarn:5   dfail:1   fail:26  skip:828 time:10590s
shard-snb        total:2584 pass:1206 dwarn:2   dfail:1   fail:12  skip:1363 time:7791s

== Logs ==

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

Re: [PATCH] drm/i915: Clear breadcrumb node when cancelling signaling

[Joonas Lahtinen]

On Wed, 2017-11-15 at 12:14 +0000, Chris Wilson wrote:
> When we call intel_engine_cancel_signaling() to stop reporting whether
> or not a request is completed via an asynchronous signal, we remove that
> request from the breadcrumb wait queue. However, we may be concurrently
> processing that request in the signaler itself, the actual operations on
> the request itself are serialised but we do not actually clear the
> waiter after removing it from the tree allowing both parties to attempt
> to do so and corrupting the rbtree. (Elsewhere removing from the
> breadcrumb wait queue could only be done on behalf of i915_wait_request,
> so this race could not happen).
> 
> Reported-by: "He, Bo" <bo.he@intel.com>
> Fixes: 9eb143bbec7d ("drm/i915: Allow a request to be cancelled")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: "He, Bo" <bo.he@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Clear breadcrumb node when cancelling signaling

[Chris Wilson]

Quoting Joonas Lahtinen (2017-11-16 11:08:18)
> On Wed, 2017-11-15 at 12:14 +0000, Chris Wilson wrote:
> > When we call intel_engine_cancel_signaling() to stop reporting whether
> > or not a request is completed via an asynchronous signal, we remove that
> > request from the breadcrumb wait queue. However, we may be concurrently
> > processing that request in the signaler itself, the actual operations on
> > the request itself are serialised but we do not actually clear the
> > waiter after removing it from the tree allowing both parties to attempt
> > to do so and corrupting the rbtree. (Elsewhere removing from the
> > breadcrumb wait queue could only be done on behalf of i915_wait_request,
> > so this race could not happen).
> > 
> > Reported-by: "He, Bo" <bo.he@intel.com>
> > Fixes: 9eb143bbec7d ("drm/i915: Allow a request to be cancelled")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: "He, Bo" <bo.he@intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Ta, I was debating how to wait for a t-b, but this is definitely a race
between canceling and signaling that needs to be fixed regardless.

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