reservation questions

[Christian König]

When adding fences to a reservation object all newly added fences must 
be created after all existing fences.

In other words when adding a shared fence the caller must guarantee that 
all existing fences of the same context must be completed before this 
new one. Because of this we can safely replace the existing fence with 
the new one.

When adding an exclusive fence the caller must guarantee that all 
existing fences of that reservation object must complete before this new 
one. Because of this we can safely throw away all existing fences as 
well as the old exclusive fence.

Regards,
Christian.

Am 06.03.2018 um 11:10 schrieb Liu, Monk:
>
> Oh, I see
>
> for(i =
>
> 0; i < fobj->shared_count; ++i) {
>
> structdma_fence *old_fence;
>
> Since it use I < fobj->shared_count, then the wild pointer access 
> won’t hit, please ignore question 1
>
> The only valuable is question 2: how we treat excl fence and shared 
> fence?
>
> e.g. when we add an excl fence to resv, how to deal with shared ? 
> current logic is just put the all
>
> do we need to wait on their signaling before putting them ?
>
> thanks
>
> /Monk
>
> *From:*Liu, Monk
> *Sent:* 2018年3月6日17:57
> *To:* dri-devel at lists.freedesktop.org; Chris Wilson 
> <chris at chris-wilson.co.uk>; Koenig, Christian <Christian.Koenig at amd.com>
> *Subject:* Re: reservation questions
>
>
> sorry, I have some mistake in previous thread, correct it as followings.
>
> 1) considering below sequence:
>
> call reservation_object_add_shared_fence,
>
> now assume old->shared_count is now 3
>
> call reservation_object_add_shared_fence,
>
> now assume old->shared_count is now 4,
>
> call reservation_object_reserve_shared,
>
> now obj->staged is new allocated, and its shared_max = 8, but not
>
> used by far.
>
> 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)
>
> 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*,
>
> since old->shared_count is 1 now, during 
> *reservation_object_add_shared_inplace*()
>
> it would go through the shared list, but the fence in shared list is 
> now wild pointer:
>
> for(i =
>
> 0; i < fobj->shared_count; ++i) {
>
> structdma_fence *old_fence;
>
>
>
> *        old_fence = **rcu_dereference_protected**(fobj->**shared**[i],*
>
> ***reservation_object_held**(obj));*
>
>
>
> if(old_fence->context== fence->context&&
>
> dma_fence_is_later(fence, old_fence)) {
>
> /* memory barrier is added by write_seqcount_begin */
>
> RCU_INIT_POINTER(fobj->shared[i], fence);
>
> write_seqcount_end(&obj->seq);
>
> preempt_enable();
>
>
>
> dma_fence_put(old_fence);
>
> return;
>
>         }
>
>
>
> if(!signaled &&
>
> dma_fence_is_signaled(old_fence)) {
>
>             signaled = old_fence;
>
>             signaled_idx = i;
>
>         }
>
>     }
>
> see that old_fence is get from fobj, and fobj is from 
> reservation_object_get_list(obj)
>
> in outside, which is obj->fence, and in add_excl_fence, all dma_fence in
>
> obj->fence is already put.
>
> /Monk
>
> ------------------------------------------------------------------------
>
> *From:*Liu, Monk
> *Sent:* Tuesday, March 6, 2018 5:45:19 PM
> *To:* dri-devel at lists.freedesktop.org 
> <mailto:dri-devel at lists.freedesktop.org>; Chris Wilson; Koenig, Christian
> *Subject:* reservation questions
>
> Hi Christian & Chris
>
> two question regarding resv:
>
> 1) considering below sequence:
>
> call reservation_object_add_shared_fence,
>
> now assume old->shared_count is now 3
>
> call reservation_object_add_shared_fence,
>
> now assume old->shared_count is now 4,
>
> call reservation_object_reserve_shared,
>
> now obj->staged is new allocated, and its shared_max = 8, but not
>
> used by far.
>
> 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)
>
> 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 !*
>
> This looks a design flaw in reservation object, shouldn't we fix it ?
>
> 2) in add_excl_fence(), it simply set old->shared_count to 0, and put 
> all shared fences of old
>
> is that correct? if excl fence is really exclusively used, why we 
> still consider both shared fence and
>
> excl fence on wait_timeout_rcu() routine, see blew description of this 
> routine
>
>
>
> /**
>
> * reservation_object_wait_timeout_rcu - Wait on reservation's objects
>
> * shared and/or exclusive fences.
>
> * @obj: the reservation object
>
> * @wait_all: if true, wait on all fences, else wait on just exclusive 
> fence
>
> * @intr: if true, do interruptible wait
>
> * @timeout: timeout value in jiffies or zero to return immediately
>
> *
>
> * RETURNS
>
> * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or
>
> * greater than zer on success.
>
> */
>
> longreservation_object_wait_timeout_rcu(structreservation_object *obj,
>
> boolwait_all,
>
> boolintr,
>
> unsigned
>
> longtimeout)
>
> thanks
>
> /Monk
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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