lock/unlock mismatch in ttm_bo.c

Christian König christian.koenig at amd.com
Wed Jan 24 08:58:51 UTC 2018


NAK, that doesn't looks correct to me. unlock_resv means that the 
function is allowed to unlock the reservation.

So the original logic seems to do exactly what it is supposed to do. 
Please keep in mind that reservation_object_trylock returns boolean, not 
negative error code.

Regards,
Christian.

Am 24.01.2018 um 06:46 schrieb Chunming Zhou:
> update the fix.
>
>
> On 2018年01月24日 11:09, Chunming Zhou wrote:
>> Hi Tom,
>>
>> Your change looks ok, as Roger suggested, you can send both dri and 
>> amd mail lists.
>>
>> In addition, when I review your patches, I found a bug as the 
>> attached, you can send it together with yours if you think that's a 
>> right fix.
>>
>> Regards,
>>
>> David Zhou
>>
>>
>> On 2018年01月24日 03:25, Tom St Denis wrote:
>>> On 22/01/18 01:42 AM, Chunming Zhou wrote:
>>>>
>>>>
>>>> On 2018年01月20日 02:23, Tom St Denis wrote:
>>>>> On 19/01/18 01:14 PM, Tom St Denis wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> In the function ttm_bo_cleanup_refs() it seems possible to get to 
>>>>>> line 551 without entering the block on 516 which means you'll be 
>>>>>> unlocking a mutex that wasn't locked.
>>>>>>
>>>>>> Now it might be that in the course of the API this pattern cannot 
>>>>>> be expressed but it's not clear from the function alone that that 
>>>>>> is the case.
>>>>>
>>>>>
>>>>> Looking further it seems the behaviour depends on locking in 
>>>>> parent callers.  That's kinda a no-no right?  Shouldn't the lock 
>>>>> be taken/released in the same function ideally?
>>>> Same feelings
>>>>
>>>> Regards,
>>>> David Zhou
>>>
>>> Attached is a patch that addresses this.
>>>
>>> I can't see any obvious race in functions that call 
>>> ttm_bo_cleanup_refs() between the time they let go of the lock and 
>>> the time it's taken again in the call.
>>>
>>> Running it on my system doesn't produce anything notable though the 
>>> KASAN with DRI_PRIME=1 issue is still there (this patch neither 
>>> causes that nor fixes it).
>>>
>>> Tom
>>
>



More information about the amd-gfx mailing list