diff mbox series

[3/4] drm/ttm: improve idle/busy handling

Message ID 20231213144222.1871-3-christian.koenig@amd.com (mailing list archive)
State New, archived
Headers show
Series [1/4] drm/ttm: return ENOSPC from ttm_bo_mem_space | expand

Commit Message

Christian König Dec. 13, 2023, 2:42 p.m. UTC
Previously we would never try to move a BO into the preferred placements when
it ever landed in a busy placement since those were considered compatible.

Rework the whole handling and finally unify the idle and busy handling.
ttm_bo_validate() is now responsible to try idle placement first and then use
the busy placement if that didn't worked.

Drawback is that we now always try the idle placement first for each
validation which might cause some additional CPU overhead on overcommit.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |   2 +-
 drivers/gpu/drm/ttm/ttm_bo.c               | 131 ++++++++-------------
 drivers/gpu/drm/ttm/ttm_resource.c         |  14 ++-
 include/drm/ttm/ttm_bo.h                   |   3 +-
 include/drm/ttm/ttm_resource.h             |   3 +-
 6 files changed, 64 insertions(+), 91 deletions(-)

Comments

kernel test robot Dec. 14, 2023, 8:30 a.m. UTC | #1
Hi Christian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7-rc5 next-20231214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/drm-ttm-replace-busy-placement-with-flags-v3/20231213-224456
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20231213144222.1871-3-christian.koenig%40amd.com
patch subject: [PATCH 3/4] drm/ttm: improve idle/busy handling
config: arm-defconfig (https://download.01.org/0day-ci/archive/20231214/202312141637.ciYxVFVl-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231214/202312141637.ciYxVFVl-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312141637.ciYxVFVl-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/ttm/ttm_resource.c:301: warning: Function parameter or member 'busy' not described in 'ttm_resource_compatible'


vim +301 drivers/gpu/drm/ttm/ttm_resource.c

544432703b2fe7 Arunpravin Paneer Selvam 2022-08-20  289  
544432703b2fe7 Arunpravin Paneer Selvam 2022-08-20  290  /**
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  291   * ttm_resource_compatible - check if resource is compatible with placement
544432703b2fe7 Arunpravin Paneer Selvam 2022-08-20  292   *
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  293   * @res: the resource to check
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  294   * @placement: the placement to check against
544432703b2fe7 Arunpravin Paneer Selvam 2022-08-20  295   *
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  296   * Returns true if the placement is compatible.
544432703b2fe7 Arunpravin Paneer Selvam 2022-08-20  297   */
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  298  bool ttm_resource_compatible(struct ttm_resource *res,
1e59504faf5d28 Christian König          2023-12-13  299  			     struct ttm_placement *placement,
1e59504faf5d28 Christian König          2023-12-13  300  			     bool busy)
98cca519df6da6 Christian König          2021-08-30 @301  {
544432703b2fe7 Arunpravin Paneer Selvam 2022-08-20  302  	struct ttm_buffer_object *bo = res->bo;
544432703b2fe7 Arunpravin Paneer Selvam 2022-08-20  303  	struct ttm_device *bdev = bo->bdev;
98cca519df6da6 Christian König          2021-08-30  304  	unsigned i;
98cca519df6da6 Christian König          2021-08-30  305  
98cca519df6da6 Christian König          2021-08-30  306  	if (res->placement & TTM_PL_FLAG_TEMPORARY)
98cca519df6da6 Christian König          2021-08-30  307  		return false;
98cca519df6da6 Christian König          2021-08-30  308  
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  309  	for (i = 0; i < placement->num_placement; i++) {
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  310  		const struct ttm_place *place = &placement->placement[i];
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  311  		struct ttm_resource_manager *man;
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  312  
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  313  		if (res->mem_type != place->mem_type)
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  314  			continue;
98cca519df6da6 Christian König          2021-08-30  315  
1e59504faf5d28 Christian König          2023-12-13  316  		if (place->flags & (busy ? TTM_PL_FLAG_IDLE : TTM_PL_FLAG_BUSY))
1e59504faf5d28 Christian König          2023-12-13  317  			continue;
1e59504faf5d28 Christian König          2023-12-13  318  
1e59504faf5d28 Christian König          2023-12-13  319  		if (place->flags & TTM_PL_FLAG_CONTIGUOUS &&
1e59504faf5d28 Christian König          2023-12-13  320  		     !(res->placement & TTM_PL_FLAG_CONTIGUOUS))
1e59504faf5d28 Christian König          2023-12-13  321  			continue;
1e59504faf5d28 Christian König          2023-12-13  322  
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  323  		man = ttm_manager_type(bdev, res->mem_type);
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  324  		if (man->func->compatible &&
9909b7ee1d1561 Somalapuram Amaranath    2023-12-13  325  		    !man->func->compatible(man, res, place, bo->base.size))
98cca519df6da6 Christian König          2021-08-30  326  			continue;
98cca519df6da6 Christian König          2021-08-30  327  
98cca519df6da6 Christian König          2021-08-30  328  		return true;
98cca519df6da6 Christian König          2021-08-30  329  	}
98cca519df6da6 Christian König          2021-08-30  330  	return false;
98cca519df6da6 Christian König          2021-08-30  331  }
98cca519df6da6 Christian König          2021-08-30  332
kernel test robot Dec. 16, 2023, 12:44 a.m. UTC | #2
Hi Christian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7-rc5 next-20231215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/drm-ttm-replace-busy-placement-with-flags-v3/20231213-224456
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20231213144222.1871-3-christian.koenig%40amd.com
patch subject: [PATCH 3/4] drm/ttm: improve idle/busy handling
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20231216/202312160809.nvmiBwrJ-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231216/202312160809.nvmiBwrJ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312160809.nvmiBwrJ-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> scripts/kernel-doc: drivers/gpu/drm/ttm/ttm_resource.c:301: warning: Function parameter or struct member 'busy' not described in 'ttm_resource_compatible'
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index aa0dd6dad068..f110dfdc4feb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -404,7 +404,7 @@  int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
 		(*bo_ptr)->placements[i].lpfn = (offset + size) >> PAGE_SHIFT;
 	}
 	r = ttm_bo_mem_space(&(*bo_ptr)->tbo, &(*bo_ptr)->placement,
-			     &(*bo_ptr)->tbo.resource, &ctx);
+			     &(*bo_ptr)->tbo.resource, &ctx, false);
 	if (r)
 		goto error;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 9a6a00b1af40..00da9a81cf6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -967,7 +967,7 @@  int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
 	placements.mem_type = TTM_PL_TT;
 	placements.flags = bo->resource->placement;
 
-	r = ttm_bo_mem_space(bo, &placement, &tmp, &ctx);
+	r = ttm_bo_mem_space(bo, &placement, &tmp, &ctx, true);
 	if (unlikely(r))
 		return r;
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index aa12bd5cfd17..17bfc252f76d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -414,7 +414,7 @@  static int ttm_bo_bounce_temp_buffer(struct ttm_buffer_object *bo,
 	hop_placement.placement = hop;
 
 	/* find space in the bounce domain */
-	ret = ttm_bo_mem_space(bo, &hop_placement, &hop_mem, ctx);
+	ret = ttm_bo_mem_space(bo, &hop_placement, &hop_mem, ctx, true);
 	if (ret)
 		return ret;
 	/* move to the bounce domain */
@@ -454,7 +454,7 @@  static int ttm_bo_evict(struct ttm_buffer_object *bo,
 		return ttm_bo_pipeline_gutting(bo);
 	}
 
-	ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx);
+	ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx, true);
 	if (ret) {
 		if (ret != -ERESTARTSYS) {
 			pr_err("Failed to find memory space for buffer 0x%p eviction\n",
@@ -724,37 +724,6 @@  static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo,
 	return ret;
 }
 
-/*
- * Repeatedly evict memory from the LRU for @mem_type until we create enough
- * space, or we've evicted everything and there isn't enough space.
- */
-static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
-				  const struct ttm_place *place,
-				  struct ttm_resource **mem,
-				  struct ttm_operation_ctx *ctx)
-{
-	struct ttm_device *bdev = bo->bdev;
-	struct ttm_resource_manager *man;
-	struct ww_acquire_ctx *ticket;
-	int ret;
-
-	man = ttm_manager_type(bdev, place->mem_type);
-	ticket = dma_resv_locking_ctx(bo->base.resv);
-	do {
-		ret = ttm_resource_alloc(bo, place, mem);
-		if (likely(!ret))
-			break;
-		if (unlikely(ret != -ENOSPC))
-			return ret;
-		ret = ttm_mem_evict_first(bdev, man, place, ctx,
-					  ticket);
-		if (unlikely(ret != 0))
-			return ret;
-	} while (1);
-
-	return ttm_bo_add_move_fence(bo, man, *mem, ctx->no_wait_gpu);
-}
-
 /**
  * ttm_bo_mem_space
  *
@@ -763,6 +732,7 @@  static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
  * @placement: Proposed new placement for the buffer object.
  * @mem: A struct ttm_resource.
  * @ctx: if and how to sleep, lock buffers and alloc memory
+ * @force_space: If we should evict buffers to force space
  *
  * Allocate memory space for the buffer object pointed to by @bo, using
  * the placement flags in @placement, potentially evicting other idle buffer objects.
@@ -776,12 +746,14 @@  static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
 int ttm_bo_mem_space(struct ttm_buffer_object *bo,
 			struct ttm_placement *placement,
 			struct ttm_resource **mem,
-			struct ttm_operation_ctx *ctx)
+			struct ttm_operation_ctx *ctx,
+			bool force_space)
 {
 	struct ttm_device *bdev = bo->bdev;
-	bool type_found = false;
+	struct ww_acquire_ctx *ticket;
 	int i, ret;
 
+	ticket = dma_resv_locking_ctx(bo->base.resv);
 	ret = dma_resv_reserve_fences(bo->base.resv, 1);
 	if (unlikely(ret))
 		return ret;
@@ -790,19 +762,30 @@  int ttm_bo_mem_space(struct ttm_buffer_object *bo,
 		const struct ttm_place *place = &placement->placement[i];
 		struct ttm_resource_manager *man;
 
-		if (place->flags & TTM_PL_FLAG_BUSY)
-			continue;
-
 		man = ttm_manager_type(bdev, place->mem_type);
 		if (!man || !ttm_resource_manager_used(man))
 			continue;
 
-		type_found = true;
-		ret = ttm_resource_alloc(bo, place, mem);
-		if (ret == -ENOSPC)
+		if (place->flags & (force_space ? TTM_PL_FLAG_IDLE :
+				    TTM_PL_FLAG_BUSY))
+			continue;
+
+		do {
+			ret = ttm_resource_alloc(bo, place, mem);
+			if (unlikely(ret != -ENOSPC))
+				return ret;
+			if (likely(!ret) || !force_space)
+				break;
+
+			ret = ttm_mem_evict_first(bdev, man, place, ctx,
+						  ticket);
+			if (unlikely(ret == -EBUSY))
+				break;
+			if (unlikely(ret))
+				return ret;
+		} while (1);
+		if (ret)
 			continue;
-		if (unlikely(ret))
-			goto error;
 
 		ret = ttm_bo_add_move_fence(bo, man, *mem, ctx->no_wait_gpu);
 		if (unlikely(ret)) {
@@ -810,45 +793,19 @@  int ttm_bo_mem_space(struct ttm_buffer_object *bo,
 			if (ret == -EBUSY)
 				continue;
 
-			goto error;
+			return ret;
 		}
 		return 0;
 	}
 
-	for (i = 0; i < placement->num_placement; ++i) {
-		const struct ttm_place *place = &placement->placement[i];
-		struct ttm_resource_manager *man;
-
-		if (place->flags & TTM_PL_FLAG_IDLE)
-			continue;
-
-		man = ttm_manager_type(bdev, place->mem_type);
-		if (!man || !ttm_resource_manager_used(man))
-			continue;
-
-		type_found = true;
-		ret = ttm_bo_mem_force_space(bo, place, mem, ctx);
-		if (likely(!ret))
-			return 0;
-
-		if (ret && ret != -EBUSY)
-			goto error;
-	}
-
-	ret = -ENOSPC;
-	if (!type_found) {
-		pr_err(TTM_PFX "No compatible memory type found\n");
-		ret = -EINVAL;
-	}
-
-error:
-	return ret;
+	return -ENOSPC;
 }
 EXPORT_SYMBOL(ttm_bo_mem_space);
 
 static int ttm_bo_move_buffer(struct ttm_buffer_object *bo,
 			      struct ttm_placement *placement,
-			      struct ttm_operation_ctx *ctx)
+			      struct ttm_operation_ctx *ctx,
+			      bool force_space)
 {
 	struct ttm_resource *mem;
 	struct ttm_place hop;
@@ -865,7 +822,7 @@  static int ttm_bo_move_buffer(struct ttm_buffer_object *bo,
 	 * stop and the driver will be called to make
 	 * the second hop.
 	 */
-	ret = ttm_bo_mem_space(bo, placement, &mem, ctx);
+	ret = ttm_bo_mem_space(bo, placement, &mem, ctx, force_space);
 	if (ret)
 		return ret;
 bounce:
@@ -902,6 +859,7 @@  int ttm_bo_validate(struct ttm_buffer_object *bo,
 		    struct ttm_placement *placement,
 		    struct ttm_operation_ctx *ctx)
 {
+	bool force_space;
 	int ret;
 
 	dma_resv_assert_held(bo->base.resv);
@@ -912,20 +870,27 @@  int ttm_bo_validate(struct ttm_buffer_object *bo,
 	if (!placement->num_placement)
 		return ttm_bo_pipeline_gutting(bo);
 
-	/* Check whether we need to move buffer. */
-	if (bo->resource && ttm_resource_compatible(bo->resource, placement))
-		return 0;
+	force_space = false;
+	do {
+		/* Check whether we need to move buffer. */
+		if (bo->resource &&
+		    ttm_resource_compatible(bo->resource, placement,
+					    force_space))
+			return 0;
+
+		/* Moving of pinned BOs is forbidden */
+		if (bo->pin_count)
+			return -EINVAL;
 
-	/* Moving of pinned BOs is forbidden */
-	if (bo->pin_count)
-		return -EINVAL;
+		ret = ttm_bo_move_buffer(bo, placement, ctx, force_space);
+		if (ret && ret != -ENOSPC)
+			return ret;
 
-	ret = ttm_bo_move_buffer(bo, placement, ctx);
+		force_space = !force_space;
+	} while (force_space);
 	/* For backward compatibility with userspace */
 	if (ret == -ENOSPC)
 		return -ENOMEM;
-	if (ret)
-		return ret;
 
 	/*
 	 * We might need to add a TTM.
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 3cf1db122980..e49e359c7a43 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -296,7 +296,8 @@  bool ttm_resource_intersects(struct ttm_device *bdev,
  * Returns true if the placement is compatible.
  */
 bool ttm_resource_compatible(struct ttm_resource *res,
-			     struct ttm_placement *placement)
+			     struct ttm_placement *placement,
+			     bool busy)
 {
 	struct ttm_buffer_object *bo = res->bo;
 	struct ttm_device *bdev = bo->bdev;
@@ -312,14 +313,19 @@  bool ttm_resource_compatible(struct ttm_resource *res,
 		if (res->mem_type != place->mem_type)
 			continue;
 
+		if (place->flags & (busy ? TTM_PL_FLAG_IDLE : TTM_PL_FLAG_BUSY))
+			continue;
+
+		if (place->flags & TTM_PL_FLAG_CONTIGUOUS &&
+		     !(res->placement & TTM_PL_FLAG_CONTIGUOUS))
+			continue;
+
 		man = ttm_manager_type(bdev, res->mem_type);
 		if (man->func->compatible &&
 		    !man->func->compatible(man, res, place, bo->base.size))
 			continue;
 
-		if ((!(place->flags & TTM_PL_FLAG_CONTIGUOUS) ||
-		     (res->placement & TTM_PL_FLAG_CONTIGUOUS)))
-			return true;
+		return true;
 	}
 	return false;
 }
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 0223a41a64b2..64d738f152d1 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -397,7 +397,8 @@  vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot);
 int ttm_bo_mem_space(struct ttm_buffer_object *bo,
 		     struct ttm_placement *placement,
 		     struct ttm_resource **mem,
-		     struct ttm_operation_ctx *ctx);
+		     struct ttm_operation_ctx *ctx,
+		     bool force_space);
 
 void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo);
 /*
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 1afa13f0c22b..514858c49848 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -366,7 +366,8 @@  bool ttm_resource_intersects(struct ttm_device *bdev,
 			     const struct ttm_place *place,
 			     size_t size);
 bool ttm_resource_compatible(struct ttm_resource *res,
-			     struct ttm_placement *placement);
+			     struct ttm_placement *placement,
+			     bool busy);
 void ttm_resource_set_bo(struct ttm_resource *res,
 			 struct ttm_buffer_object *bo);