Re: [PATCH 1/2] dma-buf: make reservation_object_copy_fences rcu save

[Maarten Lankhorst <maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]

Op 11-09-17 om 16:45 schreef Christian König:
> Am 11.09.2017 um 15:56 schrieb Maarten Lankhorst:
>> Op 11-09-17 om 14:53 schreef Christian König:
>>> Am 10.09.2017 um 09:30 schrieb Maarten Lankhorst:
>>>> Op 04-09-17 om 21:02 schreef Christian König:
>>>>> From: Christian König <christian.koenig@amd.com>
>>>>>
>>>>> Stop requiring that the src reservation object is locked for this operation.
>>>>>
>>>>> Signed-off-by: Christian König <christian.koenig@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)
>>>> Could this be implemented using reservation_object_get_fences_rcu? You're essentially duplicating its functionality.
>>> I've considered this as well, but reservation_object_get_fences_rcu() returns an array and here we need an reservation_object_list.
>> Doesn't seem too hard, below is the result from fiddling with the allocation function..
>> reservation_object_list is just a list of fences with some junk in front.
>>
>> Here you go, but only tested with the compiler. O:)
>> -----8<-----
>>   drivers/dma-buf/reservation.c | 192 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------
>>   1 file changed, 109 insertions(+), 83 deletions(-)
>
> To be honest that looks rather ugly to me for not much gain.
>
> Additional to that we loose the optimization I've stolen from the wait function.

Right now your version does exactly the same as reservation_object_get_fences_rcu,
but with a reservation_object_list instead of a fence array.

Can't you change reservation_object_get_fences_rcu to return a
reservation_object_list? All callsites look like they could be easily converted.

So the new patch series would be:
1. Make reservation_object_get_fences_rcu return a reservation_object_list
2. Add optimization
3. Convert reservation_object_copy_fences to use rcu.

Cheers,
Maarten

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx