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.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(-)

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.

Regards,
Christian.


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);
/**

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


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

Reply via email to