Yeah, that shouldn't be a problem. We just need to make sure we don't
busy wait for the BOs to become available.
Christian.
Am 24.05.19 um 07:35 schrieb Liang, Prike:
Use Abaqus torturing the amdgpu driver more times will running into locking first busy BO deadlock .Then the caller will
return EAGAIN and eventually dm_plane_helper_prepare_fb popups out pinned failed message .For this case, the patch#7
can we add EAGAIN as ERESTARTSYS which filter out the annoying error message .
Thanks,
Prike
-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
Sent: Thursday, May 23, 2019 7:04 PM
To: Zhou, David(ChunMing) <David1.Zhou@xxxxxxx>; Olsak, Marek <Marek.Olsak@xxxxxxx>; Zhou, David(ChunMing) <David1.Zhou@xxxxxxx>; Liang, Prike <Prike.Liang@xxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 06/10] drm/ttm: fix busy memory to fail other user v10
[CAUTION: External Email]
Am 23.05.19 um 12:24 schrieb zhoucm1:
On 2019年05月22日 20:59, Christian König wrote:
[CAUTION: External Email]
BOs on the LRU might be blocked during command submission and cause
OOM situations.
Avoid this by blocking for the first busy BO not locked by the same
ticket as the BO we are searching space for.
v10: completely start over with the patch since we didn't
handled a whole bunch of corner cases.
Signed-off-by: Christian König <christian.koenig@xxxxxxx>
---
drivers/gpu/drm/ttm/ttm_bo.c | 77 ++++++++++++++++++++++++++++++------
1 file changed, 66 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
b/drivers/gpu/drm/ttm/ttm_bo.c index 4c6389d849ed..861facac33d4
100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -771,32 +771,72 @@ EXPORT_SYMBOL(ttm_bo_eviction_valuable);
* b. Otherwise, trylock it.
*/
static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object
*bo,
- struct ttm_operation_ctx *ctx, bool *locked)
+ struct ttm_operation_ctx *ctx, bool *locked,
bool *busy)
{
bool ret = false;
- *locked = false;
if (bo->resv == ctx->resv) {
reservation_object_assert_held(bo->resv);
if (ctx->flags & TTM_OPT_FLAG_ALLOW_RES_EVICT
|| !list_empty(&bo->ddestroy))
ret = true;
+ *locked = false;
+ if (busy)
+ *busy = false;
} else {
- *locked = reservation_object_trylock(bo->resv);
- ret = *locked;
+ ret = reservation_object_trylock(bo->resv);
+ *locked = ret;
+ if (busy)
+ *busy = !ret;
}
return ret;
}
+/**
+ * ttm_mem_evict_wait_busy - wait for a busy BO to become available
+ *
+ * @busy_bo: BO which couldn't be locked with trylock
+ * @ctx: operation context
+ * @ticket: acquire ticket
+ *
+ * Try to lock a busy buffer object to avoid failing eviction.
+ */
+static int ttm_mem_evict_wait_busy(struct ttm_buffer_object *busy_bo,
+ struct ttm_operation_ctx *ctx,
+ struct ww_acquire_ctx *ticket) {
+ int r;
+
+ if (!busy_bo || !ticket)
+ return -EBUSY;
+
+ if (ctx->interruptible)
+ r =
+ reservation_object_lock_interruptible(busy_bo->resv,
+ ticket);
+ else
+ r = reservation_object_lock(busy_bo->resv, ticket);
+
+ /*
+ * TODO: It would be better to keep the BO locked until
allocation is at
+ * least tried one more time, but that would mean a much
larger rework
+ * of TTM.
+ */
+ if (!r)
+ reservation_object_unlock(busy_bo->resv);
+
+ return r == -EDEADLK ? -EAGAIN : r; }
+
static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
uint32_t mem_type,
const struct ttm_place *place,
- struct ttm_operation_ctx *ctx)
+ struct ttm_operation_ctx *ctx,
+ struct ww_acquire_ctx *ticket)
{
+ struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
struct ttm_bo_global *glob = bdev->glob;
struct ttm_mem_type_manager *man = &bdev->man[mem_type];
- struct ttm_buffer_object *bo = NULL;
bool locked = false;
unsigned i;
int ret;
@@ -804,8 +844,15 @@ static int ttm_mem_evict_first(struct
ttm_bo_device *bdev,
spin_lock(&glob->lru_lock);
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
list_for_each_entry(bo, &man->lru[i], lru) {
- if (!ttm_bo_evict_swapout_allowable(bo, ctx,
&locked))
+ bool busy;
+
+ if (!ttm_bo_evict_swapout_allowable(bo, ctx,
&locked,
+ &busy)) {
+ if (busy && !busy_bo &&
+ bo->resv->lock.ctx != ticket)
+ busy_bo = bo;
continue;
+ }
if (place &&
!bdev->driver->eviction_valuable(bo,
place)) {
@@ -824,8 +871,13 @@ static int ttm_mem_evict_first(struct
ttm_bo_device *bdev,
}
if (!bo) {
+ if (busy_bo)
+ ttm_bo_get(busy_bo);
spin_unlock(&glob->lru_lock);
- return -EBUSY;
+ ret = ttm_mem_evict_wait_busy(busy_bo, ctx, ticket);
If you rely on EAGAIN, why do you still try to lock busy_bo? any
negative effect if directly return EAGAIN without tring lock?
Yeah, that would burn a lot of CPU cycles because we would essentially busy wait for the BO to become unlocked.
When we only return in case of a deadlock the other thread can continue with its eviction while we reacquire all looks during EAGAIN handling.
Even directly unlocking the BO as I do here is a bit questionable. But I couldn't get the original logic with finding a new BO to evict to work correctly, that's why I have the TODO comment in the function itself as well.
Christian.
-David
+ if (busy_bo)
+ ttm_bo_put(busy_bo);
+ return ret;
}
kref_get(&bo->list_kref);
@@ -911,7 +963,8 @@ static int ttm_bo_mem_force_space(struct
ttm_buffer_object *bo,
return ret;
if (mem->mm_node)
break;
- ret = ttm_mem_evict_first(bdev, mem->mem_type, place,
ctx);
+ ret = ttm_mem_evict_first(bdev, mem->mem_type, place,
ctx,
+ bo->resv->lock.ctx);
if (unlikely(ret != 0))
return ret;
} while (1);
@@ -1426,7 +1479,8 @@ static int ttm_bo_force_list_clean(struct
ttm_bo_device *bdev,
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
while (!list_empty(&man->lru[i])) {
spin_unlock(&glob->lru_lock);
- ret = ttm_mem_evict_first(bdev, mem_type,
NULL, &ctx);
+ ret = ttm_mem_evict_first(bdev, mem_type,
NULL, &ctx,
+ NULL);
if (ret)
return ret;
spin_lock(&glob->lru_lock); @@ -1797,7
+1851,8 @@ int ttm_bo_swapout(struct ttm_bo_global *glob, struct
ttm_operation_ctx *ctx)
spin_lock(&glob->lru_lock);
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
list_for_each_entry(bo, &glob->swap_lru[i], swap) {
- if (ttm_bo_evict_swapout_allowable(bo, ctx,
&locked)) {
+ if (ttm_bo_evict_swapout_allowable(bo, ctx,
&locked,
+ NULL)) {
ret = 0;
break;
}
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
