[PATCH] dma-fence: fix dma_fence_get_rcu_safe

[Chris Wilson]

Quoting Christian König (2017-09-11 10:06:50)
> 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 at 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.

RCU only protects that the pointer remains valid. If you use
SLAB_TYPESAFE_BY_RCU, it is possible to reuse the pointer within a grace
period. It does happen and that is the point the comment is trying to
make.
-Chris