[PATCH 1/2] drm/amdgpu: Fix a bug on flag table_freed

[Eric Huang]

On 2021-05-30 12:54 p.m., Christian König wrote:
>
>
> Am 30.05.21 um 00:51 schrieb Eric Huang:
>> table_freed will be always true when mapping a memory with size
>> bigger than 2MB. The problem is page table's entries are always
>> existed, but existing mapping depends on page talbe's bo, so
>> using a check of page table's bo existed will resolve the issue.
>>
>> Signed-off-by: Eric Huang <jinhuieric.huang at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 0dee2e8797c7..95b94c95adac 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1582,9 +1582,11 @@ static int amdgpu_vm_update_ptes(struct 
>> amdgpu_vm_update_params *params,
>>                * completely covered by the range and so potentially 
>> still in use.
>>                */
>>               while (cursor.pfn < frag_start) {
>> +                /* Make sure previous mapping existed */
>> +                if (cursor.entry->base.bo)
>> +                    params->table_freed = true;
>
> In general this is the correct approach, but I would push that 
> decision into the amdgpu_vm_free_pts() function.
>
>>                   amdgpu_vm_free_pts(adev, params->vm, &cursor);
>
> So that we have here something like
>
> params->table_freed |= amdgpu_vm_free_pts(..);
>
Thank you for your review. I was thinking put the check into function 
amdgpu_vm_free_pts() since previous review, it will change returns of 
two functions amdgpu_vm_free_pts() and amdgpu_vm_free_table(). If the 
returns are not used by other functions, it seems make a simple change 
complex from my perspective. Can you share the reason of your suggestion?

Regards,
Eric
>
> Regards,
> Christian.
>
>>                   amdgpu_vm_pt_next(adev, &cursor);
>> -                params->table_freed = true;
>>               }
>>             } else if (frag >= shift) {
>