On 2017å¹´10æ??17æ?¥ 16:27, Christian König wrote:
> Am 17.10.2017 um 10:15 schrieb 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,
>
> Yes, correct so far. That's why I've added the extra check in
> amdgpu_gtt_mgr_usage().
>
>> since available is unsigned
>
> available is an atomic64_t and that is signed IIRC.
>
> We could cast mem->num_pages to signed as well to be double sure, but
> as far as I can see that should work.
I agree above, but why you cut some part of mine:" 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."
What do you think of you cut?
Regards,
David Zhou
>
> Regards,
> Christian.
>
>>
>> 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);
>>>>>>> Â }
>>>>>>
>>>>>
>>>>
>>>
>>
>
