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);

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