We can't pipeline that during eviction because the memory needs to be available immediately.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index bc2230ecb7e3..122040056a07 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -651,8 +651,16 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, placement.num_busy_placement = 0; bdev->driver->evict_flags(bo, &placement);
- if (!placement.num_placement && !placement.num_busy_placement) - return ttm_bo_pipeline_gutting(bo); + if (!placement.num_placement && !placement.num_busy_placement) { + ttm_bo_wait(bo, false, false); + + ttm_tt_destroy(bo->ttm); + + memset(&bo->mem, 0, sizeof(bo->mem)); + bo->mem.mem_type = TTM_PL_SYSTEM; + bo->ttm = NULL; + return 0; + }
evict_mem = bo->mem; evict_mem.mm_node = NULL;
Am 2020-07-23 um 5:00 a.m. schrieb Christian König:
We can't pipeline that during eviction because the memory needs to be available immediately.
Signed-off-by: Christian König christian.koenig@amd.com
Looks good to me.
Reviewed-by: Felix Kuehling Felix.Kuehling@amd.com
Alex, in this case the synchronous ttm_bo_wait would be triggering the eviction fence rather than a delayed delete.
Scheduling an eviction worker, like we currently do, would only add unnecessary latency here. The best place to do the HMM migration to system memory synchronously and minimize the wait time here may be in amdgpu_eviction_flags. That way all the SDMA copies to system memory pages would already be in the pipe by the time we get to the ttm_bo_wait.
Regards, Felix
drivers/gpu/drm/ttm/ttm_bo.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index bc2230ecb7e3..122040056a07 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -651,8 +651,16 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, placement.num_busy_placement = 0; bdev->driver->evict_flags(bo, &placement);
- if (!placement.num_placement && !placement.num_busy_placement)
return ttm_bo_pipeline_gutting(bo);
if (!placement.num_placement && !placement.num_busy_placement) {
ttm_bo_wait(bo, false, false);ttm_tt_destroy(bo->ttm);memset(&bo->mem, 0, sizeof(bo->mem));bo->mem.mem_type = TTM_PL_SYSTEM;bo->ttm = NULL;return 0;}
evict_mem = bo->mem; evict_mem.mm_node = NULL;
Am 2020-07-23 um 5:00 a.m. schrieb Christian König:
We can't pipeline that during eviction because the memory needs to be available immediately.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_bo.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index bc2230ecb7e3..122040056a07 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -651,8 +651,16 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, placement.num_busy_placement = 0; bdev->driver->evict_flags(bo, &placement);
- if (!placement.num_placement && !placement.num_busy_placement)
return ttm_bo_pipeline_gutting(bo);
- if (!placement.num_placement && !placement.num_busy_placement) {
ttm_bo_wait(bo, false, false);ttm_tt_destroy(bo->ttm);memset(&bo->mem, 0, sizeof(bo->mem));
Where does the memory in the bo->mem (ttm_mem_reg) get destroyed? It doesn't get attached to a ghost BO in this case, so someone will have to call ttm_bo_mem_put explicitly before you wipe out bo->mem.
Regards, Felix
bo->mem.mem_type = TTM_PL_SYSTEM;bo->ttm = NULL;return 0;}
evict_mem = bo->mem; evict_mem.mm_node = NULL;
On 2020-07-23 7:02 p.m., Felix Kuehling wrote:
Am 2020-07-23 um 5:00 a.m. schrieb Christian König:
We can't pipeline that during eviction because the memory needs to be available immediately.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_bo.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index bc2230ecb7e3..122040056a07 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -651,8 +651,16 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, placement.num_busy_placement = 0; bdev->driver->evict_flags(bo, &placement);
- if (!placement.num_placement && !placement.num_busy_placement)
return ttm_bo_pipeline_gutting(bo);
- if (!placement.num_placement && !placement.num_busy_placement) {
ttm_bo_wait(bo, false, false);ttm_tt_destroy(bo->ttm);memset(&bo->mem, 0, sizeof(bo->mem));Where does the memory in the bo->mem (ttm_mem_reg) get destroyed? It doesn't get attached to a ghost BO in this case, so someone will have to call ttm_bo_mem_put explicitly before you wipe out bo->mem.
After migrating to ram, svm_range_bo_unref-->amdgpu_unref_bo->ttm_bo_put->ttm_bo_release calls ttm_bo_mem_put.
vram is already freed before we signal fence, right?
Regards,
Philip
Regards, Felix
bo->mem.mem_type = TTM_PL_SYSTEM;bo->ttm = NULL;return 0;}
evict_mem = bo->mem; evict_mem.mm_node = NULL;
Am 2020-07-23 um 9:58 p.m. schrieb philip yang:
On 2020-07-23 7:02 p.m., Felix Kuehling wrote:
Am 2020-07-23 um 5:00 a.m. schrieb Christian König:
We can't pipeline that during eviction because the memory needs to be available immediately.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_bo.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index bc2230ecb7e3..122040056a07 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -651,8 +651,16 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, placement.num_busy_placement = 0; bdev->driver->evict_flags(bo, &placement); - if (!placement.num_placement && !placement.num_busy_placement) - return ttm_bo_pipeline_gutting(bo); + if (!placement.num_placement && !placement.num_busy_placement) { + ttm_bo_wait(bo, false, false);
+ ttm_tt_destroy(bo->ttm);
+ memset(&bo->mem, 0, sizeof(bo->mem));
Where does the memory in the bo->mem (ttm_mem_reg) get destroyed? It doesn't get attached to a ghost BO in this case, so someone will have to call ttm_bo_mem_put explicitly before you wipe out bo->mem.
After migrating to ram, svm_range_bo_unref-->amdgpu_unref_bo->ttm_bo_put->ttm_bo_release calls ttm_bo_mem_put.
amdgpu_bo_unref won't free anything if the reference count doesn't go to 0. And TTM is still holding a reference here. The BO won't be freed until ttm_mem_evict_first calls ttm_bo_put.
The memset above overwrites the ttm_mem_reg structure, which means it will forget about the VRAM nodes held by the BO. They need to be released first. That's what frees the space that ttm_bo_evict was trying to make available in the first place.
Regards, Felix
vram is already freed before we signal fence, right?
Regards,
Philip
Regards, Felix
+ bo->mem.mem_type = TTM_PL_SYSTEM; + bo->ttm = NULL; + return 0; + } evict_mem = bo->mem; evict_mem.mm_node = NULL;
Am 24.07.20 um 04:56 schrieb Felix Kuehling:
Am 2020-07-23 um 9:58 p.m. schrieb philip yang:
On 2020-07-23 7:02 p.m., Felix Kuehling wrote:
Am 2020-07-23 um 5:00 a.m. schrieb Christian König:
We can't pipeline that during eviction because the memory needs to be available immediately.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_bo.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index bc2230ecb7e3..122040056a07 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -651,8 +651,16 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, placement.num_busy_placement = 0; bdev->driver->evict_flags(bo, &placement); - if (!placement.num_placement && !placement.num_busy_placement) - return ttm_bo_pipeline_gutting(bo); + if (!placement.num_placement && !placement.num_busy_placement) { + ttm_bo_wait(bo, false, false);
+ ttm_tt_destroy(bo->ttm);
+ memset(&bo->mem, 0, sizeof(bo->mem));
Where does the memory in the bo->mem (ttm_mem_reg) get destroyed? It doesn't get attached to a ghost BO in this case, so someone will have to call ttm_bo_mem_put explicitly before you wipe out bo->mem.
After migrating to ram, svm_range_bo_unref-->amdgpu_unref_bo->ttm_bo_put->ttm_bo_release calls ttm_bo_mem_put.
amdgpu_bo_unref won't free anything if the reference count doesn't go to 0. And TTM is still holding a reference here. The BO won't be freed until ttm_mem_evict_first calls ttm_bo_put.
The memset above overwrites the ttm_mem_reg structure, which means it will forget about the VRAM nodes held by the BO. They need to be released first. That's what frees the space that ttm_bo_evict was trying to make available in the first place.
Yeah, Felix is right that this is a bug.
Going to send a v2 in a minute.
Thanks, Christian.
Regards, Felix
vram is already freed before we signal fence, right?
Regards,
Philip
Regards, Felix
+ bo->mem.mem_type = TTM_PL_SYSTEM; + bo->ttm = NULL; + return 0; + } evict_mem = bo->mem; evict_mem.mm_node = NULL;
dri-devel@lists.freedesktop.org
-
Christian König -
Felix Kuehling -
philip yang