Quoting Christian König (2017-09-11 10:57:57) > Am 11.09.2017 um 11:23 schrieb 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@xxxxxxx> > >>>>>> > >>>>>> 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. > > Yeah, but that is illegal with a fence objects. > > When anybody allocates fences this way it breaks at least > reservation_object_get_fences_rcu(), > reservation_object_wait_timeout_rcu() and > reservation_object_test_signaled_single(). Many, many months ago I sent patches to fix them all. -Chris
