[PATCH 4/8] drm/amdgpu: remove last_entry_used from the VM code

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

 



Am 11.12.2017 um 13:08 schrieb Christian König:
> Am 11.12.2017 um 06:52 schrieb Chunming Zhou:
>>
>>
>> On 2017å¹´12æ??09æ?¥ 00:41, Christian König wrote:
>>> Not needed any more.
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 52 
>>> +++++++++++++++++++---------------
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  1 -
>>>   2 files changed, 29 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 400a00fababd..ae5451bf5873 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -329,9 +329,6 @@ static int amdgpu_vm_alloc_levels(struct 
>>> amdgpu_device *adev,
>>>           to >= amdgpu_vm_num_entries(adev, level))
>>>           return -EINVAL;
>>>   -    if (to > parent->last_entry_used)
>>> -        parent->last_entry_used = to;
>>> -
>>>       ++level;
>>>       saddr = saddr & ((1 << shift) - 1);
>>>       eaddr = eaddr & ((1 << shift) - 1);
>>> @@ -1187,16 +1184,19 @@ static int amdgpu_vm_update_pde(struct 
>>> amdgpu_device *adev,
>>>    *
>>>    * Mark all PD level as invalid after an error.
>>>    */
>>> -static void amdgpu_vm_invalidate_level(struct amdgpu_vm *vm,
>>> -                       struct amdgpu_vm_pt *parent)
>>> +static void amdgpu_vm_invalidate_level(struct amdgpu_device *adev,
>>> +                       struct amdgpu_vm *vm,
>>> +                       struct amdgpu_vm_pt *parent,
>>> +                       unsigned level)
>> can we move level to struct amdgpu_vm_pt?
>
> I considered this as well, but then abandoned the approach and moved 
> to using it as parameter again.
>
> The general problem is that amdgpu_vm_pt is already *WAY* to big, we 
> use 60 bytes to manage 4K in the worst case.
>
> Working on getting this down to something sane again, but adding the 
> level here just to save passing it as parameter during the destruction 
> would make it worse.

Ping? Any more objections to this patch or can I commit it?

Wanted to commit those up till patch #7, then add your work to reverse 
the level and then put patch #8 on top.

Christian.

>
> Christian.
>
>> otherwise, it looks ok to me.
>>
>> Regards,
>> David Zhou
>>>   {
>>> -    unsigned pt_idx;
>>> +    unsigned pt_idx, num_entries;
>>>         /*
>>>        * Recurse into the subdirectories. This recursion is harmless 
>>> because
>>>        * we only have a maximum of 5 layers.
>>>        */
>>> -    for (pt_idx = 0; pt_idx <= parent->last_entry_used; ++pt_idx) {
>>> +    num_entries = amdgpu_vm_num_entries(adev, level);
>>> +    for (pt_idx = 0; pt_idx < num_entries; ++pt_idx) {
>>>           struct amdgpu_vm_pt *entry = &parent->entries[pt_idx];
>>>             if (!entry->base.bo)
>>> @@ -1207,7 +1207,7 @@ static void amdgpu_vm_invalidate_level(struct 
>>> amdgpu_vm *vm,
>>>           if (list_empty(&entry->base.vm_status))
>>>               list_add(&entry->base.vm_status, &vm->relocated);
>>>           spin_unlock(&vm->status_lock);
>>> -        amdgpu_vm_invalidate_level(vm, entry);
>>> +        amdgpu_vm_invalidate_level(adev, vm, entry, level + 1);
>>>       }
>>>   }
>>>   @@ -1249,7 +1249,8 @@ int amdgpu_vm_update_directories(struct 
>>> amdgpu_device *adev,
>>>                 r = amdgpu_vm_update_pde(adev, vm, pt, entry);
>>>               if (r) {
>>> -                amdgpu_vm_invalidate_level(vm, &vm->root);
>>> +                amdgpu_vm_invalidate_level(adev, vm,
>>> +                               &vm->root, 0);
>>>                   return r;
>>>               }
>>>               spin_lock(&vm->status_lock);
>>> @@ -1652,7 +1653,7 @@ static int amdgpu_vm_bo_update_mapping(struct 
>>> amdgpu_device *adev,
>>>     error_free:
>>>       amdgpu_job_free(job);
>>> -    amdgpu_vm_invalidate_level(vm, &vm->root);
>>> +    amdgpu_vm_invalidate_level(adev, vm, &vm->root, 0);
>>>       return r;
>>>   }
>>>   @@ -2716,26 +2717,31 @@ int amdgpu_vm_init(struct amdgpu_device 
>>> *adev, struct amdgpu_vm *vm,
>>>   /**
>>>    * amdgpu_vm_free_levels - free PD/PT levels
>>>    *
>>> - * @level: PD/PT starting level to free
>>> + * @adev: amdgpu device structure
>>> + * @parent: PD/PT starting level to free
>>> + * @level: level of parent structure
>>>    *
>>>    * Free the page directory or page table level and all sub levels.
>>>    */
>>> -static void amdgpu_vm_free_levels(struct amdgpu_vm_pt *level)
>>> +static void amdgpu_vm_free_levels(struct amdgpu_device *adev,
>>> +                  struct amdgpu_vm_pt *parent,
>>> +                  unsigned level)
>>>   {
>>> -    unsigned i;
>>> +    unsigned i, num_entries = amdgpu_vm_num_entries(adev, level);
>>>   -    if (level->base.bo) {
>>> -        list_del(&level->base.bo_list);
>>> -        list_del(&level->base.vm_status);
>>> -        amdgpu_bo_unref(&level->base.bo->shadow);
>>> -        amdgpu_bo_unref(&level->base.bo);
>>> +    if (parent->base.bo) {
>>> +        list_del(&parent->base.bo_list);
>>> +        list_del(&parent->base.vm_status);
>>> +        amdgpu_bo_unref(&parent->base.bo->shadow);
>>> +        amdgpu_bo_unref(&parent->base.bo);
>>>       }
>>>   -    if (level->entries)
>>> -        for (i = 0; i <= level->last_entry_used; i++)
>>> -            amdgpu_vm_free_levels(&level->entries[i]);
>>> +    if (parent->entries)
>>> +        for (i = 0; i < num_entries; i++)
>>> +            amdgpu_vm_free_levels(adev, &parent->entries[i],
>>> +                          level + 1);
>>>   -    kvfree(level->entries);
>>> +    kvfree(parent->entries);
>>>   }
>>>     /**
>>> @@ -2793,7 +2799,7 @@ void amdgpu_vm_fini(struct amdgpu_device 
>>> *adev, struct amdgpu_vm *vm)
>>>       if (r) {
>>>           dev_err(adev->dev, "Leaking page tables because BO 
>>> reservation failed\n");
>>>       } else {
>>> -        amdgpu_vm_free_levels(&vm->root);
>>> +        amdgpu_vm_free_levels(adev, &vm->root, 0);
>>>           amdgpu_bo_unreserve(root);
>>>       }
>>>       amdgpu_bo_unref(&root);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> index 43ea131dd411..7a308a1ea048 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> @@ -141,7 +141,6 @@ struct amdgpu_vm_pt {
>>>         /* array of page tables, one for each directory entry */
>>>       struct amdgpu_vm_pt        *entries;
>>> -    unsigned            last_entry_used;
>>>   };
>>>     #define AMDGPU_VM_FAULT(pasid, addr) (((u64)(pasid) << 48) | 
>>> (addr))
>>
>



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux