[Intel-gfx] [PATCH 1/6] dma-buf: remove shared fence staging in reservation object

[Christian König]

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