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(-)
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 5a37f1cc057e..bfa9d146d3d4 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -597,29 +597,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;
@@ -641,59 +642,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. @@ -708,20 +656,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; }
/**
Am 28.07.20 um 21:11 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
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);
if (ret == -EALREADY && dups) { struct ttm_validate_buffer *safe = entry; entry = list_prev_entry(entry, head);ret = ttm_bo_reserve(bo, intr, (ticket == NULL), ticket);
@@ -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);
The extra {} could be removed here as well, apart from that the patch is Reviewed-by: Christian König christian.koenig@amd.com.
Regards, Christian.
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 5a37f1cc057e..bfa9d146d3d4 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -597,29 +597,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,
{ int ret = 0;struct ww_acquire_ctx *ticket)
@@ -641,59 +642,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.
@@ -708,20 +656,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; }
/**
dri-devel@lists.freedesktop.org