Am 23.02.2018 um 10:46 schrieb He, Roger:

-----Original Message-----
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Christian K?nig
Sent: Tuesday, February 20, 2018 8:58 PM
To: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org
Subject: [PATCH 3/4] drm/ttm: handle already locked BOs during eviction and 
swapout.

This solves the problem that when we swapout a BO from a domain we sometimes 
couldn't make room for it because holding the lock blocks all other BOs with 
this reservation object.

Signed-off-by: Christian König <christian.koe...@amd.com>
---
  drivers/gpu/drm/ttm/ttm_bo.c | 33 ++++++++++++++++-----------------
  1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 
d90b1cf10b27..3a44c2ee4155 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -713,31 +713,30 @@ bool ttm_bo_eviction_valuable(struct ttm_buffer_object 
*bo,  EXPORT_SYMBOL(ttm_bo_eviction_valuable);
/**
- * Check the target bo is allowable to be evicted or swapout, including cases:
- *
- * a. if share same reservation object with ctx->resv, have assumption
- * reservation objects should already be locked, so not lock again and
- * return true directly when either the opreation allow_reserved_eviction
- * or the target bo already is in delayed free list;
- *
- * b. Otherwise, trylock it.
+ * Check if the target bo is allowed to be evicted or swapedout.
   */
  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 ret = false;
+       /* First check if we can lock it */
+       *locked = reservation_object_trylock(bo->resv);
+       if (*locked)
+               return true;
- *locked = false;
+       /* Check if it's locked because it is part of the current operation */
        if (bo->resv == ctx->resv) {
                reservation_object_assert_held(bo->resv);
-               if (ctx->allow_reserved_eviction || !list_empty(&bo->ddestroy))
-                       ret = true;
-       } else {
-               *locked = reservation_object_trylock(bo->resv);
-               ret = *locked;
+               return ctx->allow_reserved_eviction ||
+                       !list_empty(&bo->ddestroy);
        }
- return ret;
+       /* Check if it's locked because it was already evicted */
+       if (ww_mutex_is_owned_by(&bo->resv->lock, NULL))
+               return true;

For the special case: when command submission with Per-VM-BO enabled,
All BOs  a/b/c are always valid BO. After the validation of BOs a and b,
when validation of BO c, is it possible to return true and then evict BO a and 
b by mistake ?
Because a/b/c share same task_struct.

No, that's why I check the context as well. BOs explicitly reserved have a non NULL context while BOs trylocked for swapout have a NULL context.

Christian.


Thanks
Roger(Hongbo.He)

+       /* Some other thread is using it, don't touch it */
+       return false;
  }
static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
--
2.14.1

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

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

Reply via email to