ok, that's good point ... ________________________________ From: Koenig, Christian Sent: Tuesday, March 6, 2018 6:42:44 PM To: Liu, Monk; Chris Wilson; dri-devel@lists.freedesktop.org Subject: Re: reservation questions
Hi Monk, that is to remove the problem that allocating memory could fail. E.g. we only add the fence after sending the command to the hardware, so there is now way back and we need to add the fence or break memory management. So reservation_object_reserve_shared() makes sure there is a free fence slot *before* we start to prepare things for the hardware. Regards, Christian. Am 06.03.2018 um 11:19 schrieb Liu, Monk: Hi Chris another question is why we not just call "reservation_object_reserve_shared" during below add_shared_fence function, so the BUG_ON() could be avoided and caller won't need to worry when and how much time it should call reserve_shared() ? thanks ! void reservation_object_add_shared_fence(struct reservation_object *obj, struct dma_fence *fence) { struct reservation_object_list *old, *fobj = obj->staged; old = reservation_object_get_list(obj); obj->staged = NULL; if (!fobj) { BUG_ON(old->shared_count >= old->shared_max); reservation_object_add_shared_inplace(obj, old, fence); } else reservation_object_add_shared_replace(obj, old, fobj, fence); } EXPORT_SYMBOL(reservation_object_add_shared_fence); ________________________________ From: Chris Wilson <ch...@chris-wilson.co.uk><mailto:ch...@chris-wilson.co.uk> Sent: Tuesday, March 6, 2018 6:10:21 PM To: Liu, Monk; dri-devel@lists.freedesktop.org<mailto:dri-devel@lists.freedesktop.org>; Koenig, Christian Subject: Re: reservation questions Quoting Liu, Monk (2018-03-06 09:45:19) > call reservation_object_add_excl_fence, > it set obj->fence->shared_count to 0, and put all shared fence from obj->fence > without waiting signaling. > (this action looks inappropriate, I think at least before put all those shared > fences > we should dma_wait_fence() on them to make sure they are signaled) No. Serialisation of resv updates are handled by the caller, the fences are ordered asynchronously so the wait is implicit in the construction. (I.e. before the excl fence can be signaled, all of the earlier shared fences must be signaled. You can even say before the operation that the excl fence signals completion of can begin, all the shared fences must have been signaled. But that is all implicit so that we can do it asynchronously.) > call reservation_object_reserve_shared, > this time obj->staged isn't NULL, and it is freed (nothing bad now > since obj->fence points to other place), > and obj->staged set to NULL, > > call reservation_object_add_shared_fence, > this time should going through reservation_object_add_shared_inplace, > But BUG_ON(old->shared_count >= old->shared_max) will hit ! How? You only free staged iff shared_count < shared_max. You've reminded me that we should cover all this with a bunch of selftests. -Chris
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel