From patchwork Mon Nov 12 14:00:11 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 1728411 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id D9A5C3FCDE for ; Mon, 12 Nov 2012 14:10:06 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D06159F0E8 for ; Mon, 12 Nov 2012 06:10:06 -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 395849F06F for ; Mon, 12 Nov 2012 06:01:15 -0800 (PST) Received: by mail-we0-f177.google.com with SMTP id u50so2715708wey.36 for ; Mon, 12 Nov 2012 06:01:14 -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=8+KRX/L/TIvJZljpgREnViL2nkTPWQUZ8uekgyyRU+U=; b=IJFmB/YbNLa70mrUJHHJ5+7h/ybjrKPyJX5dnH6zF2744+FENQPy2wpeNiXfoPnxBq WEj0Wn9zqxpvrmFAwwHLwTMcDSr0KEPbOeWg5vJyf7JYoG9bRnbWSsqlEUitXHJBVki/ NM2zWveQrVgHu7Rl0bNaWI5azFKfG+5IpVmqz1Me/Vj26Wht3L1Y8OzZiFWLNrAxVOr7 G/Z25HNGXdsBCmvxZG+BkkYMQwQTWEZfBjuZ9qxymunVPmLW2kAwyeJTrrkCxGd7L6ib obrKGFC8V8oAxBVqLLe3aQsmxJChuvcDjcqhSc0fsjAT4mPaq+sQM6xSdLiPF73CRMTg 0wKQ== Received: by 10.216.197.166 with SMTP id t38mr7725771wen.109.1352728874838; Mon, 12 Nov 2012 06:01:14 -0800 (PST) Received: from localhost (5ED48CEF.cm-7-5c.dynamic.ziggo.nl. [94.212.140.239]) by mx.google.com with ESMTPS id eq2sm11813880wib.1.2012.11.12.06.01.08 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 12 Nov 2012 06:01:14 -0800 (PST) Received: by localhost (sSMTP sendmail emulation); Mon, 12 Nov 2012 15:01:08 +0100 From: Maarten Lankhorst To: dri-devel@lists.freedesktop.org Subject: [PATCH 10/10] drm/ttm: remove reliance on ttm_bo_wait_unreserved Date: Mon, 12 Nov 2012 15:00:11 +0100 Message-Id: <1352728811-21860-10-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 Slightly makes things more complicated, but instead of testing for unreserved and starting over, try to block and acquire reservation first, then start over. This maps a lot better to a blocking acquire operation. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/nouveau/nouveau_gem.c | 19 +++++++++--- drivers/gpu/drm/ttm/ttm_bo.c | 33 +++++++++++++++++++-- drivers/gpu/drm/ttm/ttm_execbuf_util.c | 53 ++++++++++++++++++++-------------- include/drm/ttm/ttm_bo_driver.h | 24 +++++++-------- 4 files changed, 89 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index 7b9364b..6f58604 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -321,6 +321,7 @@ validate_init(struct nouveau_channel *chan, struct drm_file *file_priv, uint32_t sequence; int trycnt = 0; int ret, i; + struct nouveau_bo *res_bo = NULL; sequence = atomic_add_return(1, &drm->ttm.validate_sequence); retry: @@ -341,6 +342,11 @@ retry: return -ENOENT; } nvbo = gem->driver_private; + if (nvbo == res_bo) { + res_bo = NULL; + drm_gem_object_unreference_unlocked(gem); + continue; + } if (nvbo->reserved_by && nvbo->reserved_by == file_priv) { NV_ERROR(drm, "multiple instances of buffer %d on " @@ -353,15 +359,18 @@ retry: ret = ttm_bo_reserve(&nvbo->bo, true, false, true, sequence); if (ret) { validate_fini(op, NULL); - if (unlikely(ret == -EAGAIN)) - ret = ttm_bo_wait_unreserved(&nvbo->bo, true); - drm_gem_object_unreference_unlocked(gem); + if (unlikely(ret == -EAGAIN)) { + ret = ttm_bo_reserve_slowpath(&nvbo->bo, true, + sequence); + if (!ret) + res_bo = nvbo; + } if (unlikely(ret)) { + drm_gem_object_unreference_unlocked(gem); if (ret != -ERESTARTSYS) NV_ERROR(drm, "fail reserve\n"); return ret; } - goto retry; } b->user_priv = (uint64_t)(unsigned long)nvbo; @@ -383,6 +392,8 @@ retry: validate_fini(op, NULL); return -EINVAL; } + if (nvbo == res_bo) + goto retry; } return 0; diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index e57dae5..bc90f6b 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -166,7 +166,8 @@ static void ttm_bo_release_list(struct kref *list_kref) ttm_mem_global_free(bdev->glob->mem_glob, acc_size); } -int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo, bool interruptible) +static int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo, + bool interruptible) { if (interruptible) { return wait_event_interruptible(bo->event_queue, @@ -176,7 +177,6 @@ int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo, bool interruptible) return 0; } } -EXPORT_SYMBOL(ttm_bo_wait_unreserved); void ttm_bo_add_to_lru(struct ttm_buffer_object *bo) { @@ -320,6 +320,35 @@ int ttm_bo_reserve(struct ttm_buffer_object *bo, return ret; } +int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo, + bool interruptible, uint32_t sequence) +{ + struct ttm_bo_global *glob = bo->glob; + int put_count = 0; + int ret; + + WARN_ON(!list_empty_careful(&bo->ddestroy)); + + ret = ttm_bo_reserve_nolru(bo, interruptible, false, false, 0); + if (likely(ret == 0)) { + /** + * Wake up waiters that may need to recheck for deadlock, + * since we unset seq_valid in ttm_bo_reserve_nolru + */ + bo->val_seq = sequence; + bo->seq_valid = true; + wake_up_all(&bo->event_queue); + + spin_lock(&glob->lru_lock); + put_count = ttm_bo_del_from_lru(bo); + spin_unlock(&glob->lru_lock); + ttm_bo_list_ref_sub(bo, put_count, true); + } + + return ret; +} +EXPORT_SYMBOL(ttm_bo_reserve_slowpath); + void ttm_bo_unreserve_locked(struct ttm_buffer_object *bo) { ttm_bo_add_to_lru(bo); diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c index b3fe824..de1504f 100644 --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c @@ -82,22 +82,6 @@ static void ttm_eu_list_ref_sub(struct list_head *list) } } -static int ttm_eu_wait_unreserved_locked(struct list_head *list, - struct ttm_buffer_object *bo) -{ - struct ttm_bo_global *glob = bo->glob; - int ret; - - ttm_eu_del_from_lru_locked(list); - spin_unlock(&glob->lru_lock); - ret = ttm_bo_wait_unreserved(bo, true); - spin_lock(&glob->lru_lock); - if (unlikely(ret != 0)) - ttm_eu_backoff_reservation_locked(list); - return ret; -} - - void ttm_eu_backoff_reservation(struct list_head *list) { struct ttm_validate_buffer *entry; @@ -145,34 +129,59 @@ int ttm_eu_reserve_buffers(struct list_head *list) entry = list_first_entry(list, struct ttm_validate_buffer, head); glob = entry->bo->glob; -retry: spin_lock(&glob->lru_lock); val_seq = entry->bo->bdev->val_seq++; +retry: list_for_each_entry(entry, list, head) { struct ttm_buffer_object *bo = entry->bo; + /* already slowpath reserved? */ + if (entry->reserved) + continue; + WARN_ON(!atomic_read(&bo->kref.refcount)); -retry_this_bo: + ret = ttm_bo_reserve_nolru(bo, true, true, true, val_seq); switch (ret) { case 0: break; case -EBUSY: - ret = ttm_eu_wait_unreserved_locked(list, bo); - if (unlikely(ret != 0)) { + ttm_eu_del_from_lru_locked(list); + spin_unlock(&glob->lru_lock); + ret = ttm_bo_reserve_nolru(bo, true, false, + true, val_seq); + spin_lock(&glob->lru_lock); + if (!ret) + break; + + if (ret != -EAGAIN) { + ttm_eu_backoff_reservation_locked(list); spin_unlock(&glob->lru_lock); ttm_eu_list_ref_sub(list); return ret; } - goto retry_this_bo; + + /* fallthrough */ case -EAGAIN: + /* uh oh, we lost out, drop every reservation and try + * to only reserve this buffer, then start over if + * this succeeds. + */ ttm_eu_backoff_reservation_locked(list); spin_unlock(&glob->lru_lock); ttm_eu_list_ref_sub(list); - ret = ttm_bo_wait_unreserved(bo, true); + ret = ttm_bo_reserve_slowpath(bo, true, val_seq); if (unlikely(ret != 0)) return ret; + entry->removed = entry->reserved = true; + spin_lock(&glob->lru_lock); + if (unlikely(atomic_read(&bo->cpu_writers) > 0)) { + ttm_eu_backoff_reservation_locked(list); + spin_unlock(&glob->lru_lock); + ttm_eu_list_ref_sub(list); + return -EBUSY; + } goto retry; default: ttm_eu_backoff_reservation_locked(list); diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index e9cdae1..26af40c 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -824,6 +824,18 @@ extern int ttm_bo_reserve(struct ttm_buffer_object *bo, bool interruptible, bool no_wait, bool use_sequence, uint32_t sequence); +/** + * ttm_bo_reserve_slowpath: + * @bo: A pointer to a struct ttm_buffer_object. + * @interruptible: Sleep interruptible if waiting. + * @sequence: Set (@bo)->sequence to this value after lock + * + * This is called after ttm_bo_reserve returns -EAGAIN and we backed off + * from all our other reservations. Because there are no other reservations + * held by us, this function cannot deadlock any more. + */ +extern int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo, + bool interruptible, uint32_t sequence); /** * ttm_bo_reserve_nolru: @@ -871,18 +883,6 @@ extern void ttm_bo_unreserve(struct ttm_buffer_object *bo); */ extern void ttm_bo_unreserve_locked(struct ttm_buffer_object *bo); -/** - * ttm_bo_wait_unreserved - * - * @bo: A pointer to a struct ttm_buffer_object. - * - * Wait for a struct ttm_buffer_object to become unreserved. - * This is typically used in the execbuf code to relax cpu-usage when - * a potential deadlock condition backoff. - */ -extern int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo, - bool interruptible); - /* * ttm_bo_util.c */