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.koe...@amd.com>
>>>
>>> Stop requiring that the src reservation object is locked for this operation.
>>>
>>> Signed-off-by: Christian König <christian.koe...@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(-)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index dec3a815455d..72ee91f34132 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -261,87 +261,43 @@ void reservation_object_add_excl_fence(struct 
reservation_object *obj,
 }
 EXPORT_SYMBOL(reservation_object_add_excl_fence);
 
-/**
-* reservation_object_copy_fences - Copy all fences from src to dst.
-* @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.
-*/
-int reservation_object_copy_fences(struct reservation_object *dst,
-                                  struct reservation_object *src)
+static bool realloc_shared(void **ptr, struct dma_fence ***pshared, unsigned 
shared_max, bool alloc_list, bool unlocked)
 {
-       struct reservation_object_list *src_list, *dst_list;
-       struct dma_fence *old, *new;
-       size_t size;
-       unsigned i;
-
-       src_list = reservation_object_get_list(src);
-
-       if (src_list) {
-               size = offsetof(typeof(*src_list),
-                               shared[src_list->shared_count]);
-               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]);
-       } else {
-               dst_list = NULL;
-       }
-
-       kfree(dst->staged);
-       dst->staged = NULL;
+       size_t sz;
+       void *newptr;
 
-       src_list = reservation_object_get_list(dst);
+       if (alloc_list)
+               sz = offsetof(struct reservation_object_list, 
shared[shared_max]);
+       else
+               sz = shared_max * sizeof(struct dma_fence *);
 
-       old = reservation_object_get_excl(dst);
-       new = reservation_object_get_excl(src);
+       newptr = krealloc(*ptr, sz, unlocked ? GFP_KERNEL : GFP_NOWAIT | 
__GFP_NOWARN);
+       if (!newptr)
+               return false;
 
-       dma_fence_get(new);
+       *ptr = newptr;
+       if (alloc_list) {
+               struct reservation_object_list *fobj = newptr;
 
-       preempt_disable();
-       write_seqcount_begin(&dst->seq);
-       /* write_seqcount_begin provides the necessary memory barrier */
-       RCU_INIT_POINTER(dst->fence_excl, new);
-       RCU_INIT_POINTER(dst->fence, dst_list);
-       write_seqcount_end(&dst->seq);
-       preempt_enable();
-
-       if (src_list)
-               kfree_rcu(src_list, rcu);
-       dma_fence_put(old);
+               fobj->shared_max = shared_max;
+               *pshared = fobj->shared;
+       } else
+               *pshared = newptr;
 
-       return 0;
+       return true;
 }
-EXPORT_SYMBOL(reservation_object_copy_fences);
 
-/**
- * reservation_object_get_fences_rcu - Get an object's shared and exclusive
- * fences without update side lock held
- * @obj: the reservation object
- * @pfence_excl: the returned exclusive fence (or NULL)
- * @pshared_count: the number of shared fences returned
- * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
- * the required size, and must be freed by caller)
- *
- * RETURNS
- * Zero or -errno
- */
-int reservation_object_get_fences_rcu(struct reservation_object *obj,
+static int __reservation_object_get_fences_rcu(struct reservation_object *obj,
                                      struct dma_fence **pfence_excl,
                                      unsigned *pshared_count,
-                                     struct dma_fence ***pshared)
+                                     struct dma_fence ***pshared,
+                                     struct reservation_object_list 
**list_shared)
 {
        struct dma_fence **shared = NULL;
        struct dma_fence *fence_excl;
        unsigned int shared_count;
        int ret = 1;
+       void *ptr = NULL;
 
        do {
                struct reservation_object_list *fobj;
@@ -359,23 +315,16 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
 
                fobj = rcu_dereference(obj->fence);
                if (fobj) {
-                       struct dma_fence **nshared;
-                       size_t sz = sizeof(*shared) * fobj->shared_max;
-
-                       nshared = krealloc(shared, sz,
-                                          GFP_NOWAIT | __GFP_NOWARN);
-                       if (!nshared) {
+                       if (!realloc_shared(&ptr, &shared, fobj->shared_max, 
list_shared, false)) {
                                rcu_read_unlock();
-                               nshared = krealloc(shared, sz, GFP_KERNEL);
-                               if (nshared) {
-                                       shared = nshared;
-                                       continue;
+                               if (!realloc_shared(&ptr, &shared, 
fobj->shared_max, list_shared, true)) {
+                                       ret = -ENOMEM;
+                                       break;
                                }
 
-                               ret = -ENOMEM;
-                               break;
+                               /* need to acquire rcu lock again and retry */
+                               continue;
                        }
-                       shared = nshared;
                        shared_count = fobj->shared_count;
 
                        for (i = 0; i < shared_count; ++i) {
@@ -398,16 +347,93 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
        } while (ret);
 
        if (!shared_count) {
-               kfree(shared);
+               kfree(ptr);
+               ptr = NULL;
                shared = NULL;
        }
 
-       *pshared_count = shared_count;
-       *pshared = shared;
-       *pfence_excl = fence_excl;
+       if (!list_shared) {
+               *pshared_count = shared_count;
+               *pshared = shared;
+       } else {
+               *list_shared = ptr;
+               if (ptr)
+                       (*list_shared)->shared_count = shared_count;
+       }
 
+       *pfence_excl = fence_excl;
        return ret;
 }
+
+
+/**
+* reservation_object_copy_fences - Copy all fences from src to dst.
+* @dst: the destination reservation object
+* @src: the source reservation object
+*
+* 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)
+{
+       struct reservation_object_list *old_list, *dst_list;
+       struct dma_fence *excl, *old;
+       unsigned i;
+       int ret;
+
+       ret = __reservation_object_get_fences_rcu(src, &excl, NULL,
+                                                 NULL, &dst_list);
+       if (ret)
+               return ret;
+
+       kfree(dst->staged);
+       dst->staged = NULL;
+
+       old = rcu_dereference_protected(dst->fence_excl,
+                                       reservation_object_held(dst));
+
+       old_list = rcu_dereference_protected(dst->fence,
+                                            reservation_object_held(dst));
+
+       preempt_disable();
+       write_seqcount_begin(&dst->seq);
+       /* write_seqcount_begin provides the necessary memory barrier */
+       RCU_INIT_POINTER(dst->fence_excl, excl);
+       RCU_INIT_POINTER(dst->fence, dst_list);
+       write_seqcount_end(&dst->seq);
+       preempt_enable();
+
+       if (old_list) {
+               for (i = 0; i < old_list->shared_count; i++)
+                       dma_fence_put(old_list->shared[i]);
+
+               kfree_rcu(old_list, rcu);
+       }
+       dma_fence_put(old);
+
+       return 0;
+}
+EXPORT_SYMBOL(reservation_object_copy_fences);
+
+/**
+ * reservation_object_get_fences_rcu - Get an object's shared and exclusive
+ * fences without update side lock held
+ * @obj: the reservation object
+ * @pfence_excl: the returned exclusive fence (or NULL)
+ * @pshared_count: the number of shared fences returned
+ * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
+ * the required size, and must be freed by caller)
+ *
+ * RETURNS
+ * Zero or -errno
+ */
+int reservation_object_get_fences_rcu(struct reservation_object *obj,
+                                     struct dma_fence **pfence_excl,
+                                     unsigned *pshared_count,
+                                     struct dma_fence ***pshared)
+{
+       return __reservation_object_get_fences_rcu(obj, pfence_excl, 
pshared_count, pshared, NULL);
+}
 EXPORT_SYMBOL_GPL(reservation_object_get_fences_rcu);
 
 /**

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to