From patchwork Mon Nov 12 14:00:05 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 1728351 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id D20BADFE80 for ; Mon, 12 Nov 2012 14:03:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C1D219F0BA for ; Mon, 12 Nov 2012 06:03:30 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-we0-f177.google.com (mail-we0-f177.google.com [74.125.82.177]) by gabe.freedesktop.org (Postfix) with ESMTP id CCE1F9F01D for ; Mon, 12 Nov 2012 06:00:38 -0800 (PST) Received: by mail-we0-f177.google.com with SMTP id u50so2715708wey.36 for ; Mon, 12 Nov 2012 06:00:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=q6eybG1kcyez9NB0TAHgBI9yqyT05vC1H0CQbi4ukzU=; b=JnrPucEuWAUMKkB9LFqynfapivGtB5UcprbwVKYulv53mtGuhUbWfF8l2xlTYyOM71 2JrxfaIoJDgD6Ud7tn70B2Obx02b/UPOxmOGaauIEftbxQ7JIV897kz0oIRX43g/Tdqo LdW22K0CHsEaqhiUEKZ4EV8I+v3ZWGlMKLUmjgbJxbhAL8ygcFgPyHfvr0VluOELPocb wkBm8dzTSp+kleTtuTotvjt/PFdfFaV2ns0q3H4pF7FCGcImX2bLSFzoq5t4k7yLgKWT W4mDZI6Eo3XO7Cr1KPUpoeuRchMzEfkgd+QZkTvmwRW4QeiCCFfA19EjpKeK/aRiJSYc wfiQ== Received: by 10.180.99.5 with SMTP id em5mr15117680wib.8.1352728838473; Mon, 12 Nov 2012 06:00:38 -0800 (PST) Received: from localhost (5ED48CEF.cm-7-5c.dynamic.ziggo.nl. [94.212.140.239]) by mx.google.com with ESMTPS id ec3sm2522199wib.10.2012.11.12.06.00.31 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 12 Nov 2012 06:00:33 -0800 (PST) Received: by localhost (sSMTP sendmail emulation); Mon, 12 Nov 2012 15:00:30 +0100 From: Maarten Lankhorst To: dri-devel@lists.freedesktop.org Subject: [PATCH 04/10] drm/ttm: change fence_lock to inner lock, v3 Date: Mon, 12 Nov 2012 15:00:05 +0100 Message-Id: <1352728811-21860-4-git-send-email-maarten.lankhorst@canonical.com> X-Mailer: git-send-email 1.8.0 In-Reply-To: <1352728811-21860-1-git-send-email-maarten.lankhorst@canonical.com> References: <1352728811-21860-1-git-send-email-maarten.lankhorst@canonical.com> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org I changed the hierarchy to make fence_lock the most inner lock, instead of outer lock. This will simplify things slightly, and hopefully makes it easier to make fence_lock global at one point should it be needed. To make things clearer, I change the order around in ttm_bo_cleanup_refs and ttm_bo_cleanup_refs_or_queue. A reservation is taken first, then fence lock is taken and a wait is attempted. Signed-off-by: Maarten Lankhorst v2: - fix conflict with upstream race fix, simplifies ttm_bo_cleanup_refs v3: - change removal of fence_lock to making it a inner lock instead --- drivers/gpu/drm/ttm/ttm_bo.c | 95 ++++++++++++++++------------------ drivers/gpu/drm/ttm/ttm_execbuf_util.c | 4 +- 2 files changed, 48 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index a3383a7..70285ff 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -478,28 +478,26 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo) { struct ttm_bo_device *bdev = bo->bdev; struct ttm_bo_global *glob = bo->glob; - struct ttm_bo_driver *driver; + struct ttm_bo_driver *driver = bdev->driver; void *sync_obj = NULL; int put_count; int ret; - spin_lock(&bdev->fence_lock); - (void) ttm_bo_wait(bo, false, false, true); - if (!bo->sync_obj) { - - spin_lock(&glob->lru_lock); - - /** - * Lock inversion between bo:reserve and bdev::fence_lock here, - * but that's OK, since we're only trylocking. - */ - - ret = ttm_bo_reserve_locked(bo, false, true, false, 0); + spin_lock(&glob->lru_lock); + ret = ttm_bo_reserve_locked(bo, false, true, false, 0); + if (!ret) { + spin_lock(&bdev->fence_lock); + ret = ttm_bo_wait(bo, false, false, true); - if (unlikely(ret == -EBUSY)) + if (unlikely(ret == -EBUSY)) { + sync_obj = driver->sync_obj_ref(bo->sync_obj); + spin_unlock(&bdev->fence_lock); + atomic_set(&bo->reserved, 0); + wake_up_all(&bo->event_queue); goto queue; - + } spin_unlock(&bdev->fence_lock); + put_count = ttm_bo_del_from_lru(bo); atomic_set(&bo->reserved, 0); @@ -509,18 +507,11 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo) ttm_bo_list_ref_sub(bo, put_count, true); return; - } else { - spin_lock(&glob->lru_lock); } queue: - driver = bdev->driver; - if (bo->sync_obj) - sync_obj = driver->sync_obj_ref(bo->sync_obj); - kref_get(&bo->list_kref); list_add_tail(&bo->ddestroy, &bdev->ddestroy); spin_unlock(&glob->lru_lock); - spin_unlock(&bdev->fence_lock); if (sync_obj) { driver->sync_obj_flush(sync_obj); @@ -546,54 +537,60 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, bool no_wait_gpu) { struct ttm_bo_device *bdev = bo->bdev; + struct ttm_bo_driver *driver = bdev->driver; struct ttm_bo_global *glob = bo->glob; int put_count; int ret = 0; + void *sync_obj; retry: - spin_lock(&bdev->fence_lock); - ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu); - spin_unlock(&bdev->fence_lock); + spin_lock(&glob->lru_lock); - if (unlikely(ret != 0)) - return ret; + ret = ttm_bo_reserve_locked(bo, interruptible, + no_wait_reserve, false, 0); -retry_reserve: - spin_lock(&glob->lru_lock); + if (unlikely(ret)) { + spin_unlock(&glob->lru_lock); + return ret; + } if (unlikely(list_empty(&bo->ddestroy))) { + atomic_set(&bo->reserved, 0); + wake_up_all(&bo->event_queue); spin_unlock(&glob->lru_lock); return 0; } - ret = ttm_bo_reserve_locked(bo, false, true, false, 0); - - if (unlikely(ret == -EBUSY)) { - spin_unlock(&glob->lru_lock); - if (likely(!no_wait_reserve)) - ret = ttm_bo_wait_unreserved(bo, interruptible); - if (unlikely(ret != 0)) + spin_lock(&bdev->fence_lock); + ret = ttm_bo_wait(bo, false, false, true); + if (ret) { + if (no_wait_gpu) { + spin_unlock(&bdev->fence_lock); + atomic_set(&bo->reserved, 0); + wake_up_all(&bo->event_queue); + spin_unlock(&glob->lru_lock); return ret; + } - goto retry_reserve; - } - - BUG_ON(ret != 0); - - /** - * We can re-check for sync object without taking - * the bo::lock since setting the sync object requires - * also bo::reserved. A busy object at this point may - * be caused by another thread recently starting an accelerated - * eviction. - */ + /** + * Take a reference to the fence and unreserve, if the wait + * was succesful and no new sync_obj was attached, + * ttm_bo_wait in retry will return ret = 0, and end the loop. + */ - if (unlikely(bo->sync_obj)) { + sync_obj = driver->sync_obj_ref(&bo->sync_obj); + spin_unlock(&bdev->fence_lock); atomic_set(&bo->reserved, 0); wake_up_all(&bo->event_queue); spin_unlock(&glob->lru_lock); + + ret = driver->sync_obj_wait(bo->sync_obj, false, interruptible); + driver->sync_obj_unref(&sync_obj); + if (ret) + return ret; goto retry; } + spin_unlock(&bdev->fence_lock); put_count = ttm_bo_del_from_lru(bo); list_del_init(&bo->ddestroy); diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c index 1986d00..cd9e452 100644 --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c @@ -213,8 +213,8 @@ void ttm_eu_fence_buffer_objects(struct list_head *list, void *sync_obj) driver = bdev->driver; glob = bo->glob; - spin_lock(&bdev->fence_lock); spin_lock(&glob->lru_lock); + spin_lock(&bdev->fence_lock); list_for_each_entry(entry, list, head) { bo = entry->bo; @@ -223,8 +223,8 @@ void ttm_eu_fence_buffer_objects(struct list_head *list, void *sync_obj) ttm_bo_unreserve_locked(bo); entry->reserved = false; } - spin_unlock(&glob->lru_lock); spin_unlock(&bdev->fence_lock); + spin_unlock(&glob->lru_lock); list_for_each_entry(entry, list, head) { if (entry->old_sync_obj)