Re: [PATCH 2/4] drm/ttm: use the parent resv for ghost objects v2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 08.10.19 um 11:25 schrieb Daniel Vetter:
On Thu, Aug 29, 2019 at 04:29:15PM +0200, Christian König wrote:
This way we can even pipeline imported BO evictions.

v2: Limit this to only cases when the parent object uses a separate
     reservation object as well. This fixes another OOM problem.

Signed-off-by: Christian König <christian.koenig@xxxxxxx>
Since I read quite a bit of ttm I figured I'll review this too, but I'm
totally lost. And git blame gives me at best commits with one-liner commit
messages, and the docs aren't explaining much at all either (and generally
they didn't get updated at all with all the changes in the past years).

I have a vague idea of what you're doing here, but not enough to do review
with any confidence. And from other ttm patches from amd it feels a lot
like we have essentially a bus factor of 1 for all things ttm :-/

Yeah, that's one of a couple of reasons why I want to get rid of TTM in the long term.

Basically this is a bug fix for delay freeing ttm objects. When we hang the ttm object on a ghost object to be freed and the ttm object is an imported DMA-buf we run into the problem that we want to drop the mapping, but have the wrong lock taken (the lock of the ghost and not of the parent).

Regards,
Christian.

-Daniel

---
  drivers/gpu/drm/ttm/ttm_bo_util.c | 16 +++++++++-------
  1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index fe81c565e7ef..2ebe9fe7f6c8 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -517,7 +517,9 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
  	kref_init(&fbo->base.kref);
  	fbo->base.destroy = &ttm_transfered_destroy;
  	fbo->base.acc_size = 0;
-	fbo->base.base.resv = &fbo->base.base._resv;
+	if (bo->base.resv == &bo->base._resv)
+		fbo->base.base.resv = &fbo->base.base._resv;
+
  	dma_resv_init(fbo->base.base.resv);
  	ret = dma_resv_trylock(fbo->base.base.resv);
  	WARN_ON(!ret);
@@ -716,7 +718,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
  		if (ret)
  			return ret;
- dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
+		dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
/**
  		 * If we're not moving to fixed memory, the TTM object
@@ -729,7 +731,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
  		else
  			bo->ttm = NULL;
- ttm_bo_unreserve(ghost_obj);
+		dma_resv_unlock(&ghost_obj->base._resv);
  		ttm_bo_put(ghost_obj);
  	}
@@ -772,7 +774,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
  		if (ret)
  			return ret;
- dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
+		dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
/**
  		 * If we're not moving to fixed memory, the TTM object
@@ -785,7 +787,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
  		else
  			bo->ttm = NULL;
- ttm_bo_unreserve(ghost_obj);
+		dma_resv_unlock(&ghost_obj->base._resv);
  		ttm_bo_put(ghost_obj);
} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
@@ -841,7 +843,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
  	if (ret)
  		return ret;
- ret = dma_resv_copy_fences(ghost->base.resv, bo->base.resv);
+	ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv);
  	/* Last resort, wait for the BO to be idle when we are OOM */
  	if (ret)
  		ttm_bo_wait(bo, false, false);
@@ -850,7 +852,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
  	bo->mem.mem_type = TTM_PL_SYSTEM;
  	bo->ttm = NULL;
- ttm_bo_unreserve(ghost);
+	dma_resv_unlock(&ghost->base._resv);
  	ttm_bo_put(ghost);
return 0;
--
2.17.1


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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux