From patchwork Tue Jan 2 15:12:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 10140867 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4390560362 for ; Tue, 2 Jan 2018 15:13:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4FA8326E49 for ; Tue, 2 Jan 2018 15:13:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 445992861E; Tue, 2 Jan 2018 15:13:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 986AF26E49 for ; Tue, 2 Jan 2018 15:13:23 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0D9B689548; Tue, 2 Jan 2018 15:13:23 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 38AFE89428 for ; Tue, 2 Jan 2018 15:13:20 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 10184657-1500050 for multiple; Tue, 02 Jan 2018 15:13:01 +0000 Received: by haswell.alporthouse.com (sSMTP sendmail emulation); Tue, 02 Jan 2018 15:13:01 +0000 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 2 Jan 2018 15:12:28 +0000 Message-Id: <20180102151235.3949-12-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.15.1 In-Reply-To: <20180102151235.3949-1-chris@chris-wilson.co.uk> References: <20180102151235.3949-1-chris@chris-wilson.co.uk> X-Originating-IP: 78.156.65.138 X-Country: code=GB country="United Kingdom" ip=78.156.65.138 Subject: [Intel-gfx] [PATCH 12/19] drm/i915: Drop request reference for the signaler thread X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP If we remember to cancel the signaler on a request when retiring it (after we know that the request has been signaled), we do not need to carry an additional request in the signaler itself. This prevents an issue whereby the signaler threads may be delayed and hold on to thousands of request references, causing severe memory fragmentation and premature oom (most noticeable on 32b snb due to the limited GFP_KERNEL and frequent use of inter-engine fences). Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem_request.c | 6 +- drivers/gpu/drm/i915/intel_breadcrumbs.c | 111 +++++++++++++++---------------- 2 files changed, 59 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index a24c6a87953c..af2493164686 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -441,7 +441,10 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request) spin_lock_irq(&request->lock); if (request->waitboost) atomic_dec(&request->i915->gt_pm.rps.num_waiters); - dma_fence_signal_locked(&request->fence); + if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &request->fence.flags)) + dma_fence_signal_locked(&request->fence); + if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags)) + intel_engine_cancel_signaling(request); spin_unlock_irq(&request->lock); i915_priotree_fini(request->i915, &request->priotree); @@ -730,6 +733,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine, /* No zalloc, must clear what we need by hand */ req->global_seqno = 0; + req->signaling.wait.seqno = 0; req->file_priv = NULL; req->batch = NULL; req->capture_list = NULL; diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 58c624f982d9..6cfffa68f71a 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -642,6 +642,38 @@ static void signaler_set_rtpriority(void) sched_setscheduler_nocheck(current, SCHED_FIFO, ¶m); } +static void __intel_engine_remove_signal(struct intel_engine_cs *engine, + struct drm_i915_gem_request *request) +{ + struct intel_breadcrumbs *b = &engine->breadcrumbs; + + lockdep_assert_held(&b->rb_lock); + + /* + * Wake up all other completed waiters and select the + * next bottom-half for the next user interrupt. + */ + __intel_engine_remove_wait(engine, &request->signaling.wait); + + /* + * Find the next oldest signal. Note that as we have + * not been holding the lock, another client may + * have installed an even older signal than the one + * we just completed - so double check we are still + * the oldest before picking the next one. + */ + if (request->signaling.wait.seqno) { + if (request == rcu_access_pointer(b->first_signal)) { + struct rb_node *rb = rb_next(&request->signaling.node); + rcu_assign_pointer(b->first_signal, + rb ? to_signaler(rb) : NULL); + } + + rb_erase(&request->signaling.node, &b->signals); + request->signaling.wait.seqno = 0; + } +} + static int intel_breadcrumbs_signaler(void *arg) { struct intel_engine_cs *engine = arg; @@ -670,36 +702,18 @@ static int intel_breadcrumbs_signaler(void *arg) request = i915_gem_request_get_rcu(request); rcu_read_unlock(); if (signal_complete(request)) { - local_bh_disable(); - dma_fence_signal(&request->fence); - local_bh_enable(); /* kick start the tasklets */ - - spin_lock_irq(&b->rb_lock); - - /* Wake up all other completed waiters and select the - * next bottom-half for the next user interrupt. - */ - __intel_engine_remove_wait(engine, - &request->signaling.wait); - - /* Find the next oldest signal. Note that as we have - * not been holding the lock, another client may - * have installed an even older signal than the one - * we just completed - so double check we are still - * the oldest before picking the next one. - */ - if (request == rcu_access_pointer(b->first_signal)) { - struct rb_node *rb = - rb_next(&request->signaling.node); - rcu_assign_pointer(b->first_signal, - rb ? to_signaler(rb) : NULL); + if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, + &request->fence.flags)) { + local_bh_disable(); + dma_fence_signal(&request->fence); + local_bh_enable(); /* kick start the tasklets */ } - rb_erase(&request->signaling.node, &b->signals); - RB_CLEAR_NODE(&request->signaling.node); - - spin_unlock_irq(&b->rb_lock); - i915_gem_request_put(request); + if (request->signaling.wait.seqno) { + spin_lock_irq(&b->rb_lock); + __intel_engine_remove_signal(engine, request); + spin_unlock_irq(&b->rb_lock); + } /* If the engine is saturated we may be continually * processing completed requests. This angers the @@ -710,19 +724,17 @@ static int intel_breadcrumbs_signaler(void *arg) */ do_schedule = need_resched(); } + i915_gem_request_put(request); if (unlikely(do_schedule)) { if (kthread_should_park()) kthread_parkme(); - if (unlikely(kthread_should_stop())) { - i915_gem_request_put(request); + if (unlikely(kthread_should_stop())) break; - } schedule(); } - i915_gem_request_put(request); } while (1); __set_current_state(TASK_RUNNING); @@ -751,12 +763,12 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request, if (!seqno) return; + spin_lock(&b->rb_lock); + + GEM_BUG_ON(request->signaling.wait.seqno); request->signaling.wait.tsk = b->signaler; request->signaling.wait.request = request; request->signaling.wait.seqno = seqno; - i915_gem_request_get(request); - - spin_lock(&b->rb_lock); /* First add ourselves into the list of waiters, but register our * bottom-half as the signaller thread. As per usual, only the oldest @@ -795,7 +807,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request, rcu_assign_pointer(b->first_signal, request); } else { __intel_engine_remove_wait(engine, &request->signaling.wait); - i915_gem_request_put(request); + request->signaling.wait.seqno = 0; wakeup = false; } @@ -807,32 +819,17 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request, void intel_engine_cancel_signaling(struct drm_i915_gem_request *request) { - struct intel_engine_cs *engine = request->engine; - struct intel_breadcrumbs *b = &engine->breadcrumbs; - GEM_BUG_ON(!irqs_disabled()); lockdep_assert_held(&request->lock); - GEM_BUG_ON(!request->signaling.wait.seqno); - spin_lock(&b->rb_lock); + if (request->signaling.wait.seqno) { + struct intel_engine_cs *engine = request->engine; + struct intel_breadcrumbs *b = &engine->breadcrumbs; - if (!RB_EMPTY_NODE(&request->signaling.node)) { - if (request == rcu_access_pointer(b->first_signal)) { - struct rb_node *rb = - rb_next(&request->signaling.node); - rcu_assign_pointer(b->first_signal, - rb ? to_signaler(rb) : NULL); - } - rb_erase(&request->signaling.node, &b->signals); - RB_CLEAR_NODE(&request->signaling.node); - i915_gem_request_put(request); + spin_lock(&b->rb_lock); + __intel_engine_remove_signal(engine, request); + spin_unlock(&b->rb_lock); } - - __intel_engine_remove_wait(engine, &request->signaling.wait); - - spin_unlock(&b->rb_lock); - - request->signaling.wait.seqno = 0; } int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)