[PATCH] dma-fence: fix dma_fence_get_rcu_safe
Chris Wilson
chris at chris-wilson.co.uk
Mon Sep 11 08:59:31 UTC 2017
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().
-Chris
More information about the dri-devel
mailing list