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 >> >
