Am 09.08.2018 um 14:08 schrieb Chris Wilson:
> Quoting Christian König (2018-08-09 12:37:08)
>> void reservation_object_add_shared_fence(struct reservation_object *obj,
>> struct dma_fence *fence)
>> {
>> - struct reservation_object_list *old, *fobj = obj->staged;
>> + struct reservation_object_list *fobj;
>> + unsigned int i;
>>
>> - old = reservation_object_get_list(obj);
>> - obj->staged = NULL;
>> + dma_fence_get(fence);
>> +
>> + fobj = reservation_object_get_list(obj);
>>
>> - 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);
>> + preempt_disable();
>> + write_seqcount_begin(&obj->seq);
>> +
>> + 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_signaled(old_fence)) {
> Are you happy with the possibility that the shared[] may contain two
> fences belonging to the same context? That was a sticking point earlier.
Yeah, that is fixed by now. I've removed the dependency on this in our
VM handling code quite a while ago.
>
>> + /* 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);
> You can rearrange this to have a single exit.
Good point, going to rearrange the code.
Christian.
>
> 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_signaled(old_fence)) {
> dma_fence_put(old_fence);
> goto replace;
> }
> }
> fobj->shared_count++;
> replace:
> /*
> * memory barrier is added by write_seqcount_begin,
> * fobj->shared_count is protected by this lock too
> */
> RCU_INIT_POINTER(fobj->shared[i], fence);
> write_seqcount_end(&obj->seq);
> preempt_enable();
> }
> -Chris
