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