reservation questions

[Liu, Monk]

Yeah should be 0, typo sorry


I use 3dmark test and successfully triggered a case in reserve_shared:


if (obj->staged!=NULL) {

BUG();

}

kfree(obj->staged)


Previously I cannot figure out why the hell this BUG() could be hit, finally the scenario comes up,

you only need one add_excl_fence() after reserved_shared(), and next time the reserve_shared will

go hit that BUG(), but that's okay, since it only frees the obj->staged that allocated right before this calling

in the previous reserve_shared(), and no one refers to it now.




________________________________
From: Koenig, Christian
Sent: Tuesday, March 6, 2018 6:05:27 PM
To: Liu, Monk; dri-devel at lists.freedesktop.org; Chris Wilson
Subject: Re: reservation questions

Am 06.03.2018 um 10:56 schrieb Liu, Monk:

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)

The exclusive fence replaces all shared fences, e.g. the exclusive operation needs to wait for all shared fences before it can access the protected object. Because of this we can drop all shared fences when a new exclusive fence is set.


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

Why would old->shared_count be 1 now? As far as I can see it should be zero.

Christian.

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) {
        struct dma_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.
*/
long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
                     bool wait_all, bool intr,
                     unsigned long timeout)


thanks
/Monk





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