Am 24.01.2018 um 10:10 schrieb Christian König:
Am 24.01.2018 um 10:06 schrieb Chunming Zhou:
On 2018年01月24日 16:58, Christian König wrote:
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.
'unlock_resv = true' means ttm_bo_cleanup_refs() is locked outside,
then reservation_object_trylock will return false, then original code
'if (unlock_resv && !reservation_object_trylock(bo->resv))' will
always be true, and then return directly.
Is my understanding right? is that expected?
Ah, crap you are right. That must be "!unlock_resv && !reserv...".
No wait a moment. Your initial assumption seems to be incorrect:
'unlock_resv = true' means ttm_bo_cleanup_refs() is locked outside
When unlock_resv = false then that means ttm_bo_cleanup_refs() is locked
outside.
And when we assume this, the original code is indeed right.
Christian.
But why do you want to set unlock_resv to true?
Regards,
Christian.
Regards,
David Zhou
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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
