From patchwork Wed Nov 15 16:55:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 10059881 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 E22686019D for ; Wed, 15 Nov 2017 16:55:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E57E02A05E for ; Wed, 15 Nov 2017 16:55:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D90872A06D; Wed, 15 Nov 2017 16:55:30 +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 546322A05E for ; Wed, 15 Nov 2017 16:55:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 48DF66E628; Wed, 15 Nov 2017 16:55:29 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 827D86E628; Wed, 15 Nov 2017 16:55:27 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from localhost (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP (TLS) id 9604561-1500050 for multiple; Wed, 15 Nov 2017 16:55:23 +0000 MIME-Version: 1.0 From: Chris Wilson User-Agent: alot/0.3.6 To: christian.koenig@amd.com, =?utf-8?q?Christian_K=C3=B6nig?= , dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org References: <20171030145904.18584-1-deathsimple@vodafone.de> <20171030145904.18584-2-deathsimple@vodafone.de> <150998532204.10527.4781311190552781355@mail.alporthouse.com> <4fc05061-e92d-1702-a0f6-ee337b7d3eb3@gmail.com> <151067004520.20436.1513830153061032440@mail.alporthouse.com> In-Reply-To: <151067004520.20436.1513830153061032440@mail.alporthouse.com> Message-ID: <151076492143.19172.10013343108476631614@mail.alporthouse.com> Subject: Re: [PATCH 2/2] dma-buf: try to replace a signaled fence in reservation_object_add_shared_inplace Date: Wed, 15 Nov 2017 16:55:21 +0000 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Quoting Chris Wilson (2017-11-14 14:34:05) > Quoting Christian König (2017-11-14 14:24:44) > > Am 06.11.2017 um 17:22 schrieb Chris Wilson: > > > Quoting Christian König (2017-10-30 14:59:04) > > >> @@ -126,17 +127,28 @@ reservation_object_add_shared_inplace(struct reservation_object *obj, > > >> dma_fence_put(old_fence); > > >> return; > > >> } > > >> + > > >> + if (!signaled && dma_fence_is_signaled(old_fence)) { > > >> + signaled = old_fence; > > >> + signaled_idx = i; > > >> + } > > > How much do we care about only keeping one fence per-ctx here? You could > > > rearrange this to break on old_fence->context == fence->context || > > > dma_fence_is_signaled(old_fence) then everyone can use the final block. > > > > Yeah, that is what David Zhou suggested as well. > > > > I've rejected this approach for now cause I think we still have cases > > where we rely on one fence per ctx (but I'm not 100% sure). > > > > I changed patch #1 in this series as you suggest and going to send that > > out once more in a minute. > > > > Can we get this upstream as is for now? I won't have much more time > > working on this. > > Sure, we are only discussing how we might make it look tidier, pure > micro-optimisation with the caveat of losing the one-fence-per-ctx > guarantee. Ah, one thing to note is that extra checking pushed one of our corner case tests over its time limit. If we can completely forgo the one-fence-per-ctx here, what works really well in testing is i.e. don't check when not replacing the shared[], on creating the new buffer we then discard all the old fences. It should work for amdgpu as well since you do a ht to coalesce redundant fences before queuing. -Chris diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index 5319ac478918..5755e95fab1b 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -104,39 +104,19 @@ reservation_object_add_shared_inplace(struct reservation_object *obj, struct reservation_object_list *fobj, struct dma_fence *fence) { - struct dma_fence *replace = NULL; - u32 ctx = fence->context; - u32 i; - dma_fence_get(fence); preempt_disable(); write_seqcount_begin(&obj->seq); - for (i = 0; i < fobj->shared_count; ++i) { - struct dma_fence *check; - - check = rcu_dereference_protected(fobj->shared[i], - reservation_object_held(obj)); - - if (check->context == ctx || dma_fence_is_signaled(check)) { - replace = old_fence; - break; - } - } - /* * memory barrier is added by write_seqcount_begin, * fobj->shared_count is protected by this lock too */ - RCU_INIT_POINTER(fobj->shared[i], fence); - if (!replace) - fobj->shared_count++; + RCU_INIT_POINTER(fobj->shared[fobj->shared_count++], fence); write_seqcount_end(&obj->seq); preempt_enable(); - - dma_fence_put(replace); } static void