On 2017å¹´10æ??17æ?¥ 16:34, Chunming Zhou wrote:
>
>
> 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?
Ah, sorry, atomic64_t is singed, so the checking will fail, this case
will not happen.
Reviewed-by: Chunming Zhou <david1.zhou at amd.com>
>
> 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);
>>>>>>>> Â }
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
