On 2017å¹´09æ??07æ?¥ 15:13, Christian König wrote:
> Ping? David can you take a look?
>
> Alex is on vacation and that is a rather important bug fix.
>
> Thanks,
> Christian.
>
> Am 04.09.2017 um 21:02 schrieb Christian König:
>> From: Christian König <christian.koenig at amd.com>
>>
>> Stop requiring that the src reservation object is locked for this
>> operation.
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>> drivers/dma-buf/reservation.c | 56
>> ++++++++++++++++++++++++++++++++-----------
>> 1 file changed, 42 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/dma-buf/reservation.c
>> b/drivers/dma-buf/reservation.c
>> index dec3a81..b44d9d7 100644
>> --- a/drivers/dma-buf/reservation.c
>> +++ b/drivers/dma-buf/reservation.c
>> @@ -266,8 +266,7 @@ EXPORT_SYMBOL(reservation_object_add_excl_fence);
>> * @dst: the destination reservation object
>> * @src: the source reservation object
>> *
>> -* Copy all fences from src to dst. Both src->lock as well as
>> dst-lock must be
>> -* held.
>> +* Copy all fences from src to dst. dst-lock must be held.
>> */
>> int reservation_object_copy_fences(struct reservation_object *dst,
>> struct reservation_object *src)
>> @@ -277,33 +276,62 @@ int reservation_object_copy_fences(struct
>> reservation_object *dst,
>> size_t size;
>> unsigned i;
>> - src_list = reservation_object_get_list(src);
>> + rcu_read_lock();
>> + src_list = rcu_dereference(src->fence);
>> +retry:
>> if (src_list) {
>> - size = offsetof(typeof(*src_list),
>> - shared[src_list->shared_count]);
>> + unsigned shared_count = src_list->shared_count;
>> +
>> + size = offsetof(typeof(*src_list), shared[shared_count]);
>> + rcu_read_unlock();
>> +
>> dst_list = kmalloc(size, GFP_KERNEL);
>> if (!dst_list)
>> return -ENOMEM;
>> - dst_list->shared_count = src_list->shared_count;
>> - dst_list->shared_max = src_list->shared_count;
>> - for (i = 0; i < src_list->shared_count; ++i)
>> - dst_list->shared[i] =
>> - dma_fence_get(src_list->shared[i]);
>> + rcu_read_lock();
>> + src_list = rcu_dereference(src->fence);
>> + if (!src_list || src_list->shared_count > shared_count) {
>> + kfree(dst_list);
>> + goto retry;
>> + }
>> +
>> + dst_list->shared_count = 0;
>> + dst_list->shared_max = shared_count;
>> + for (i = 0; i < src_list->shared_count; ++i) {
>> + struct dma_fence *fence;
>> +
>> + fence = rcu_dereference(src_list->shared[i]);
>> + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>> + &fence->flags))
seems here is duplicated with the below dma_fence_is_signaled, can it be
removed?
And I'm not sure the locking, but it looks good, so Acked-by: Chunming
Zhou <david1.zhou at amd.com>
>> + continue;
>> +
>> + if (!dma_fence_get_rcu(fence)) {
>> + kfree(dst_list);
>> + src_list = rcu_dereference(src->fence);
>> + goto retry;
>> + }
>> +
>> + if (dma_fence_is_signaled(fence)) {
>> + dma_fence_put(fence);
>> + continue;
>> + }
>> +
>> + dst_list->shared[dst_list->shared_count++] = fence;
>> + }
>> } else {
>> dst_list = NULL;
>> }
>> + new = dma_fence_get_rcu_safe(&src->fence_excl);
>> + rcu_read_unlock();
>> +
>> kfree(dst->staged);
>> dst->staged = NULL;
>> src_list = reservation_object_get_list(dst);
>> -
>> old = reservation_object_get_excl(dst);
>> - new = reservation_object_get_excl(src);
>> -
>> - dma_fence_get(new);
>> preempt_disable();
>> write_seqcount_begin(&dst->seq);
>
>
