reservation questions

[Liu, Monk]

ok, that's good point ...

________________________________
From: Koenig, Christian
Sent: Tuesday, March 6, 2018 6:42:44 PM
To: Liu, Monk; Chris Wilson; dri-devel at 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 <chris at chris-wilson.co.uk><mailto:chris at chris-wilson.co.uk>
Sent: Tuesday, March 6, 2018 6:10:21 PM
To: Liu, Monk; dri-devel at lists.freedesktop.org<mailto:dri-devel at 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180306/2b878295/attachment-0001.html>