[PATCH 06/13] drm/amdgpu: use the new drm_exec object for CS v2

[Christian König]

Am 20.06.23 um 06:14 schrieb Tatsuyuki Ishi:
> On 6/20/23 13:07, Tatsuyuki Ishi wrote:
>>> @@ -1296,9 +1271,8 @@ static int amdgpu_cs_submit(struct 
>>> amdgpu_cs_parser *p,
>>>        */
>>>       r = 0;
>>>       amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
>>> -        struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo);
>>> -
>>> -        r |= !amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm, 
>>> e->range);
>>> +        r |= !amdgpu_ttm_tt_get_user_pages_done(e->bo->tbo.ttm,
>>> +                            e->range);
>>>           e->range = NULL;
>>
>> e->range = NULL; needs to be removed, or it's causing *massive* 
>> memory leaks.
>
> Actually, I quoted the wrong hunk, the correct one is below.

Ah, yes that makes more sense. Going to take a look.

Thanks,
Christian.

>
>> @@ -928,18 +874,56 @@ static int amdgpu_cs_parser_bos(struct 
>> amdgpu_cs_parser *p,
>>          e->user_invalidated = userpage_invalidated;
>>      }
>>
>> -    r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true,
>> -                   &duplicates);
>> -    if (unlikely(r != 0)) {
>> -        if (r != -ERESTARTSYS)
>> -            DRM_ERROR("ttm_eu_reserve_buffers failed.\n");
>> -        goto out_free_user_pages;
>> +    drm_exec_while_not_all_locked(&p->exec) {
>> +        r = amdgpu_vm_lock_pd(&fpriv->vm, &p->exec);
>> +        drm_exec_continue_on_contention(&p->exec);
>> +        if (unlikely(r))
>> +            goto out_free_user_pages;
>> +
>> +        amdgpu_bo_list_for_each_entry(e, p->bo_list) {
>> +            r = drm_exec_prepare_obj(&p->exec, &e->bo->tbo.base, 2);
>> +            drm_exec_break_on_contention(&p->exec);
>> +            if (unlikely(r))
>> +                goto out_free_user_pages;
>> +
>> +            e->bo_va = amdgpu_vm_bo_find(vm, e->bo);
>> +            e->range = NULL;
>
> This causes the leak.
>
>> +        }
>> +        drm_exec_continue_on_contention(&p->exec);
>> +
>> +        if (p->uf_bo) {
>> +            r = drm_exec_prepare_obj(&p->exec, &p->uf_bo->tbo.base,
>> +                         2);
>> +            drm_exec_continue_on_contention(&p->exec);
>> +            if (unlikely(r))
>> +                goto out_free_user_pages;
>> +        }
>>      }
>
> Tatsuyuki