[05/59] drm/ttm/amdgpu: consolidate ttm reserve paths
diff mbox series

Message ID 20200804025632.3868079-6-airlied@gmail.com
State New
Headers show
Series
  • ttm misc cleanups, mem refactoring, rename objects. (v2)
Related show

Commit Message

Dave Airlie Aug. 4, 2020, 2:55 a.m. UTC
From: Dave Airlie <airlied@redhat.com>

Drop the WARN_ON and consolidate the two paths into one.

Use the consolidate slowpath in the execbuf utils code.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
 drivers/gpu/drm/ttm/ttm_execbuf_util.c     | 12 +--
 include/drm/ttm/ttm_bo_driver.h            | 91 ++++------------------
 3 files changed, 20 insertions(+), 85 deletions(-)

Comments

Christian König Aug. 4, 2020, 10:33 a.m. UTC | #1
Am 04.08.20 um 04:55 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Drop the WARN_ON and consolidate the two paths into one.
>
> Use the consolidate slowpath in the execbuf utils code.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
>   drivers/gpu/drm/ttm/ttm_execbuf_util.c     | 12 +--
>   include/drm/ttm/ttm_bo_driver.h            | 91 ++++------------------
>   3 files changed, 20 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index afa5189dba7d..e01e8903741e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -160,7 +160,7 @@ static inline int amdgpu_bo_reserve(struct amdgpu_bo *bo, bool no_intr)
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>   	int r;
>   
> -	r = __ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL);
> +	r = ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL);
>   	if (unlikely(r != 0)) {
>   		if (r != -ERESTARTSYS)
>   			dev_err(adev->dev, "%p reserve failed\n", bo);
> diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> index 1797f04c0534..8a8f1a6a83a6 100644
> --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> @@ -93,7 +93,7 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
>   	list_for_each_entry(entry, list, head) {
>   		struct ttm_buffer_object *bo = entry->bo;
>   
> -		ret = __ttm_bo_reserve(bo, intr, (ticket == NULL), ticket);
> +		ret = ttm_bo_reserve(bo, intr, (ticket == NULL), ticket);
>   		if (ret == -EALREADY && dups) {
>   			struct ttm_validate_buffer *safe = entry;
>   			entry = list_prev_entry(entry, head);
> @@ -119,13 +119,7 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
>   		ttm_eu_backoff_reservation_reverse(list, entry);
>   
>   		if (ret == -EDEADLK) {
> -			if (intr) {
> -				ret = dma_resv_lock_slow_interruptible(bo->base.resv,
> -										 ticket);
> -			} else {
> -				dma_resv_lock_slow(bo->base.resv, ticket);
> -				ret = 0;
> -			}
> +			ret = ttm_bo_reserve_slowpath(bo, intr, ticket);
>   		}
>   
>   		if (!ret && entry->num_shared)
> @@ -133,8 +127,6 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
>   								entry->num_shared);
>   
>   		if (unlikely(ret != 0)) {
> -			if (ret == -EINTR)
> -				ret = -ERESTARTSYS;
>   			if (ticket) {
>   				ww_acquire_done(ticket);
>   				ww_acquire_fini(ticket);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 09211ecbf84f..c20fef4da1d3 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -588,29 +588,30 @@ int ttm_mem_io_lock(struct ttm_mem_type_manager *man, bool interruptible);
>   void ttm_mem_io_unlock(struct ttm_mem_type_manager *man);
>   
>   /**
> - * __ttm_bo_reserve:
> + * ttm_bo_reserve:
>    *
>    * @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.
>    * @ticket: ticket used to acquire the ww_mutex.
>    *
> - * Will not remove reserved buffers from the lru lists.
> - * Otherwise identical to ttm_bo_reserve.
> + * Locks a buffer object for validation. (Or prevents other processes from
> + * locking it for validation), while taking a number of measures to prevent
> + * deadlocks.
>    *
>    * 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).
> + * try again.
>    * -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(struct ttm_buffer_object *bo,
> -				   bool interruptible, bool no_wait,
> -				   struct ww_acquire_ctx *ticket)
> +static inline int ttm_bo_reserve(struct ttm_buffer_object *bo,
> +				 bool interruptible, bool no_wait,
> +				 struct ww_acquire_ctx *ticket)
>   {
>   	int ret = 0;
>   
> @@ -632,59 +633,6 @@ static inline int __ttm_bo_reserve(struct ttm_buffer_object *bo,
>   	return ret;
>   }
>   
> -/**
> - * ttm_bo_reserve:
> - *
> - * @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.
> - * @ticket: ticket used to acquire the ww_mutex.
> - *
> - * Locks a buffer object for validation. (Or prevents other processes from
> - * locking it for validation) and removes it from lru lists, while taking
> - * a number of measures to prevent deadlocks.
> - *
> - * Deadlocks may occur when two processes try to reserve multiple buffers in
> - * different order, either by will or as a result of a buffer being evicted
> - * to make room for a buffer already reserved. (Buffers are reserved before
> - * they are evicted). The following algorithm prevents such deadlocks from
> - * occurring:
> - * 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_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,
> - * and if the current validation sequence is greater than that of the process
> - * holding the reservation, the function returns -EDEADLK. Otherwise it sleeps
> - * waiting for the buffer to become unreserved, after which it retries
> - * reserving.
> - * The caller should, when receiving an -EDEADLK error
> - * release all its buffer reservations, wait for @bo to become unreserved, and
> - * then rerun the validation with the same validation sequence. This procedure
> - * will always guarantee that the process with the lowest validation sequence
> - * will eventually succeed, preventing both deadlocks and starvation.
> - *
> - * 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(struct ttm_buffer_object *bo,
> -				 bool interruptible, bool no_wait,
> -				 struct ww_acquire_ctx *ticket)
> -{
> -	WARN_ON(!kref_read(&bo->kref));
> -
> -	return __ttm_bo_reserve(bo, interruptible, no_wait, ticket);
> -}
> -
>   /**
>    * ttm_bo_reserve_slowpath:
>    * @bo: A pointer to a struct ttm_buffer_object.
> @@ -699,20 +647,15 @@ static inline int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo,
>   					  bool interruptible,
>   					  struct ww_acquire_ctx *ticket)
>   {
> -	int ret = 0;
> -
> -	WARN_ON(!kref_read(&bo->kref));
> -
> -	if (interruptible)
> -		ret = dma_resv_lock_slow_interruptible(bo->base.resv,
> -								 ticket);
> -	else
> -		dma_resv_lock_slow(bo->base.resv, ticket);
> -
> -	if (ret == -EINTR)
> -		ret = -ERESTARTSYS;
> -
> -	return ret;
> +	if (interruptible) {
> +		int ret = dma_resv_lock_slow_interruptible(bo->base.resv,
> +							   ticket);
> +		if (ret == -EINTR)
> +			ret = -ERESTARTSYS;
> +		return ret;
> +	}
> +	dma_resv_lock_slow(bo->base.resv, ticket);
> +	return 0;
>   }
>   
>   /**

Patch
diff mbox series

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index afa5189dba7d..e01e8903741e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -160,7 +160,7 @@  static inline int amdgpu_bo_reserve(struct amdgpu_bo *bo, bool no_intr)
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
 	int r;
 
-	r = __ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL);
+	r = ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL);
 	if (unlikely(r != 0)) {
 		if (r != -ERESTARTSYS)
 			dev_err(adev->dev, "%p reserve failed\n", bo);
diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
index 1797f04c0534..8a8f1a6a83a6 100644
--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -93,7 +93,7 @@  int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
 	list_for_each_entry(entry, list, head) {
 		struct ttm_buffer_object *bo = entry->bo;
 
-		ret = __ttm_bo_reserve(bo, intr, (ticket == NULL), ticket);
+		ret = ttm_bo_reserve(bo, intr, (ticket == NULL), ticket);
 		if (ret == -EALREADY && dups) {
 			struct ttm_validate_buffer *safe = entry;
 			entry = list_prev_entry(entry, head);
@@ -119,13 +119,7 @@  int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
 		ttm_eu_backoff_reservation_reverse(list, entry);
 
 		if (ret == -EDEADLK) {
-			if (intr) {
-				ret = dma_resv_lock_slow_interruptible(bo->base.resv,
-										 ticket);
-			} else {
-				dma_resv_lock_slow(bo->base.resv, ticket);
-				ret = 0;
-			}
+			ret = ttm_bo_reserve_slowpath(bo, intr, ticket);
 		}
 
 		if (!ret && entry->num_shared)
@@ -133,8 +127,6 @@  int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
 								entry->num_shared);
 
 		if (unlikely(ret != 0)) {
-			if (ret == -EINTR)
-				ret = -ERESTARTSYS;
 			if (ticket) {
 				ww_acquire_done(ticket);
 				ww_acquire_fini(ticket);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 09211ecbf84f..c20fef4da1d3 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -588,29 +588,30 @@  int ttm_mem_io_lock(struct ttm_mem_type_manager *man, bool interruptible);
 void ttm_mem_io_unlock(struct ttm_mem_type_manager *man);
 
 /**
- * __ttm_bo_reserve:
+ * ttm_bo_reserve:
  *
  * @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.
  * @ticket: ticket used to acquire the ww_mutex.
  *
- * Will not remove reserved buffers from the lru lists.
- * Otherwise identical to ttm_bo_reserve.
+ * Locks a buffer object for validation. (Or prevents other processes from
+ * locking it for validation), while taking a number of measures to prevent
+ * deadlocks.
  *
  * 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).
+ * try again.
  * -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(struct ttm_buffer_object *bo,
-				   bool interruptible, bool no_wait,
-				   struct ww_acquire_ctx *ticket)
+static inline int ttm_bo_reserve(struct ttm_buffer_object *bo,
+				 bool interruptible, bool no_wait,
+				 struct ww_acquire_ctx *ticket)
 {
 	int ret = 0;
 
@@ -632,59 +633,6 @@  static inline int __ttm_bo_reserve(struct ttm_buffer_object *bo,
 	return ret;
 }
 
-/**
- * ttm_bo_reserve:
- *
- * @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.
- * @ticket: ticket used to acquire the ww_mutex.
- *
- * Locks a buffer object for validation. (Or prevents other processes from
- * locking it for validation) and removes it from lru lists, while taking
- * a number of measures to prevent deadlocks.
- *
- * Deadlocks may occur when two processes try to reserve multiple buffers in
- * different order, either by will or as a result of a buffer being evicted
- * to make room for a buffer already reserved. (Buffers are reserved before
- * they are evicted). The following algorithm prevents such deadlocks from
- * occurring:
- * 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_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,
- * and if the current validation sequence is greater than that of the process
- * holding the reservation, the function returns -EDEADLK. Otherwise it sleeps
- * waiting for the buffer to become unreserved, after which it retries
- * reserving.
- * The caller should, when receiving an -EDEADLK error
- * release all its buffer reservations, wait for @bo to become unreserved, and
- * then rerun the validation with the same validation sequence. This procedure
- * will always guarantee that the process with the lowest validation sequence
- * will eventually succeed, preventing both deadlocks and starvation.
- *
- * 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(struct ttm_buffer_object *bo,
-				 bool interruptible, bool no_wait,
-				 struct ww_acquire_ctx *ticket)
-{
-	WARN_ON(!kref_read(&bo->kref));
-
-	return __ttm_bo_reserve(bo, interruptible, no_wait, ticket);
-}
-
 /**
  * ttm_bo_reserve_slowpath:
  * @bo: A pointer to a struct ttm_buffer_object.
@@ -699,20 +647,15 @@  static inline int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo,
 					  bool interruptible,
 					  struct ww_acquire_ctx *ticket)
 {
-	int ret = 0;
-
-	WARN_ON(!kref_read(&bo->kref));
-
-	if (interruptible)
-		ret = dma_resv_lock_slow_interruptible(bo->base.resv,
-								 ticket);
-	else
-		dma_resv_lock_slow(bo->base.resv, ticket);
-
-	if (ret == -EINTR)
-		ret = -ERESTARTSYS;
-
-	return ret;
+	if (interruptible) {
+		int ret = dma_resv_lock_slow_interruptible(bo->base.resv,
+							   ticket);
+		if (ret == -EINTR)
+			ret = -ERESTARTSYS;
+		return ret;
+	}
+	dma_resv_lock_slow(bo->base.resv, ticket);
+	return 0;
 }
 
 /**