On 31/10/17 04:14 PM, Michel Dänzer wrote: > On 31/10/17 04:03 PM, Christian König wrote: >> Am 31.10.2017 um 09:43 schrieb Michel Dänzer: >>> On 31/10/17 09:37 AM, Christian König wrote: >>>> From: Christian König <christian.koenig at amd.com> >>>> >>>> The bo structure is freed up in case of an error, so we can't do any >>>> accounting if that happens. >>>> >>>> Signed-off-by: Christian König <christian.koenig at amd.com> >>>> --- >>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 +++--- >>>>  1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>>> index e44b880eefdd..ff6f842655d1 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>>> @@ -369,6 +369,9 @@ static int amdgpu_bo_do_create(struct >>>> amdgpu_device *adev, >>>>      r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, type, >>>>                   &bo->placement, page_align, !kernel, NULL, >>>>                   acc_size, sg, resv, &amdgpu_ttm_bo_destroy); >>>> +   if (unlikely(r != 0)) >>>> +       return r; >>>> + >>>>      bytes_moved = atomic64_read(&adev->num_bytes_moved) - >>>>                initial_bytes_moved; >>>>      if (adev->mc.visible_vram_size < adev->mc.real_vram_size && >>>> @@ -378,9 +381,6 @@ static int amdgpu_bo_do_create(struct >>>> amdgpu_device *adev, >>>>      else >>>>          amdgpu_cs_report_moved_bytes(adev, bytes_moved, 0); >>>>  -   if (unlikely(r != 0)) >>>> -       return r; >>>> - >>>>      if (kernel) >>>>          bo->tbo.priority = 1; >>>>  >>> Hmm. If it's at least theoretically possible that ttm_bo_init_reserved >>> moves some BOs before returning an error, we should instead make the >>> accounting safe WRT bo having been freed. But assuming it's not possible, >> >> I have made patches for this back in April and send them to the list, >> but the value is rather low and you need to change a lot of places in >> other drivers as well. > > At least in this case, simply moving the > > if (adev->mc.visible_vram_size < adev->mc.real_vram_size && > bo->tbo.mem.mem_type == TTM_PL_VRAM && > bo->tbo.mem.start < adev->mc.visible_vram_size >> PAGE_SHIFT) > > test before the ttm_bo_init_reserved call, [...] ... except that's not possible, because at least bo->tbo.mem.start isn't decided yet before calling ttm_bo_init_reserved. Never mind the noise. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
