[PATCH 2/2] drm/amdgpu: allow GTT overcommit during bind

[Chunming Zhou]

On 2017年10月17日 15:46, Christian König wrote:
> Am 17.10.2017 um 06:11 schrieb Chunming Zhou:
>>
>>
>> On 2017年10月16日 19:40, Christian König wrote:
>>> Am 16.10.2017 um 11:42 schrieb Chunming Zhou:
>>>>
>>>>
>>>> On 2017年10月16日 17:26, Christian König wrote:
>>>>> From: Christian König <christian.koenig at amd.com>
>>>>>
>>>>> While binding BOs to GART we need to allow a bit overcommit in the 
>>>>> GTT
>>>>> domain.
>>>> If allowing overcommit, will the new node not over the GART mc 
>>>> range? Which is also allowed?
>>> No that is checked separately by drm_mm_insert_node_in_range().
>>>
>>> This is just to cover the case when we have a BO in GTT space which 
>>> needs to be bound into the GART table.
>> Sorry, I missed that even gart BO is also without node during creating.
>> One nitpick, atomic64_sub(mem->num_pages, &mgr->available) will be 
>> calculated twice for one gart bo create and pin, which results in 
>> available isn't correct.
>
> Yeah, that is true but not as problematic as you think.
>
> The BO is transfered from the old mem object (without GART mapping) to 
> the new mem object (with GART mapping). While doing this the old mem 
> object is released, so we actually don't leak the memory.
>
> It's just that for a moment the BO is accounted twice, once for the 
> old location and once for the new one.
If in memory pressure, I think we could allocate memory over the mgr 
limitation.
For example, the limitation is 1024M, the BO allocation is 800M, when 
the second counting, the available will be -576M, since available is 
unsigned, if another process allocation(800M) is coming at this moment, 
the bo_create is still successful, the binding will be fail. But the 
total allocation will be over the mgr limitation.

Regards,
David Zhou
>
> But we had that behavior previously as well, so that is not something 
> introduced with this patch.
>
> Regards,
> Christian.
>
>>
>> Regards,
>> David Zhou
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Regards,
>>>> David Zhou
>>>>>   Otherwise we can never use the full GART space when GART 
>>>>> size=GTT size.
>>>>>
>>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 8 +++++---
>>>>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> index 0d15eb7d31d7..33535d347734 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> @@ -169,7 +169,8 @@ static int amdgpu_gtt_mgr_new(struct 
>>>>> ttm_mem_type_manager *man,
>>>>>       int r;
>>>>>         spin_lock(&mgr->lock);
>>>>> -    if (atomic64_read(&mgr->available) < mem->num_pages) {
>>>>> +    if ((&tbo->mem == mem || tbo->mem.mem_type != TTM_PL_TT) &&
>>>>> +        atomic64_read(&mgr->available) < mem->num_pages) {
>>>>>           spin_unlock(&mgr->lock);
>>>>>           return 0;
>>>>>       }
>>>>> @@ -244,8 +245,9 @@ static void amdgpu_gtt_mgr_del(struct 
>>>>> ttm_mem_type_manager *man,
>>>>>   uint64_t amdgpu_gtt_mgr_usage(struct ttm_mem_type_manager *man)
>>>>>   {
>>>>>       struct amdgpu_gtt_mgr *mgr = man->priv;
>>>>> +    s64 result = man->size - atomic64_read(&mgr->available);
>>>>>   -    return (u64)(man->size - atomic64_read(&mgr->available)) * 
>>>>> PAGE_SIZE;
>>>>> +    return (result > 0 ? result : 0) * PAGE_SIZE;
>>>>>   }
>>>>>     /**
>>>>> @@ -265,7 +267,7 @@ static void amdgpu_gtt_mgr_debug(struct 
>>>>> ttm_mem_type_manager *man,
>>>>>       drm_mm_print(&mgr->mm, printer);
>>>>>       spin_unlock(&mgr->lock);
>>>>>   -    drm_printf(printer, "man size:%llu pages, gtt 
>>>>> available:%llu pages, usage:%lluMB\n",
>>>>> +    drm_printf(printer, "man size:%llu pages, gtt available:%lld 
>>>>> pages, usage:%lluMB\n",
>>>>>              man->size, (u64)atomic64_read(&mgr->available),
>>>>>              amdgpu_gtt_mgr_usage(man) >> 20);
>>>>>   }
>>>>
>>>
>>
>