Re: [PATCH 1/8] dma-buf: remove shared fence staging in reservation object

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ping once more! Adding a few more AMD people.

Any comments on this?

Thanks,
Christian.

Am 12.10.18 um 10:22 schrieb Christian König:
Ping! Adding a few people directly and more mailing lists.

Can I get an acked-by/rb for this? It's only a cleanup and not much functional change.

Regards,
Christian.

Am 04.10.2018 um 15:12 schrieb Christian König:
No need for that any more. Just replace the list when there isn't enough
room any more for the additional fence.

Signed-off-by: Christian König <christian.koenig@xxxxxxx>
---
  drivers/dma-buf/reservation.c | 178 ++++++++++++++----------------------------
  include/linux/reservation.h   |   4 -
  2 files changed, 58 insertions(+), 124 deletions(-)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 6c95f61a32e7..5825fc336a13 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -68,105 +68,23 @@ EXPORT_SYMBOL(reservation_seqcount_string);
   */
  int reservation_object_reserve_shared(struct reservation_object *obj)
  {
-    struct reservation_object_list *fobj, *old;
-    u32 max;
+    struct reservation_object_list *old, *new;
+    unsigned int i, j, k, max;
        old = reservation_object_get_list(obj);
        if (old && old->shared_max) {
-        if (old->shared_count < old->shared_max) {
-            /* perform an in-place update */
-            kfree(obj->staged);
-            obj->staged = NULL;
+        if (old->shared_count < old->shared_max)
              return 0;
-        } else
+        else
              max = old->shared_max * 2;
-    } else
-        max = 4;
-
-    /*
-     * resize obj->staged or allocate if it doesn't exist,
-     * noop if already correct size
-     */
-    fobj = krealloc(obj->staged, offsetof(typeof(*fobj), shared[max]),
-            GFP_KERNEL);
-    if (!fobj)
-        return -ENOMEM;
-
-    obj->staged = fobj;
-    fobj->shared_max = max;
-    return 0;
-}
-EXPORT_SYMBOL(reservation_object_reserve_shared);
-
-static void
-reservation_object_add_shared_inplace(struct reservation_object *obj,
-                      struct reservation_object_list *fobj,
-                      struct dma_fence *fence)
-{
-    struct dma_fence *signaled = NULL;
-    u32 i, signaled_idx;
-
-    dma_fence_get(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) {
-            /* 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);
-            return;
-        }
-
-        if (!signaled && dma_fence_is_signaled(old_fence)) {
-            signaled = old_fence;
-            signaled_idx = i;
-        }
-    }
-
-    /*
-     * memory barrier is added by write_seqcount_begin,
-     * fobj->shared_count is protected by this lock too
-     */
-    if (signaled) {
-        RCU_INIT_POINTER(fobj->shared[signaled_idx], fence);
      } else {
-        BUG_ON(fobj->shared_count >= fobj->shared_max);
- RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
-        fobj->shared_count++;
+        max = 4;
      }
  -    write_seqcount_end(&obj->seq);
-    preempt_enable();
-
-    dma_fence_put(signaled);
-}
-
-static void
-reservation_object_add_shared_replace(struct reservation_object *obj,
-                      struct reservation_object_list *old,
-                      struct reservation_object_list *fobj,
-                      struct dma_fence *fence)
-{
-    unsigned i, j, k;
-
-    dma_fence_get(fence);
-
-    if (!old) {
-        RCU_INIT_POINTER(fobj->shared[0], fence);
-        fobj->shared_count = 1;
-        goto done;
-    }
+    new = kmalloc(offsetof(typeof(*new), shared[max]), GFP_KERNEL);
+    if (!new)
+        return -ENOMEM;
        /*
       * no need to bump fence refcounts, rcu_read access
@@ -174,46 +92,45 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
       * references from the old struct are carried over to
       * the new.
       */
-    for (i = 0, j = 0, k = fobj->shared_max; i < old->shared_count; ++i) {
-        struct dma_fence *check;
+    for (i = 0, j = 0, k = max; i < (old ? old->shared_count : 0); ++i) {
+        struct dma_fence *fence;
  -        check = rcu_dereference_protected(old->shared[i],
-                        reservation_object_held(obj));
-
-        if (check->context == fence->context ||
-            dma_fence_is_signaled(check))
-            RCU_INIT_POINTER(fobj->shared[--k], check);
+        fence = rcu_dereference_protected(old->shared[i],
+                          reservation_object_held(obj));
+        if (dma_fence_is_signaled(fence))
+            RCU_INIT_POINTER(new->shared[--k], fence);
          else
-            RCU_INIT_POINTER(fobj->shared[j++], check);
+            RCU_INIT_POINTER(new->shared[j++], fence);
      }
-    fobj->shared_count = j;
-    RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
-    fobj->shared_count++;
+    new->shared_count = j;
+    new->shared_max = max;
  -done:
      preempt_disable();
      write_seqcount_begin(&obj->seq);
      /*
       * RCU_INIT_POINTER can be used here,
       * seqcount provides the necessary barriers
       */
-    RCU_INIT_POINTER(obj->fence, fobj);
+    RCU_INIT_POINTER(obj->fence, new);
      write_seqcount_end(&obj->seq);
      preempt_enable();
        if (!old)
-        return;
+        return 0;
        /* Drop the references to the signaled fences */
-    for (i = k; i < fobj->shared_max; ++i) {
-        struct dma_fence *f;
+    for (i = k; i < new->shared_max; ++i) {
+        struct dma_fence *fence;
  -        f = rcu_dereference_protected(fobj->shared[i],
-                          reservation_object_held(obj));
-        dma_fence_put(f);
+        fence = rcu_dereference_protected(new->shared[i],
+                          reservation_object_held(obj));
+        dma_fence_put(fence);
      }
      kfree_rcu(old, rcu);
+
+    return 0;
  }
+EXPORT_SYMBOL(reservation_object_reserve_shared);
    /**
   * reservation_object_add_shared_fence - Add a fence to a shared slot
@@ -226,15 +143,39 @@ reservation_object_add_shared_replace(struct reservation_object *obj,   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)
-        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)) {
+            dma_fence_put(old_fence);
+            goto replace;
+        }
+    }
+
+    BUG_ON(fobj->shared_count >= fobj->shared_max);
+    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();
  }
  EXPORT_SYMBOL(reservation_object_add_shared_fence);
  @@ -343,9 +284,6 @@ int reservation_object_copy_fences(struct reservation_object *dst,
      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);
  diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index 02166e815afb..54cf6773a14c 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -68,7 +68,6 @@ struct reservation_object_list {
   * @seq: sequence count for managing RCU read-side synchronization
   * @fence_excl: the exclusive fence, if there is one currently
   * @fence: list of current shared fences
- * @staged: staged copy of shared fences for RCU updates
   */
  struct reservation_object {
      struct ww_mutex lock;
@@ -76,7 +75,6 @@ struct reservation_object {
        struct dma_fence __rcu *fence_excl;
      struct reservation_object_list __rcu *fence;
-    struct reservation_object_list *staged;
  };
    #define reservation_object_held(obj) lockdep_is_held(&(obj)->lock.base) @@ -95,7 +93,6 @@ reservation_object_init(struct reservation_object *obj)       __seqcount_init(&obj->seq, reservation_seqcount_string, &reservation_seqcount_class);
      RCU_INIT_POINTER(obj->fence, NULL);
      RCU_INIT_POINTER(obj->fence_excl, NULL);
-    obj->staged = NULL;
  }
    /**
@@ -124,7 +121,6 @@ reservation_object_fini(struct reservation_object *obj)
            kfree(fobj);
      }
-    kfree(obj->staged);
        ww_mutex_destroy(&obj->lock);
  }


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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux