Re: [PATCH WW 09/13] drm/ttm: inline ttm_bo_reserve and related calls

On Thu, Jun 27, 2013 at 01:48:24PM +0200, Maarten Lankhorst wrote:

Makes lockdep a lot more useful.

Signed-off-by: Maarten Lankhorst maarten.lankhorst@canonical.com

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 ++++++++++++++++++++-------------

The function movement in the header makes the diff a bit hard to read. So if possible I think that should be avoided. Anyway I think I've spotted a few kerneldoc updates (like s/EAGAIN/EDEADLK/) which should be part of the main conversion patch. -Daniel

3 files changed, 117 insertions(+), 172 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 5f9fe80..a8a27f5 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 7392da5..6c91178 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 ec18c5f..984fc2d 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -33,6 +33,7 @@ #include <ttm/ttm_bo_api.h> #include <ttm/ttm_memory.h> #include <ttm/ttm_module.h> +#include <ttm/ttm_placement.h> #include <drm/drm_mm.h> #include <drm/drm_global.h> #include <linux/workqueue.h> @@ -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

-- 1.8.3.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel