From ecff665f5e3f1c6909353e00b9420e45ae23d995 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Thu, 27 Jun 2013 13:48:17 +0200 Subject: drm/ttm: make ttm reservation calls behave like reservation calls This commit converts the source of the val_seq counter to the ww_mutex api. The reservation objects are converted later, because there is still a lockdep splat in nouveau that has to resolved first. Signed-off-by: Maarten Lankhorst Reviewed-by: Jerome Glisse Signed-off-by: Dave Airlie --- include/drm/ttm/ttm_bo_api.h | 2 +- include/drm/ttm/ttm_bo_driver.h | 32 +++++++++++++++++++++++--------- include/drm/ttm/ttm_execbuf_util.h | 13 ++++++++++--- 3 files changed, 34 insertions(+), 13 deletions(-) (limited to 'include/drm/ttm') diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 3cb5d848fb66..0a992b016fe9 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -234,7 +234,7 @@ struct ttm_buffer_object { struct list_head ddestroy; struct list_head swap; struct list_head io_reserve_lru; - uint32_t val_seq; + unsigned long val_seq; bool seq_valid; /** diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index 9c8dca79808e..ec18c5f3e6e9 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -38,6 +38,7 @@ #include #include #include +#include struct ttm_backend_func { /** @@ -778,7 +779,7 @@ extern void ttm_mem_io_unlock(struct ttm_mem_type_manager *man); * @bo: A pointer to a struct ttm_buffer_object. * @interruptible: Sleep interruptible if waiting. * @no_wait: Don't sleep while trying to reserve, rather return -EBUSY. - * @use_sequence: If @bo is already reserved, Only sleep waiting for + * @use_ticket: If @bo is already reserved, Only sleep waiting for * it to become unreserved if @sequence < (@bo)->sequence. * * Locks a buffer object for validation. (Or prevents other processes from @@ -819,7 +820,8 @@ extern void ttm_mem_io_unlock(struct ttm_mem_type_manager *man); */ extern int ttm_bo_reserve(struct ttm_buffer_object *bo, bool interruptible, - bool no_wait, bool use_sequence, uint32_t sequence); + bool no_wait, bool use_ticket, + struct ww_acquire_ctx *ticket); /** * ttm_bo_reserve_slowpath_nolru: @@ -836,7 +838,7 @@ extern int ttm_bo_reserve(struct ttm_buffer_object *bo, */ extern int ttm_bo_reserve_slowpath_nolru(struct ttm_buffer_object *bo, bool interruptible, - uint32_t sequence); + struct ww_acquire_ctx *ticket); /** @@ -850,7 +852,8 @@ extern int ttm_bo_reserve_slowpath_nolru(struct ttm_buffer_object *bo, * 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); + bool interruptible, + struct ww_acquire_ctx *ticket); /** * ttm_bo_reserve_nolru: @@ -876,8 +879,8 @@ extern int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo, */ extern int ttm_bo_reserve_nolru(struct ttm_buffer_object *bo, bool interruptible, - bool no_wait, bool use_sequence, - uint32_t sequence); + bool no_wait, bool use_ticket, + struct ww_acquire_ctx *ticket); /** * ttm_bo_unreserve @@ -889,14 +892,25 @@ extern int ttm_bo_reserve_nolru(struct ttm_buffer_object *bo, extern void ttm_bo_unreserve(struct ttm_buffer_object *bo); /** - * ttm_bo_unreserve_locked + * ttm_bo_unreserve_ticket + * @bo: A pointer to a struct ttm_buffer_object. + * @ticket: ww_acquire_ctx used for reserving * + * Unreserve a previous reservation of @bo made with @ticket. + */ +extern void ttm_bo_unreserve_ticket(struct ttm_buffer_object *bo, + struct ww_acquire_ctx *ticket); + +/** + * ttm_bo_unreserve_locked * @bo: A pointer to a struct ttm_buffer_object. + * @ticket: ww_acquire_ctx used for reserving, or NULL * - * Unreserve a previous reservation of @bo. + * Unreserve a previous reservation of @bo made with @ticket. * Needs to be called with struct ttm_bo_global::lru_lock held. */ -extern void ttm_bo_unreserve_locked(struct ttm_buffer_object *bo); +extern void ttm_bo_unreserve_ticket_locked(struct ttm_buffer_object *bo, + struct ww_acquire_ctx *ticket); /* * ttm_bo_util.c diff --git a/include/drm/ttm/ttm_execbuf_util.h b/include/drm/ttm/ttm_execbuf_util.h index 547e19f06e57..ba71ef91f4d5 100644 --- a/include/drm/ttm/ttm_execbuf_util.h +++ b/include/drm/ttm/ttm_execbuf_util.h @@ -33,6 +33,7 @@ #include #include +#include /** * struct ttm_validate_buffer @@ -57,17 +58,20 @@ struct ttm_validate_buffer { /** * function ttm_eu_backoff_reservation * + * @ticket: ww_acquire_ctx from reserve call * @list: thread private list of ttm_validate_buffer structs. * * Undoes all buffer validation reservations for bos pointed to by * the list entries. */ -extern void ttm_eu_backoff_reservation(struct list_head *list); +extern void ttm_eu_backoff_reservation(struct ww_acquire_ctx *ticket, + struct list_head *list); /** * function ttm_eu_reserve_buffers * + * @ticket: [out] ww_acquire_ctx returned by call. * @list: thread private list of ttm_validate_buffer structs. * * Tries to reserve bos pointed to by the list entries for validation. @@ -90,11 +94,13 @@ extern void ttm_eu_backoff_reservation(struct list_head *list); * has failed. */ -extern int ttm_eu_reserve_buffers(struct list_head *list); +extern int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, + struct list_head *list); /** * function ttm_eu_fence_buffer_objects. * + * @ticket: ww_acquire_ctx from reserve call * @list: thread private list of ttm_validate_buffer structs. * @sync_obj: The new sync object for the buffers. * @@ -104,6 +110,7 @@ extern int ttm_eu_reserve_buffers(struct list_head *list); * */ -extern void ttm_eu_fence_buffer_objects(struct list_head *list, void *sync_obj); +extern void ttm_eu_fence_buffer_objects(struct ww_acquire_ctx *ticket, + struct list_head *list, void *sync_obj); #endif -- cgit v1.2.3 From 5e338405119a80aa59e811626739122d1c15045d Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Thu, 27 Jun 2013 13:48:19 +0200 Subject: drm/ttm: convert to the reservation api Now that the code is compatible in semantics, flip the switch. Use ww_mutex instead of the homegrown implementation. ww_mutex uses -EDEADLK to signal that the caller has to back off, and -EALREADY to indicate this buffer is already held by the caller. ttm used -EAGAIN and -EDEADLK for those, respectively. So some changes were needed to handle this correctly. Signed-off-by: Maarten Lankhorst Reviewed-by: Jerome Glisse Signed-off-by: Dave Airlie --- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- drivers/gpu/drm/qxl/qxl_object.h | 5 - drivers/gpu/drm/ttm/ttm_bo.c | 190 +++++++++------------------------ drivers/gpu/drm/ttm/ttm_bo_util.c | 6 +- drivers/gpu/drm/ttm/ttm_execbuf_util.c | 43 +++----- include/drm/ttm/ttm_bo_api.h | 25 ++--- include/drm/ttm/ttm_execbuf_util.h | 1 - 7 files changed, 79 insertions(+), 193 deletions(-) (limited to 'include/drm/ttm') diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index e35d4688711c..2b2077d1e2f7 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -368,7 +368,7 @@ retry: ret = ttm_bo_reserve(&nvbo->bo, true, false, true, &op->ticket); if (ret) { validate_fini_no_ticket(op, NULL); - if (unlikely(ret == -EAGAIN)) { + if (unlikely(ret == -EDEADLK)) { ret = ttm_bo_reserve_slowpath(&nvbo->bo, true, &op->ticket); if (!ret) diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h index b4fd89fbd8b7..ee7ad79ce781 100644 --- a/drivers/gpu/drm/qxl/qxl_object.h +++ b/drivers/gpu/drm/qxl/qxl_object.h @@ -57,11 +57,6 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo) return bo->tbo.num_pages << PAGE_SHIFT; } -static inline bool qxl_bo_is_reserved(struct qxl_bo *bo) -{ - return !!atomic_read(&bo->tbo.reserved); -} - static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo) { return bo->tbo.addr_space_offset; diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index b912375b9c18..5f9fe8044afc 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -150,6 +150,9 @@ static void ttm_bo_release_list(struct kref *list_kref) if (bo->ttm) ttm_tt_destroy(bo->ttm); atomic_dec(&bo->glob->bo_count); + if (bo->resv == &bo->ttm_resv) + reservation_object_fini(&bo->ttm_resv); + if (bo->destroy) bo->destroy(bo); else { @@ -158,18 +161,6 @@ static void ttm_bo_release_list(struct kref *list_kref) ttm_mem_global_free(bdev->glob->mem_glob, acc_size); } -static int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo, - bool interruptible) -{ - if (interruptible) { - return wait_event_interruptible(bo->event_queue, - !ttm_bo_is_reserved(bo)); - } else { - wait_event(bo->event_queue, !ttm_bo_is_reserved(bo)); - return 0; - } -} - void ttm_bo_add_to_lru(struct ttm_buffer_object *bo) { struct ttm_bo_device *bdev = bo->bdev; @@ -218,65 +209,27 @@ int ttm_bo_reserve_nolru(struct ttm_buffer_object *bo, bool no_wait, bool use_ticket, struct ww_acquire_ctx *ticket) { - int ret; + int ret = 0; - while (unlikely(atomic_xchg(&bo->reserved, 1) != 0)) { - /** - * Deadlock avoidance for multi-bo reserving. - */ - if (use_ticket && bo->seq_valid) { - /** - * We've already reserved this one. - */ - if (unlikely(ticket->stamp == bo->val_seq)) - return -EDEADLK; - /** - * Already reserved by a thread that will not back - * off for us. We need to back off. - */ - if (unlikely(ticket->stamp - bo->val_seq <= LONG_MAX)) - return -EAGAIN; - } + if (no_wait) { + bool success; - if (no_wait) + /* not valid any more, fix your locking! */ + if (WARN_ON(ticket)) return -EBUSY; - ret = ttm_bo_wait_unreserved(bo, interruptible); - - if (unlikely(ret)) - return ret; - } - - if (use_ticket) { - bool wake_up = false; - - /** - * Wake up waiters that may need to recheck for deadlock, - * if we decreased the sequence number. - */ - if (unlikely((bo->val_seq - ticket->stamp <= LONG_MAX) - || !bo->seq_valid)) - wake_up = true; - - /* - * In the worst case with memory ordering these values can be - * seen in the wrong order. However since we call wake_up_all - * in that case, this will hopefully not pose a problem, - * and the worst case would only cause someone to accidentally - * hit -EAGAIN in ttm_bo_reserve when they see old value of - * val_seq. However this would only happen if seq_valid was - * written before val_seq was, and just means some slightly - * increased cpu usage - */ - bo->val_seq = ticket->stamp; - bo->seq_valid = true; - if (wake_up) - wake_up_all(&bo->event_queue); - } else { - bo->seq_valid = false; + success = ww_mutex_trylock(&bo->resv->lock); + return success ? 0 : -EBUSY; } - return 0; + if (interruptible) + ret = ww_mutex_lock_interruptible(&bo->resv->lock, + ticket); + else + ret = ww_mutex_lock(&bo->resv->lock, ticket); + if (ret == -EINTR) + return -ERESTARTSYS; + return ret; } EXPORT_SYMBOL(ttm_bo_reserve); @@ -313,50 +266,27 @@ int ttm_bo_reserve(struct ttm_buffer_object *bo, return ret; } -int ttm_bo_reserve_slowpath_nolru(struct ttm_buffer_object *bo, - bool interruptible, - struct ww_acquire_ctx *ticket) -{ - bool wake_up = false; - int ret; - - while (unlikely(atomic_xchg(&bo->reserved, 1) != 0)) { - WARN_ON(bo->seq_valid && ticket->stamp == bo->val_seq); - - ret = ttm_bo_wait_unreserved(bo, interruptible); - - if (unlikely(ret)) - return ret; - } - - if (bo->val_seq - ticket->stamp < LONG_MAX || !bo->seq_valid) - wake_up = true; - - /** - * Wake up waiters that may need to recheck for deadlock, - * if we decreased the sequence number. - */ - bo->val_seq = ticket->stamp; - bo->seq_valid = true; - if (wake_up) - wake_up_all(&bo->event_queue); - - return 0; -} - int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo, bool interruptible, struct ww_acquire_ctx *ticket) { struct ttm_bo_global *glob = bo->glob; - int put_count, ret; + int put_count = 0; + int ret = 0; + + if (interruptible) + ret = ww_mutex_lock_slow_interruptible(&bo->resv->lock, + ticket); + else + ww_mutex_lock_slow(&bo->resv->lock, ticket); - ret = ttm_bo_reserve_slowpath_nolru(bo, interruptible, ticket); - if (likely(!ret)) { + if (likely(ret == 0)) { 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); - } + } else if (ret == -EINTR) + ret = -ERESTARTSYS; + return ret; } EXPORT_SYMBOL(ttm_bo_reserve_slowpath); @@ -364,8 +294,7 @@ EXPORT_SYMBOL(ttm_bo_reserve_slowpath); void ttm_bo_unreserve_ticket_locked(struct ttm_buffer_object *bo, struct ww_acquire_ctx *ticket) { ttm_bo_add_to_lru(bo); - atomic_set(&bo->reserved, 0); - wake_up_all(&bo->event_queue); + ww_mutex_unlock(&bo->resv->lock); } void ttm_bo_unreserve(struct ttm_buffer_object *bo) @@ -558,17 +487,7 @@ static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo) } ttm_bo_mem_put(bo, &bo->mem); - atomic_set(&bo->reserved, 0); - wake_up_all(&bo->event_queue); - - /* - * Since the final reference to this bo may not be dropped by - * the current task we have to put a memory barrier here to make - * sure the changes done in this function are always visible. - * - * This function only needs protection against the final kref_put. - */ - smp_mb__before_atomic_dec(); + ww_mutex_unlock (&bo->resv->lock); } static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo) @@ -600,10 +519,8 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo) sync_obj = driver->sync_obj_ref(bo->sync_obj); spin_unlock(&bdev->fence_lock); - if (!ret) { - atomic_set(&bo->reserved, 0); - wake_up_all(&bo->event_queue); - } + if (!ret) + ww_mutex_unlock(&bo->resv->lock); kref_get(&bo->list_kref); list_add_tail(&bo->ddestroy, &bdev->ddestroy); @@ -653,8 +570,7 @@ static int ttm_bo_cleanup_refs_and_unlock(struct ttm_buffer_object *bo, 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); + ww_mutex_unlock(&bo->resv->lock); spin_unlock(&glob->lru_lock); ret = driver->sync_obj_wait(sync_obj, false, interruptible); @@ -692,8 +608,7 @@ static int ttm_bo_cleanup_refs_and_unlock(struct ttm_buffer_object *bo, spin_unlock(&bdev->fence_lock); if (ret || unlikely(list_empty(&bo->ddestroy))) { - atomic_set(&bo->reserved, 0); - wake_up_all(&bo->event_queue); + ww_mutex_unlock(&bo->resv->lock); spin_unlock(&glob->lru_lock); return ret; } @@ -1253,6 +1168,7 @@ int ttm_bo_init(struct ttm_bo_device *bdev, int ret = 0; unsigned long num_pages; struct ttm_mem_global *mem_glob = bdev->glob->mem_glob; + bool locked; ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false); if (ret) { @@ -1279,8 +1195,6 @@ int ttm_bo_init(struct ttm_bo_device *bdev, kref_init(&bo->kref); kref_init(&bo->list_kref); atomic_set(&bo->cpu_writers, 0); - atomic_set(&bo->reserved, 1); - init_waitqueue_head(&bo->event_queue); INIT_LIST_HEAD(&bo->lru); INIT_LIST_HEAD(&bo->ddestroy); INIT_LIST_HEAD(&bo->swap); @@ -1298,37 +1212,34 @@ int ttm_bo_init(struct ttm_bo_device *bdev, bo->mem.bus.io_reserved_count = 0; bo->priv_flags = 0; bo->mem.placement = (TTM_PL_FLAG_SYSTEM | TTM_PL_FLAG_CACHED); - bo->seq_valid = false; bo->persistent_swap_storage = persistent_swap_storage; bo->acc_size = acc_size; bo->sg = sg; + bo->resv = &bo->ttm_resv; + reservation_object_init(bo->resv); atomic_inc(&bo->glob->bo_count); ret = ttm_bo_check_placement(bo, placement); - if (unlikely(ret != 0)) - goto out_err; /* * For ttm_bo_type_device buffers, allocate * address space from the device. */ - if (bo->type == ttm_bo_type_device || - bo->type == ttm_bo_type_sg) { + if (likely(!ret) && + (bo->type == ttm_bo_type_device || + bo->type == ttm_bo_type_sg)) ret = ttm_bo_setup_vm(bo); - if (ret) - goto out_err; - } - ret = ttm_bo_validate(bo, placement, interruptible, false); - if (ret) - goto out_err; + locked = ww_mutex_trylock(&bo->resv->lock); + WARN_ON(!locked); - ttm_bo_unreserve(bo); - return 0; + if (likely(!ret)) + ret = ttm_bo_validate(bo, placement, interruptible, false); -out_err: ttm_bo_unreserve(bo); - ttm_bo_unref(&bo); + + if (unlikely(ret)) + ttm_bo_unref(&bo); return ret; } @@ -1941,8 +1852,7 @@ out: * already swapped buffer. */ - atomic_set(&bo->reserved, 0); - wake_up_all(&bo->event_queue); + ww_mutex_unlock(&bo->resv->lock); kref_put(&bo->list_kref, ttm_bo_release_list); return ret; } diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index af894584dd90..319cf4127c5b 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -433,6 +433,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, struct ttm_buffer_object *fbo; struct ttm_bo_device *bdev = bo->bdev; struct ttm_bo_driver *driver = bdev->driver; + int ret; fbo = kmalloc(sizeof(*fbo), GFP_KERNEL); if (!fbo) @@ -445,7 +446,6 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, * TODO: Explicit member copy would probably be better here. */ - init_waitqueue_head(&fbo->event_queue); INIT_LIST_HEAD(&fbo->ddestroy); INIT_LIST_HEAD(&fbo->lru); INIT_LIST_HEAD(&fbo->swap); @@ -463,6 +463,10 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, kref_init(&fbo->kref); fbo->destroy = &ttm_transfered_destroy; fbo->acc_size = 0; + fbo->resv = &fbo->ttm_resv; + reservation_object_init(fbo->resv); + ret = ww_mutex_trylock(&fbo->resv->lock); + WARN_ON(!ret); *new_obj = fbo; return 0; diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c index efcb734e5543..7392da557be2 100644 --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c @@ -48,8 +48,7 @@ static void ttm_eu_backoff_reservation_locked(struct list_head *list, entry->removed = false; } else { - atomic_set(&bo->reserved, 0); - wake_up_all(&bo->event_queue); + ww_mutex_unlock(&bo->resv->lock); } } } @@ -134,8 +133,6 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, glob = entry->bo->glob; ww_acquire_init(ticket, &reservation_ww_class); - spin_lock(&glob->lru_lock); - retry: list_for_each_entry(entry, list, head) { struct ttm_buffer_object *bo = entry->bo; @@ -144,42 +141,34 @@ retry: if (entry->reserved) continue; - ret = ttm_bo_reserve_nolru(bo, true, true, true, ticket); - switch (ret) { - case 0: - break; - case -EBUSY: - ttm_eu_del_from_lru_locked(list); - spin_unlock(&glob->lru_lock); - ret = ttm_bo_reserve_nolru(bo, true, false, - true, ticket); - spin_lock(&glob->lru_lock); - - if (!ret) - break; - if (unlikely(ret != -EAGAIN)) - goto err; + ret = ttm_bo_reserve_nolru(bo, true, false, true, ticket); - /* fallthrough */ - case -EAGAIN: + if (ret == -EDEADLK) { + /* uh oh, we lost out, drop every reservation and try + * to only reserve this buffer, then start over if + * this succeeds. + */ + spin_lock(&glob->lru_lock); ttm_eu_backoff_reservation_locked(list, ticket); spin_unlock(&glob->lru_lock); ttm_eu_list_ref_sub(list); - ret = ttm_bo_reserve_slowpath_nolru(bo, true, ticket); - if (unlikely(ret != 0)) + ret = ww_mutex_lock_slow_interruptible(&bo->resv->lock, + ticket); + if (unlikely(ret != 0)) { + if (ret == -EINTR) + ret = -ERESTARTSYS; goto err_fini; + } - spin_lock(&glob->lru_lock); entry->reserved = true; if (unlikely(atomic_read(&bo->cpu_writers) > 0)) { ret = -EBUSY; goto err; } goto retry; - default: + } else if (ret) goto err; - } entry->reserved = true; if (unlikely(atomic_read(&bo->cpu_writers) > 0)) { @@ -189,12 +178,14 @@ retry: } ww_acquire_done(ticket); + spin_lock(&glob->lru_lock); ttm_eu_del_from_lru_locked(list); spin_unlock(&glob->lru_lock); ttm_eu_list_ref_sub(list); return 0; err: + spin_lock(&glob->lru_lock); ttm_eu_backoff_reservation_locked(list, ticket); spin_unlock(&glob->lru_lock); ttm_eu_list_ref_sub(list); diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 0a992b016fe9..31ad860c10cd 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -39,6 +39,7 @@ #include #include #include +#include struct ttm_bo_device; @@ -153,7 +154,6 @@ struct ttm_tt; * Lru lists may keep one refcount, the delayed delete list, and kref != 0 * keeps one refcount. When this refcount reaches zero, * the object is destroyed. - * @event_queue: Queue for processes waiting on buffer object status change. * @mem: structure describing current placement. * @persistent_swap_storage: Usually the swap storage is deleted for buffers * pinned in physical memory. If this behaviour is not desired, this member @@ -164,12 +164,6 @@ struct ttm_tt; * @lru: List head for the lru list. * @ddestroy: List head for the delayed destroy list. * @swap: List head for swap LRU list. - * @val_seq: Sequence of the validation holding the @reserved lock. - * Used to avoid starvation when many processes compete to validate the - * buffer. This member is protected by the bo_device::lru_lock. - * @seq_valid: The value of @val_seq is valid. This value is protected by - * the bo_device::lru_lock. - * @reserved: Deadlock-free lock used for synchronization state transitions. * @sync_obj: Pointer to a synchronization object. * @priv_flags: Flags describing buffer object internal state. * @vm_rb: Rb node for the vm rb tree. @@ -209,10 +203,9 @@ struct ttm_buffer_object { struct kref kref; struct kref list_kref; - wait_queue_head_t event_queue; /** - * Members protected by the bo::reserved lock. + * Members protected by the bo::resv::reserved lock. */ struct ttm_mem_reg mem; @@ -234,15 +227,6 @@ struct ttm_buffer_object { struct list_head ddestroy; struct list_head swap; struct list_head io_reserve_lru; - unsigned long val_seq; - bool seq_valid; - - /** - * Members protected by the bdev::lru_lock - * only when written to. - */ - - atomic_t reserved; /** * Members protected by struct buffer_object_device::fence_lock @@ -272,6 +256,9 @@ struct ttm_buffer_object { uint32_t cur_placement; struct sg_table *sg; + + struct reservation_object *resv; + struct reservation_object ttm_resv; }; /** @@ -736,7 +723,7 @@ extern void ttm_bo_swapout_all(struct ttm_bo_device *bdev); */ static inline bool ttm_bo_is_reserved(struct ttm_buffer_object *bo) { - return atomic_read(&bo->reserved); + return ww_mutex_is_locked(&bo->resv->lock); } #endif diff --git a/include/drm/ttm/ttm_execbuf_util.h b/include/drm/ttm/ttm_execbuf_util.h index ba71ef91f4d5..ec8a1d306510 100644 --- a/include/drm/ttm/ttm_execbuf_util.h +++ b/include/drm/ttm/ttm_execbuf_util.h @@ -33,7 +33,6 @@ #include #include -#include /** * struct ttm_validate_buffer -- cgit v1.2.3 From 3482032457f50cae196f6397ebec7f5f2ad3cf7d Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Thu, 27 Jun 2013 13:48:24 +0200 Subject: drm/ttm: inline ttm_bo_reserve and related calls Makes lockdep a lot more useful. Signed-off-by: Maarten Lankhorst Signed-off-by: Dave Airlie --- drivers/gpu/drm/ttm/ttm_bo.c | 105 ++------------------ drivers/gpu/drm/ttm/ttm_execbuf_util.c | 9 +- include/drm/ttm/ttm_bo_driver.h | 175 ++++++++++++++++++++------------- 3 files changed, 117 insertions(+), 172 deletions(-) (limited to 'include/drm/ttm') diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 5f9fe8044afc..a8a27f51e419 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -182,6 +182,7 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo) } } } +EXPORT_SYMBOL(ttm_bo_add_to_lru); int ttm_bo_del_from_lru(struct ttm_buffer_object *bo) { @@ -204,35 +205,6 @@ int ttm_bo_del_from_lru(struct ttm_buffer_object *bo) return put_count; } -int ttm_bo_reserve_nolru(struct ttm_buffer_object *bo, - bool interruptible, - bool no_wait, bool use_ticket, - struct ww_acquire_ctx *ticket) -{ - int ret = 0; - - if (no_wait) { - bool success; - - /* not valid any more, fix your locking! */ - if (WARN_ON(ticket)) - return -EBUSY; - - success = ww_mutex_trylock(&bo->resv->lock); - return success ? 0 : -EBUSY; - } - - if (interruptible) - ret = ww_mutex_lock_interruptible(&bo->resv->lock, - ticket); - else - ret = ww_mutex_lock(&bo->resv->lock, ticket); - if (ret == -EINTR) - return -ERESTARTSYS; - return ret; -} -EXPORT_SYMBOL(ttm_bo_reserve); - static void ttm_bo_ref_bug(struct kref *list_kref) { BUG(); @@ -245,77 +217,16 @@ void ttm_bo_list_ref_sub(struct ttm_buffer_object *bo, int count, (never_free) ? ttm_bo_ref_bug : ttm_bo_release_list); } -int ttm_bo_reserve(struct ttm_buffer_object *bo, - bool interruptible, - bool no_wait, bool use_ticket, - struct ww_acquire_ctx *ticket) -{ - struct ttm_bo_global *glob = bo->glob; - int put_count = 0; - int ret; - - ret = ttm_bo_reserve_nolru(bo, interruptible, no_wait, use_ticket, - ticket); - if (likely(ret == 0)) { - 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; -} - -int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo, - bool interruptible, struct ww_acquire_ctx *ticket) -{ - struct ttm_bo_global *glob = bo->glob; - int put_count = 0; - int ret = 0; - - if (interruptible) - ret = ww_mutex_lock_slow_interruptible(&bo->resv->lock, - ticket); - else - ww_mutex_lock_slow(&bo->resv->lock, ticket); - - if (likely(ret == 0)) { - 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); - } else if (ret == -EINTR) - ret = -ERESTARTSYS; - - return ret; -} -EXPORT_SYMBOL(ttm_bo_reserve_slowpath); - -void ttm_bo_unreserve_ticket_locked(struct ttm_buffer_object *bo, struct ww_acquire_ctx *ticket) +void ttm_bo_del_sub_from_lru(struct ttm_buffer_object *bo) { - ttm_bo_add_to_lru(bo); - ww_mutex_unlock(&bo->resv->lock); -} - -void ttm_bo_unreserve(struct ttm_buffer_object *bo) -{ - struct ttm_bo_global *glob = bo->glob; - - spin_lock(&glob->lru_lock); - ttm_bo_unreserve_ticket_locked(bo, NULL); - spin_unlock(&glob->lru_lock); -} -EXPORT_SYMBOL(ttm_bo_unreserve); - -void ttm_bo_unreserve_ticket(struct ttm_buffer_object *bo, struct ww_acquire_ctx *ticket) -{ - struct ttm_bo_global *glob = bo->glob; + int put_count; - spin_lock(&glob->lru_lock); - ttm_bo_unreserve_ticket_locked(bo, ticket); - spin_unlock(&glob->lru_lock); + spin_lock(&bo->glob->lru_lock); + put_count = ttm_bo_del_from_lru(bo); + spin_unlock(&bo->glob->lru_lock); + ttm_bo_list_ref_sub(bo, put_count, true); } -EXPORT_SYMBOL(ttm_bo_unreserve_ticket); +EXPORT_SYMBOL(ttm_bo_del_sub_from_lru); /* * Call bo->mutex locked. diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c index 7392da557be2..6c911789ae5c 100644 --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c @@ -44,12 +44,10 @@ static void ttm_eu_backoff_reservation_locked(struct list_head *list, entry->reserved = false; if (entry->removed) { - ttm_bo_unreserve_ticket_locked(bo, ticket); + ttm_bo_add_to_lru(bo); entry->removed = false; - - } else { - ww_mutex_unlock(&bo->resv->lock); } + ww_mutex_unlock(&bo->resv->lock); } } @@ -220,7 +218,8 @@ void ttm_eu_fence_buffer_objects(struct ww_acquire_ctx *ticket, bo = entry->bo; entry->old_sync_obj = bo->sync_obj; bo->sync_obj = driver->sync_obj_ref(sync_obj); - ttm_bo_unreserve_ticket_locked(bo, ticket); + ttm_bo_add_to_lru(bo); + ww_mutex_unlock(&bo->resv->lock); entry->reserved = false; } spin_unlock(&bdev->fence_lock); diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index ec18c5f3e6e9..984fc2d571a1 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -772,6 +773,55 @@ extern int ttm_mem_io_lock(struct ttm_mem_type_manager *man, bool interruptible); extern void ttm_mem_io_unlock(struct ttm_mem_type_manager *man); +extern void ttm_bo_del_sub_from_lru(struct ttm_buffer_object *bo); +extern void ttm_bo_add_to_lru(struct ttm_buffer_object *bo); + +/** + * ttm_bo_reserve_nolru: + * + * @bo: A pointer to a struct ttm_buffer_object. + * @interruptible: Sleep interruptible if waiting. + * @no_wait: Don't sleep while trying to reserve, rather return -EBUSY. + * @use_ticket: If @bo is already reserved, Only sleep waiting for + * it to become unreserved if @ticket->stamp is older. + * + * Will not remove reserved buffers from the lru lists. + * Otherwise identical to ttm_bo_reserve. + * + * Returns: + * -EDEADLK: The reservation may cause a deadlock. + * Release all buffer reservations, wait for @bo to become unreserved and + * try again. (only if use_sequence == 1). + * -ERESTARTSYS: A wait for the buffer to become unreserved was interrupted by + * a signal. Release all buffer reservations and return to user-space. + * -EBUSY: The function needed to sleep, but @no_wait was true + * -EALREADY: Bo already reserved using @ticket. This error code will only + * be returned if @use_ticket is set to true. + */ +static inline int ttm_bo_reserve_nolru(struct ttm_buffer_object *bo, + bool interruptible, + bool no_wait, bool use_ticket, + struct ww_acquire_ctx *ticket) +{ + int ret = 0; + + if (no_wait) { + bool success; + if (WARN_ON(ticket)) + return -EBUSY; + + success = ww_mutex_trylock(&bo->resv->lock); + return success ? 0 : -EBUSY; + } + + if (interruptible) + ret = ww_mutex_lock_interruptible(&bo->resv->lock, ticket); + else + ret = ww_mutex_lock(&bo->resv->lock, ticket); + if (ret == -EINTR) + return -ERESTARTSYS; + return ret; +} /** * ttm_bo_reserve: @@ -780,7 +830,7 @@ extern void ttm_mem_io_unlock(struct ttm_mem_type_manager *man); * @interruptible: Sleep interruptible if waiting. * @no_wait: Don't sleep while trying to reserve, rather return -EBUSY. * @use_ticket: If @bo is already reserved, Only sleep waiting for - * it to become unreserved if @sequence < (@bo)->sequence. + * it to become unreserved if @ticket->stamp is older. * * Locks a buffer object for validation. (Or prevents other processes from * locking it for validation) and removes it from lru lists, while taking @@ -794,7 +844,7 @@ extern void ttm_mem_io_unlock(struct ttm_mem_type_manager *man); * Processes attempting to reserve multiple buffers other than for eviction, * (typically execbuf), should first obtain a unique 32-bit * validation sequence number, - * and call this function with @use_sequence == 1 and @sequence == the unique + * and call this function with @use_ticket == 1 and @ticket->stamp == the unique * sequence number. If upon call of this function, the buffer object is already * reserved, the validation sequence is checked against the validation * sequence of the process currently reserving the buffer, @@ -809,37 +859,31 @@ extern void ttm_mem_io_unlock(struct ttm_mem_type_manager *man); * will eventually succeed, preventing both deadlocks and starvation. * * Returns: - * -EAGAIN: The reservation may cause a deadlock. + * -EDEADLK: The reservation may cause a deadlock. * Release all buffer reservations, wait for @bo to become unreserved and * try again. (only if use_sequence == 1). * -ERESTARTSYS: A wait for the buffer to become unreserved was interrupted by * a signal. Release all buffer reservations and return to user-space. * -EBUSY: The function needed to sleep, but @no_wait was true - * -EDEADLK: Bo already reserved using @sequence. This error code will only - * be returned if @use_sequence is set to true. + * -EALREADY: Bo already reserved using @ticket. This error code will only + * be returned if @use_ticket is set to true. */ -extern int ttm_bo_reserve(struct ttm_buffer_object *bo, - bool interruptible, - bool no_wait, bool use_ticket, - struct ww_acquire_ctx *ticket); +static inline int ttm_bo_reserve(struct ttm_buffer_object *bo, + bool interruptible, + bool no_wait, bool use_ticket, + struct ww_acquire_ctx *ticket) +{ + int ret; -/** - * ttm_bo_reserve_slowpath_nolru: - * @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. - * - * Will not remove reserved buffers from the lru lists. - * Otherwise identical to ttm_bo_reserve_slowpath. - */ -extern int ttm_bo_reserve_slowpath_nolru(struct ttm_buffer_object *bo, - bool interruptible, - struct ww_acquire_ctx *ticket); + WARN_ON(!atomic_read(&bo->kref.refcount)); + + ret = ttm_bo_reserve_nolru(bo, interruptible, no_wait, use_ticket, + ticket); + if (likely(ret == 0)) + ttm_bo_del_sub_from_lru(bo); + return ret; +} /** * ttm_bo_reserve_slowpath: @@ -851,45 +895,27 @@ extern int ttm_bo_reserve_slowpath_nolru(struct ttm_buffer_object *bo, * 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, - struct ww_acquire_ctx *ticket); +static inline int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo, + bool interruptible, + struct ww_acquire_ctx *ticket) +{ + int ret = 0; -/** - * ttm_bo_reserve_nolru: - * - * @bo: A pointer to a struct ttm_buffer_object. - * @interruptible: Sleep interruptible if waiting. - * @no_wait: Don't sleep while trying to reserve, rather return -EBUSY. - * @use_sequence: If @bo is already reserved, Only sleep waiting for - * it to become unreserved if @sequence < (@bo)->sequence. - * - * Will not remove reserved buffers from the lru lists. - * Otherwise identical to ttm_bo_reserve. - * - * Returns: - * -EAGAIN: The reservation may cause a deadlock. - * Release all buffer reservations, wait for @bo to become unreserved and - * try again. (only if use_sequence == 1). - * -ERESTARTSYS: A wait for the buffer to become unreserved was interrupted by - * a signal. Release all buffer reservations and return to user-space. - * -EBUSY: The function needed to sleep, but @no_wait was true - * -EDEADLK: Bo already reserved using @sequence. This error code will only - * be returned if @use_sequence is set to true. - */ -extern int ttm_bo_reserve_nolru(struct ttm_buffer_object *bo, - bool interruptible, - bool no_wait, bool use_ticket, - struct ww_acquire_ctx *ticket); + WARN_ON(!atomic_read(&bo->kref.refcount)); -/** - * ttm_bo_unreserve - * - * @bo: A pointer to a struct ttm_buffer_object. - * - * Unreserve a previous reservation of @bo. - */ -extern void ttm_bo_unreserve(struct ttm_buffer_object *bo); + if (interruptible) + ret = ww_mutex_lock_slow_interruptible(&bo->resv->lock, + ticket); + else + ww_mutex_lock_slow(&bo->resv->lock, ticket); + + if (likely(ret == 0)) + ttm_bo_del_sub_from_lru(bo); + else if (ret == -EINTR) + ret = -ERESTARTSYS; + + return ret; +} /** * ttm_bo_unreserve_ticket @@ -898,19 +924,28 @@ extern void ttm_bo_unreserve(struct ttm_buffer_object *bo); * * Unreserve a previous reservation of @bo made with @ticket. */ -extern void ttm_bo_unreserve_ticket(struct ttm_buffer_object *bo, - struct ww_acquire_ctx *ticket); +static inline void ttm_bo_unreserve_ticket(struct ttm_buffer_object *bo, + struct ww_acquire_ctx *t) +{ + if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) { + spin_lock(&bo->glob->lru_lock); + ttm_bo_add_to_lru(bo); + spin_unlock(&bo->glob->lru_lock); + } + ww_mutex_unlock(&bo->resv->lock); +} /** - * ttm_bo_unreserve_locked + * ttm_bo_unreserve + * * @bo: A pointer to a struct ttm_buffer_object. - * @ticket: ww_acquire_ctx used for reserving, or NULL * - * Unreserve a previous reservation of @bo made with @ticket. - * Needs to be called with struct ttm_bo_global::lru_lock held. + * Unreserve a previous reservation of @bo. */ -extern void ttm_bo_unreserve_ticket_locked(struct ttm_buffer_object *bo, - struct ww_acquire_ctx *ticket); +static inline void ttm_bo_unreserve(struct ttm_buffer_object *bo) +{ + ttm_bo_unreserve_ticket(bo, NULL); +} /* * ttm_bo_util.c -- cgit v1.2.3 From 8f262540e61c7caaff791bde8336196ac7aca77a Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Thu, 27 Jun 2013 13:48:28 +0200 Subject: drm/ttm: get rid of ttm_bo_is_reserved Signed-off-by: Maarten Lankhorst Signed-off-by: Dave Airlie --- include/drm/ttm/ttm_bo_api.h | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'include/drm/ttm') diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 31ad860c10cd..8a6aa56ece52 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -712,18 +712,4 @@ extern ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp, extern void ttm_bo_swapout_all(struct ttm_bo_device *bdev); -/** - * ttm_bo_is_reserved - return an indication if a ttm buffer object is reserved - * - * @bo: The buffer object to check. - * - * This function returns an indication if a bo is reserved or not, and should - * only be used to print an error when it is not from incorrect api usage, since - * there's no guarantee that it is the caller that is holding the reservation. - */ -static inline bool ttm_bo_is_reserved(struct ttm_buffer_object *bo) -{ - return ww_mutex_is_locked(&bo->resv->lock); -} - #endif -- cgit v1.2.3