Re: [PATCH] dma-fence: fix dma_fence_get_rcu_safe

["Christian König" <christian.koenig@amd.com>]

Am 11.09.2017 um 10:59 schrieb Chris Wilson:
> Quoting Christian König (2017-09-11 09:50:40)
>> Sorry for the delayed response, but your mail somehow ended up in the
>> Spam folder.
>>
>> Am 04.09.2017 um 15:40 schrieb Chris Wilson:
>>> Quoting Christian König (2017-09-04 14:27:33)
>>>> From: Christian König <christian.koenig@amd.com>
>>>>
>>>> The logic is buggy and unnecessary complex. When dma_fence_get_rcu() fails to
>>>> acquire a reference it doesn't necessary mean that there is no fence at all.
>>>>
>>>> It usually mean that the fence was replaced by a new one and in this situation
>>>> we certainly want to have the new one as result and *NOT* NULL.
>>> Which is not guaranteed by the code you wrote either.
>>>
>>> The point of the comment is that the mb is only inside the successful
>>> kref_atomic_inc_unless_zero, and that only after that mb do you know
>>> whether or not you have the current fence.
>>>
>>> You can argue that you want to replace the
>>>        if (!dma_fence_get_rcu())
>>>                return NULL
>>> with
>>>        if (!dma_fence_get_rcu()
>>>                continue;
>>> but it would be incorrect to say that by simply ignoring the
>>> post-condition check that you do have the right fence.
>> You are completely missing the point here.
>>
>> It is irrelevant if you have the current fence or not when you return.
>> You can only guarantee that it is the current fence when you take a look
>> and that is exactly what we want to avoid.
>>
>> So the existing code is complete nonsense. Instead what we need to
>> guarantee is that we return *ANY* fence which we can grab a reference for.
> Not quite. We can grab a reference on a fence that was already freed and
> reused between the rcu_dereference() and dma_fence_get_rcu().

Reusing a memory structure before the RCU grace period is completed is 
illegal, otherwise the whole RCU approach won't work.

So that case can't happen.

Regards,
Christian.

> -Chris