[PATCH] drm/amdgpu: Allow to create BO lists in CS ioctl.

Christian König christian.koenig at amd.com
Wed Jul 11 15:42:00 UTC 2018


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.



More information about the amd-gfx mailing list