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.
> + /* 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.
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
