On 2018å¹´05æ??03æ?¥ 18:14, Christian König wrote:
> Am 24.04.2018 um 09:35 schrieb Chunming Zhou:
>> Shadow BO is located on GTT and its parent (PT and PD) BO could
>> located on VRAM.
>> In some case, the BO on GTT could be evicted but the parent did not.
>> This may
>> cause the shadow BO not be put in the evict list and could not be
>> invalidate
>> correctly.
>>
>> Change-Id: Iad10d9a3031fa2b243879b9e58ee4d8c527eb433
>> Signed-off-by: Chunming Zhou <david1.zhou at amd.com>
>> Reported-by: Shaoyun Liu <Shaoyun.Liu at amd.com>
>
> Way to much memory usage and to complicated.
>
> What we need to do is just to handle the real BO as evicted when the
> shadow BO is evicted.
>
> Something like the following at the start of amdgpu_vm_bo_invalidate()
> should be sufficient:
>
> if (bo->parent && bo->parent->shadow == bo)
> Â Â Â bo = bo->parent;
clever, will send a patch soon.
Regards,
David Zhou
>
> Regards,
> Christian.
>
>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c    | 5 -----
>> Â drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |Â 1 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    | 11 +++++++++++
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h    | 1 +
>> Â 4 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index e1756b68a17b..9c9f6fc5c994 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -462,11 +462,6 @@ static int amdgpu_cs_validate(void *param,
>> struct amdgpu_bo *bo)
>> Â Â Â Â Â do {
>> Â Â Â Â Â Â Â Â Â r = amdgpu_cs_bo_validate(p, bo);
>> Â Â Â Â Â } while (r == -ENOMEM && amdgpu_cs_try_evict(p, bo));
>> -Â Â Â if (r)
>> -Â Â Â Â Â Â Â return r;
>> -
>> -Â Â Â if (bo->shadow)
>> -Â Â Â Â Â Â Â r = amdgpu_cs_bo_validate(p, bo->shadow);
>> Â Â Â Â Â Â return r;
>> Â }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index 540e03fa159f..8078da36aec7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -58,6 +58,7 @@ struct amdgpu_bo_va_mapping {
>> Â /* User space allocated BO in a VM */
>> Â struct amdgpu_bo_va {
>>      struct amdgpu_vm_bo_base   base;
>> +   struct amdgpu_vm_bo_base   shadow_base;
>> Â Â Â Â Â Â /* protected by bo being reserved */
>>      unsigned           ref_count;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index c75b96433ee7..026147cc5104 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -450,8 +450,13 @@ static int amdgpu_vm_alloc_levels(struct
>> amdgpu_device *adev,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â pt->parent = amdgpu_bo_ref(parent->base.bo);
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â amdgpu_vm_bo_base_init(&entry->base, vm, pt);
>> +Â Â Â Â Â Â Â Â Â Â Â amdgpu_vm_bo_base_init(&entry->shadow_base, vm,
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â pt->shadow);
>> Â Â Â Â Â Â Â Â Â Â Â Â Â spin_lock(&vm->status_lock);
>> Â Â Â Â Â Â Â Â Â Â Â Â Â list_move(&entry->base.vm_status, &vm->relocated);
>> +Â Â Â Â Â Â Â Â Â Â Â if (pt->shadow)
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â list_move(&entry->shadow_base.vm_status,
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &vm->relocated);
>> Â Â Â Â Â Â Â Â Â Â Â Â Â spin_unlock(&vm->status_lock);
>> Â Â Â Â Â Â Â Â Â }
>> Â @@ -1875,6 +1880,7 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct
>> amdgpu_device *adev,
>> Â Â Â Â Â Â Â Â Â return NULL;
>> Â Â Â Â Â }
>> Â Â Â Â Â amdgpu_vm_bo_base_init(&bo_va->base, vm, bo);
>> +Â Â Â amdgpu_vm_bo_base_init(&bo_va->shadow_base, vm, bo ? bo->shadow
>> : NULL);
>> Â Â Â Â Â Â bo_va->ref_count = 1;
>> Â Â Â Â Â INIT_LIST_HEAD(&bo_va->valids);
>> @@ -2220,9 +2226,11 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>> Â Â Â Â Â struct amdgpu_vm *vm = bo_va->base.vm;
>> Â Â Â Â Â Â list_del(&bo_va->base.bo_list);
>> +Â Â Â list_del(&bo_va->shadow_base.bo_list);
>> Â Â Â Â Â Â spin_lock(&vm->status_lock);
>> Â Â Â Â Â list_del(&bo_va->base.vm_status);
>> +Â Â Â list_del(&bo_va->shadow_base.vm_status);
>> Â Â Â Â Â spin_unlock(&vm->status_lock);
>> Â Â Â Â Â Â list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
>> @@ -2456,6 +2464,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev,
>> struct amdgpu_vm *vm,
>> Â Â Â Â Â Â Â Â Â goto error_unreserve;
>> Â Â Â Â Â Â amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
>> +Â Â Â amdgpu_vm_bo_base_init(&vm->root.shadow_base, vm, root->shadow);
>> Â Â Â Â Â amdgpu_bo_unreserve(vm->root.base.bo);
>> Â Â Â Â Â Â if (pasid) {
>> @@ -2575,6 +2584,8 @@ static void amdgpu_vm_free_levels(struct
>> amdgpu_device *adev,
>> Â Â Â Â Â if (parent->base.bo) {
>> Â Â Â Â Â Â Â Â Â list_del(&parent->base.bo_list);
>> Â Â Â Â Â Â Â Â Â list_del(&parent->base.vm_status);
>> +Â Â Â Â Â Â Â list_del(&parent->shadow_base.bo_list);
>> +Â Â Â Â Â Â Â list_del(&parent->shadow_base.vm_status);
>> Â Â Â Â Â Â Â Â Â amdgpu_bo_unref(&parent->base.bo->shadow);
>> Â Â Â Â Â Â Â Â Â amdgpu_bo_unref(&parent->base.bo);
>> Â Â Â Â Â }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index 30f080364c97..f4ae6c6b28b8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -153,6 +153,7 @@ struct amdgpu_vm_bo_base {
>> Â Â struct amdgpu_vm_pt {
>>      struct amdgpu_vm_bo_base   base;
>> +   struct amdgpu_vm_bo_base   shadow_base;
>>      bool               huge;
>> Â Â Â Â Â Â /* array of page tables, one for each directory entry */
>
