| Message ID | 20200804025632.3868079-6-airlied@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ttm misc cleanups, mem refactoring, rename objects. (v2) | expand |
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; > } > > /**
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; } /**