* [PATCH 01/12] drm/ttm: add ttm_resource_fini
@ 2021-08-30 8:56 Christian König
2021-08-30 8:56 ` [PATCH 02/12] drm/ttm: add back a reference to the bdev to the res manager Christian König
` (10 more replies)
0 siblings, 11 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 8:56 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
Make sure we call the common cleanup function in all
implementations of the resource manager.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 2 ++
drivers/gpu/drm/nouveau/nouveau_mem.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_mem.h | 3 ++-
drivers/gpu/drm/nouveau/nouveau_ttm.c | 9 +++++----
drivers/gpu/drm/ttm/ttm_range_manager.c | 2 ++
drivers/gpu/drm/ttm/ttm_resource.c | 6 ++++++
drivers/gpu/drm/ttm/ttm_sys_manager.c | 1 +
drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 2 ++
drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 3 ++-
include/drm/ttm/ttm_resource.h | 3 +++
12 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 543000304a1c..77cfb64dd312 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -169,6 +169,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
return 0;
err_free:
+ ttm_resource_fini(man, &node->base.base);
kfree(node);
err_out:
@@ -200,6 +201,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
if (!(res->placement & TTM_PL_FLAG_TEMPORARY))
atomic64_sub(res->num_pages, &mgr->used);
+ ttm_resource_fini(man, res);
kfree(node);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
index d02c8637f909..ffddec08e931 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
@@ -95,6 +95,7 @@ static void amdgpu_preempt_mgr_del(struct ttm_resource_manager *man,
struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man);
atomic64_sub(res->num_pages, &mgr->used);
+ ttm_resource_fini(man, res);
kfree(res);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 2fd77c36a1ff..0184210744a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -476,6 +476,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
while (i--)
drm_mm_remove_node(&node->mm_nodes[i]);
spin_unlock(&mgr->lock);
+ ttm_resource_fini(man, &node->base);
kvfree(node);
error_sub:
@@ -515,6 +516,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
atomic64_sub(usage, &mgr->usage);
atomic64_sub(vis_usage, &mgr->vis_usage);
+ ttm_resource_fini(man, res);
kvfree(node);
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_mem.c b/drivers/gpu/drm/nouveau/nouveau_mem.c
index 0de6549fb875..5b610906bec8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_mem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_mem.c
@@ -175,11 +175,12 @@ nouveau_mem_vram(struct ttm_resource *reg, bool contig, u8 page)
}
void
-nouveau_mem_del(struct ttm_resource *reg)
+nouveau_mem_del(struct ttm_resource_manager *man, struct ttm_resource *reg)
{
struct nouveau_mem *mem = nouveau_mem(reg);
nouveau_mem_fini(mem);
+ ttm_resource_fini(man, reg);
kfree(mem);
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_mem.h b/drivers/gpu/drm/nouveau/nouveau_mem.h
index 2c01166a90f2..325551eba5cd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_mem.h
+++ b/drivers/gpu/drm/nouveau/nouveau_mem.h
@@ -23,7 +23,8 @@ nouveau_mem(struct ttm_resource *reg)
int nouveau_mem_new(struct nouveau_cli *, u8 kind, u8 comp,
struct ttm_resource **);
-void nouveau_mem_del(struct ttm_resource *);
+void nouveau_mem_del(struct ttm_resource_manager *man,
+ struct ttm_resource *);
int nouveau_mem_vram(struct ttm_resource *, bool contig, u8 page);
int nouveau_mem_host(struct ttm_resource *, struct ttm_tt *);
void nouveau_mem_fini(struct nouveau_mem *);
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index f4c2e46b6fe1..34421174ed59 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -36,9 +36,10 @@
#include <core/tegra.h>
static void
-nouveau_manager_del(struct ttm_resource_manager *man, struct ttm_resource *reg)
+nouveau_manager_del(struct ttm_resource_manager *man,
+ struct ttm_resource *reg)
{
- nouveau_mem_del(reg);
+ nouveau_mem_del(man, reg);
}
static int
@@ -62,7 +63,7 @@ nouveau_vram_manager_new(struct ttm_resource_manager *man,
ret = nouveau_mem_vram(*res, nvbo->contig, nvbo->page);
if (ret) {
- nouveau_mem_del(*res);
+ nouveau_mem_del(man, *res);
return ret;
}
@@ -118,7 +119,7 @@ nv04_gart_manager_new(struct ttm_resource_manager *man,
ret = nvif_vmm_get(&mem->cli->vmm.vmm, PTES, false, 12, 0,
(long)(*res)->num_pages << PAGE_SHIFT, &mem->vma[0]);
if (ret) {
- nouveau_mem_del(*res);
+ nouveau_mem_del(man, *res);
return ret;
}
diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c b/drivers/gpu/drm/ttm/ttm_range_manager.c
index f4b08a8705b3..69962b5769c5 100644
--- a/drivers/gpu/drm/ttm/ttm_range_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
@@ -89,6 +89,7 @@ static int ttm_range_man_alloc(struct ttm_resource_manager *man,
spin_unlock(&rman->lock);
if (unlikely(ret)) {
+ ttm_resource_fini(man, *res);
kfree(node);
return ret;
}
@@ -108,6 +109,7 @@ static void ttm_range_man_free(struct ttm_resource_manager *man,
drm_mm_remove_node(&node->mm_nodes[0]);
spin_unlock(&rman->lock);
+ ttm_resource_fini(man, res);
kfree(node);
}
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 2431717376e7..2e0db43ff99c 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -44,6 +44,12 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
}
EXPORT_SYMBOL(ttm_resource_init);
+void ttm_resource_fini(struct ttm_resource_manager *man,
+ struct ttm_resource *res)
+{
+}
+EXPORT_SYMBOL(ttm_resource_fini);
+
int ttm_resource_alloc(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource **res_ptr)
diff --git a/drivers/gpu/drm/ttm/ttm_sys_manager.c b/drivers/gpu/drm/ttm/ttm_sys_manager.c
index 63aca52f75e1..135394dcca95 100644
--- a/drivers/gpu/drm/ttm/ttm_sys_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_sys_manager.c
@@ -23,6 +23,7 @@ static int ttm_sys_man_alloc(struct ttm_resource_manager *man,
static void ttm_sys_man_free(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
+ ttm_resource_fini(man, res);
kfree(res);
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
index b2c4af331c9d..bfd686bb8d19 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
@@ -116,6 +116,7 @@ static int vmw_gmrid_man_get_node(struct ttm_resource_manager *man,
gman->used_gmr_pages -= (*res)->num_pages;
spin_unlock(&gman->lock);
ida_free(&gman->gmr_ida, id);
+ ttm_resource_fini(man, *res);
kfree(*res);
return -ENOSPC;
}
@@ -129,6 +130,7 @@ static void vmw_gmrid_man_put_node(struct ttm_resource_manager *man,
spin_lock(&gman->lock);
gman->used_gmr_pages -= res->num_pages;
spin_unlock(&gman->lock);
+ ttm_resource_fini(man, res);
kfree(res);
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
index 2a3d3468e4e0..4fcbd94ccc11 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
@@ -104,6 +104,7 @@ static int vmw_thp_get_node(struct ttm_resource_manager *man,
spin_unlock(&rman->lock);
if (unlikely(ret)) {
+ ttm_resource_fini(man, &node->base);
kfree(node);
} else {
node->base.start = node->mm_nodes[0].start;
@@ -122,7 +123,7 @@ static void vmw_thp_put_node(struct ttm_resource_manager *man,
spin_lock(&rman->lock);
drm_mm_remove_node(&node->mm_nodes[0]);
spin_unlock(&rman->lock);
-
+ ttm_resource_fini(man, res);
kfree(node);
}
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 140b6b9a8bbe..dd6929f0c4f6 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -262,6 +262,9 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man)
void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource *res);
+void ttm_resource_fini(struct ttm_resource_manager *man,
+ struct ttm_resource *res);
+
int ttm_resource_alloc(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource **res);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 02/12] drm/ttm: add back a reference to the bdev to the res manager
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
@ 2021-08-30 8:56 ` Christian König
2021-08-30 8:56 ` [PATCH 03/12] drm/ttm: add a weak BO reference to the resource v3 Christian König
` (9 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 8:56 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
It is simply a lot cleaner to have this around instead of adding
the device throughout the call chain.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_ttm.c | 4 ++--
drivers/gpu/drm/ttm/ttm_range_manager.c | 2 +-
drivers/gpu/drm/ttm/ttm_resource.c | 3 +++
drivers/gpu/drm/ttm/ttm_sys_manager.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 2 +-
include/drm/ttm/ttm_resource.h | 8 ++++----
10 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 77cfb64dd312..1dae68caa974 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -293,7 +293,8 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size)
man->use_tt = true;
man->func = &amdgpu_gtt_mgr_func;
- ttm_resource_manager_init(man, gtt_size >> PAGE_SHIFT);
+ ttm_resource_manager_init(man, &adev->mman.bdev,
+ gtt_size >> PAGE_SHIFT);
start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
index ffddec08e931..6f7189d32f0a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
@@ -153,7 +153,7 @@ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev)
man->use_tt = true;
man->func = &amdgpu_preempt_mgr_func;
- ttm_resource_manager_init(man, (1 << 30));
+ ttm_resource_manager_init(man, &adev->mman.bdev, (1 << 30));
atomic64_set(&mgr->used, 0);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 0184210744a7..c1e97c865c2b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -699,7 +699,8 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev)
struct amdgpu_vram_mgr *mgr = &adev->mman.vram_mgr;
struct ttm_resource_manager *man = &mgr->manager;
- ttm_resource_manager_init(man, adev->gmc.real_vram_size >> PAGE_SHIFT);
+ ttm_resource_manager_init(man, &adev->mman.bdev,
+ adev->gmc.real_vram_size >> PAGE_SHIFT);
man->func = &amdgpu_vram_mgr_func;
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 34421174ed59..6adee64b2332 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -164,7 +164,7 @@ nouveau_ttm_init_vram(struct nouveau_drm *drm)
man->func = &nouveau_vram_manager;
- ttm_resource_manager_init(man,
+ ttm_resource_manager_init(man, &drm->ttm.bdev,
drm->gem.vram_available >> PAGE_SHIFT);
ttm_set_driver_manager(&drm->ttm.bdev, TTM_PL_VRAM, man);
ttm_resource_manager_set_used(man, true);
@@ -211,7 +211,7 @@ nouveau_ttm_init_gtt(struct nouveau_drm *drm)
man->func = func;
man->use_tt = true;
- ttm_resource_manager_init(man, size_pages);
+ ttm_resource_manager_init(man, &drm->ttm.bdev, size_pages);
ttm_set_driver_manager(&drm->ttm.bdev, TTM_PL_TT, man);
ttm_resource_manager_set_used(man, true);
return 0;
diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c b/drivers/gpu/drm/ttm/ttm_range_manager.c
index 69962b5769c5..5c62cbb2a205 100644
--- a/drivers/gpu/drm/ttm/ttm_range_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
@@ -156,7 +156,7 @@ int ttm_range_man_init(struct ttm_device *bdev,
man->func = &ttm_range_manager_func;
- ttm_resource_manager_init(man, p_size);
+ ttm_resource_manager_init(man, bdev, p_size);
drm_mm_init(&rman->mm, 0, p_size);
spin_lock_init(&rman->lock);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 2e0db43ff99c..122f19e6968b 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -77,16 +77,19 @@ EXPORT_SYMBOL(ttm_resource_free);
* ttm_resource_manager_init
*
* @man: memory manager object to init
+ * @bdev: ttm device this manager belongs to
* @p_size: size managed area in pages.
*
* Initialise core parts of a manager object.
*/
void ttm_resource_manager_init(struct ttm_resource_manager *man,
+ struct ttm_device *bdev,
unsigned long p_size)
{
unsigned i;
spin_lock_init(&man->move_lock);
+ man->bdev = bdev;
man->size = p_size;
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
diff --git a/drivers/gpu/drm/ttm/ttm_sys_manager.c b/drivers/gpu/drm/ttm/ttm_sys_manager.c
index 135394dcca95..2ced169513cb 100644
--- a/drivers/gpu/drm/ttm/ttm_sys_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_sys_manager.c
@@ -43,7 +43,7 @@ void ttm_sys_man_init(struct ttm_device *bdev)
man->use_tt = true;
man->func = &ttm_sys_manager_func;
- ttm_resource_manager_init(man, 0);
+ ttm_resource_manager_init(man, bdev, 0);
ttm_set_driver_manager(bdev, TTM_PL_SYSTEM, man);
ttm_resource_manager_set_used(man, true);
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
index bfd686bb8d19..4fe4eeb95bf3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
@@ -150,7 +150,7 @@ int vmw_gmrid_man_init(struct vmw_private *dev_priv, int type)
man->func = &vmw_gmrid_manager_func;
/* TODO: This is most likely not correct */
man->use_tt = true;
- ttm_resource_manager_init(man, 0);
+ ttm_resource_manager_init(man, &dev_priv->bdev, 0);
spin_lock_init(&gman->lock);
gman->used_gmr_pages = 0;
ida_init(&gman->gmr_ida);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
index 4fcbd94ccc11..b8cd89cd624c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
@@ -135,7 +135,7 @@ int vmw_thp_init(struct vmw_private *dev_priv)
if (!rman)
return -ENOMEM;
- ttm_resource_manager_init(&rman->manager,
+ ttm_resource_manager_init(&rman->manager, &dev_priv->bdev,
dev_priv->vram_size >> PAGE_SHIFT);
rman->manager.func = &vmw_thp_func;
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index dd6929f0c4f6..c004672789b6 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -102,11 +102,9 @@ struct ttm_resource_manager_func {
* struct ttm_resource_manager
*
* @use_type: The memory type is enabled.
- * @flags: TTM_MEMTYPE_XX flags identifying the traits of the memory
- * managed by this memory type.
- * @gpu_offset: If used, the GPU offset of the first managed page of
- * fixed memory or the first managed location in an aperture.
+ * @use_tt: allocate TTM object for this resources
* @size: Size of the managed region.
+ * @bdev: ttm device this manager belongs to
* @func: structure pointer implementing the range manager. See above
* @move_lock: lock for move fence
* static information. bdev::driver::io_mem_free is never used.
@@ -121,6 +119,7 @@ struct ttm_resource_manager {
*/
bool use_type;
bool use_tt;
+ struct ttm_device *bdev;
uint64_t size;
const struct ttm_resource_manager_func *func;
spinlock_t move_lock;
@@ -271,6 +270,7 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res);
void ttm_resource_manager_init(struct ttm_resource_manager *man,
+ struct ttm_device *bdev,
unsigned long p_size);
int ttm_resource_manager_evict_all(struct ttm_device *bdev,
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 03/12] drm/ttm: add a weak BO reference to the resource v3
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
2021-08-30 8:56 ` [PATCH 02/12] drm/ttm: add back a reference to the bdev to the res manager Christian König
@ 2021-08-30 8:56 ` Christian König
2021-08-30 8:56 ` [PATCH 04/12] drm/ttm: add common accounting to the resource mgr Christian König
` (8 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 8:56 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
Keep track for which BO a resource was allocated.
This is necessary to move the LRU handling into the resources.
A bit problematic is i915 since it tries to use the resource
interface without a BO which is illegal from the conceptional
point of view.
v2: Document that this is a weak reference and add a workaround for i915
v3: further document that this is protected by ttm_device::lru_lock and
clarify the i915 workaround
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/i915/intel_region_ttm.c | 22 ++++++++++++----------
drivers/gpu/drm/ttm/ttm_bo_util.c | 7 +++++--
drivers/gpu/drm/ttm/ttm_resource.c | 9 +++++++++
include/drm/ttm/ttm_resource.h | 4 ++++
4 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c b/drivers/gpu/drm/i915/intel_region_ttm.c
index 27fe0668d094..1b2a61b085f7 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -75,24 +75,26 @@ intel_region_ttm_node_reserve(struct intel_memory_region *mem,
int ret;
/*
- * Having to use a mock_bo is unfortunate but stems from some
- * drivers having private managers that insist to know what the
- * allocate memory is intended for, using it to send private
- * data to the manager. Also recently the bo has been used to send
- * alignment info to the manager. Assume that apart from the latter,
- * none of the managers we use will ever access the buffer object
- * members, hoping we can pass the alignment info in the
- * struct ttm_place in the future.
+ * This is essential an illegal use the of the TTM resource manager
+ * backend and should be fixed in the future. For now work around by
+ * using a mock_bo with at least the mandatory fields initialized.
+ *
+ * The resource should be ignored by TTM since it can't grab a
+ * reference to the BO during eviction.
*/
place.fpfn = offset >> PAGE_SHIFT;
place.lpfn = place.fpfn + (size >> PAGE_SHIFT);
mock_bo.base.size = size;
+ mock_bo.bdev = &mem->i915->bdev;
ret = man->func->alloc(man, &mock_bo, &place, &res);
if (ret == -ENOSPC)
- ret = -ENXIO;
+ return ERR_PTR(-ENXIO);
+ if (ret)
+ return ERR_PTR(ret);
- return ret ? ERR_PTR(ret) : res;
+ res->bo = NULL;
+ return 0;
}
/**
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 763fa6f4e07d..c5d02edaefc0 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -242,6 +242,11 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
if (bo->type != ttm_bo_type_sg)
fbo->base.base.resv = &fbo->base.base._resv;
+ if (fbo->base.resource) {
+ ttm_resource_set_bo(fbo->base.resource, &fbo->base);
+ bo->resource = NULL;
+ }
+
dma_resv_init(&fbo->base.base._resv);
fbo->base.base.dev = NULL;
ret = dma_resv_trylock(&fbo->base.base._resv);
@@ -510,7 +515,6 @@ static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo,
ghost_obj->ttm = NULL;
else
bo->ttm = NULL;
- bo->resource = NULL;
dma_resv_unlock(&ghost_obj->base._resv);
ttm_bo_put(ghost_obj);
@@ -638,7 +642,6 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
dma_resv_unlock(&ghost->base._resv);
ttm_bo_put(ghost);
bo->ttm = ttm;
- bo->resource = NULL;
ttm_bo_assign_mem(bo, sys_res);
return 0;
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 122f19e6968b..a4c495da0040 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -41,6 +41,7 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
res->bus.offset = 0;
res->bus.is_iomem = false;
res->bus.caching = ttm_cached;
+ res->bo = bo;
}
EXPORT_SYMBOL(ttm_resource_init);
@@ -73,6 +74,14 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res)
}
EXPORT_SYMBOL(ttm_resource_free);
+void ttm_resource_set_bo(struct ttm_resource *res,
+ struct ttm_buffer_object *bo)
+{
+ spin_lock(&bo->bdev->lru_lock);
+ res->bo = bo;
+ spin_unlock(&bo->bdev->lru_lock);
+}
+
/**
* ttm_resource_manager_init
*
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index c004672789b6..e8080192cae4 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -160,6 +160,7 @@ struct ttm_bus_placement {
* @mem_type: Resource type of the allocation.
* @placement: Placement flags.
* @bus: Placement on io bus accessible to the CPU
+ * @bo: weak reference to the BO, protected by ttm_device::lru_lock
*
* Structure indicating the placement and space resources used by a
* buffer object.
@@ -170,6 +171,7 @@ struct ttm_resource {
uint32_t mem_type;
uint32_t placement;
struct ttm_bus_placement bus;
+ struct ttm_buffer_object *bo;
};
/**
@@ -268,6 +270,8 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource **res);
void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res);
+void ttm_resource_set_bo(struct ttm_resource *res,
+ struct ttm_buffer_object *bo);
void ttm_resource_manager_init(struct ttm_resource_manager *man,
struct ttm_device *bdev,
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 04/12] drm/ttm: add common accounting to the resource mgr
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
2021-08-30 8:56 ` [PATCH 02/12] drm/ttm: add back a reference to the bdev to the res manager Christian König
2021-08-30 8:56 ` [PATCH 03/12] drm/ttm: add a weak BO reference to the resource v3 Christian König
@ 2021-08-30 8:56 ` Christian König
2021-08-31 12:52 ` Daniel Vetter
2021-08-30 8:57 ` [PATCH 05/12] drm/ttm: move the LRU into resource handling Christian König
` (7 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Christian König @ 2021-08-30 8:56 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
It makes sense to have this in the common manager for debugging and
accounting of how much resources are used.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/ttm/ttm_resource.c | 8 ++++++++
include/drm/ttm/ttm_resource.h | 18 ++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index a4c495da0040..426e6841fc89 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -33,6 +33,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource *res)
{
+ struct ttm_resource_manager *man;
+
res->start = 0;
res->num_pages = PFN_UP(bo->base.size);
res->mem_type = place->mem_type;
@@ -42,12 +44,16 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
res->bus.is_iomem = false;
res->bus.caching = ttm_cached;
res->bo = bo;
+
+ man = ttm_manager_type(bo->bdev, place->mem_type);
+ atomic64_add(bo->base.size, &man->usage);
}
EXPORT_SYMBOL(ttm_resource_init);
void ttm_resource_fini(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
+ atomic64_sub(res->bo->base.size, &man->usage);
}
EXPORT_SYMBOL(ttm_resource_fini);
@@ -100,6 +106,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
spin_lock_init(&man->move_lock);
man->bdev = bdev;
man->size = p_size;
+ atomic64_set(&man->usage, 0);
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
INIT_LIST_HEAD(&man->lru[i]);
@@ -172,6 +179,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
drm_printf(p, " use_type: %d\n", man->use_type);
drm_printf(p, " use_tt: %d\n", man->use_tt);
drm_printf(p, " size: %llu\n", man->size);
+ drm_printf(p, " usage: %llu\n", atomic64_read(&man->usage));
if (man->func->debug)
man->func->debug(man, p);
}
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index e8080192cae4..526fe359c603 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -27,6 +27,7 @@
#include <linux/types.h>
#include <linux/mutex.h>
+#include <linux/atomic.h>
#include <linux/dma-buf-map.h>
#include <linux/dma-fence.h>
#include <drm/drm_print.h>
@@ -110,6 +111,7 @@ struct ttm_resource_manager_func {
* static information. bdev::driver::io_mem_free is never used.
* @lru: The lru list for this memory type.
* @move: The fence of the last pipelined move operation.
+ * @usage: How much of the region is used.
*
* This structure is used to identify and manage memory types for a device.
*/
@@ -134,6 +136,9 @@ struct ttm_resource_manager {
* Protected by @move_lock.
*/
struct dma_fence *move;
+
+ /* Own protection */
+ atomic64_t usage;
};
/**
@@ -260,6 +265,19 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man)
man->move = NULL;
}
+/**
+ * ttm_resource_manager_usage
+ *
+ * @man: A memory manager object.
+ *
+ * Return how many resources are currently used.
+ */
+static inline uint64_t
+ttm_resource_manager_usage(struct ttm_resource_manager *man)
+{
+ return atomic64_read(&man->usage);
+}
+
void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource *res);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 05/12] drm/ttm: move the LRU into resource handling
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
` (2 preceding siblings ...)
2021-08-30 8:56 ` [PATCH 04/12] drm/ttm: add common accounting to the resource mgr Christian König
@ 2021-08-30 8:57 ` Christian König
2021-08-30 16:53 ` Andrey Grodzovsky
2021-08-30 8:57 ` [PATCH 06/12] drm/ttm: add resource iterator Christian König
` (6 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Christian König @ 2021-08-30 8:57 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
This way we finally fix the problem that new resource are
not immediately evict-able after allocation.
That has caused numerous problems including OOM on GDS handling
and not being able to use TTM as general resource manager.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 105 ++------------------
drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
drivers/gpu/drm/ttm/ttm_device.c | 8 +-
drivers/gpu/drm/ttm/ttm_resource.c | 127 ++++++++++++++++++++++++
include/drm/ttm/ttm_bo_api.h | 16 ---
include/drm/ttm/ttm_bo_driver.h | 29 +-----
include/drm/ttm/ttm_resource.h | 35 +++++++
9 files changed, 181 insertions(+), 150 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 6362e861a3f5..70d2cbb1dbb4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
if (vm->bulk_moveable) {
spin_lock(&adev->mman.bdev.lru_lock);
- ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move);
+ ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
spin_unlock(&adev->mman.bdev.lru_lock);
return;
}
- memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move));
+ ttm_lru_bulk_move_init(&vm->lru_bulk_move);
spin_lock(&adev->mman.bdev.lru_lock);
list_for_each_entry(bo_base, &vm->idle, vm_status) {
@@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
if (!bo->parent)
continue;
- ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource,
- &vm->lru_bulk_move);
+ ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move);
if (shadow)
ttm_bo_move_to_lru_tail(&shadow->tbo,
- shadow->tbo.resource,
&vm->lru_bulk_move);
}
spin_unlock(&adev->mman.bdev.lru_lock);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index e646aac9d7a4..41f0de841d72 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -471,7 +471,7 @@ static void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj)
bo->priority = I915_TTM_PRIO_NO_PAGES;
}
- ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
+ ttm_bo_move_to_lru_tail(bo, NULL);
spin_unlock(&bo->bdev->lru_lock);
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 49f4bc97c35a..d5c6e096fd31 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -69,98 +69,16 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
}
}
-static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
-{
- struct ttm_device *bdev = bo->bdev;
-
- list_del_init(&bo->lru);
-
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(bo);
-}
-
-static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
- struct ttm_buffer_object *bo)
-{
- if (!pos->first)
- pos->first = bo;
- pos->last = bo;
-}
-
void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
- struct ttm_resource *mem,
struct ttm_lru_bulk_move *bulk)
{
- struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man;
-
- if (!bo->deleted)
- dma_resv_assert_held(bo->base.resv);
-
- if (bo->pin_count) {
- ttm_bo_del_from_lru(bo);
- return;
- }
-
- if (!mem)
- return;
-
- man = ttm_manager_type(bdev, mem->mem_type);
- list_move_tail(&bo->lru, &man->lru[bo->priority]);
-
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(bo);
-
- if (bulk && !bo->pin_count) {
- switch (bo->resource->mem_type) {
- case TTM_PL_TT:
- ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo);
- break;
+ dma_resv_assert_held(bo->base.resv);
- case TTM_PL_VRAM:
- ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo);
- break;
- }
- }
+ if (bo->resource)
+ ttm_resource_move_to_lru_tail(bo->resource, bulk);
}
EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
-void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
-{
- unsigned i;
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
- struct ttm_resource_manager *man;
-
- if (!pos->first)
- continue;
-
- dma_resv_assert_held(pos->first->base.resv);
- dma_resv_assert_held(pos->last->base.resv);
-
- man = ttm_manager_type(pos->first->bdev, TTM_PL_TT);
- list_bulk_move_tail(&man->lru[i], &pos->first->lru,
- &pos->last->lru);
- }
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
- struct ttm_resource_manager *man;
-
- if (!pos->first)
- continue;
-
- dma_resv_assert_held(pos->first->base.resv);
- dma_resv_assert_held(pos->last->base.resv);
-
- man = ttm_manager_type(pos->first->bdev, TTM_PL_VRAM);
- list_bulk_move_tail(&man->lru[i], &pos->first->lru,
- &pos->last->lru);
- }
-}
-EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
-
static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
struct ttm_resource *mem, bool evict,
struct ttm_operation_ctx *ctx,
@@ -342,7 +260,6 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
return ret;
}
- ttm_bo_del_from_lru(bo);
list_del_init(&bo->ddestroy);
spin_unlock(&bo->bdev->lru_lock);
ttm_bo_cleanup_memtype_use(bo);
@@ -443,7 +360,7 @@ static void ttm_bo_release(struct kref *kref)
*/
if (bo->pin_count) {
bo->pin_count = 0;
- ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
+ ttm_resource_move_to_lru_tail(bo->resource, NULL);
}
kref_init(&bo->kref);
@@ -456,7 +373,6 @@ static void ttm_bo_release(struct kref *kref)
}
spin_lock(&bo->bdev->lru_lock);
- ttm_bo_del_from_lru(bo);
list_del(&bo->ddestroy);
spin_unlock(&bo->bdev->lru_lock);
@@ -670,15 +586,17 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
struct ww_acquire_ctx *ticket)
{
struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
+ struct ttm_resource *res;
bool locked = false;
unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- list_for_each_entry(bo, &man->lru[i], lru) {
+ list_for_each_entry(res, &man->lru[i], lru) {
bool busy;
+ bo = res->bo;
if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
&locked, &busy)) {
if (busy && !busy_bo && ticket !=
@@ -696,7 +614,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
}
/* If the inner loop terminated early, we have our candidate */
- if (&bo->lru != &man->lru[i])
+ if (&res->lru != &man->lru[i])
break;
bo = NULL;
@@ -870,9 +788,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
}
error:
- if (bo->resource->mem_type == TTM_PL_SYSTEM && !bo->pin_count)
- ttm_bo_move_to_lru_tail_unlocked(bo);
-
return ret;
}
EXPORT_SYMBOL(ttm_bo_mem_space);
@@ -1012,7 +927,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
kref_init(&bo->kref);
- INIT_LIST_HEAD(&bo->lru);
INIT_LIST_HEAD(&bo->ddestroy);
bo->bdev = bdev;
bo->type = type;
@@ -1062,8 +976,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
return ret;
}
- ttm_bo_move_to_lru_tail_unlocked(bo);
-
return ret;
}
EXPORT_SYMBOL(ttm_bo_init_reserved);
@@ -1165,7 +1077,6 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
return 0;
}
- ttm_bo_del_from_lru(bo);
/* TODO: Cleanup the locking */
spin_unlock(&bo->bdev->lru_lock);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index c5d02edaefc0..49b4bedf8715 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -232,7 +232,6 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
atomic_inc(&ttm_glob.bo_count);
INIT_LIST_HEAD(&fbo->base.ddestroy);
- INIT_LIST_HEAD(&fbo->base.lru);
fbo->base.moving = NULL;
drm_vma_node_reset(&fbo->base.base.vma_node);
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 519deea8e39b..9e0dfceff68c 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -134,6 +134,7 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
{
struct ttm_resource_manager *man;
struct ttm_buffer_object *bo;
+ struct ttm_resource *res;
unsigned i, j;
int ret;
@@ -144,8 +145,11 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
continue;
for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
- list_for_each_entry(bo, &man->lru[j], lru) {
- uint32_t num_pages = PFN_UP(bo->base.size);
+ list_for_each_entry(res, &man->lru[j], lru) {
+ uint32_t num_pages;
+
+ bo = res->bo;
+ num_pages = PFN_UP(bo->base.size);
ret = ttm_bo_swapout(bo, ctx, gfp_flags);
/* ttm_bo_swapout has dropped the lru_lock */
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 426e6841fc89..355c542758b5 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -29,6 +29,115 @@
#include <drm/ttm/ttm_resource.h>
#include <drm/ttm/ttm_bo_driver.h>
+/**
+ * ttm_lru_bulk_move_init - initialize a bulk move structure
+ * @bulk: the structure to init
+ *
+ * For now just memset the structure to zero.
+ */
+void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk)
+{
+ memset(bulk, 0, sizeof(*bulk));
+}
+EXPORT_SYMBOL(ttm_lru_bulk_move_init);
+
+/**
+ * ttm_lru_bulk_move_tail
+ *
+ * @bulk: bulk move structure
+ *
+ * Bulk move BOs to the LRU tail, only valid to use when driver makes sure that
+ * resource order never changes. Should be called with ttm_device::lru_lock held.
+ */
+void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
+{
+ unsigned i;
+
+ for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
+ struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
+ struct ttm_resource_manager *man;
+
+ if (!pos->first)
+ continue;
+
+ dma_resv_assert_held(pos->first->bo->base.resv);
+ dma_resv_assert_held(pos->last->bo->base.resv);
+
+ man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_TT);
+ list_bulk_move_tail(&man->lru[i], &pos->first->lru,
+ &pos->last->lru);
+ }
+
+ for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
+ struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
+ struct ttm_resource_manager *man;
+
+ if (!pos->first)
+ continue;
+
+ dma_resv_assert_held(pos->first->bo->base.resv);
+ dma_resv_assert_held(pos->last->bo->base.resv);
+
+ man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_VRAM);
+ list_bulk_move_tail(&man->lru[i], &pos->first->lru,
+ &pos->last->lru);
+ }
+}
+EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
+
+/* Record a resource position in a bulk move structure */
+static void ttm_lru_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
+ struct ttm_resource *res)
+{
+ if (!pos->first)
+ pos->first = res;
+ pos->last = res;
+}
+
+/* Remove a resource from the LRU */
+static void ttm_resource_del_from_lru(struct ttm_resource *res)
+{
+ struct ttm_device *bdev = res->bo->bdev;
+
+ list_del_init(&res->lru);
+
+ if (bdev->funcs->del_from_lru_notify)
+ bdev->funcs->del_from_lru_notify(res->bo);
+}
+
+/* Move a resource to the LRU tail and track the bulk position */
+void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
+ struct ttm_lru_bulk_move *bulk)
+{
+ struct ttm_buffer_object *bo = res->bo;
+ struct ttm_device *bdev = bo->bdev;
+ struct ttm_resource_manager *man;
+
+ if (bo->pin_count) {
+ ttm_resource_del_from_lru(res);
+ return;
+ }
+
+ man = ttm_manager_type(bdev, res->mem_type);
+ list_move_tail(&res->lru, &man->lru[bo->priority]);
+
+ if (bdev->funcs->del_from_lru_notify)
+ bdev->funcs->del_from_lru_notify(bo);
+
+ if (!bulk)
+ return;
+
+ switch (res->mem_type) {
+ case TTM_PL_TT:
+ ttm_lru_bulk_move_set_pos(&bulk->tt[bo->priority], res);
+ break;
+
+ case TTM_PL_VRAM:
+ ttm_lru_bulk_move_set_pos(&bulk->vram[bo->priority], res);
+ break;
+ }
+}
+
void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource *res)
@@ -44,15 +153,33 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
res->bus.is_iomem = false;
res->bus.caching = ttm_cached;
res->bo = bo;
+ INIT_LIST_HEAD(&res->lru);
man = ttm_manager_type(bo->bdev, place->mem_type);
atomic64_add(bo->base.size, &man->usage);
+
+ spin_lock(&bo->bdev->lru_lock);
+ ttm_resource_move_to_lru_tail(res, NULL);
+ spin_unlock(&bo->bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_init);
+/**
+ * ttm_resource_fini
+ *
+ * @res: the resource to clean up
+ *
+ * Make sure the resource is removed from the LRU before destruction.
+ */
void ttm_resource_fini(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
+ struct ttm_device *bdev = res->bo->bdev;
+
+ spin_lock(&bdev->lru_lock);
+ ttm_resource_del_from_lru(res);
+ spin_unlock(&bdev->lru_lock);
+
atomic64_sub(res->bo->base.size, &man->usage);
}
EXPORT_SYMBOL(ttm_resource_fini);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index f681bbdbc698..0928d8cfb45a 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -56,8 +56,6 @@ struct ttm_placement;
struct ttm_place;
-struct ttm_lru_bulk_move;
-
/**
* enum ttm_bo_type
*
@@ -95,7 +93,6 @@ struct ttm_tt;
* @ttm: TTM structure holding system pages.
* @evicted: Whether the object was evicted without user-space knowing.
* @deleted: True if the object is only a zombie and already deleted.
- * @lru: List head for the lru list.
* @ddestroy: List head for the delayed destroy list.
* @swap: List head for swap LRU list.
* @moving: Fence set when BO is moving
@@ -144,7 +141,6 @@ struct ttm_buffer_object {
* Members protected by the bdev::lru_lock.
*/
- struct list_head lru;
struct list_head ddestroy;
/**
@@ -308,7 +304,6 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
* ttm_bo_move_to_lru_tail
*
* @bo: The buffer object.
- * @mem: Resource object.
* @bulk: optional bulk move structure to remember BO positions
*
* Move this BO to the tail of all lru lists used to lookup and reserve an
@@ -316,19 +311,8 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
* held, and is used to make a BO less likely to be considered for eviction.
*/
void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
- struct ttm_resource *mem,
struct ttm_lru_bulk_move *bulk);
-/**
- * ttm_bo_bulk_move_lru_tail
- *
- * @bulk: bulk move structure
- *
- * Bulk move BOs to the LRU tail, only valid to use when driver makes sure that
- * BO order never changes. Should be called with ttm_global::lru_lock held.
- */
-void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk);
-
/**
* ttm_bo_lock_delayed_workqueue
*
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 68d6069572aa..fba2f7d3d34e 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -45,33 +45,6 @@
#include "ttm_tt.h"
#include "ttm_pool.h"
-/**
- * struct ttm_lru_bulk_move_pos
- *
- * @first: first BO in the bulk move range
- * @last: last BO in the bulk move range
- *
- * Positions for a lru bulk move.
- */
-struct ttm_lru_bulk_move_pos {
- struct ttm_buffer_object *first;
- struct ttm_buffer_object *last;
-};
-
-/**
- * struct ttm_lru_bulk_move
- *
- * @tt: first/last lru entry for BOs in the TT domain
- * @vram: first/last lru entry for BOs in the VRAM domain
- * @swap: first/last lru entry for BOs on the swap list
- *
- * Helper structure for bulk moves on the LRU list.
- */
-struct ttm_lru_bulk_move {
- struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
- struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
-};
-
/*
* ttm_bo.c
*/
@@ -182,7 +155,7 @@ static inline void
ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo)
{
spin_lock(&bo->bdev->lru_lock);
- ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
+ ttm_bo_move_to_lru_tail(bo, NULL);
spin_unlock(&bo->bdev->lru_lock);
}
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 526fe359c603..5f9797f9d64a 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -26,10 +26,12 @@
#define _TTM_RESOURCE_H_
#include <linux/types.h>
+#include <linux/list.h>
#include <linux/mutex.h>
#include <linux/atomic.h>
#include <linux/dma-buf-map.h>
#include <linux/dma-fence.h>
+
#include <drm/drm_print.h>
#include <drm/ttm/ttm_caching.h>
#include <drm/ttm/ttm_kmap_iter.h>
@@ -177,6 +179,33 @@ struct ttm_resource {
uint32_t placement;
struct ttm_bus_placement bus;
struct ttm_buffer_object *bo;
+ struct list_head lru;
+};
+
+/**
+ * struct ttm_lru_bulk_move_pos
+ *
+ * @first: first res in the bulk move range
+ * @last: last res in the bulk move range
+ *
+ * Positions for a lru bulk move.
+ */
+struct ttm_lru_bulk_move_pos {
+ struct ttm_resource *first;
+ struct ttm_resource *last;
+};
+
+/**
+ * struct ttm_lru_bulk_move
+ *
+ * @tt: first/last lru entry for resources in the TT domain
+ * @vram: first/last lru entry for resources in the VRAM domain
+ *
+ * Helper structure for bulk moves on the LRU list.
+ */
+struct ttm_lru_bulk_move {
+ struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
+ struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
};
/**
@@ -278,6 +307,12 @@ ttm_resource_manager_usage(struct ttm_resource_manager *man)
return atomic64_read(&man->usage);
}
+void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk);
+void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk);
+
+void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
+ struct ttm_lru_bulk_move *bulk);
+
void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource *res);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 06/12] drm/ttm: add resource iterator
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
` (3 preceding siblings ...)
2021-08-30 8:57 ` [PATCH 05/12] drm/ttm: move the LRU into resource handling Christian König
@ 2021-08-30 8:57 ` Christian König
2021-08-30 8:57 ` [PATCH 07/12] drm/radeon: use ttm_resource_manager_debug Christian König
` (5 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 8:57 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
Instead of duplicating that at different places add an iterator over all
the resources in a resource manager.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/ttm/ttm_bo.c | 41 +++++++++++----------------
drivers/gpu/drm/ttm/ttm_device.c | 34 ++++++++++++----------
drivers/gpu/drm/ttm/ttm_resource.c | 45 ++++++++++++++++++++++++++++++
include/drm/ttm/ttm_resource.h | 23 +++++++++++++++
4 files changed, 103 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d5c6e096fd31..8ca418d4a059 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -586,38 +586,29 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
struct ww_acquire_ctx *ticket)
{
struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
+ struct ttm_resource_cursor cursor;
struct ttm_resource *res;
bool locked = false;
- unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- list_for_each_entry(res, &man->lru[i], lru) {
- bool busy;
-
- bo = res->bo;
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
- &locked, &busy)) {
- if (busy && !busy_bo && ticket !=
- dma_resv_locking_ctx(bo->base.resv))
- busy_bo = bo;
- continue;
- }
-
- if (!ttm_bo_get_unless_zero(bo)) {
- if (locked)
- dma_resv_unlock(bo->base.resv);
- continue;
- }
- break;
+ ttm_resource_manager_for_each_res(man, &cursor, res) {
+ bool busy;
+
+ if (!ttm_bo_evict_swapout_allowable(res->bo, ctx, place,
+ &locked, &busy)) {
+ if (busy && !busy_bo && ticket !=
+ dma_resv_locking_ctx(bo->base.resv))
+ busy_bo = res->bo;
+ continue;
}
- /* If the inner loop terminated early, we have our candidate */
- if (&res->lru != &man->lru[i])
- break;
-
- bo = NULL;
+ if (!ttm_bo_get_unless_zero(res->bo)) {
+ if (locked)
+ dma_resv_unlock(res->bo->base.resv);
+ continue;
+ }
+ bo = res->bo;
}
if (!bo) {
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 9e0dfceff68c..c019d8dcd82c 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -132,10 +132,11 @@ EXPORT_SYMBOL(ttm_global_swapout);
int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
gfp_t gfp_flags)
{
+ struct ttm_resource_cursor cursor;
struct ttm_resource_manager *man;
struct ttm_buffer_object *bo;
struct ttm_resource *res;
- unsigned i, j;
+ unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
@@ -144,20 +145,23 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
if (!man || !man->use_tt)
continue;
- for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
- list_for_each_entry(res, &man->lru[j], lru) {
- uint32_t num_pages;
-
- bo = res->bo;
- num_pages = PFN_UP(bo->base.size);
-
- ret = ttm_bo_swapout(bo, ctx, gfp_flags);
- /* ttm_bo_swapout has dropped the lru_lock */
- if (!ret)
- return num_pages;
- if (ret != -EBUSY)
- return ret;
- }
+ ttm_resource_manager_for_each_res(man, &cursor, res) {
+ uint32_t num_pages;
+
+ bo = res->bo;
+ num_pages = PFN_UP(bo->base.size);
+ if (!bo->ttm ||
+ bo->ttm->page_flags & TTM_PAGE_FLAG_SG ||
+ bo->ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)
+ continue;
+
+ num_pages = bo->ttm->num_pages;
+ ret = ttm_bo_swapout(bo, ctx, gfp_flags);
+ /* ttm_bo_swapout has dropped the lru_lock */
+ if (!ret)
+ return num_pages;
+ if (ret != -EBUSY)
+ return ret;
}
}
spin_unlock(&bdev->lru_lock);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 355c542758b5..6e5678ad3a13 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -312,6 +312,51 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
}
EXPORT_SYMBOL(ttm_resource_manager_debug);
+/**
+ * ttm_resource_manager_first
+ *
+ * @man: resource manager to iterate over
+ * @cursor: cursor to record the position
+ *
+ * Returns the first resource from the resource manager.
+ */
+struct ttm_resource *
+ttm_resource_manager_first(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor)
+{
+ struct ttm_resource *res;
+
+ for (cursor->priority = 0; cursor->priority < TTM_MAX_BO_PRIORITY;
+ ++cursor->priority)
+ list_for_each_entry(res, &man->lru[cursor->priority], lru)
+ return res;
+
+ return NULL;
+}
+
+/**
+ * ttm_resource_manager_next
+ *
+ * @man: resource manager to iterate over
+ * @cursor: cursor to record the position
+ *
+ * Returns the next resource from the resource manager.
+ */
+struct ttm_resource *
+ttm_resource_manager_next(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor,
+ struct ttm_resource *res)
+{
+ list_for_each_entry_continue(res, &man->lru[cursor->priority], lru)
+ return res;
+
+ for (; cursor->priority < TTM_MAX_BO_PRIORITY; ++cursor->priority)
+ list_for_each_entry(res, &man->lru[cursor->priority], lru)
+ return res;
+
+ return NULL;
+}
+
static void ttm_kmap_iter_iomap_map_local(struct ttm_kmap_iter *iter,
struct dma_buf_map *dmap,
pgoff_t i)
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 5f9797f9d64a..fc2da89c9c41 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -182,6 +182,17 @@ struct ttm_resource {
struct list_head lru;
};
+/**
+ * struct ttm_resource_cursor
+ *
+ * @priority: the current priority
+ *
+ * Cursor to iterate over the resources in a manager.
+ */
+struct ttm_resource_cursor {
+ unsigned int priority;
+};
+
/**
* struct ttm_lru_bulk_move_pos
*
@@ -336,6 +347,18 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev,
void ttm_resource_manager_debug(struct ttm_resource_manager *man,
struct drm_printer *p);
+struct ttm_resource *
+ttm_resource_manager_first(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor);
+struct ttm_resource *
+ttm_resource_manager_next(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor,
+ struct ttm_resource *res);
+
+#define ttm_resource_manager_for_each_res(man, cursor, res) \
+ for (res = ttm_resource_manager_first(man, cursor); res; \
+ res = ttm_resource_manager_next(man, cursor, res))
+
struct ttm_kmap_iter *
ttm_kmap_iter_iomap_init(struct ttm_kmap_iter_iomap *iter_io,
struct io_mapping *iomap,
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 07/12] drm/radeon: use ttm_resource_manager_debug
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
` (4 preceding siblings ...)
2021-08-30 8:57 ` [PATCH 06/12] drm/ttm: add resource iterator Christian König
@ 2021-08-30 8:57 ` Christian König
2021-08-30 8:57 ` [PATCH 08/12] drm/radeon: remove resource accounting Christian König
` (4 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 8:57 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
Instead of calling the debug operation directly.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 7793249bc549..9c45d8eb8680 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -802,7 +802,7 @@ static int radeon_mm_vram_dump_table_show(struct seq_file *m, void *unused)
TTM_PL_VRAM);
struct drm_printer p = drm_seq_file_printer(m);
- man->func->debug(man, &p);
+ ttm_resource_manager_debug(man, &p);
return 0;
}
@@ -820,7 +820,7 @@ static int radeon_mm_gtt_dump_table_show(struct seq_file *m, void *unused)
TTM_PL_TT);
struct drm_printer p = drm_seq_file_printer(m);
- man->func->debug(man, &p);
+ ttm_resource_manager_debug(man, &p);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 08/12] drm/radeon: remove resource accounting
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
` (5 preceding siblings ...)
2021-08-30 8:57 ` [PATCH 07/12] drm/radeon: use ttm_resource_manager_debug Christian König
@ 2021-08-30 8:57 ` Christian König
2021-08-30 8:57 ` [PATCH 09/12] drm/amdgpu: use ttm_resource_manager_debug Christian König
` (3 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 8:57 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
Use the one provided by TTM instead.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/radeon/radeon.h | 2 --
drivers/gpu/drm/radeon/radeon_kms.c | 7 ++++--
drivers/gpu/drm/radeon/radeon_object.c | 30 +++-----------------------
drivers/gpu/drm/radeon/radeon_object.h | 1 -
drivers/gpu/drm/radeon/radeon_ttm.c | 18 ++--------------
5 files changed, 10 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 895776c421d4..08f83bf2c330 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2462,8 +2462,6 @@ struct radeon_device {
struct radeon_vm_manager vm_manager;
struct mutex gpu_clock_mutex;
/* memory stats */
- atomic64_t vram_usage;
- atomic64_t gtt_usage;
atomic64_t num_bytes_moved;
atomic_t gpu_reset_counter;
/* ACPI interface */
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 0473583dcdac..e3643379b607 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -241,6 +241,7 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
struct drm_radeon_info *info = data;
struct radeon_mode_info *minfo = &rdev->mode_info;
uint32_t *value, value_tmp, *value_ptr, value_size;
+ struct ttm_resource_manager *man;
uint64_t value64;
struct drm_crtc *crtc;
int i, found;
@@ -550,12 +551,14 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
case RADEON_INFO_VRAM_USAGE:
value = (uint32_t*)&value64;
value_size = sizeof(uint64_t);
- value64 = atomic64_read(&rdev->vram_usage);
+ man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM);
+ value64 = ttm_resource_manager_usage(man);
break;
case RADEON_INFO_GTT_USAGE:
value = (uint32_t*)&value64;
value_size = sizeof(uint64_t);
- value64 = atomic64_read(&rdev->gtt_usage);
+ man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_TT);
+ value64 = ttm_resource_manager_usage(man);
break;
case RADEON_INFO_ACTIVE_CU_COUNT:
if (rdev->family >= CHIP_BONAIRE)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 56ede9d63b12..c9bbed2a25ad 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -49,27 +49,6 @@ static void radeon_bo_clear_surface_reg(struct radeon_bo *bo);
* function are calling it.
*/
-static void radeon_update_memory_usage(struct ttm_buffer_object *bo,
- unsigned int mem_type, int sign)
-{
- struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
-
- switch (mem_type) {
- case TTM_PL_TT:
- if (sign > 0)
- atomic64_add(bo->base.size, &rdev->gtt_usage);
- else
- atomic64_sub(bo->base.size, &rdev->gtt_usage);
- break;
- case TTM_PL_VRAM:
- if (sign > 0)
- atomic64_add(bo->base.size, &rdev->vram_usage);
- else
- atomic64_sub(bo->base.size, &rdev->vram_usage);
- break;
- }
-}
-
static void radeon_ttm_bo_destroy(struct ttm_buffer_object *tbo)
{
struct radeon_bo *bo;
@@ -434,7 +413,9 @@ void radeon_bo_fini(struct radeon_device *rdev)
static u64 radeon_bo_get_threshold_for_moves(struct radeon_device *rdev)
{
u64 real_vram_size = rdev->mc.real_vram_size;
- u64 vram_usage = atomic64_read(&rdev->vram_usage);
+ struct ttm_resource_manager *man =
+ ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM);
+ u64 vram_usage = ttm_resource_manager_usage(man);
/* This function is based on the current VRAM usage.
*
@@ -725,15 +706,10 @@ int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
}
void radeon_bo_move_notify(struct ttm_buffer_object *bo,
- unsigned int old_type,
struct ttm_resource *new_mem)
{
struct radeon_bo *rbo;
- radeon_update_memory_usage(bo, old_type, -1);
- if (new_mem)
- radeon_update_memory_usage(bo, new_mem->mem_type, 1);
-
if (!radeon_ttm_bo_is_radeon_bo(bo))
return;
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 1afc7992ef91..0b64e202577b 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -161,7 +161,6 @@ extern void radeon_bo_get_tiling_flags(struct radeon_bo *bo,
extern int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
bool force_drop);
extern void radeon_bo_move_notify(struct ttm_buffer_object *bo,
- unsigned int old_type,
struct ttm_resource *new_mem);
extern vm_fault_t radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo);
extern int radeon_bo_get_surface_reg(struct radeon_bo *bo);
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 9c45d8eb8680..e8d883c7118b 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -199,7 +199,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
struct ttm_resource *old_mem = bo->resource;
struct radeon_device *rdev;
struct radeon_bo *rbo;
- int r, old_type;
+ int r;
if (new_mem->mem_type == TTM_PL_TT) {
r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem);
@@ -216,9 +216,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
if (WARN_ON_ONCE(rbo->tbo.pin_count > 0))
return -EINVAL;
- /* Save old type for statistics update */
- old_type = old_mem->mem_type;
-
rdev = radeon_get_rdev(bo->bdev);
if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) {
ttm_bo_move_null(bo, new_mem);
@@ -264,7 +261,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
out:
/* update statistics */
atomic64_add(bo->base.size, &rdev->num_bytes_moved);
- radeon_bo_move_notify(bo, old_type, new_mem);
+ radeon_bo_move_notify(bo, new_mem);
return 0;
}
@@ -679,16 +676,6 @@ bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev,
return !!(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
}
-static void
-radeon_bo_delete_mem_notify(struct ttm_buffer_object *bo)
-{
- unsigned int old_type = TTM_PL_SYSTEM;
-
- if (bo->resource)
- old_type = bo->resource->mem_type;
- radeon_bo_move_notify(bo, old_type, NULL);
-}
-
static struct ttm_device_funcs radeon_bo_driver = {
.ttm_tt_create = &radeon_ttm_tt_create,
.ttm_tt_populate = &radeon_ttm_tt_populate,
@@ -697,7 +684,6 @@ static struct ttm_device_funcs radeon_bo_driver = {
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = &radeon_evict_flags,
.move = &radeon_bo_move,
- .delete_mem_notify = &radeon_bo_delete_mem_notify,
.io_mem_reserve = &radeon_ttm_io_mem_reserve,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 09/12] drm/amdgpu: use ttm_resource_manager_debug
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
` (6 preceding siblings ...)
2021-08-30 8:57 ` [PATCH 08/12] drm/radeon: remove resource accounting Christian König
@ 2021-08-30 8:57 ` Christian König
2021-08-30 8:57 ` [PATCH 10/12] drm/amdgpu: remove GTT accounting Christian König
` (2 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 8:57 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
Instead of calling the debug operation directly.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 489e22190e29..fdf4c46d2073 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2045,7 +2045,7 @@ static int amdgpu_mm_vram_table_show(struct seq_file *m, void *unused)
TTM_PL_VRAM);
struct drm_printer p = drm_seq_file_printer(m);
- man->func->debug(man, &p);
+ ttm_resource_manager_debug(man, &p);
return 0;
}
@@ -2063,7 +2063,7 @@ static int amdgpu_mm_tt_table_show(struct seq_file *m, void *unused)
TTM_PL_TT);
struct drm_printer p = drm_seq_file_printer(m);
- man->func->debug(man, &p);
+ ttm_resource_manager_debug(man, &p);
return 0;
}
@@ -2074,7 +2074,7 @@ static int amdgpu_mm_gds_table_show(struct seq_file *m, void *unused)
AMDGPU_PL_GDS);
struct drm_printer p = drm_seq_file_printer(m);
- man->func->debug(man, &p);
+ ttm_resource_manager_debug(man, &p);
return 0;
}
@@ -2085,7 +2085,7 @@ static int amdgpu_mm_gws_table_show(struct seq_file *m, void *unused)
AMDGPU_PL_GWS);
struct drm_printer p = drm_seq_file_printer(m);
- man->func->debug(man, &p);
+ ttm_resource_manager_debug(man, &p);
return 0;
}
@@ -2096,7 +2096,7 @@ static int amdgpu_mm_oa_table_show(struct seq_file *m, void *unused)
AMDGPU_PL_OA);
struct drm_printer p = drm_seq_file_printer(m);
- man->func->debug(man, &p);
+ ttm_resource_manager_debug(man, &p);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 10/12] drm/amdgpu: remove GTT accounting
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
` (7 preceding siblings ...)
2021-08-30 8:57 ` [PATCH 09/12] drm/amdgpu: use ttm_resource_manager_debug Christian König
@ 2021-08-30 8:57 ` Christian König
2021-08-30 8:57 ` [PATCH 11/12] drm/amdgpu: remove VRAM accounting Christian König
2021-08-30 8:57 ` [PATCH 12/12] drm/amdgpu: drop amdgpu_gtt_node Christian König
10 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 8:57 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
This is provided by TTM now.
Also switch man->size to bytes instead of pages and fix the double
printing of size and usage in debugfs.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 50 +++++----------------
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 5 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
3 files changed, 12 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 1dae68caa974..7085494c6a9f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev,
struct ttm_resource_manager *man;
man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
- return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE);
+ return sysfs_emit(buf, "%llu\n", man->size);
}
/**
@@ -80,7 +80,7 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev,
struct ttm_resource_manager *man;
man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
- return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(man));
+ return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
}
static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
@@ -132,20 +132,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
struct amdgpu_gtt_node *node;
int r;
- if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
- atomic64_add_return(num_pages, &mgr->used) > man->size) {
- atomic64_sub(num_pages, &mgr->used);
- return -ENOSPC;
- }
-
node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
- if (!node) {
- r = -ENOMEM;
- goto err_out;
- }
+ if (!node)
+ return -ENOMEM;
node->tbo = tbo;
ttm_resource_init(tbo, place, &node->base.base);
+ if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
+ ttm_resource_manager_usage(man) > man->size) {
+ r = -ENOSPC;
+ goto err_free;
+ }
if (place->lpfn) {
spin_lock(&mgr->lock);
@@ -171,11 +168,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
err_free:
ttm_resource_fini(man, &node->base.base);
kfree(node);
-
-err_out:
- if (!(place->flags & TTM_PL_FLAG_TEMPORARY))
- atomic64_sub(num_pages, &mgr->used);
-
return r;
}
@@ -198,27 +190,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
drm_mm_remove_node(&node->base.mm_nodes[0]);
spin_unlock(&mgr->lock);
- if (!(res->placement & TTM_PL_FLAG_TEMPORARY))
- atomic64_sub(res->num_pages, &mgr->used);
-
ttm_resource_fini(man, res);
kfree(node);
}
-/**
- * amdgpu_gtt_mgr_usage - return usage of GTT domain
- *
- * @man: TTM memory type manager
- *
- * Return how many bytes are used in the GTT domain
- */
-uint64_t amdgpu_gtt_mgr_usage(struct ttm_resource_manager *man)
-{
- struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
-
- return atomic64_read(&mgr->used) * PAGE_SIZE;
-}
-
/**
* amdgpu_gtt_mgr_recover - re-init gart
*
@@ -265,9 +240,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man,
spin_lock(&mgr->lock);
drm_mm_print(&mgr->mm, printer);
spin_unlock(&mgr->lock);
-
- drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n",
- man->size, atomic64_read(&mgr->used));
}
static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {
@@ -293,14 +265,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size)
man->use_tt = true;
man->func = &amdgpu_gtt_mgr_func;
- ttm_resource_manager_init(man, &adev->mman.bdev,
- gtt_size >> PAGE_SHIFT);
+ ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size);
start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
drm_mm_init(&mgr->mm, start, size);
spin_lock_init(&mgr->lock);
- atomic64_set(&mgr->used, 0);
ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_TT, &mgr->manager);
ttm_resource_manager_set_used(man, true);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 20b049ad61c1..bd8e28e9ca7c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -671,7 +671,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
ui64 = amdgpu_vram_mgr_vis_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM));
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_GTT_USAGE:
- ui64 = amdgpu_gtt_mgr_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_TT));
+ ui64 = ttm_resource_manager_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_TT));
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_GDS_CONFIG: {
struct drm_amdgpu_info_gds gds_info;
@@ -730,8 +730,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
mem.gtt.total_heap_size *= PAGE_SIZE;
mem.gtt.usable_heap_size = mem.gtt.total_heap_size -
atomic64_read(&adev->gart_pin_size);
- mem.gtt.heap_usage =
- amdgpu_gtt_mgr_usage(gtt_man);
+ mem.gtt.heap_usage = ttm_resource_manager_usage(gtt_man);
mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
return copy_to_user(out, &mem,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 3205fd520060..af0faf9b36f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -52,7 +52,6 @@ struct amdgpu_gtt_mgr {
struct ttm_resource_manager manager;
struct drm_mm mm;
spinlock_t lock;
- atomic64_t used;
};
struct amdgpu_preempt_mgr {
@@ -114,7 +113,6 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev);
void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);
bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem);
-uint64_t amdgpu_gtt_mgr_usage(struct ttm_resource_manager *man);
int amdgpu_gtt_mgr_recover(struct ttm_resource_manager *man);
uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 11/12] drm/amdgpu: remove VRAM accounting
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
` (8 preceding siblings ...)
2021-08-30 8:57 ` [PATCH 10/12] drm/amdgpu: remove GTT accounting Christian König
@ 2021-08-30 8:57 ` Christian König
2021-08-30 8:57 ` [PATCH 12/12] drm/amdgpu: drop amdgpu_gtt_node Christian König
10 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 8:57 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
This is provided by TTM now.
Also switch man->size to bytes instead of pages and fix the double
printing of size and usage in debugfs.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 56 +++++++-------------
7 files changed, 26 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 7b46ba551cb2..c2d5aef2ae48 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -520,9 +520,10 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int dma_buf_fd,
uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev *kgd)
{
struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
- struct ttm_resource_manager *vram_man = ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM);
+ struct ttm_resource_manager *vram_man =
+ ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM);
- return amdgpu_vram_mgr_usage(vram_man);
+ return ttm_resource_manager_usage(vram_man);
}
uint64_t amdgpu_amdkfd_get_hive_id(struct kgd_dev *kgd)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index a152363b0254..c2f17d4df748 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -315,7 +315,7 @@ static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev,
}
total_vram = adev->gmc.real_vram_size - atomic64_read(&adev->vram_pin_size);
- used_vram = amdgpu_vram_mgr_usage(vram_man);
+ used_vram = ttm_resource_manager_usage(vram_man);
free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram;
spin_lock(&adev->mm_stats.lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index bd8e28e9ca7c..9e9d4f4a8743 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -665,7 +665,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
ui64 = atomic64_read(&adev->num_vram_cpu_page_faults);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_VRAM_USAGE:
- ui64 = amdgpu_vram_mgr_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM));
+ ui64 = ttm_resource_manager_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM));
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_VIS_VRAM_USAGE:
ui64 = amdgpu_vram_mgr_vis_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM));
@@ -711,8 +711,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
mem.vram.usable_heap_size = adev->gmc.real_vram_size -
atomic64_read(&adev->vram_pin_size) -
AMDGPU_VM_RESERVED_VRAM;
- mem.vram.heap_usage =
- amdgpu_vram_mgr_usage(vram_man);
+ mem.vram.heap_usage = ttm_resource_manager_usage(vram_man);
mem.vram.max_allocation = mem.vram.usable_heap_size * 3 / 4;
mem.cpu_accessible_vram.total_heap_size =
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index fdf4c46d2073..f3222a396fd1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1872,7 +1872,7 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable)
size = adev->gmc.real_vram_size;
else
size = adev->gmc.visible_vram_size;
- man->size = size >> PAGE_SHIFT;
+ man->size = size;
adev->mman.buffer_funcs_enabled = enable;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index af0faf9b36f0..ef84a1a78fef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -44,7 +44,6 @@ struct amdgpu_vram_mgr {
spinlock_t lock;
struct list_head reservations_pending;
struct list_head reserved_pages;
- atomic64_t usage;
atomic64_t vis_usage;
};
@@ -127,7 +126,6 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
void amdgpu_vram_mgr_free_sgt(struct device *dev,
enum dma_data_direction dir,
struct sg_table *sgt);
-uint64_t amdgpu_vram_mgr_usage(struct ttm_resource_manager *man);
uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_resource_manager *man);
int amdgpu_vram_mgr_reserve_range(struct ttm_resource_manager *man,
uint64_t start, uint64_t size);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 12a7cc2f01cd..1e157e4e99a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -568,7 +568,7 @@ static int amdgpu_virt_write_vf2pf_data(struct amdgpu_device *adev)
vf2pf_info->driver_cert = 0;
vf2pf_info->os_info.all = 0;
- vf2pf_info->fb_usage = amdgpu_vram_mgr_usage(vram_man) >> 20;
+ vf2pf_info->fb_usage = ttm_resource_manager_usage(vram_man) >> 20;
vf2pf_info->fb_vis_usage = amdgpu_vram_mgr_vis_usage(vram_man) >> 20;
vf2pf_info->fb_size = adev->gmc.real_vram_size >> 20;
vf2pf_info->fb_vis_size = adev->gmc.visible_vram_size >> 20;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index c1e97c865c2b..87596c03ced2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -99,7 +99,7 @@ static ssize_t amdgpu_mem_info_vram_used_show(struct device *dev,
struct ttm_resource_manager *man;
man = ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM);
- return sysfs_emit(buf, "%llu\n", amdgpu_vram_mgr_usage(man));
+ return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
}
/**
@@ -255,7 +255,7 @@ static void amdgpu_vram_mgr_do_reserve(struct ttm_resource_manager *man)
vis_usage = amdgpu_vram_mgr_vis_size(adev, &rsv->mm_node);
atomic64_add(vis_usage, &mgr->vis_usage);
- atomic64_add(rsv->mm_node.size << PAGE_SHIFT, &mgr->usage);
+ atomic64_add(rsv->mm_node.size << PAGE_SHIFT, &man->usage);
list_move(&rsv->node, &mgr->reserved_pages);
}
}
@@ -382,19 +382,13 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
lpfn = place->lpfn;
if (!lpfn)
- lpfn = man->size;
+ lpfn = man->size >> PAGE_SHIFT;
max_bytes = adev->gmc.mc_vram_size;
if (tbo->type != ttm_bo_type_kernel)
max_bytes -= AMDGPU_VM_RESERVED_VRAM;
- /* bail out quickly if there's likely not enough VRAM for this BO */
mem_bytes = tbo->base.size;
- if (atomic64_add_return(mem_bytes, &mgr->usage) > max_bytes) {
- r = -ENOSPC;
- goto error_sub;
- }
-
if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
pages_per_node = ~0ul;
num_nodes = 1;
@@ -412,13 +406,17 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
node = kvmalloc(struct_size(node, mm_nodes, num_nodes),
GFP_KERNEL | __GFP_ZERO);
- if (!node) {
- r = -ENOMEM;
- goto error_sub;
- }
+ if (!node)
+ return -ENOMEM;
ttm_resource_init(tbo, place, &node->base);
+ /* bail out quickly if there's likely not enough VRAM for this BO */
+ if (ttm_resource_manager_usage(man) > max_bytes) {
+ r = -ENOSPC;
+ goto error_fini;
+ }
+
mode = DRM_MM_INSERT_BEST;
if (place->flags & TTM_PL_FLAG_TOPDOWN)
mode = DRM_MM_INSERT_HIGH;
@@ -476,11 +474,10 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
while (i--)
drm_mm_remove_node(&node->mm_nodes[i]);
spin_unlock(&mgr->lock);
+error_fini:
ttm_resource_fini(man, &node->base);
kvfree(node);
-error_sub:
- atomic64_sub(mem_bytes, &mgr->usage);
return r;
}
@@ -498,7 +495,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
struct amdgpu_device *adev = to_amdgpu_device(mgr);
- uint64_t usage = 0, vis_usage = 0;
+ uint64_t vis_usage = 0;
unsigned i, pages;
spin_lock(&mgr->lock);
@@ -507,13 +504,11 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
struct drm_mm_node *mm = &node->mm_nodes[i];
drm_mm_remove_node(mm);
- usage += mm->size << PAGE_SHIFT;
vis_usage += amdgpu_vram_mgr_vis_size(adev, mm);
}
amdgpu_vram_mgr_do_reserve(man);
spin_unlock(&mgr->lock);
- atomic64_sub(usage, &mgr->usage);
atomic64_sub(vis_usage, &mgr->vis_usage);
ttm_resource_fini(man, res);
@@ -631,20 +626,6 @@ void amdgpu_vram_mgr_free_sgt(struct device *dev,
kfree(sgt);
}
-/**
- * amdgpu_vram_mgr_usage - how many bytes are used in this domain
- *
- * @man: TTM memory type manager
- *
- * Returns how many bytes are used in this domain.
- */
-uint64_t amdgpu_vram_mgr_usage(struct ttm_resource_manager *man)
-{
- struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
-
- return atomic64_read(&mgr->usage);
-}
-
/**
* amdgpu_vram_mgr_vis_usage - how many bytes are used in the visible part
*
@@ -672,13 +653,12 @@ static void amdgpu_vram_mgr_debug(struct ttm_resource_manager *man,
{
struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
+ drm_printf(printer, " vis usage:%llu\n",
+ amdgpu_vram_mgr_vis_usage(man));
+
spin_lock(&mgr->lock);
drm_mm_print(&mgr->mm, printer);
spin_unlock(&mgr->lock);
-
- drm_printf(printer, "man size:%llu pages, ram usage:%lluMB, vis usage:%lluMB\n",
- man->size, amdgpu_vram_mgr_usage(man) >> 20,
- amdgpu_vram_mgr_vis_usage(man) >> 20);
}
static const struct ttm_resource_manager_func amdgpu_vram_mgr_func = {
@@ -700,11 +680,11 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev)
struct ttm_resource_manager *man = &mgr->manager;
ttm_resource_manager_init(man, &adev->mman.bdev,
- adev->gmc.real_vram_size >> PAGE_SHIFT);
+ adev->gmc.real_vram_size);
man->func = &amdgpu_vram_mgr_func;
- drm_mm_init(&mgr->mm, 0, man->size);
+ drm_mm_init(&mgr->mm, 0, man->size >> PAGE_SHIFT);
spin_lock_init(&mgr->lock);
INIT_LIST_HEAD(&mgr->reservations_pending);
INIT_LIST_HEAD(&mgr->reserved_pages);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 12/12] drm/amdgpu: drop amdgpu_gtt_node
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
` (9 preceding siblings ...)
2021-08-30 8:57 ` [PATCH 11/12] drm/amdgpu: remove VRAM accounting Christian König
@ 2021-08-30 8:57 ` Christian König
10 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 8:57 UTC (permalink / raw)
To: thomas.hellstrom, dri-devel, andrey.grodzovsky
We have the BO pointer in the base structure now as well.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 ++++++++-------------
1 file changed, 18 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 7085494c6a9f..22506e429a62 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -26,23 +26,12 @@
#include "amdgpu.h"
-struct amdgpu_gtt_node {
- struct ttm_buffer_object *tbo;
- struct ttm_range_mgr_node base;
-};
-
static inline struct amdgpu_gtt_mgr *
to_gtt_mgr(struct ttm_resource_manager *man)
{
return container_of(man, struct amdgpu_gtt_mgr, manager);
}
-static inline struct amdgpu_gtt_node *
-to_amdgpu_gtt_node(struct ttm_resource *res)
-{
- return container_of(res, struct amdgpu_gtt_node, base.base);
-}
-
/**
* DOC: mem_info_gtt_total
*
@@ -107,9 +96,9 @@ const struct attribute_group amdgpu_gtt_mgr_attr_group = {
*/
bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *res)
{
- struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res);
+ struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
- return drm_mm_node_allocated(&node->base.mm_nodes[0]);
+ return drm_mm_node_allocated(&node->mm_nodes[0]);
}
/**
@@ -129,15 +118,14 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
{
struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
uint32_t num_pages = PFN_UP(tbo->base.size);
- struct amdgpu_gtt_node *node;
+ struct ttm_range_mgr_node *node;
int r;
- node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
+ node = kzalloc(struct_size(node, mm_nodes, 1), GFP_KERNEL);
if (!node)
return -ENOMEM;
- node->tbo = tbo;
- ttm_resource_init(tbo, place, &node->base.base);
+ ttm_resource_init(tbo, place, &node->base);
if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
ttm_resource_manager_usage(man) > man->size) {
r = -ENOSPC;
@@ -146,8 +134,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
if (place->lpfn) {
spin_lock(&mgr->lock);
- r = drm_mm_insert_node_in_range(&mgr->mm,
- &node->base.mm_nodes[0],
+ r = drm_mm_insert_node_in_range(&mgr->mm, &node->mm_nodes[0],
num_pages, tbo->page_alignment,
0, place->fpfn, place->lpfn,
DRM_MM_INSERT_BEST);
@@ -155,18 +142,18 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
if (unlikely(r))
goto err_free;
- node->base.base.start = node->base.mm_nodes[0].start;
+ node->base.start = node->mm_nodes[0].start;
} else {
- node->base.mm_nodes[0].start = 0;
- node->base.mm_nodes[0].size = node->base.base.num_pages;
- node->base.base.start = AMDGPU_BO_INVALID_OFFSET;
+ node->mm_nodes[0].start = 0;
+ node->mm_nodes[0].size = node->base.num_pages;
+ node->base.start = AMDGPU_BO_INVALID_OFFSET;
}
- *res = &node->base.base;
+ *res = &node->base;
return 0;
err_free:
- ttm_resource_fini(man, &node->base.base);
+ ttm_resource_fini(man, &node->base);
kfree(node);
return r;
}
@@ -182,12 +169,12 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
- struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res);
+ struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
spin_lock(&mgr->lock);
- if (drm_mm_node_allocated(&node->base.mm_nodes[0]))
- drm_mm_remove_node(&node->base.mm_nodes[0]);
+ if (drm_mm_node_allocated(&node->mm_nodes[0]))
+ drm_mm_remove_node(&node->mm_nodes[0]);
spin_unlock(&mgr->lock);
ttm_resource_fini(man, res);
@@ -204,16 +191,16 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
int amdgpu_gtt_mgr_recover(struct ttm_resource_manager *man)
{
struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
+ struct ttm_range_mgr_node *node;
struct amdgpu_device *adev;
- struct amdgpu_gtt_node *node;
struct drm_mm_node *mm_node;
int r = 0;
adev = container_of(mgr, typeof(*adev), mman.gtt_mgr);
spin_lock(&mgr->lock);
drm_mm_for_each_node(mm_node, &mgr->mm) {
- node = container_of(mm_node, typeof(*node), base.mm_nodes[0]);
- r = amdgpu_ttm_recover_gart(node->tbo);
+ node = container_of(mm_node, typeof(*node), mm_nodes[0]);
+ r = amdgpu_ttm_recover_gart(node->base.bo);
if (r)
break;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 05/12] drm/ttm: move the LRU into resource handling
2021-08-30 8:57 ` [PATCH 05/12] drm/ttm: move the LRU into resource handling Christian König
@ 2021-08-30 16:53 ` Andrey Grodzovsky
2021-08-30 16:55 ` Christian König
0 siblings, 1 reply; 19+ messages in thread
From: Andrey Grodzovsky @ 2021-08-30 16:53 UTC (permalink / raw)
To: Christian König, thomas.hellstrom, dri-devel
On 2021-08-30 4:57 a.m., Christian König wrote:
> This way we finally fix the problem that new resource are
> not immediately evict-able after allocation.
>
> That has caused numerous problems including OOM on GDS handling
> and not being able to use TTM as general resource manager.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +-
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
> drivers/gpu/drm/ttm/ttm_bo.c | 105 ++------------------
> drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
> drivers/gpu/drm/ttm/ttm_device.c | 8 +-
> drivers/gpu/drm/ttm/ttm_resource.c | 127 ++++++++++++++++++++++++
> include/drm/ttm/ttm_bo_api.h | 16 ---
> include/drm/ttm/ttm_bo_driver.h | 29 +-----
> include/drm/ttm/ttm_resource.h | 35 +++++++
> 9 files changed, 181 insertions(+), 150 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 6362e861a3f5..70d2cbb1dbb4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
>
> if (vm->bulk_moveable) {
> spin_lock(&adev->mman.bdev.lru_lock);
> - ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move);
> + ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
> spin_unlock(&adev->mman.bdev.lru_lock);
> return;
> }
>
> - memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move));
> + ttm_lru_bulk_move_init(&vm->lru_bulk_move);
>
> spin_lock(&adev->mman.bdev.lru_lock);
> list_for_each_entry(bo_base, &vm->idle, vm_status) {
> @@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
> if (!bo->parent)
> continue;
>
> - ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource,
> - &vm->lru_bulk_move);
> + ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move);
> if (shadow)
> ttm_bo_move_to_lru_tail(&shadow->tbo,
> - shadow->tbo.resource,
> &vm->lru_bulk_move);
> }
> spin_unlock(&adev->mman.bdev.lru_lock);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index e646aac9d7a4..41f0de841d72 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -471,7 +471,7 @@ static void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj)
> bo->priority = I915_TTM_PRIO_NO_PAGES;
> }
>
> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
> + ttm_bo_move_to_lru_tail(bo, NULL);
> spin_unlock(&bo->bdev->lru_lock);
> }
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 49f4bc97c35a..d5c6e096fd31 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -69,98 +69,16 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
> }
> }
>
> -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
> -{
> - struct ttm_device *bdev = bo->bdev;
> -
> - list_del_init(&bo->lru);
> -
> - if (bdev->funcs->del_from_lru_notify)
> - bdev->funcs->del_from_lru_notify(bo);
> -}
> -
> -static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
> - struct ttm_buffer_object *bo)
> -{
> - if (!pos->first)
> - pos->first = bo;
> - pos->last = bo;
> -}
> -
> void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
> - struct ttm_resource *mem,
> struct ttm_lru_bulk_move *bulk)
> {
> - struct ttm_device *bdev = bo->bdev;
> - struct ttm_resource_manager *man;
> -
> - if (!bo->deleted)
> - dma_resv_assert_held(bo->base.resv);
> -
> - if (bo->pin_count) {
> - ttm_bo_del_from_lru(bo);
> - return;
> - }
> -
> - if (!mem)
> - return;
> -
> - man = ttm_manager_type(bdev, mem->mem_type);
> - list_move_tail(&bo->lru, &man->lru[bo->priority]);
> -
> - if (bdev->funcs->del_from_lru_notify)
> - bdev->funcs->del_from_lru_notify(bo);
> -
> - if (bulk && !bo->pin_count) {
> - switch (bo->resource->mem_type) {
> - case TTM_PL_TT:
> - ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo);
> - break;
> + dma_resv_assert_held(bo->base.resv);
>
> - case TTM_PL_VRAM:
> - ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo);
> - break;
> - }
> - }
> + if (bo->resource)
> + ttm_resource_move_to_lru_tail(bo->resource, bulk);
> }
> EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
>
> -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
> -{
> - unsigned i;
> -
> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> - struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
> - struct ttm_resource_manager *man;
> -
> - if (!pos->first)
> - continue;
> -
> - dma_resv_assert_held(pos->first->base.resv);
> - dma_resv_assert_held(pos->last->base.resv);
> -
> - man = ttm_manager_type(pos->first->bdev, TTM_PL_TT);
> - list_bulk_move_tail(&man->lru[i], &pos->first->lru,
> - &pos->last->lru);
> - }
> -
> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> - struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
> - struct ttm_resource_manager *man;
> -
> - if (!pos->first)
> - continue;
> -
> - dma_resv_assert_held(pos->first->base.resv);
> - dma_resv_assert_held(pos->last->base.resv);
> -
> - man = ttm_manager_type(pos->first->bdev, TTM_PL_VRAM);
> - list_bulk_move_tail(&man->lru[i], &pos->first->lru,
> - &pos->last->lru);
> - }
> -}
> -EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
> -
> static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> struct ttm_resource *mem, bool evict,
> struct ttm_operation_ctx *ctx,
> @@ -342,7 +260,6 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
> return ret;
> }
>
> - ttm_bo_del_from_lru(bo);
Here and in other places this was removed, I assume ttm_resource_fini
should replace it but I don't see where exactly this takes place.
Andrey
> list_del_init(&bo->ddestroy);
> spin_unlock(&bo->bdev->lru_lock);
> ttm_bo_cleanup_memtype_use(bo);
> @@ -443,7 +360,7 @@ static void ttm_bo_release(struct kref *kref)
> */
> if (bo->pin_count) {
> bo->pin_count = 0;
> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
> + ttm_resource_move_to_lru_tail(bo->resource, NULL);
> }
>
> kref_init(&bo->kref);
> @@ -456,7 +373,6 @@ static void ttm_bo_release(struct kref *kref)
> }
>
> spin_lock(&bo->bdev->lru_lock);
> - ttm_bo_del_from_lru(bo);
> list_del(&bo->ddestroy);
> spin_unlock(&bo->bdev->lru_lock);
>
> @@ -670,15 +586,17 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
> struct ww_acquire_ctx *ticket)
> {
> struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
> + struct ttm_resource *res;
> bool locked = false;
> unsigned i;
> int ret;
>
> spin_lock(&bdev->lru_lock);
> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> - list_for_each_entry(bo, &man->lru[i], lru) {
> + list_for_each_entry(res, &man->lru[i], lru) {
> bool busy;
>
> + bo = res->bo;
> if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
> &locked, &busy)) {
> if (busy && !busy_bo && ticket !=
> @@ -696,7 +614,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
> }
>
> /* If the inner loop terminated early, we have our candidate */
> - if (&bo->lru != &man->lru[i])
> + if (&res->lru != &man->lru[i])
> break;
>
> bo = NULL;
> @@ -870,9 +788,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
> }
>
> error:
> - if (bo->resource->mem_type == TTM_PL_SYSTEM && !bo->pin_count)
> - ttm_bo_move_to_lru_tail_unlocked(bo);
> -
> return ret;
> }
> EXPORT_SYMBOL(ttm_bo_mem_space);
> @@ -1012,7 +927,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
> bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
>
> kref_init(&bo->kref);
> - INIT_LIST_HEAD(&bo->lru);
> INIT_LIST_HEAD(&bo->ddestroy);
> bo->bdev = bdev;
> bo->type = type;
> @@ -1062,8 +976,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
> return ret;
> }
>
> - ttm_bo_move_to_lru_tail_unlocked(bo);
> -
> return ret;
> }
> EXPORT_SYMBOL(ttm_bo_init_reserved);
> @@ -1165,7 +1077,6 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
> return 0;
> }
>
> - ttm_bo_del_from_lru(bo);
> /* TODO: Cleanup the locking */
> spin_unlock(&bo->bdev->lru_lock);
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index c5d02edaefc0..49b4bedf8715 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -232,7 +232,6 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
>
> atomic_inc(&ttm_glob.bo_count);
> INIT_LIST_HEAD(&fbo->base.ddestroy);
> - INIT_LIST_HEAD(&fbo->base.lru);
> fbo->base.moving = NULL;
> drm_vma_node_reset(&fbo->base.base.vma_node);
>
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index 519deea8e39b..9e0dfceff68c 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -134,6 +134,7 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
> {
> struct ttm_resource_manager *man;
> struct ttm_buffer_object *bo;
> + struct ttm_resource *res;
> unsigned i, j;
> int ret;
>
> @@ -144,8 +145,11 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
> continue;
>
> for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
> - list_for_each_entry(bo, &man->lru[j], lru) {
> - uint32_t num_pages = PFN_UP(bo->base.size);
> + list_for_each_entry(res, &man->lru[j], lru) {
> + uint32_t num_pages;
> +
> + bo = res->bo;
> + num_pages = PFN_UP(bo->base.size);
>
> ret = ttm_bo_swapout(bo, ctx, gfp_flags);
> /* ttm_bo_swapout has dropped the lru_lock */
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index 426e6841fc89..355c542758b5 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -29,6 +29,115 @@
> #include <drm/ttm/ttm_resource.h>
> #include <drm/ttm/ttm_bo_driver.h>
>
> +/**
> + * ttm_lru_bulk_move_init - initialize a bulk move structure
> + * @bulk: the structure to init
> + *
> + * For now just memset the structure to zero.
> + */
> +void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk)
> +{
> + memset(bulk, 0, sizeof(*bulk));
> +}
> +EXPORT_SYMBOL(ttm_lru_bulk_move_init);
> +
> +/**
> + * ttm_lru_bulk_move_tail
> + *
> + * @bulk: bulk move structure
> + *
> + * Bulk move BOs to the LRU tail, only valid to use when driver makes sure that
> + * resource order never changes. Should be called with ttm_device::lru_lock held.
> + */
> +void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
> +{
> + unsigned i;
> +
> + for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> + struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
> + struct ttm_resource_manager *man;
> +
> + if (!pos->first)
> + continue;
> +
> + dma_resv_assert_held(pos->first->bo->base.resv);
> + dma_resv_assert_held(pos->last->bo->base.resv);
> +
> + man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_TT);
> + list_bulk_move_tail(&man->lru[i], &pos->first->lru,
> + &pos->last->lru);
> + }
> +
> + for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> + struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
> + struct ttm_resource_manager *man;
> +
> + if (!pos->first)
> + continue;
> +
> + dma_resv_assert_held(pos->first->bo->base.resv);
> + dma_resv_assert_held(pos->last->bo->base.resv);
> +
> + man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_VRAM);
> + list_bulk_move_tail(&man->lru[i], &pos->first->lru,
> + &pos->last->lru);
> + }
> +}
> +EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
> +
> +/* Record a resource position in a bulk move structure */
> +static void ttm_lru_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
> + struct ttm_resource *res)
> +{
> + if (!pos->first)
> + pos->first = res;
> + pos->last = res;
> +}
> +
> +/* Remove a resource from the LRU */
> +static void ttm_resource_del_from_lru(struct ttm_resource *res)
> +{
> + struct ttm_device *bdev = res->bo->bdev;
> +
> + list_del_init(&res->lru);
> +
> + if (bdev->funcs->del_from_lru_notify)
> + bdev->funcs->del_from_lru_notify(res->bo);
> +}
> +
> +/* Move a resource to the LRU tail and track the bulk position */
> +void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
> + struct ttm_lru_bulk_move *bulk)
> +{
> + struct ttm_buffer_object *bo = res->bo;
> + struct ttm_device *bdev = bo->bdev;
> + struct ttm_resource_manager *man;
> +
> + if (bo->pin_count) {
> + ttm_resource_del_from_lru(res);
> + return;
> + }
> +
> + man = ttm_manager_type(bdev, res->mem_type);
> + list_move_tail(&res->lru, &man->lru[bo->priority]);
> +
> + if (bdev->funcs->del_from_lru_notify)
> + bdev->funcs->del_from_lru_notify(bo);
> +
> + if (!bulk)
> + return;
> +
> + switch (res->mem_type) {
> + case TTM_PL_TT:
> + ttm_lru_bulk_move_set_pos(&bulk->tt[bo->priority], res);
> + break;
> +
> + case TTM_PL_VRAM:
> + ttm_lru_bulk_move_set_pos(&bulk->vram[bo->priority], res);
> + break;
> + }
> +}
> +
> void ttm_resource_init(struct ttm_buffer_object *bo,
> const struct ttm_place *place,
> struct ttm_resource *res)
> @@ -44,15 +153,33 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
> res->bus.is_iomem = false;
> res->bus.caching = ttm_cached;
> res->bo = bo;
> + INIT_LIST_HEAD(&res->lru);
>
> man = ttm_manager_type(bo->bdev, place->mem_type);
> atomic64_add(bo->base.size, &man->usage);
> +
> + spin_lock(&bo->bdev->lru_lock);
> + ttm_resource_move_to_lru_tail(res, NULL);
> + spin_unlock(&bo->bdev->lru_lock);
> }
> EXPORT_SYMBOL(ttm_resource_init);
>
> +/**
> + * ttm_resource_fini
> + *
> + * @res: the resource to clean up
> + *
> + * Make sure the resource is removed from the LRU before destruction.
> + */
> void ttm_resource_fini(struct ttm_resource_manager *man,
> struct ttm_resource *res)
> {
> + struct ttm_device *bdev = res->bo->bdev;
> +
> + spin_lock(&bdev->lru_lock);
> + ttm_resource_del_from_lru(res);
> + spin_unlock(&bdev->lru_lock);
> +
> atomic64_sub(res->bo->base.size, &man->usage);
> }
> EXPORT_SYMBOL(ttm_resource_fini);
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index f681bbdbc698..0928d8cfb45a 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -56,8 +56,6 @@ struct ttm_placement;
>
> struct ttm_place;
>
> -struct ttm_lru_bulk_move;
> -
> /**
> * enum ttm_bo_type
> *
> @@ -95,7 +93,6 @@ struct ttm_tt;
> * @ttm: TTM structure holding system pages.
> * @evicted: Whether the object was evicted without user-space knowing.
> * @deleted: True if the object is only a zombie and already deleted.
> - * @lru: List head for the lru list.
> * @ddestroy: List head for the delayed destroy list.
> * @swap: List head for swap LRU list.
> * @moving: Fence set when BO is moving
> @@ -144,7 +141,6 @@ struct ttm_buffer_object {
> * Members protected by the bdev::lru_lock.
> */
>
> - struct list_head lru;
> struct list_head ddestroy;
>
> /**
> @@ -308,7 +304,6 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
> * ttm_bo_move_to_lru_tail
> *
> * @bo: The buffer object.
> - * @mem: Resource object.
> * @bulk: optional bulk move structure to remember BO positions
> *
> * Move this BO to the tail of all lru lists used to lookup and reserve an
> @@ -316,19 +311,8 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
> * held, and is used to make a BO less likely to be considered for eviction.
> */
> void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
> - struct ttm_resource *mem,
> struct ttm_lru_bulk_move *bulk);
>
> -/**
> - * ttm_bo_bulk_move_lru_tail
> - *
> - * @bulk: bulk move structure
> - *
> - * Bulk move BOs to the LRU tail, only valid to use when driver makes sure that
> - * BO order never changes. Should be called with ttm_global::lru_lock held.
> - */
> -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk);
> -
> /**
> * ttm_bo_lock_delayed_workqueue
> *
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 68d6069572aa..fba2f7d3d34e 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -45,33 +45,6 @@
> #include "ttm_tt.h"
> #include "ttm_pool.h"
>
> -/**
> - * struct ttm_lru_bulk_move_pos
> - *
> - * @first: first BO in the bulk move range
> - * @last: last BO in the bulk move range
> - *
> - * Positions for a lru bulk move.
> - */
> -struct ttm_lru_bulk_move_pos {
> - struct ttm_buffer_object *first;
> - struct ttm_buffer_object *last;
> -};
> -
> -/**
> - * struct ttm_lru_bulk_move
> - *
> - * @tt: first/last lru entry for BOs in the TT domain
> - * @vram: first/last lru entry for BOs in the VRAM domain
> - * @swap: first/last lru entry for BOs on the swap list
> - *
> - * Helper structure for bulk moves on the LRU list.
> - */
> -struct ttm_lru_bulk_move {
> - struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
> - struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
> -};
> -
> /*
> * ttm_bo.c
> */
> @@ -182,7 +155,7 @@ static inline void
> ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo)
> {
> spin_lock(&bo->bdev->lru_lock);
> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
> + ttm_bo_move_to_lru_tail(bo, NULL);
> spin_unlock(&bo->bdev->lru_lock);
> }
>
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index 526fe359c603..5f9797f9d64a 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -26,10 +26,12 @@
> #define _TTM_RESOURCE_H_
>
> #include <linux/types.h>
> +#include <linux/list.h>
> #include <linux/mutex.h>
> #include <linux/atomic.h>
> #include <linux/dma-buf-map.h>
> #include <linux/dma-fence.h>
> +
> #include <drm/drm_print.h>
> #include <drm/ttm/ttm_caching.h>
> #include <drm/ttm/ttm_kmap_iter.h>
> @@ -177,6 +179,33 @@ struct ttm_resource {
> uint32_t placement;
> struct ttm_bus_placement bus;
> struct ttm_buffer_object *bo;
> + struct list_head lru;
> +};
> +
> +/**
> + * struct ttm_lru_bulk_move_pos
> + *
> + * @first: first res in the bulk move range
> + * @last: last res in the bulk move range
> + *
> + * Positions for a lru bulk move.
> + */
> +struct ttm_lru_bulk_move_pos {
> + struct ttm_resource *first;
> + struct ttm_resource *last;
> +};
> +
> +/**
> + * struct ttm_lru_bulk_move
> + *
> + * @tt: first/last lru entry for resources in the TT domain
> + * @vram: first/last lru entry for resources in the VRAM domain
> + *
> + * Helper structure for bulk moves on the LRU list.
> + */
> +struct ttm_lru_bulk_move {
> + struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
> + struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
> };
>
> /**
> @@ -278,6 +307,12 @@ ttm_resource_manager_usage(struct ttm_resource_manager *man)
> return atomic64_read(&man->usage);
> }
>
> +void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk);
> +void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk);
> +
> +void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
> + struct ttm_lru_bulk_move *bulk);
> +
> void ttm_resource_init(struct ttm_buffer_object *bo,
> const struct ttm_place *place,
> struct ttm_resource *res);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 05/12] drm/ttm: move the LRU into resource handling
2021-08-30 16:53 ` Andrey Grodzovsky
@ 2021-08-30 16:55 ` Christian König
2021-08-30 17:00 ` Andrey Grodzovsky
0 siblings, 1 reply; 19+ messages in thread
From: Christian König @ 2021-08-30 16:55 UTC (permalink / raw)
To: Andrey Grodzovsky, thomas.hellstrom, dri-devel
Am 30.08.21 um 18:53 schrieb Andrey Grodzovsky:
>
> On 2021-08-30 4:57 a.m., Christian König wrote:
>> This way we finally fix the problem that new resource are
>> not immediately evict-able after allocation.
>>
>> That has caused numerous problems including OOM on GDS handling
>> and not being able to use TTM as general resource manager.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +-
>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
>> drivers/gpu/drm/ttm/ttm_bo.c | 105 ++------------------
>> drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
>> drivers/gpu/drm/ttm/ttm_device.c | 8 +-
>> drivers/gpu/drm/ttm/ttm_resource.c | 127 ++++++++++++++++++++++++
>> include/drm/ttm/ttm_bo_api.h | 16 ---
>> include/drm/ttm/ttm_bo_driver.h | 29 +-----
>> include/drm/ttm/ttm_resource.h | 35 +++++++
>> 9 files changed, 181 insertions(+), 150 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 6362e861a3f5..70d2cbb1dbb4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct
>> amdgpu_device *adev,
>> if (vm->bulk_moveable) {
>> spin_lock(&adev->mman.bdev.lru_lock);
>> - ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move);
>> + ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
>> spin_unlock(&adev->mman.bdev.lru_lock);
>> return;
>> }
>> - memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move));
>> + ttm_lru_bulk_move_init(&vm->lru_bulk_move);
>> spin_lock(&adev->mman.bdev.lru_lock);
>> list_for_each_entry(bo_base, &vm->idle, vm_status) {
>> @@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct
>> amdgpu_device *adev,
>> if (!bo->parent)
>> continue;
>> - ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource,
>> - &vm->lru_bulk_move);
>> + ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move);
>> if (shadow)
>> ttm_bo_move_to_lru_tail(&shadow->tbo,
>> - shadow->tbo.resource,
>> &vm->lru_bulk_move);
>> }
>> spin_unlock(&adev->mman.bdev.lru_lock);
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> index e646aac9d7a4..41f0de841d72 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> @@ -471,7 +471,7 @@ static void i915_ttm_adjust_lru(struct
>> drm_i915_gem_object *obj)
>> bo->priority = I915_TTM_PRIO_NO_PAGES;
>> }
>> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
>> + ttm_bo_move_to_lru_tail(bo, NULL);
>> spin_unlock(&bo->bdev->lru_lock);
>> }
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>> b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 49f4bc97c35a..d5c6e096fd31 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -69,98 +69,16 @@ static void ttm_bo_mem_space_debug(struct
>> ttm_buffer_object *bo,
>> }
>> }
>> -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>> -{
>> - struct ttm_device *bdev = bo->bdev;
>> -
>> - list_del_init(&bo->lru);
>> -
>> - if (bdev->funcs->del_from_lru_notify)
>> - bdev->funcs->del_from_lru_notify(bo);
>> -}
>> -
>> -static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
>> - struct ttm_buffer_object *bo)
>> -{
>> - if (!pos->first)
>> - pos->first = bo;
>> - pos->last = bo;
>> -}
>> -
>> void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
>> - struct ttm_resource *mem,
>> struct ttm_lru_bulk_move *bulk)
>> {
>> - struct ttm_device *bdev = bo->bdev;
>> - struct ttm_resource_manager *man;
>> -
>> - if (!bo->deleted)
>> - dma_resv_assert_held(bo->base.resv);
>> -
>> - if (bo->pin_count) {
>> - ttm_bo_del_from_lru(bo);
>> - return;
>> - }
>> -
>> - if (!mem)
>> - return;
>> -
>> - man = ttm_manager_type(bdev, mem->mem_type);
>> - list_move_tail(&bo->lru, &man->lru[bo->priority]);
>> -
>> - if (bdev->funcs->del_from_lru_notify)
>> - bdev->funcs->del_from_lru_notify(bo);
>> -
>> - if (bulk && !bo->pin_count) {
>> - switch (bo->resource->mem_type) {
>> - case TTM_PL_TT:
>> - ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo);
>> - break;
>> + dma_resv_assert_held(bo->base.resv);
>> - case TTM_PL_VRAM:
>> - ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo);
>> - break;
>> - }
>> - }
>> + if (bo->resource)
>> + ttm_resource_move_to_lru_tail(bo->resource, bulk);
>> }
>> EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
>> -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
>> -{
>> - unsigned i;
>> -
>> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>> - struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
>> - struct ttm_resource_manager *man;
>> -
>> - if (!pos->first)
>> - continue;
>> -
>> - dma_resv_assert_held(pos->first->base.resv);
>> - dma_resv_assert_held(pos->last->base.resv);
>> -
>> - man = ttm_manager_type(pos->first->bdev, TTM_PL_TT);
>> - list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>> - &pos->last->lru);
>> - }
>> -
>> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>> - struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
>> - struct ttm_resource_manager *man;
>> -
>> - if (!pos->first)
>> - continue;
>> -
>> - dma_resv_assert_held(pos->first->base.resv);
>> - dma_resv_assert_held(pos->last->base.resv);
>> -
>> - man = ttm_manager_type(pos->first->bdev, TTM_PL_VRAM);
>> - list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>> - &pos->last->lru);
>> - }
>> -}
>> -EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
>> -
>> static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>> struct ttm_resource *mem, bool evict,
>> struct ttm_operation_ctx *ctx,
>> @@ -342,7 +260,6 @@ static int ttm_bo_cleanup_refs(struct
>> ttm_buffer_object *bo,
>> return ret;
>> }
>> - ttm_bo_del_from_lru(bo);
>
>
> Here and in other places this was removed, I assume ttm_resource_fini
> should replace it but I don't see where exactly this takes place.
Take a look at function ttm_resource_fini().
Christian.
>
> Andrey
>
>
>> list_del_init(&bo->ddestroy);
>> spin_unlock(&bo->bdev->lru_lock);
>> ttm_bo_cleanup_memtype_use(bo);
>> @@ -443,7 +360,7 @@ static void ttm_bo_release(struct kref *kref)
>> */
>> if (bo->pin_count) {
>> bo->pin_count = 0;
>> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
>> + ttm_resource_move_to_lru_tail(bo->resource, NULL);
>> }
>> kref_init(&bo->kref);
>> @@ -456,7 +373,6 @@ static void ttm_bo_release(struct kref *kref)
>> }
>> spin_lock(&bo->bdev->lru_lock);
>> - ttm_bo_del_from_lru(bo);
>> list_del(&bo->ddestroy);
>> spin_unlock(&bo->bdev->lru_lock);
>> @@ -670,15 +586,17 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
>> struct ww_acquire_ctx *ticket)
>> {
>> struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
>> + struct ttm_resource *res;
>> bool locked = false;
>> unsigned i;
>> int ret;
>> spin_lock(&bdev->lru_lock);
>> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>> - list_for_each_entry(bo, &man->lru[i], lru) {
>> + list_for_each_entry(res, &man->lru[i], lru) {
>> bool busy;
>> + bo = res->bo;
>> if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
>> &locked, &busy)) {
>> if (busy && !busy_bo && ticket !=
>> @@ -696,7 +614,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
>> }
>> /* If the inner loop terminated early, we have our
>> candidate */
>> - if (&bo->lru != &man->lru[i])
>> + if (&res->lru != &man->lru[i])
>> break;
>> bo = NULL;
>> @@ -870,9 +788,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>> }
>> error:
>> - if (bo->resource->mem_type == TTM_PL_SYSTEM && !bo->pin_count)
>> - ttm_bo_move_to_lru_tail_unlocked(bo);
>> -
>> return ret;
>> }
>> EXPORT_SYMBOL(ttm_bo_mem_space);
>> @@ -1012,7 +927,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>> bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
>> kref_init(&bo->kref);
>> - INIT_LIST_HEAD(&bo->lru);
>> INIT_LIST_HEAD(&bo->ddestroy);
>> bo->bdev = bdev;
>> bo->type = type;
>> @@ -1062,8 +976,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>> return ret;
>> }
>> - ttm_bo_move_to_lru_tail_unlocked(bo);
>> -
>> return ret;
>> }
>> EXPORT_SYMBOL(ttm_bo_init_reserved);
>> @@ -1165,7 +1077,6 @@ int ttm_bo_swapout(struct ttm_buffer_object
>> *bo, struct ttm_operation_ctx *ctx,
>> return 0;
>> }
>> - ttm_bo_del_from_lru(bo);
>> /* TODO: Cleanup the locking */
>> spin_unlock(&bo->bdev->lru_lock);
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> index c5d02edaefc0..49b4bedf8715 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> @@ -232,7 +232,6 @@ static int ttm_buffer_object_transfer(struct
>> ttm_buffer_object *bo,
>> atomic_inc(&ttm_glob.bo_count);
>> INIT_LIST_HEAD(&fbo->base.ddestroy);
>> - INIT_LIST_HEAD(&fbo->base.lru);
>> fbo->base.moving = NULL;
>> drm_vma_node_reset(&fbo->base.base.vma_node);
>> diff --git a/drivers/gpu/drm/ttm/ttm_device.c
>> b/drivers/gpu/drm/ttm/ttm_device.c
>> index 519deea8e39b..9e0dfceff68c 100644
>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>> @@ -134,6 +134,7 @@ int ttm_device_swapout(struct ttm_device *bdev,
>> struct ttm_operation_ctx *ctx,
>> {
>> struct ttm_resource_manager *man;
>> struct ttm_buffer_object *bo;
>> + struct ttm_resource *res;
>> unsigned i, j;
>> int ret;
>> @@ -144,8 +145,11 @@ int ttm_device_swapout(struct ttm_device
>> *bdev, struct ttm_operation_ctx *ctx,
>> continue;
>> for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
>> - list_for_each_entry(bo, &man->lru[j], lru) {
>> - uint32_t num_pages = PFN_UP(bo->base.size);
>> + list_for_each_entry(res, &man->lru[j], lru) {
>> + uint32_t num_pages;
>> +
>> + bo = res->bo;
>> + num_pages = PFN_UP(bo->base.size);
>> ret = ttm_bo_swapout(bo, ctx, gfp_flags);
>> /* ttm_bo_swapout has dropped the lru_lock */
>> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
>> b/drivers/gpu/drm/ttm/ttm_resource.c
>> index 426e6841fc89..355c542758b5 100644
>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>> @@ -29,6 +29,115 @@
>> #include <drm/ttm/ttm_resource.h>
>> #include <drm/ttm/ttm_bo_driver.h>
>> +/**
>> + * ttm_lru_bulk_move_init - initialize a bulk move structure
>> + * @bulk: the structure to init
>> + *
>> + * For now just memset the structure to zero.
>> + */
>> +void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk)
>> +{
>> + memset(bulk, 0, sizeof(*bulk));
>> +}
>> +EXPORT_SYMBOL(ttm_lru_bulk_move_init);
>> +
>> +/**
>> + * ttm_lru_bulk_move_tail
>> + *
>> + * @bulk: bulk move structure
>> + *
>> + * Bulk move BOs to the LRU tail, only valid to use when driver
>> makes sure that
>> + * resource order never changes. Should be called with
>> ttm_device::lru_lock held.
>> + */
>> +void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
>> +{
>> + unsigned i;
>> +
>> + for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>> + struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
>> + struct ttm_resource_manager *man;
>> +
>> + if (!pos->first)
>> + continue;
>> +
>> + dma_resv_assert_held(pos->first->bo->base.resv);
>> + dma_resv_assert_held(pos->last->bo->base.resv);
>> +
>> + man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_TT);
>> + list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>> + &pos->last->lru);
>> + }
>> +
>> + for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>> + struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
>> + struct ttm_resource_manager *man;
>> +
>> + if (!pos->first)
>> + continue;
>> +
>> + dma_resv_assert_held(pos->first->bo->base.resv);
>> + dma_resv_assert_held(pos->last->bo->base.resv);
>> +
>> + man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_VRAM);
>> + list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>> + &pos->last->lru);
>> + }
>> +}
>> +EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
>> +
>> +/* Record a resource position in a bulk move structure */
>> +static void ttm_lru_bulk_move_set_pos(struct ttm_lru_bulk_move_pos
>> *pos,
>> + struct ttm_resource *res)
>> +{
>> + if (!pos->first)
>> + pos->first = res;
>> + pos->last = res;
>> +}
>> +
>> +/* Remove a resource from the LRU */
>> +static void ttm_resource_del_from_lru(struct ttm_resource *res)
>> +{
>> + struct ttm_device *bdev = res->bo->bdev;
>> +
>> + list_del_init(&res->lru);
>> +
>> + if (bdev->funcs->del_from_lru_notify)
>> + bdev->funcs->del_from_lru_notify(res->bo);
>> +}
>> +
>> +/* Move a resource to the LRU tail and track the bulk position */
>> +void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
>> + struct ttm_lru_bulk_move *bulk)
>> +{
>> + struct ttm_buffer_object *bo = res->bo;
>> + struct ttm_device *bdev = bo->bdev;
>> + struct ttm_resource_manager *man;
>> +
>> + if (bo->pin_count) {
>> + ttm_resource_del_from_lru(res);
>> + return;
>> + }
>> +
>> + man = ttm_manager_type(bdev, res->mem_type);
>> + list_move_tail(&res->lru, &man->lru[bo->priority]);
>> +
>> + if (bdev->funcs->del_from_lru_notify)
>> + bdev->funcs->del_from_lru_notify(bo);
>> +
>> + if (!bulk)
>> + return;
>> +
>> + switch (res->mem_type) {
>> + case TTM_PL_TT:
>> + ttm_lru_bulk_move_set_pos(&bulk->tt[bo->priority], res);
>> + break;
>> +
>> + case TTM_PL_VRAM:
>> + ttm_lru_bulk_move_set_pos(&bulk->vram[bo->priority], res);
>> + break;
>> + }
>> +}
>> +
>> void ttm_resource_init(struct ttm_buffer_object *bo,
>> const struct ttm_place *place,
>> struct ttm_resource *res)
>> @@ -44,15 +153,33 @@ void ttm_resource_init(struct ttm_buffer_object
>> *bo,
>> res->bus.is_iomem = false;
>> res->bus.caching = ttm_cached;
>> res->bo = bo;
>> + INIT_LIST_HEAD(&res->lru);
>> man = ttm_manager_type(bo->bdev, place->mem_type);
>> atomic64_add(bo->base.size, &man->usage);
>> +
>> + spin_lock(&bo->bdev->lru_lock);
>> + ttm_resource_move_to_lru_tail(res, NULL);
>> + spin_unlock(&bo->bdev->lru_lock);
>> }
>> EXPORT_SYMBOL(ttm_resource_init);
>> +/**
>> + * ttm_resource_fini
>> + *
>> + * @res: the resource to clean up
>> + *
>> + * Make sure the resource is removed from the LRU before destruction.
>> + */
>> void ttm_resource_fini(struct ttm_resource_manager *man,
>> struct ttm_resource *res)
>> {
>> + struct ttm_device *bdev = res->bo->bdev;
>> +
>> + spin_lock(&bdev->lru_lock);
>> + ttm_resource_del_from_lru(res);
>> + spin_unlock(&bdev->lru_lock);
>> +
>> atomic64_sub(res->bo->base.size, &man->usage);
>> }
>> EXPORT_SYMBOL(ttm_resource_fini);
>> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
>> index f681bbdbc698..0928d8cfb45a 100644
>> --- a/include/drm/ttm/ttm_bo_api.h
>> +++ b/include/drm/ttm/ttm_bo_api.h
>> @@ -56,8 +56,6 @@ struct ttm_placement;
>> struct ttm_place;
>> -struct ttm_lru_bulk_move;
>> -
>> /**
>> * enum ttm_bo_type
>> *
>> @@ -95,7 +93,6 @@ struct ttm_tt;
>> * @ttm: TTM structure holding system pages.
>> * @evicted: Whether the object was evicted without user-space
>> knowing.
>> * @deleted: True if the object is only a zombie and already deleted.
>> - * @lru: List head for the lru list.
>> * @ddestroy: List head for the delayed destroy list.
>> * @swap: List head for swap LRU list.
>> * @moving: Fence set when BO is moving
>> @@ -144,7 +141,6 @@ struct ttm_buffer_object {
>> * Members protected by the bdev::lru_lock.
>> */
>> - struct list_head lru;
>> struct list_head ddestroy;
>> /**
>> @@ -308,7 +304,6 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
>> * ttm_bo_move_to_lru_tail
>> *
>> * @bo: The buffer object.
>> - * @mem: Resource object.
>> * @bulk: optional bulk move structure to remember BO positions
>> *
>> * Move this BO to the tail of all lru lists used to lookup and
>> reserve an
>> @@ -316,19 +311,8 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
>> * held, and is used to make a BO less likely to be considered for
>> eviction.
>> */
>> void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
>> - struct ttm_resource *mem,
>> struct ttm_lru_bulk_move *bulk);
>> -/**
>> - * ttm_bo_bulk_move_lru_tail
>> - *
>> - * @bulk: bulk move structure
>> - *
>> - * Bulk move BOs to the LRU tail, only valid to use when driver
>> makes sure that
>> - * BO order never changes. Should be called with
>> ttm_global::lru_lock held.
>> - */
>> -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk);
>> -
>> /**
>> * ttm_bo_lock_delayed_workqueue
>> *
>> diff --git a/include/drm/ttm/ttm_bo_driver.h
>> b/include/drm/ttm/ttm_bo_driver.h
>> index 68d6069572aa..fba2f7d3d34e 100644
>> --- a/include/drm/ttm/ttm_bo_driver.h
>> +++ b/include/drm/ttm/ttm_bo_driver.h
>> @@ -45,33 +45,6 @@
>> #include "ttm_tt.h"
>> #include "ttm_pool.h"
>> -/**
>> - * struct ttm_lru_bulk_move_pos
>> - *
>> - * @first: first BO in the bulk move range
>> - * @last: last BO in the bulk move range
>> - *
>> - * Positions for a lru bulk move.
>> - */
>> -struct ttm_lru_bulk_move_pos {
>> - struct ttm_buffer_object *first;
>> - struct ttm_buffer_object *last;
>> -};
>> -
>> -/**
>> - * struct ttm_lru_bulk_move
>> - *
>> - * @tt: first/last lru entry for BOs in the TT domain
>> - * @vram: first/last lru entry for BOs in the VRAM domain
>> - * @swap: first/last lru entry for BOs on the swap list
>> - *
>> - * Helper structure for bulk moves on the LRU list.
>> - */
>> -struct ttm_lru_bulk_move {
>> - struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
>> - struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
>> -};
>> -
>> /*
>> * ttm_bo.c
>> */
>> @@ -182,7 +155,7 @@ static inline void
>> ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo)
>> {
>> spin_lock(&bo->bdev->lru_lock);
>> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
>> + ttm_bo_move_to_lru_tail(bo, NULL);
>> spin_unlock(&bo->bdev->lru_lock);
>> }
>> diff --git a/include/drm/ttm/ttm_resource.h
>> b/include/drm/ttm/ttm_resource.h
>> index 526fe359c603..5f9797f9d64a 100644
>> --- a/include/drm/ttm/ttm_resource.h
>> +++ b/include/drm/ttm/ttm_resource.h
>> @@ -26,10 +26,12 @@
>> #define _TTM_RESOURCE_H_
>> #include <linux/types.h>
>> +#include <linux/list.h>
>> #include <linux/mutex.h>
>> #include <linux/atomic.h>
>> #include <linux/dma-buf-map.h>
>> #include <linux/dma-fence.h>
>> +
>> #include <drm/drm_print.h>
>> #include <drm/ttm/ttm_caching.h>
>> #include <drm/ttm/ttm_kmap_iter.h>
>> @@ -177,6 +179,33 @@ struct ttm_resource {
>> uint32_t placement;
>> struct ttm_bus_placement bus;
>> struct ttm_buffer_object *bo;
>> + struct list_head lru;
>> +};
>> +
>> +/**
>> + * struct ttm_lru_bulk_move_pos
>> + *
>> + * @first: first res in the bulk move range
>> + * @last: last res in the bulk move range
>> + *
>> + * Positions for a lru bulk move.
>> + */
>> +struct ttm_lru_bulk_move_pos {
>> + struct ttm_resource *first;
>> + struct ttm_resource *last;
>> +};
>> +
>> +/**
>> + * struct ttm_lru_bulk_move
>> + *
>> + * @tt: first/last lru entry for resources in the TT domain
>> + * @vram: first/last lru entry for resources in the VRAM domain
>> + *
>> + * Helper structure for bulk moves on the LRU list.
>> + */
>> +struct ttm_lru_bulk_move {
>> + struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
>> + struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
>> };
>> /**
>> @@ -278,6 +307,12 @@ ttm_resource_manager_usage(struct
>> ttm_resource_manager *man)
>> return atomic64_read(&man->usage);
>> }
>> +void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk);
>> +void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk);
>> +
>> +void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
>> + struct ttm_lru_bulk_move *bulk);
>> +
>> void ttm_resource_init(struct ttm_buffer_object *bo,
>> const struct ttm_place *place,
>> struct ttm_resource *res);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 05/12] drm/ttm: move the LRU into resource handling
2021-08-30 16:55 ` Christian König
@ 2021-08-30 17:00 ` Andrey Grodzovsky
2021-08-30 17:02 ` Christian König
0 siblings, 1 reply; 19+ messages in thread
From: Andrey Grodzovsky @ 2021-08-30 17:00 UTC (permalink / raw)
To: Christian König, thomas.hellstrom, dri-devel
On 2021-08-30 12:55 p.m., Christian König wrote:
> Am 30.08.21 um 18:53 schrieb Andrey Grodzovsky:
>>
>> On 2021-08-30 4:57 a.m., Christian König wrote:
>>> This way we finally fix the problem that new resource are
>>> not immediately evict-able after allocation.
>>>
>>> That has caused numerous problems including OOM on GDS handling
>>> and not being able to use TTM as general resource manager.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +-
>>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
>>> drivers/gpu/drm/ttm/ttm_bo.c | 105 ++------------------
>>> drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
>>> drivers/gpu/drm/ttm/ttm_device.c | 8 +-
>>> drivers/gpu/drm/ttm/ttm_resource.c | 127
>>> ++++++++++++++++++++++++
>>> include/drm/ttm/ttm_bo_api.h | 16 ---
>>> include/drm/ttm/ttm_bo_driver.h | 29 +-----
>>> include/drm/ttm/ttm_resource.h | 35 +++++++
>>> 9 files changed, 181 insertions(+), 150 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 6362e861a3f5..70d2cbb1dbb4 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct
>>> amdgpu_device *adev,
>>> if (vm->bulk_moveable) {
>>> spin_lock(&adev->mman.bdev.lru_lock);
>>> - ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move);
>>> + ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
>>> spin_unlock(&adev->mman.bdev.lru_lock);
>>> return;
>>> }
>>> - memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move));
>>> + ttm_lru_bulk_move_init(&vm->lru_bulk_move);
>>> spin_lock(&adev->mman.bdev.lru_lock);
>>> list_for_each_entry(bo_base, &vm->idle, vm_status) {
>>> @@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct
>>> amdgpu_device *adev,
>>> if (!bo->parent)
>>> continue;
>>> - ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource,
>>> - &vm->lru_bulk_move);
>>> + ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move);
>>> if (shadow)
>>> ttm_bo_move_to_lru_tail(&shadow->tbo,
>>> - shadow->tbo.resource,
>>> &vm->lru_bulk_move);
>>> }
>>> spin_unlock(&adev->mman.bdev.lru_lock);
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> index e646aac9d7a4..41f0de841d72 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> @@ -471,7 +471,7 @@ static void i915_ttm_adjust_lru(struct
>>> drm_i915_gem_object *obj)
>>> bo->priority = I915_TTM_PRIO_NO_PAGES;
>>> }
>>> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
>>> + ttm_bo_move_to_lru_tail(bo, NULL);
>>> spin_unlock(&bo->bdev->lru_lock);
>>> }
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>> index 49f4bc97c35a..d5c6e096fd31 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>> @@ -69,98 +69,16 @@ static void ttm_bo_mem_space_debug(struct
>>> ttm_buffer_object *bo,
>>> }
>>> }
>>> -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>>> -{
>>> - struct ttm_device *bdev = bo->bdev;
>>> -
>>> - list_del_init(&bo->lru);
>>> -
>>> - if (bdev->funcs->del_from_lru_notify)
>>> - bdev->funcs->del_from_lru_notify(bo);
>>> -}
>>> -
>>> -static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos
>>> *pos,
>>> - struct ttm_buffer_object *bo)
>>> -{
>>> - if (!pos->first)
>>> - pos->first = bo;
>>> - pos->last = bo;
>>> -}
>>> -
>>> void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
>>> - struct ttm_resource *mem,
>>> struct ttm_lru_bulk_move *bulk)
>>> {
>>> - struct ttm_device *bdev = bo->bdev;
>>> - struct ttm_resource_manager *man;
>>> -
>>> - if (!bo->deleted)
>>> - dma_resv_assert_held(bo->base.resv);
>>> -
>>> - if (bo->pin_count) {
>>> - ttm_bo_del_from_lru(bo);
>>> - return;
>>> - }
>>> -
>>> - if (!mem)
>>> - return;
>>> -
>>> - man = ttm_manager_type(bdev, mem->mem_type);
>>> - list_move_tail(&bo->lru, &man->lru[bo->priority]);
>>> -
>>> - if (bdev->funcs->del_from_lru_notify)
>>> - bdev->funcs->del_from_lru_notify(bo);
>>> -
>>> - if (bulk && !bo->pin_count) {
>>> - switch (bo->resource->mem_type) {
>>> - case TTM_PL_TT:
>>> - ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo);
>>> - break;
>>> + dma_resv_assert_held(bo->base.resv);
>>> - case TTM_PL_VRAM:
>>> - ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo);
>>> - break;
>>> - }
>>> - }
>>> + if (bo->resource)
>>> + ttm_resource_move_to_lru_tail(bo->resource, bulk);
>>> }
>>> EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
>>> -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
>>> -{
>>> - unsigned i;
>>> -
>>> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>> - struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
>>> - struct ttm_resource_manager *man;
>>> -
>>> - if (!pos->first)
>>> - continue;
>>> -
>>> - dma_resv_assert_held(pos->first->base.resv);
>>> - dma_resv_assert_held(pos->last->base.resv);
>>> -
>>> - man = ttm_manager_type(pos->first->bdev, TTM_PL_TT);
>>> - list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>>> - &pos->last->lru);
>>> - }
>>> -
>>> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>> - struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
>>> - struct ttm_resource_manager *man;
>>> -
>>> - if (!pos->first)
>>> - continue;
>>> -
>>> - dma_resv_assert_held(pos->first->base.resv);
>>> - dma_resv_assert_held(pos->last->base.resv);
>>> -
>>> - man = ttm_manager_type(pos->first->bdev, TTM_PL_VRAM);
>>> - list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>>> - &pos->last->lru);
>>> - }
>>> -}
>>> -EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
>>> -
>>> static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>>> struct ttm_resource *mem, bool evict,
>>> struct ttm_operation_ctx *ctx,
>>> @@ -342,7 +260,6 @@ static int ttm_bo_cleanup_refs(struct
>>> ttm_buffer_object *bo,
>>> return ret;
>>> }
>>> - ttm_bo_del_from_lru(bo);
>>
>>
>> Here and in other places this was removed, I assume ttm_resource_fini
>> should replace it but I don't see where exactly this takes place.
>
> Take a look at function ttm_resource_fini().
>
> Christian.
What I mean is that I don't see where ttm_resource_fini will be called to
compensate for removal of ttm_bo_del_from_lru here.
Andrey
>
>>
>> Andrey
>>
>>
>>> list_del_init(&bo->ddestroy);
>>> spin_unlock(&bo->bdev->lru_lock);
>>> ttm_bo_cleanup_memtype_use(bo);
>>> @@ -443,7 +360,7 @@ static void ttm_bo_release(struct kref *kref)
>>> */
>>> if (bo->pin_count) {
>>> bo->pin_count = 0;
>>> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
>>> + ttm_resource_move_to_lru_tail(bo->resource, NULL);
>>> }
>>> kref_init(&bo->kref);
>>> @@ -456,7 +373,6 @@ static void ttm_bo_release(struct kref *kref)
>>> }
>>> spin_lock(&bo->bdev->lru_lock);
>>> - ttm_bo_del_from_lru(bo);
>>> list_del(&bo->ddestroy);
>>> spin_unlock(&bo->bdev->lru_lock);
>>> @@ -670,15 +586,17 @@ int ttm_mem_evict_first(struct ttm_device
>>> *bdev,
>>> struct ww_acquire_ctx *ticket)
>>> {
>>> struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
>>> + struct ttm_resource *res;
>>> bool locked = false;
>>> unsigned i;
>>> int ret;
>>> spin_lock(&bdev->lru_lock);
>>> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>> - list_for_each_entry(bo, &man->lru[i], lru) {
>>> + list_for_each_entry(res, &man->lru[i], lru) {
>>> bool busy;
>>> + bo = res->bo;
>>> if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
>>> &locked, &busy)) {
>>> if (busy && !busy_bo && ticket !=
>>> @@ -696,7 +614,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
>>> }
>>> /* If the inner loop terminated early, we have our
>>> candidate */
>>> - if (&bo->lru != &man->lru[i])
>>> + if (&res->lru != &man->lru[i])
>>> break;
>>> bo = NULL;
>>> @@ -870,9 +788,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>>> }
>>> error:
>>> - if (bo->resource->mem_type == TTM_PL_SYSTEM && !bo->pin_count)
>>> - ttm_bo_move_to_lru_tail_unlocked(bo);
>>> -
>>> return ret;
>>> }
>>> EXPORT_SYMBOL(ttm_bo_mem_space);
>>> @@ -1012,7 +927,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>>> bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
>>> kref_init(&bo->kref);
>>> - INIT_LIST_HEAD(&bo->lru);
>>> INIT_LIST_HEAD(&bo->ddestroy);
>>> bo->bdev = bdev;
>>> bo->type = type;
>>> @@ -1062,8 +976,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>>> return ret;
>>> }
>>> - ttm_bo_move_to_lru_tail_unlocked(bo);
>>> -
>>> return ret;
>>> }
>>> EXPORT_SYMBOL(ttm_bo_init_reserved);
>>> @@ -1165,7 +1077,6 @@ int ttm_bo_swapout(struct ttm_buffer_object
>>> *bo, struct ttm_operation_ctx *ctx,
>>> return 0;
>>> }
>>> - ttm_bo_del_from_lru(bo);
>>> /* TODO: Cleanup the locking */
>>> spin_unlock(&bo->bdev->lru_lock);
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
>>> b/drivers/gpu/drm/ttm/ttm_bo_util.c
>>> index c5d02edaefc0..49b4bedf8715 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>>> @@ -232,7 +232,6 @@ static int ttm_buffer_object_transfer(struct
>>> ttm_buffer_object *bo,
>>> atomic_inc(&ttm_glob.bo_count);
>>> INIT_LIST_HEAD(&fbo->base.ddestroy);
>>> - INIT_LIST_HEAD(&fbo->base.lru);
>>> fbo->base.moving = NULL;
>>> drm_vma_node_reset(&fbo->base.base.vma_node);
>>> diff --git a/drivers/gpu/drm/ttm/ttm_device.c
>>> b/drivers/gpu/drm/ttm/ttm_device.c
>>> index 519deea8e39b..9e0dfceff68c 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>> @@ -134,6 +134,7 @@ int ttm_device_swapout(struct ttm_device *bdev,
>>> struct ttm_operation_ctx *ctx,
>>> {
>>> struct ttm_resource_manager *man;
>>> struct ttm_buffer_object *bo;
>>> + struct ttm_resource *res;
>>> unsigned i, j;
>>> int ret;
>>> @@ -144,8 +145,11 @@ int ttm_device_swapout(struct ttm_device
>>> *bdev, struct ttm_operation_ctx *ctx,
>>> continue;
>>> for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
>>> - list_for_each_entry(bo, &man->lru[j], lru) {
>>> - uint32_t num_pages = PFN_UP(bo->base.size);
>>> + list_for_each_entry(res, &man->lru[j], lru) {
>>> + uint32_t num_pages;
>>> +
>>> + bo = res->bo;
>>> + num_pages = PFN_UP(bo->base.size);
>>> ret = ttm_bo_swapout(bo, ctx, gfp_flags);
>>> /* ttm_bo_swapout has dropped the lru_lock */
>>> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
>>> b/drivers/gpu/drm/ttm/ttm_resource.c
>>> index 426e6841fc89..355c542758b5 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>>> @@ -29,6 +29,115 @@
>>> #include <drm/ttm/ttm_resource.h>
>>> #include <drm/ttm/ttm_bo_driver.h>
>>> +/**
>>> + * ttm_lru_bulk_move_init - initialize a bulk move structure
>>> + * @bulk: the structure to init
>>> + *
>>> + * For now just memset the structure to zero.
>>> + */
>>> +void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk)
>>> +{
>>> + memset(bulk, 0, sizeof(*bulk));
>>> +}
>>> +EXPORT_SYMBOL(ttm_lru_bulk_move_init);
>>> +
>>> +/**
>>> + * ttm_lru_bulk_move_tail
>>> + *
>>> + * @bulk: bulk move structure
>>> + *
>>> + * Bulk move BOs to the LRU tail, only valid to use when driver
>>> makes sure that
>>> + * resource order never changes. Should be called with
>>> ttm_device::lru_lock held.
>>> + */
>>> +void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
>>> +{
>>> + unsigned i;
>>> +
>>> + for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>> + struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
>>> + struct ttm_resource_manager *man;
>>> +
>>> + if (!pos->first)
>>> + continue;
>>> +
>>> + dma_resv_assert_held(pos->first->bo->base.resv);
>>> + dma_resv_assert_held(pos->last->bo->base.resv);
>>> +
>>> + man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_TT);
>>> + list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>>> + &pos->last->lru);
>>> + }
>>> +
>>> + for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>> + struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
>>> + struct ttm_resource_manager *man;
>>> +
>>> + if (!pos->first)
>>> + continue;
>>> +
>>> + dma_resv_assert_held(pos->first->bo->base.resv);
>>> + dma_resv_assert_held(pos->last->bo->base.resv);
>>> +
>>> + man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_VRAM);
>>> + list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>>> + &pos->last->lru);
>>> + }
>>> +}
>>> +EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
>>> +
>>> +/* Record a resource position in a bulk move structure */
>>> +static void ttm_lru_bulk_move_set_pos(struct ttm_lru_bulk_move_pos
>>> *pos,
>>> + struct ttm_resource *res)
>>> +{
>>> + if (!pos->first)
>>> + pos->first = res;
>>> + pos->last = res;
>>> +}
>>> +
>>> +/* Remove a resource from the LRU */
>>> +static void ttm_resource_del_from_lru(struct ttm_resource *res)
>>> +{
>>> + struct ttm_device *bdev = res->bo->bdev;
>>> +
>>> + list_del_init(&res->lru);
>>> +
>>> + if (bdev->funcs->del_from_lru_notify)
>>> + bdev->funcs->del_from_lru_notify(res->bo);
>>> +}
>>> +
>>> +/* Move a resource to the LRU tail and track the bulk position */
>>> +void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
>>> + struct ttm_lru_bulk_move *bulk)
>>> +{
>>> + struct ttm_buffer_object *bo = res->bo;
>>> + struct ttm_device *bdev = bo->bdev;
>>> + struct ttm_resource_manager *man;
>>> +
>>> + if (bo->pin_count) {
>>> + ttm_resource_del_from_lru(res);
>>> + return;
>>> + }
>>> +
>>> + man = ttm_manager_type(bdev, res->mem_type);
>>> + list_move_tail(&res->lru, &man->lru[bo->priority]);
>>> +
>>> + if (bdev->funcs->del_from_lru_notify)
>>> + bdev->funcs->del_from_lru_notify(bo);
>>> +
>>> + if (!bulk)
>>> + return;
>>> +
>>> + switch (res->mem_type) {
>>> + case TTM_PL_TT:
>>> + ttm_lru_bulk_move_set_pos(&bulk->tt[bo->priority], res);
>>> + break;
>>> +
>>> + case TTM_PL_VRAM:
>>> + ttm_lru_bulk_move_set_pos(&bulk->vram[bo->priority], res);
>>> + break;
>>> + }
>>> +}
>>> +
>>> void ttm_resource_init(struct ttm_buffer_object *bo,
>>> const struct ttm_place *place,
>>> struct ttm_resource *res)
>>> @@ -44,15 +153,33 @@ void ttm_resource_init(struct ttm_buffer_object
>>> *bo,
>>> res->bus.is_iomem = false;
>>> res->bus.caching = ttm_cached;
>>> res->bo = bo;
>>> + INIT_LIST_HEAD(&res->lru);
>>> man = ttm_manager_type(bo->bdev, place->mem_type);
>>> atomic64_add(bo->base.size, &man->usage);
>>> +
>>> + spin_lock(&bo->bdev->lru_lock);
>>> + ttm_resource_move_to_lru_tail(res, NULL);
>>> + spin_unlock(&bo->bdev->lru_lock);
>>> }
>>> EXPORT_SYMBOL(ttm_resource_init);
>>> +/**
>>> + * ttm_resource_fini
>>> + *
>>> + * @res: the resource to clean up
>>> + *
>>> + * Make sure the resource is removed from the LRU before destruction.
>>> + */
>>> void ttm_resource_fini(struct ttm_resource_manager *man,
>>> struct ttm_resource *res)
>>> {
>>> + struct ttm_device *bdev = res->bo->bdev;
>>> +
>>> + spin_lock(&bdev->lru_lock);
>>> + ttm_resource_del_from_lru(res);
>>> + spin_unlock(&bdev->lru_lock);
>>> +
>>> atomic64_sub(res->bo->base.size, &man->usage);
>>> }
>>> EXPORT_SYMBOL(ttm_resource_fini);
>>> diff --git a/include/drm/ttm/ttm_bo_api.h
>>> b/include/drm/ttm/ttm_bo_api.h
>>> index f681bbdbc698..0928d8cfb45a 100644
>>> --- a/include/drm/ttm/ttm_bo_api.h
>>> +++ b/include/drm/ttm/ttm_bo_api.h
>>> @@ -56,8 +56,6 @@ struct ttm_placement;
>>> struct ttm_place;
>>> -struct ttm_lru_bulk_move;
>>> -
>>> /**
>>> * enum ttm_bo_type
>>> *
>>> @@ -95,7 +93,6 @@ struct ttm_tt;
>>> * @ttm: TTM structure holding system pages.
>>> * @evicted: Whether the object was evicted without user-space
>>> knowing.
>>> * @deleted: True if the object is only a zombie and already deleted.
>>> - * @lru: List head for the lru list.
>>> * @ddestroy: List head for the delayed destroy list.
>>> * @swap: List head for swap LRU list.
>>> * @moving: Fence set when BO is moving
>>> @@ -144,7 +141,6 @@ struct ttm_buffer_object {
>>> * Members protected by the bdev::lru_lock.
>>> */
>>> - struct list_head lru;
>>> struct list_head ddestroy;
>>> /**
>>> @@ -308,7 +304,6 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
>>> * ttm_bo_move_to_lru_tail
>>> *
>>> * @bo: The buffer object.
>>> - * @mem: Resource object.
>>> * @bulk: optional bulk move structure to remember BO positions
>>> *
>>> * Move this BO to the tail of all lru lists used to lookup and
>>> reserve an
>>> @@ -316,19 +311,8 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
>>> * held, and is used to make a BO less likely to be considered for
>>> eviction.
>>> */
>>> void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
>>> - struct ttm_resource *mem,
>>> struct ttm_lru_bulk_move *bulk);
>>> -/**
>>> - * ttm_bo_bulk_move_lru_tail
>>> - *
>>> - * @bulk: bulk move structure
>>> - *
>>> - * Bulk move BOs to the LRU tail, only valid to use when driver
>>> makes sure that
>>> - * BO order never changes. Should be called with
>>> ttm_global::lru_lock held.
>>> - */
>>> -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk);
>>> -
>>> /**
>>> * ttm_bo_lock_delayed_workqueue
>>> *
>>> diff --git a/include/drm/ttm/ttm_bo_driver.h
>>> b/include/drm/ttm/ttm_bo_driver.h
>>> index 68d6069572aa..fba2f7d3d34e 100644
>>> --- a/include/drm/ttm/ttm_bo_driver.h
>>> +++ b/include/drm/ttm/ttm_bo_driver.h
>>> @@ -45,33 +45,6 @@
>>> #include "ttm_tt.h"
>>> #include "ttm_pool.h"
>>> -/**
>>> - * struct ttm_lru_bulk_move_pos
>>> - *
>>> - * @first: first BO in the bulk move range
>>> - * @last: last BO in the bulk move range
>>> - *
>>> - * Positions for a lru bulk move.
>>> - */
>>> -struct ttm_lru_bulk_move_pos {
>>> - struct ttm_buffer_object *first;
>>> - struct ttm_buffer_object *last;
>>> -};
>>> -
>>> -/**
>>> - * struct ttm_lru_bulk_move
>>> - *
>>> - * @tt: first/last lru entry for BOs in the TT domain
>>> - * @vram: first/last lru entry for BOs in the VRAM domain
>>> - * @swap: first/last lru entry for BOs on the swap list
>>> - *
>>> - * Helper structure for bulk moves on the LRU list.
>>> - */
>>> -struct ttm_lru_bulk_move {
>>> - struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
>>> - struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
>>> -};
>>> -
>>> /*
>>> * ttm_bo.c
>>> */
>>> @@ -182,7 +155,7 @@ static inline void
>>> ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo)
>>> {
>>> spin_lock(&bo->bdev->lru_lock);
>>> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
>>> + ttm_bo_move_to_lru_tail(bo, NULL);
>>> spin_unlock(&bo->bdev->lru_lock);
>>> }
>>> diff --git a/include/drm/ttm/ttm_resource.h
>>> b/include/drm/ttm/ttm_resource.h
>>> index 526fe359c603..5f9797f9d64a 100644
>>> --- a/include/drm/ttm/ttm_resource.h
>>> +++ b/include/drm/ttm/ttm_resource.h
>>> @@ -26,10 +26,12 @@
>>> #define _TTM_RESOURCE_H_
>>> #include <linux/types.h>
>>> +#include <linux/list.h>
>>> #include <linux/mutex.h>
>>> #include <linux/atomic.h>
>>> #include <linux/dma-buf-map.h>
>>> #include <linux/dma-fence.h>
>>> +
>>> #include <drm/drm_print.h>
>>> #include <drm/ttm/ttm_caching.h>
>>> #include <drm/ttm/ttm_kmap_iter.h>
>>> @@ -177,6 +179,33 @@ struct ttm_resource {
>>> uint32_t placement;
>>> struct ttm_bus_placement bus;
>>> struct ttm_buffer_object *bo;
>>> + struct list_head lru;
>>> +};
>>> +
>>> +/**
>>> + * struct ttm_lru_bulk_move_pos
>>> + *
>>> + * @first: first res in the bulk move range
>>> + * @last: last res in the bulk move range
>>> + *
>>> + * Positions for a lru bulk move.
>>> + */
>>> +struct ttm_lru_bulk_move_pos {
>>> + struct ttm_resource *first;
>>> + struct ttm_resource *last;
>>> +};
>>> +
>>> +/**
>>> + * struct ttm_lru_bulk_move
>>> + *
>>> + * @tt: first/last lru entry for resources in the TT domain
>>> + * @vram: first/last lru entry for resources in the VRAM domain
>>> + *
>>> + * Helper structure for bulk moves on the LRU list.
>>> + */
>>> +struct ttm_lru_bulk_move {
>>> + struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
>>> + struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
>>> };
>>> /**
>>> @@ -278,6 +307,12 @@ ttm_resource_manager_usage(struct
>>> ttm_resource_manager *man)
>>> return atomic64_read(&man->usage);
>>> }
>>> +void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk);
>>> +void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk);
>>> +
>>> +void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
>>> + struct ttm_lru_bulk_move *bulk);
>>> +
>>> void ttm_resource_init(struct ttm_buffer_object *bo,
>>> const struct ttm_place *place,
>>> struct ttm_resource *res);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 05/12] drm/ttm: move the LRU into resource handling
2021-08-30 17:00 ` Andrey Grodzovsky
@ 2021-08-30 17:02 ` Christian König
0 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2021-08-30 17:02 UTC (permalink / raw)
To: Andrey Grodzovsky, thomas.hellstrom, dri-devel
Am 30.08.21 um 19:00 schrieb Andrey Grodzovsky:
>
> On 2021-08-30 12:55 p.m., Christian König wrote:
>> Am 30.08.21 um 18:53 schrieb Andrey Grodzovsky:
>>>
>>> On 2021-08-30 4:57 a.m., Christian König wrote:
>>>> This way we finally fix the problem that new resource are
>>>> not immediately evict-able after allocation.
>>>>
>>>> That has caused numerous problems including OOM on GDS handling
>>>> and not being able to use TTM as general resource manager.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +-
>>>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
>>>> drivers/gpu/drm/ttm/ttm_bo.c | 105 ++------------------
>>>> drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
>>>> drivers/gpu/drm/ttm/ttm_device.c | 8 +-
>>>> drivers/gpu/drm/ttm/ttm_resource.c | 127
>>>> ++++++++++++++++++++++++
>>>> include/drm/ttm/ttm_bo_api.h | 16 ---
>>>> include/drm/ttm/ttm_bo_driver.h | 29 +-----
>>>> include/drm/ttm/ttm_resource.h | 35 +++++++
>>>> 9 files changed, 181 insertions(+), 150 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> index 6362e861a3f5..70d2cbb1dbb4 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> @@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct
>>>> amdgpu_device *adev,
>>>> if (vm->bulk_moveable) {
>>>> spin_lock(&adev->mman.bdev.lru_lock);
>>>> - ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move);
>>>> + ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
>>>> spin_unlock(&adev->mman.bdev.lru_lock);
>>>> return;
>>>> }
>>>> - memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move));
>>>> + ttm_lru_bulk_move_init(&vm->lru_bulk_move);
>>>> spin_lock(&adev->mman.bdev.lru_lock);
>>>> list_for_each_entry(bo_base, &vm->idle, vm_status) {
>>>> @@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct
>>>> amdgpu_device *adev,
>>>> if (!bo->parent)
>>>> continue;
>>>> - ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource,
>>>> - &vm->lru_bulk_move);
>>>> + ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move);
>>>> if (shadow)
>>>> ttm_bo_move_to_lru_tail(&shadow->tbo,
>>>> - shadow->tbo.resource,
>>>> &vm->lru_bulk_move);
>>>> }
>>>> spin_unlock(&adev->mman.bdev.lru_lock);
>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>> index e646aac9d7a4..41f0de841d72 100644
>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>> @@ -471,7 +471,7 @@ static void i915_ttm_adjust_lru(struct
>>>> drm_i915_gem_object *obj)
>>>> bo->priority = I915_TTM_PRIO_NO_PAGES;
>>>> }
>>>> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
>>>> + ttm_bo_move_to_lru_tail(bo, NULL);
>>>> spin_unlock(&bo->bdev->lru_lock);
>>>> }
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> index 49f4bc97c35a..d5c6e096fd31 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> @@ -69,98 +69,16 @@ static void ttm_bo_mem_space_debug(struct
>>>> ttm_buffer_object *bo,
>>>> }
>>>> }
>>>> -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>>>> -{
>>>> - struct ttm_device *bdev = bo->bdev;
>>>> -
>>>> - list_del_init(&bo->lru);
>>>> -
>>>> - if (bdev->funcs->del_from_lru_notify)
>>>> - bdev->funcs->del_from_lru_notify(bo);
>>>> -}
>>>> -
>>>> -static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos
>>>> *pos,
>>>> - struct ttm_buffer_object *bo)
>>>> -{
>>>> - if (!pos->first)
>>>> - pos->first = bo;
>>>> - pos->last = bo;
>>>> -}
>>>> -
>>>> void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
>>>> - struct ttm_resource *mem,
>>>> struct ttm_lru_bulk_move *bulk)
>>>> {
>>>> - struct ttm_device *bdev = bo->bdev;
>>>> - struct ttm_resource_manager *man;
>>>> -
>>>> - if (!bo->deleted)
>>>> - dma_resv_assert_held(bo->base.resv);
>>>> -
>>>> - if (bo->pin_count) {
>>>> - ttm_bo_del_from_lru(bo);
>>>> - return;
>>>> - }
>>>> -
>>>> - if (!mem)
>>>> - return;
>>>> -
>>>> - man = ttm_manager_type(bdev, mem->mem_type);
>>>> - list_move_tail(&bo->lru, &man->lru[bo->priority]);
>>>> -
>>>> - if (bdev->funcs->del_from_lru_notify)
>>>> - bdev->funcs->del_from_lru_notify(bo);
>>>> -
>>>> - if (bulk && !bo->pin_count) {
>>>> - switch (bo->resource->mem_type) {
>>>> - case TTM_PL_TT:
>>>> - ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo);
>>>> - break;
>>>> + dma_resv_assert_held(bo->base.resv);
>>>> - case TTM_PL_VRAM:
>>>> - ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo);
>>>> - break;
>>>> - }
>>>> - }
>>>> + if (bo->resource)
>>>> + ttm_resource_move_to_lru_tail(bo->resource, bulk);
>>>> }
>>>> EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
>>>> -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
>>>> -{
>>>> - unsigned i;
>>>> -
>>>> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>>> - struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
>>>> - struct ttm_resource_manager *man;
>>>> -
>>>> - if (!pos->first)
>>>> - continue;
>>>> -
>>>> - dma_resv_assert_held(pos->first->base.resv);
>>>> - dma_resv_assert_held(pos->last->base.resv);
>>>> -
>>>> - man = ttm_manager_type(pos->first->bdev, TTM_PL_TT);
>>>> - list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>>>> - &pos->last->lru);
>>>> - }
>>>> -
>>>> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>>> - struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
>>>> - struct ttm_resource_manager *man;
>>>> -
>>>> - if (!pos->first)
>>>> - continue;
>>>> -
>>>> - dma_resv_assert_held(pos->first->base.resv);
>>>> - dma_resv_assert_held(pos->last->base.resv);
>>>> -
>>>> - man = ttm_manager_type(pos->first->bdev, TTM_PL_VRAM);
>>>> - list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>>>> - &pos->last->lru);
>>>> - }
>>>> -}
>>>> -EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
>>>> -
>>>> static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>>>> struct ttm_resource *mem, bool evict,
>>>> struct ttm_operation_ctx *ctx,
>>>> @@ -342,7 +260,6 @@ static int ttm_bo_cleanup_refs(struct
>>>> ttm_buffer_object *bo,
>>>> return ret;
>>>> }
>>>> - ttm_bo_del_from_lru(bo);
>>>
>>>
>>> Here and in other places this was removed, I assume ttm_resource_fini
>>> should replace it but I don't see where exactly this takes place.
>>
>> Take a look at function ttm_resource_fini().
>>
>> Christian.
>
>
> What I mean is that I don't see where ttm_resource_fini will be called to
> compensate for removal of ttm_bo_del_from_lru here.
That always called during normal resource destruction, e.g. when we move
a BO from A to B or destroy a BO.
Otherwise we would leak memory and notice really fast.
Christian.
>
> Andrey
>
>
>>
>>>
>>> Andrey
>>>
>>>
>>>> list_del_init(&bo->ddestroy);
>>>> spin_unlock(&bo->bdev->lru_lock);
>>>> ttm_bo_cleanup_memtype_use(bo);
>>>> @@ -443,7 +360,7 @@ static void ttm_bo_release(struct kref *kref)
>>>> */
>>>> if (bo->pin_count) {
>>>> bo->pin_count = 0;
>>>> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
>>>> + ttm_resource_move_to_lru_tail(bo->resource, NULL);
>>>> }
>>>> kref_init(&bo->kref);
>>>> @@ -456,7 +373,6 @@ static void ttm_bo_release(struct kref *kref)
>>>> }
>>>> spin_lock(&bo->bdev->lru_lock);
>>>> - ttm_bo_del_from_lru(bo);
>>>> list_del(&bo->ddestroy);
>>>> spin_unlock(&bo->bdev->lru_lock);
>>>> @@ -670,15 +586,17 @@ int ttm_mem_evict_first(struct ttm_device
>>>> *bdev,
>>>> struct ww_acquire_ctx *ticket)
>>>> {
>>>> struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
>>>> + struct ttm_resource *res;
>>>> bool locked = false;
>>>> unsigned i;
>>>> int ret;
>>>> spin_lock(&bdev->lru_lock);
>>>> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>>> - list_for_each_entry(bo, &man->lru[i], lru) {
>>>> + list_for_each_entry(res, &man->lru[i], lru) {
>>>> bool busy;
>>>> + bo = res->bo;
>>>> if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
>>>> &locked, &busy)) {
>>>> if (busy && !busy_bo && ticket !=
>>>> @@ -696,7 +614,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
>>>> }
>>>> /* If the inner loop terminated early, we have our
>>>> candidate */
>>>> - if (&bo->lru != &man->lru[i])
>>>> + if (&res->lru != &man->lru[i])
>>>> break;
>>>> bo = NULL;
>>>> @@ -870,9 +788,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>>>> }
>>>> error:
>>>> - if (bo->resource->mem_type == TTM_PL_SYSTEM && !bo->pin_count)
>>>> - ttm_bo_move_to_lru_tail_unlocked(bo);
>>>> -
>>>> return ret;
>>>> }
>>>> EXPORT_SYMBOL(ttm_bo_mem_space);
>>>> @@ -1012,7 +927,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>>>> bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
>>>> kref_init(&bo->kref);
>>>> - INIT_LIST_HEAD(&bo->lru);
>>>> INIT_LIST_HEAD(&bo->ddestroy);
>>>> bo->bdev = bdev;
>>>> bo->type = type;
>>>> @@ -1062,8 +976,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>>>> return ret;
>>>> }
>>>> - ttm_bo_move_to_lru_tail_unlocked(bo);
>>>> -
>>>> return ret;
>>>> }
>>>> EXPORT_SYMBOL(ttm_bo_init_reserved);
>>>> @@ -1165,7 +1077,6 @@ int ttm_bo_swapout(struct ttm_buffer_object
>>>> *bo, struct ttm_operation_ctx *ctx,
>>>> return 0;
>>>> }
>>>> - ttm_bo_del_from_lru(bo);
>>>> /* TODO: Cleanup the locking */
>>>> spin_unlock(&bo->bdev->lru_lock);
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
>>>> b/drivers/gpu/drm/ttm/ttm_bo_util.c
>>>> index c5d02edaefc0..49b4bedf8715 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>>>> @@ -232,7 +232,6 @@ static int ttm_buffer_object_transfer(struct
>>>> ttm_buffer_object *bo,
>>>> atomic_inc(&ttm_glob.bo_count);
>>>> INIT_LIST_HEAD(&fbo->base.ddestroy);
>>>> - INIT_LIST_HEAD(&fbo->base.lru);
>>>> fbo->base.moving = NULL;
>>>> drm_vma_node_reset(&fbo->base.base.vma_node);
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_device.c
>>>> b/drivers/gpu/drm/ttm/ttm_device.c
>>>> index 519deea8e39b..9e0dfceff68c 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>>> @@ -134,6 +134,7 @@ int ttm_device_swapout(struct ttm_device *bdev,
>>>> struct ttm_operation_ctx *ctx,
>>>> {
>>>> struct ttm_resource_manager *man;
>>>> struct ttm_buffer_object *bo;
>>>> + struct ttm_resource *res;
>>>> unsigned i, j;
>>>> int ret;
>>>> @@ -144,8 +145,11 @@ int ttm_device_swapout(struct ttm_device
>>>> *bdev, struct ttm_operation_ctx *ctx,
>>>> continue;
>>>> for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
>>>> - list_for_each_entry(bo, &man->lru[j], lru) {
>>>> - uint32_t num_pages = PFN_UP(bo->base.size);
>>>> + list_for_each_entry(res, &man->lru[j], lru) {
>>>> + uint32_t num_pages;
>>>> +
>>>> + bo = res->bo;
>>>> + num_pages = PFN_UP(bo->base.size);
>>>> ret = ttm_bo_swapout(bo, ctx, gfp_flags);
>>>> /* ttm_bo_swapout has dropped the lru_lock */
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
>>>> b/drivers/gpu/drm/ttm/ttm_resource.c
>>>> index 426e6841fc89..355c542758b5 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>>>> @@ -29,6 +29,115 @@
>>>> #include <drm/ttm/ttm_resource.h>
>>>> #include <drm/ttm/ttm_bo_driver.h>
>>>> +/**
>>>> + * ttm_lru_bulk_move_init - initialize a bulk move structure
>>>> + * @bulk: the structure to init
>>>> + *
>>>> + * For now just memset the structure to zero.
>>>> + */
>>>> +void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk)
>>>> +{
>>>> + memset(bulk, 0, sizeof(*bulk));
>>>> +}
>>>> +EXPORT_SYMBOL(ttm_lru_bulk_move_init);
>>>> +
>>>> +/**
>>>> + * ttm_lru_bulk_move_tail
>>>> + *
>>>> + * @bulk: bulk move structure
>>>> + *
>>>> + * Bulk move BOs to the LRU tail, only valid to use when driver
>>>> makes sure that
>>>> + * resource order never changes. Should be called with
>>>> ttm_device::lru_lock held.
>>>> + */
>>>> +void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
>>>> +{
>>>> + unsigned i;
>>>> +
>>>> + for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>>> + struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
>>>> + struct ttm_resource_manager *man;
>>>> +
>>>> + if (!pos->first)
>>>> + continue;
>>>> +
>>>> + dma_resv_assert_held(pos->first->bo->base.resv);
>>>> + dma_resv_assert_held(pos->last->bo->base.resv);
>>>> +
>>>> + man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_TT);
>>>> + list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>>>> + &pos->last->lru);
>>>> + }
>>>> +
>>>> + for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>>> + struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
>>>> + struct ttm_resource_manager *man;
>>>> +
>>>> + if (!pos->first)
>>>> + continue;
>>>> +
>>>> + dma_resv_assert_held(pos->first->bo->base.resv);
>>>> + dma_resv_assert_held(pos->last->bo->base.resv);
>>>> +
>>>> + man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_VRAM);
>>>> + list_bulk_move_tail(&man->lru[i], &pos->first->lru,
>>>> + &pos->last->lru);
>>>> + }
>>>> +}
>>>> +EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
>>>> +
>>>> +/* Record a resource position in a bulk move structure */
>>>> +static void ttm_lru_bulk_move_set_pos(struct ttm_lru_bulk_move_pos
>>>> *pos,
>>>> + struct ttm_resource *res)
>>>> +{
>>>> + if (!pos->first)
>>>> + pos->first = res;
>>>> + pos->last = res;
>>>> +}
>>>> +
>>>> +/* Remove a resource from the LRU */
>>>> +static void ttm_resource_del_from_lru(struct ttm_resource *res)
>>>> +{
>>>> + struct ttm_device *bdev = res->bo->bdev;
>>>> +
>>>> + list_del_init(&res->lru);
>>>> +
>>>> + if (bdev->funcs->del_from_lru_notify)
>>>> + bdev->funcs->del_from_lru_notify(res->bo);
>>>> +}
>>>> +
>>>> +/* Move a resource to the LRU tail and track the bulk position */
>>>> +void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
>>>> + struct ttm_lru_bulk_move *bulk)
>>>> +{
>>>> + struct ttm_buffer_object *bo = res->bo;
>>>> + struct ttm_device *bdev = bo->bdev;
>>>> + struct ttm_resource_manager *man;
>>>> +
>>>> + if (bo->pin_count) {
>>>> + ttm_resource_del_from_lru(res);
>>>> + return;
>>>> + }
>>>> +
>>>> + man = ttm_manager_type(bdev, res->mem_type);
>>>> + list_move_tail(&res->lru, &man->lru[bo->priority]);
>>>> +
>>>> + if (bdev->funcs->del_from_lru_notify)
>>>> + bdev->funcs->del_from_lru_notify(bo);
>>>> +
>>>> + if (!bulk)
>>>> + return;
>>>> +
>>>> + switch (res->mem_type) {
>>>> + case TTM_PL_TT:
>>>> + ttm_lru_bulk_move_set_pos(&bulk->tt[bo->priority], res);
>>>> + break;
>>>> +
>>>> + case TTM_PL_VRAM:
>>>> + ttm_lru_bulk_move_set_pos(&bulk->vram[bo->priority], res);
>>>> + break;
>>>> + }
>>>> +}
>>>> +
>>>> void ttm_resource_init(struct ttm_buffer_object *bo,
>>>> const struct ttm_place *place,
>>>> struct ttm_resource *res)
>>>> @@ -44,15 +153,33 @@ void ttm_resource_init(struct
>>>> ttm_buffer_object *bo,
>>>> res->bus.is_iomem = false;
>>>> res->bus.caching = ttm_cached;
>>>> res->bo = bo;
>>>> + INIT_LIST_HEAD(&res->lru);
>>>> man = ttm_manager_type(bo->bdev, place->mem_type);
>>>> atomic64_add(bo->base.size, &man->usage);
>>>> +
>>>> + spin_lock(&bo->bdev->lru_lock);
>>>> + ttm_resource_move_to_lru_tail(res, NULL);
>>>> + spin_unlock(&bo->bdev->lru_lock);
>>>> }
>>>> EXPORT_SYMBOL(ttm_resource_init);
>>>> +/**
>>>> + * ttm_resource_fini
>>>> + *
>>>> + * @res: the resource to clean up
>>>> + *
>>>> + * Make sure the resource is removed from the LRU before destruction.
>>>> + */
>>>> void ttm_resource_fini(struct ttm_resource_manager *man,
>>>> struct ttm_resource *res)
>>>> {
>>>> + struct ttm_device *bdev = res->bo->bdev;
>>>> +
>>>> + spin_lock(&bdev->lru_lock);
>>>> + ttm_resource_del_from_lru(res);
>>>> + spin_unlock(&bdev->lru_lock);
>>>> +
>>>> atomic64_sub(res->bo->base.size, &man->usage);
>>>> }
>>>> EXPORT_SYMBOL(ttm_resource_fini);
>>>> diff --git a/include/drm/ttm/ttm_bo_api.h
>>>> b/include/drm/ttm/ttm_bo_api.h
>>>> index f681bbdbc698..0928d8cfb45a 100644
>>>> --- a/include/drm/ttm/ttm_bo_api.h
>>>> +++ b/include/drm/ttm/ttm_bo_api.h
>>>> @@ -56,8 +56,6 @@ struct ttm_placement;
>>>> struct ttm_place;
>>>> -struct ttm_lru_bulk_move;
>>>> -
>>>> /**
>>>> * enum ttm_bo_type
>>>> *
>>>> @@ -95,7 +93,6 @@ struct ttm_tt;
>>>> * @ttm: TTM structure holding system pages.
>>>> * @evicted: Whether the object was evicted without user-space
>>>> knowing.
>>>> * @deleted: True if the object is only a zombie and already
>>>> deleted.
>>>> - * @lru: List head for the lru list.
>>>> * @ddestroy: List head for the delayed destroy list.
>>>> * @swap: List head for swap LRU list.
>>>> * @moving: Fence set when BO is moving
>>>> @@ -144,7 +141,6 @@ struct ttm_buffer_object {
>>>> * Members protected by the bdev::lru_lock.
>>>> */
>>>> - struct list_head lru;
>>>> struct list_head ddestroy;
>>>> /**
>>>> @@ -308,7 +304,6 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
>>>> * ttm_bo_move_to_lru_tail
>>>> *
>>>> * @bo: The buffer object.
>>>> - * @mem: Resource object.
>>>> * @bulk: optional bulk move structure to remember BO positions
>>>> *
>>>> * Move this BO to the tail of all lru lists used to lookup and
>>>> reserve an
>>>> @@ -316,19 +311,8 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
>>>> * held, and is used to make a BO less likely to be considered
>>>> for eviction.
>>>> */
>>>> void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
>>>> - struct ttm_resource *mem,
>>>> struct ttm_lru_bulk_move *bulk);
>>>> -/**
>>>> - * ttm_bo_bulk_move_lru_tail
>>>> - *
>>>> - * @bulk: bulk move structure
>>>> - *
>>>> - * Bulk move BOs to the LRU tail, only valid to use when driver
>>>> makes sure that
>>>> - * BO order never changes. Should be called with
>>>> ttm_global::lru_lock held.
>>>> - */
>>>> -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk);
>>>> -
>>>> /**
>>>> * ttm_bo_lock_delayed_workqueue
>>>> *
>>>> diff --git a/include/drm/ttm/ttm_bo_driver.h
>>>> b/include/drm/ttm/ttm_bo_driver.h
>>>> index 68d6069572aa..fba2f7d3d34e 100644
>>>> --- a/include/drm/ttm/ttm_bo_driver.h
>>>> +++ b/include/drm/ttm/ttm_bo_driver.h
>>>> @@ -45,33 +45,6 @@
>>>> #include "ttm_tt.h"
>>>> #include "ttm_pool.h"
>>>> -/**
>>>> - * struct ttm_lru_bulk_move_pos
>>>> - *
>>>> - * @first: first BO in the bulk move range
>>>> - * @last: last BO in the bulk move range
>>>> - *
>>>> - * Positions for a lru bulk move.
>>>> - */
>>>> -struct ttm_lru_bulk_move_pos {
>>>> - struct ttm_buffer_object *first;
>>>> - struct ttm_buffer_object *last;
>>>> -};
>>>> -
>>>> -/**
>>>> - * struct ttm_lru_bulk_move
>>>> - *
>>>> - * @tt: first/last lru entry for BOs in the TT domain
>>>> - * @vram: first/last lru entry for BOs in the VRAM domain
>>>> - * @swap: first/last lru entry for BOs on the swap list
>>>> - *
>>>> - * Helper structure for bulk moves on the LRU list.
>>>> - */
>>>> -struct ttm_lru_bulk_move {
>>>> - struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
>>>> - struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
>>>> -};
>>>> -
>>>> /*
>>>> * ttm_bo.c
>>>> */
>>>> @@ -182,7 +155,7 @@ static inline void
>>>> ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo)
>>>> {
>>>> spin_lock(&bo->bdev->lru_lock);
>>>> - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
>>>> + ttm_bo_move_to_lru_tail(bo, NULL);
>>>> spin_unlock(&bo->bdev->lru_lock);
>>>> }
>>>> diff --git a/include/drm/ttm/ttm_resource.h
>>>> b/include/drm/ttm/ttm_resource.h
>>>> index 526fe359c603..5f9797f9d64a 100644
>>>> --- a/include/drm/ttm/ttm_resource.h
>>>> +++ b/include/drm/ttm/ttm_resource.h
>>>> @@ -26,10 +26,12 @@
>>>> #define _TTM_RESOURCE_H_
>>>> #include <linux/types.h>
>>>> +#include <linux/list.h>
>>>> #include <linux/mutex.h>
>>>> #include <linux/atomic.h>
>>>> #include <linux/dma-buf-map.h>
>>>> #include <linux/dma-fence.h>
>>>> +
>>>> #include <drm/drm_print.h>
>>>> #include <drm/ttm/ttm_caching.h>
>>>> #include <drm/ttm/ttm_kmap_iter.h>
>>>> @@ -177,6 +179,33 @@ struct ttm_resource {
>>>> uint32_t placement;
>>>> struct ttm_bus_placement bus;
>>>> struct ttm_buffer_object *bo;
>>>> + struct list_head lru;
>>>> +};
>>>> +
>>>> +/**
>>>> + * struct ttm_lru_bulk_move_pos
>>>> + *
>>>> + * @first: first res in the bulk move range
>>>> + * @last: last res in the bulk move range
>>>> + *
>>>> + * Positions for a lru bulk move.
>>>> + */
>>>> +struct ttm_lru_bulk_move_pos {
>>>> + struct ttm_resource *first;
>>>> + struct ttm_resource *last;
>>>> +};
>>>> +
>>>> +/**
>>>> + * struct ttm_lru_bulk_move
>>>> + *
>>>> + * @tt: first/last lru entry for resources in the TT domain
>>>> + * @vram: first/last lru entry for resources in the VRAM domain
>>>> + *
>>>> + * Helper structure for bulk moves on the LRU list.
>>>> + */
>>>> +struct ttm_lru_bulk_move {
>>>> + struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
>>>> + struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
>>>> };
>>>> /**
>>>> @@ -278,6 +307,12 @@ ttm_resource_manager_usage(struct
>>>> ttm_resource_manager *man)
>>>> return atomic64_read(&man->usage);
>>>> }
>>>> +void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk);
>>>> +void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk);
>>>> +
>>>> +void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
>>>> + struct ttm_lru_bulk_move *bulk);
>>>> +
>>>> void ttm_resource_init(struct ttm_buffer_object *bo,
>>>> const struct ttm_place *place,
>>>> struct ttm_resource *res);
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 04/12] drm/ttm: add common accounting to the resource mgr
2021-08-30 8:56 ` [PATCH 04/12] drm/ttm: add common accounting to the resource mgr Christian König
@ 2021-08-31 12:52 ` Daniel Vetter
2021-08-31 12:57 ` Christian König
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2021-08-31 12:52 UTC (permalink / raw)
To: Christian König; +Cc: thomas.hellstrom, dri-devel, andrey.grodzovsky
On Mon, Aug 30, 2021 at 10:56:59AM +0200, Christian König wrote:
> It makes sense to have this in the common manager for debugging and
> accounting of how much resources are used.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/ttm/ttm_resource.c | 8 ++++++++
> include/drm/ttm/ttm_resource.h | 18 ++++++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index a4c495da0040..426e6841fc89 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -33,6 +33,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
> const struct ttm_place *place,
> struct ttm_resource *res)
> {
> + struct ttm_resource_manager *man;
> +
> res->start = 0;
> res->num_pages = PFN_UP(bo->base.size);
> res->mem_type = place->mem_type;
> @@ -42,12 +44,16 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
> res->bus.is_iomem = false;
> res->bus.caching = ttm_cached;
> res->bo = bo;
> +
> + man = ttm_manager_type(bo->bdev, place->mem_type);
> + atomic64_add(bo->base.size, &man->usage);
> }
> EXPORT_SYMBOL(ttm_resource_init);
>
> void ttm_resource_fini(struct ttm_resource_manager *man,
> struct ttm_resource *res)
> {
> + atomic64_sub(res->bo->base.size, &man->usage);
> }
> EXPORT_SYMBOL(ttm_resource_fini);
>
> @@ -100,6 +106,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
> spin_lock_init(&man->move_lock);
> man->bdev = bdev;
> man->size = p_size;
> + atomic64_set(&man->usage, 0);
>
> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
> INIT_LIST_HEAD(&man->lru[i]);
> @@ -172,6 +179,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
> drm_printf(p, " use_type: %d\n", man->use_type);
> drm_printf(p, " use_tt: %d\n", man->use_tt);
> drm_printf(p, " size: %llu\n", man->size);
> + drm_printf(p, " usage: %llu\n", atomic64_read(&man->usage));
> if (man->func->debug)
> man->func->debug(man, p);
> }
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index e8080192cae4..526fe359c603 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -27,6 +27,7 @@
>
> #include <linux/types.h>
> #include <linux/mutex.h>
> +#include <linux/atomic.h>
> #include <linux/dma-buf-map.h>
> #include <linux/dma-fence.h>
> #include <drm/drm_print.h>
> @@ -110,6 +111,7 @@ struct ttm_resource_manager_func {
> * static information. bdev::driver::io_mem_free is never used.
> * @lru: The lru list for this memory type.
> * @move: The fence of the last pipelined move operation.
> + * @usage: How much of the region is used.
> *
> * This structure is used to identify and manage memory types for a device.
> */
> @@ -134,6 +136,9 @@ struct ttm_resource_manager {
> * Protected by @move_lock.
> */
> struct dma_fence *move;
> +
> + /* Own protection */
> + atomic64_t usage;
Shouldn't we keep track of this together with the lru updates, under the
same spinlock?
Otherwise this usage here just becomes kinda meaningless I think, and just
for some debugging. I really don't like sprinkling random atomic_t around
(mostly because i915-gem code has gone totally overboard with them, with
complete disregard to complexity of the result).
-Daniel
> };
>
> /**
> @@ -260,6 +265,19 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man)
> man->move = NULL;
> }
>
> +/**
> + * ttm_resource_manager_usage
> + *
> + * @man: A memory manager object.
> + *
> + * Return how many resources are currently used.
> + */
> +static inline uint64_t
> +ttm_resource_manager_usage(struct ttm_resource_manager *man)
> +{
> + return atomic64_read(&man->usage);
> +}
> +
> void ttm_resource_init(struct ttm_buffer_object *bo,
> const struct ttm_place *place,
> struct ttm_resource *res);
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 04/12] drm/ttm: add common accounting to the resource mgr
2021-08-31 12:52 ` Daniel Vetter
@ 2021-08-31 12:57 ` Christian König
2021-08-31 13:15 ` Daniel Vetter
0 siblings, 1 reply; 19+ messages in thread
From: Christian König @ 2021-08-31 12:57 UTC (permalink / raw)
To: Daniel Vetter; +Cc: thomas.hellstrom, dri-devel, andrey.grodzovsky
Am 31.08.21 um 14:52 schrieb Daniel Vetter:
> On Mon, Aug 30, 2021 at 10:56:59AM +0200, Christian König wrote:
>> It makes sense to have this in the common manager for debugging and
>> accounting of how much resources are used.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>> drivers/gpu/drm/ttm/ttm_resource.c | 8 ++++++++
>> include/drm/ttm/ttm_resource.h | 18 ++++++++++++++++++
>> 2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
>> index a4c495da0040..426e6841fc89 100644
>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>> @@ -33,6 +33,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
>> const struct ttm_place *place,
>> struct ttm_resource *res)
>> {
>> + struct ttm_resource_manager *man;
>> +
>> res->start = 0;
>> res->num_pages = PFN_UP(bo->base.size);
>> res->mem_type = place->mem_type;
>> @@ -42,12 +44,16 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
>> res->bus.is_iomem = false;
>> res->bus.caching = ttm_cached;
>> res->bo = bo;
>> +
>> + man = ttm_manager_type(bo->bdev, place->mem_type);
>> + atomic64_add(bo->base.size, &man->usage);
>> }
>> EXPORT_SYMBOL(ttm_resource_init);
>>
>> void ttm_resource_fini(struct ttm_resource_manager *man,
>> struct ttm_resource *res)
>> {
>> + atomic64_sub(res->bo->base.size, &man->usage);
>> }
>> EXPORT_SYMBOL(ttm_resource_fini);
>>
>> @@ -100,6 +106,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
>> spin_lock_init(&man->move_lock);
>> man->bdev = bdev;
>> man->size = p_size;
>> + atomic64_set(&man->usage, 0);
>>
>> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
>> INIT_LIST_HEAD(&man->lru[i]);
>> @@ -172,6 +179,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
>> drm_printf(p, " use_type: %d\n", man->use_type);
>> drm_printf(p, " use_tt: %d\n", man->use_tt);
>> drm_printf(p, " size: %llu\n", man->size);
>> + drm_printf(p, " usage: %llu\n", atomic64_read(&man->usage));
>> if (man->func->debug)
>> man->func->debug(man, p);
>> }
>> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
>> index e8080192cae4..526fe359c603 100644
>> --- a/include/drm/ttm/ttm_resource.h
>> +++ b/include/drm/ttm/ttm_resource.h
>> @@ -27,6 +27,7 @@
>>
>> #include <linux/types.h>
>> #include <linux/mutex.h>
>> +#include <linux/atomic.h>
>> #include <linux/dma-buf-map.h>
>> #include <linux/dma-fence.h>
>> #include <drm/drm_print.h>
>> @@ -110,6 +111,7 @@ struct ttm_resource_manager_func {
>> * static information. bdev::driver::io_mem_free is never used.
>> * @lru: The lru list for this memory type.
>> * @move: The fence of the last pipelined move operation.
>> + * @usage: How much of the region is used.
>> *
>> * This structure is used to identify and manage memory types for a device.
>> */
>> @@ -134,6 +136,9 @@ struct ttm_resource_manager {
>> * Protected by @move_lock.
>> */
>> struct dma_fence *move;
>> +
>> + /* Own protection */
>> + atomic64_t usage;
> Shouldn't we keep track of this together with the lru updates, under the
> same spinlock?
Mhm, what should that be good for? As far as I know we use it for two
use cases:
1. Early abort when size-usage < requested.
2. Statistics for debugging.
Especially the first use case is rather important under memory pressure
to avoid costly acquiring of a contended lock.
> Otherwise this usage here just becomes kinda meaningless I think, and just
> for some debugging. I really don't like sprinkling random atomic_t around
> (mostly because i915-gem code has gone totally overboard with them, with
> complete disregard to complexity of the result).
Well this here just replaces what drivers did anyway and the cost of
inc/dec an atomic is pretty much negligible.
Christian.
> -Daniel
>
>> };
>>
>> /**
>> @@ -260,6 +265,19 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man)
>> man->move = NULL;
>> }
>>
>> +/**
>> + * ttm_resource_manager_usage
>> + *
>> + * @man: A memory manager object.
>> + *
>> + * Return how many resources are currently used.
>> + */
>> +static inline uint64_t
>> +ttm_resource_manager_usage(struct ttm_resource_manager *man)
>> +{
>> + return atomic64_read(&man->usage);
>> +}
>> +
>> void ttm_resource_init(struct ttm_buffer_object *bo,
>> const struct ttm_place *place,
>> struct ttm_resource *res);
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 04/12] drm/ttm: add common accounting to the resource mgr
2021-08-31 12:57 ` Christian König
@ 2021-08-31 13:15 ` Daniel Vetter
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-08-31 13:15 UTC (permalink / raw)
To: Christian König
Cc: Daniel Vetter, thomas.hellstrom, dri-devel, andrey.grodzovsky
On Tue, Aug 31, 2021 at 02:57:05PM +0200, Christian König wrote:
> Am 31.08.21 um 14:52 schrieb Daniel Vetter:
> > On Mon, Aug 30, 2021 at 10:56:59AM +0200, Christian König wrote:
> > > It makes sense to have this in the common manager for debugging and
> > > accounting of how much resources are used.
> > >
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > ---
> > > drivers/gpu/drm/ttm/ttm_resource.c | 8 ++++++++
> > > include/drm/ttm/ttm_resource.h | 18 ++++++++++++++++++
> > > 2 files changed, 26 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> > > index a4c495da0040..426e6841fc89 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_resource.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> > > @@ -33,6 +33,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
> > > const struct ttm_place *place,
> > > struct ttm_resource *res)
> > > {
> > > + struct ttm_resource_manager *man;
> > > +
> > > res->start = 0;
> > > res->num_pages = PFN_UP(bo->base.size);
> > > res->mem_type = place->mem_type;
> > > @@ -42,12 +44,16 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
> > > res->bus.is_iomem = false;
> > > res->bus.caching = ttm_cached;
> > > res->bo = bo;
> > > +
> > > + man = ttm_manager_type(bo->bdev, place->mem_type);
> > > + atomic64_add(bo->base.size, &man->usage);
> > > }
> > > EXPORT_SYMBOL(ttm_resource_init);
> > > void ttm_resource_fini(struct ttm_resource_manager *man,
> > > struct ttm_resource *res)
> > > {
> > > + atomic64_sub(res->bo->base.size, &man->usage);
> > > }
> > > EXPORT_SYMBOL(ttm_resource_fini);
> > > @@ -100,6 +106,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
> > > spin_lock_init(&man->move_lock);
> > > man->bdev = bdev;
> > > man->size = p_size;
> > > + atomic64_set(&man->usage, 0);
> > > for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
> > > INIT_LIST_HEAD(&man->lru[i]);
> > > @@ -172,6 +179,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
> > > drm_printf(p, " use_type: %d\n", man->use_type);
> > > drm_printf(p, " use_tt: %d\n", man->use_tt);
> > > drm_printf(p, " size: %llu\n", man->size);
> > > + drm_printf(p, " usage: %llu\n", atomic64_read(&man->usage));
> > > if (man->func->debug)
> > > man->func->debug(man, p);
> > > }
> > > diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> > > index e8080192cae4..526fe359c603 100644
> > > --- a/include/drm/ttm/ttm_resource.h
> > > +++ b/include/drm/ttm/ttm_resource.h
> > > @@ -27,6 +27,7 @@
> > > #include <linux/types.h>
> > > #include <linux/mutex.h>
> > > +#include <linux/atomic.h>
> > > #include <linux/dma-buf-map.h>
> > > #include <linux/dma-fence.h>
> > > #include <drm/drm_print.h>
> > > @@ -110,6 +111,7 @@ struct ttm_resource_manager_func {
> > > * static information. bdev::driver::io_mem_free is never used.
> > > * @lru: The lru list for this memory type.
> > > * @move: The fence of the last pipelined move operation.
> > > + * @usage: How much of the region is used.
> > > *
> > > * This structure is used to identify and manage memory types for a device.
> > > */
> > > @@ -134,6 +136,9 @@ struct ttm_resource_manager {
> > > * Protected by @move_lock.
> > > */
> > > struct dma_fence *move;
> > > +
> > > + /* Own protection */
Please document struct members with the inline kerneldoc style, that way
the comment here shows up in the kerneldoc too.
Would be good to just convert the entire struct I think.
> > > + atomic64_t usage;
> > Shouldn't we keep track of this together with the lru updates, under the
> > same spinlock?
>
> Mhm, what should that be good for? As far as I know we use it for two use
> cases:
> 1. Early abort when size-usage < requested.
But doesn't have all kinds of potential temporary leaks again? Except this
time around we can't even fix them eventually, because these allocations
aren't on the lru list at all.
> 2. Statistics for debugging.
>
> Especially the first use case is rather important under memory pressure to
> avoid costly acquiring of a contended lock.
Or maybe I'm just not understanding where this matters? Don't you need to
grab the lru lock anyway under memory pressure?
> > Otherwise this usage here just becomes kinda meaningless I think, and just
> > for some debugging. I really don't like sprinkling random atomic_t around
> > (mostly because i915-gem code has gone totally overboard with them, with
> > complete disregard to complexity of the result).
>
> Well this here just replaces what drivers did anyway and the cost of inc/dec
> an atomic is pretty much negligible.
Complexity in understanding the locking. The cpu has not problem with this
stuff ofc.
-Daniel
>
> Christian.
>
> > -Daniel
> >
> > > };
> > > /**
> > > @@ -260,6 +265,19 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man)
> > > man->move = NULL;
> > > }
> > > +/**
> > > + * ttm_resource_manager_usage
> > > + *
> > > + * @man: A memory manager object.
> > > + *
> > > + * Return how many resources are currently used.
> > > + */
> > > +static inline uint64_t
> > > +ttm_resource_manager_usage(struct ttm_resource_manager *man)
> > > +{
> > > + return atomic64_read(&man->usage);
> > > +}
> > > +
> > > void ttm_resource_init(struct ttm_buffer_object *bo,
> > > const struct ttm_place *place,
> > > struct ttm_resource *res);
> > > --
> > > 2.25.1
> > >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-08-31 13:15 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-30 8:56 [PATCH 01/12] drm/ttm: add ttm_resource_fini Christian König
2021-08-30 8:56 ` [PATCH 02/12] drm/ttm: add back a reference to the bdev to the res manager Christian König
2021-08-30 8:56 ` [PATCH 03/12] drm/ttm: add a weak BO reference to the resource v3 Christian König
2021-08-30 8:56 ` [PATCH 04/12] drm/ttm: add common accounting to the resource mgr Christian König
2021-08-31 12:52 ` Daniel Vetter
2021-08-31 12:57 ` Christian König
2021-08-31 13:15 ` Daniel Vetter
2021-08-30 8:57 ` [PATCH 05/12] drm/ttm: move the LRU into resource handling Christian König
2021-08-30 16:53 ` Andrey Grodzovsky
2021-08-30 16:55 ` Christian König
2021-08-30 17:00 ` Andrey Grodzovsky
2021-08-30 17:02 ` Christian König
2021-08-30 8:57 ` [PATCH 06/12] drm/ttm: add resource iterator Christian König
2021-08-30 8:57 ` [PATCH 07/12] drm/radeon: use ttm_resource_manager_debug Christian König
2021-08-30 8:57 ` [PATCH 08/12] drm/radeon: remove resource accounting Christian König
2021-08-30 8:57 ` [PATCH 09/12] drm/amdgpu: use ttm_resource_manager_debug Christian König
2021-08-30 8:57 ` [PATCH 10/12] drm/amdgpu: remove GTT accounting Christian König
2021-08-30 8:57 ` [PATCH 11/12] drm/amdgpu: remove VRAM accounting Christian König
2021-08-30 8:57 ` [PATCH 12/12] drm/amdgpu: drop amdgpu_gtt_node Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.