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); >>>>>  } >>>> >>> >> >