From: Chris Wilson Date: Fri, 8 Dec 2017 12:10:33 +0000 (+0000) Subject: drm/i915: Stop listening to request resubmission from the signaler kthread X-Git-Tag: v5.15~6763^2~28^2~2116 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=776bc27fd8ab67a675cb0041d3af361af5d0e290;p=platform%2Fkernel%2Flinux-starfive.git drm/i915: Stop listening to request resubmission from the signaler kthread The intent here was that we would be listening to i915_gem_request_unsubmit in order to cancel the signaler quickly and release the reference on the request. Cancelling the signaler is done directly via intel_engine_cancel_signaling (called from unsubmit), but that does not directly wake up the signaling thread, and neither does setting the request->global_seqno back to zero wake up listeners to the request->execute waitqueue. So the only time that listening to the request->execute waitqueue would wake up the signaling kthread would be on the request resubmission, during which time we would already receive wake ups from rejoining the global breadcrumbs wait rbtree. Trying to wake up to release the request remains an issue. If the signaling was cancelled and no other request required signaling, then it is possible for us to shutdown with the reference on the request still held. To ensure that we do not try to shutdown, leaking that request, we kick the signaling threads whenever we disarm the breadcrumbs, i.e. on parking the engine when idle. v2: We do need to be sure to release the last reference on stopping the kthread; asserting that it has been dropped already is insufficient. Fixes: d6a2289d9d6b ("drm/i915: Remove the preempted request from the execution queue") Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Michał Winiarski Link: https://patchwork.freedesktop.org/patch/msgid/20171208121033.5236-1-chris@chris-wilson.co.uk Acked-by: Daniel Vetter Reviewed-by: Tvrtko Ursulin --- diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 24c6fef..a774069 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -217,7 +217,7 @@ void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine) struct intel_wait *wait, *n; if (!b->irq_armed) - return; + goto wakeup_signaler; /* * We only disarm the irq when we are idle (all requests completed), @@ -242,6 +242,14 @@ void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine) b->waiters = RB_ROOT; spin_unlock_irq(&b->rb_lock); + + /* + * The signaling thread may be asleep holding a reference to a request, + * that had its signaling cancelled prior to being preempted. We need + * to kick the signaler, just in case, to release any such reference. + */ +wakeup_signaler: + wake_up_process(b->signaler); } static bool use_fake_irq(const struct intel_breadcrumbs *b) @@ -686,23 +694,15 @@ static int intel_breadcrumbs_signaler(void *arg) } if (unlikely(do_schedule)) { - DEFINE_WAIT(exec); - if (kthread_should_park()) kthread_parkme(); - if (kthread_should_stop()) { - GEM_BUG_ON(request); + if (unlikely(kthread_should_stop())) { + i915_gem_request_put(request); break; } - if (request) - add_wait_queue(&request->execute, &exec); - schedule(); - - if (request) - remove_wait_queue(&request->execute, &exec); } i915_gem_request_put(request); } while (1);