Am 11.07.2018 um 17:37 schrieb Andrey Grodzovsky:
On 07/11/2018 11:05 AM, Christian König wrote:
Am 11.07.2018 um 16:51 schrieb Andrey Grodzovsky:
[SNIP]
+
+ bool destroy_bo_list;
I think you don't need this. IIRC the bo_list structure was reference
counted.
So all you need to do is to make sure that the temporary bo_list you
create has a reference count of 1 and so is destroyed when the CS
IOCTL calls amdgpu_bo_list_put() on it.
That would simplify the patch quite a bit.
amdgpu_bo_list_destroy is essential because it removes the list from
idr struct bo_list_handles, amdgpu_bo_list_put doesn't do it.
Regarding the refcount, it's 2 because it's 1 on list create in
amdgpu_cs_bo_handles_chunk->amdgpu_bo_list_create and then it's
incremented
to 2 in amdgpu_cs_parser_bos->amdgpu_bo_list_get. So by calling
amdgpu_bo_list_put and then amdgpu_bo_list_destroy the count properly
drops down to 0.
Why is the bo_list on the idr in the first place?
That sounds like it is unnecessary and maybe even quite dangerous when
somebody guesses the temporary allocated id and messes it from another
CPU at the same time while the CS IOCTL is processing things.
Christian.
Andrey
Apart from that looks really good to me, especially that you only
need a new chunk ID for the UAPI is quite nice.
Thanks,
Christian.
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
