[PATCH 1/4] dma-buf: dma_fence_put is NULL save

[PATCH 1/4] dma-buf: dma_fence_put is NULL save

[Christian König]

From: Christian König <christian.koenig@amd.com>

No need to check.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/reservation.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 9d4316d..dfcd4ef3 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -195,8 +195,7 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
 	if (old)
 		kfree_rcu(old, rcu);
 
-	if (old_fence)
-		dma_fence_put(old_fence);
+	dma_fence_put(old_fence);
 }
 
 /**
@@ -258,8 +257,7 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
 		dma_fence_put(rcu_dereference_protected(old->shared[i],
 						reservation_object_held(obj)));
 
-	if (old_fence)
-		dma_fence_put(old_fence);
+	dma_fence_put(old_fence);
 }
 EXPORT_SYMBOL(reservation_object_add_excl_fence);
 
-- 
2.7.4

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

[PATCH 2/4] dma-buf: add reservation_object_copy_fences

[Christian König]

From: Christian König <christian.koenig@amd.com>

Allows us to copy all the fences in a reservation object to another one.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/reservation.c | 58 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/reservation.h   |  3 +++
 2 files changed, 61 insertions(+)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index dfcd4ef3..302c137 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -262,6 +262,64 @@ 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)
+{
+	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);
+
+	/*
+	 * resize dst->fence or allocate if it doesn't exist,
+	 * noop if already correct size
+	 */
+	size = offsetof(typeof(*src_list), shared[src_list->shared_count]);
+	dst_list = kmalloc(size, GFP_KERNEL);
+	if (!dst_list)
+		return -ENOMEM;
+
+	kfree(dst->staged);
+	dst->staged = NULL;
+
+	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]);
+
+	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);
+	/* 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);
+
+	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
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index 2b5a467..cc4e9ec 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -234,6 +234,9 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
 				      unsigned *pshared_count,
 				      struct dma_fence ***pshared);
 
+int reservation_object_copy_fences(struct reservation_object *dst,
+				   struct reservation_object *src);
+
 long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
 					 bool wait_all, bool intr,
 					 unsigned long timeout);
-- 
2.7.4

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

[PATCH 3/4] drm/ttm: remove nonsense wait in ttm_bo_cleanup_refs_and_unlock

[Christian König]

From: Christian König <christian.koenig@amd.com>

With shared reservation objects the assumption that no fence
could have been added isn't true any more.

Additional to that the BO is about to be destroyed, so removing the
fences now has no advantage whatsoever.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index a54683c..46dd2d6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -505,13 +505,6 @@ static int ttm_bo_cleanup_refs_and_unlock(struct ttm_buffer_object *bo,
 			spin_unlock(&glob->lru_lock);
 			return 0;
 		}
-
-		/*
-		 * remove sync_obj with ttm_bo_wait, the wait should be
-		 * finished, and no new wait object should have been added.
-		 */
-		ret = ttm_bo_wait(bo, false, true);
-		WARN_ON(ret);
 	}
 
 	if (ret || unlikely(list_empty(&bo->ddestroy))) {
-- 
2.7.4

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

[PATCH 4/4] drm/ttm: individualize BO reservation obj when they are freed

[Christian König]

From: Christian König <christian.koenig@amd.com>

Use the BOs reservation object when it is put on the ddelete list. This way we
avoid delaying freeing up the BO because of new fences on the reservation object.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 54 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 46dd2d6..8f04c74 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -394,14 +394,33 @@ static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
 	ww_mutex_unlock (&bo->resv->lock);
 }
 
+static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo)
+{
+	int r;
+
+	if (bo->resv == &bo->ttm_resv)
+		return 0;
+
+	reservation_object_init(&bo->ttm_resv);
+	BUG_ON(reservation_object_lock(&bo->ttm_resv, NULL));
+
+	r = reservation_object_copy_fences(&bo->ttm_resv, bo->resv);
+	if (r) {
+		reservation_object_unlock(&bo->ttm_resv);
+		reservation_object_fini(&bo->ttm_resv);
+	}
+
+	return r;
+}
+
 static void ttm_bo_flush_all_fences(struct ttm_buffer_object *bo)
 {
 	struct reservation_object_list *fobj;
 	struct dma_fence *fence;
 	int i;
 
-	fobj = reservation_object_get_list(bo->resv);
-	fence = reservation_object_get_excl(bo->resv);
+	fobj = reservation_object_get_list(&bo->ttm_resv);
+	fence = reservation_object_get_excl(&bo->ttm_resv);
 	if (fence && !fence->ops->signaled)
 		dma_fence_enable_sw_signaling(fence);
 
@@ -430,8 +449,19 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo)
 			ttm_bo_cleanup_memtype_use(bo);
 
 			return;
-		} else
-			ttm_bo_flush_all_fences(bo);
+		}
+
+		ret = ttm_bo_individualize_resv(bo);
+		if (ret) {
+			/* Last resort, if we fail to allocate memory for the
+			 * fences block for the BO to become idle and free it.
+			 */
+			spin_unlock(&glob->lru_lock);
+			ttm_bo_wait(bo, true, true);
+			ttm_bo_cleanup_memtype_use(bo);
+			return;
+		}
+		ttm_bo_flush_all_fences(bo);
 
 		/*
 		 * Make NO_EVICT bos immediately available to
@@ -443,6 +473,8 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo)
 			ttm_bo_add_to_lru(bo);
 		}
 
+		if (bo->resv != &bo->ttm_resv)
+			reservation_object_unlock(&bo->ttm_resv);
 		__ttm_bo_unreserve(bo);
 	}
 
@@ -471,17 +503,25 @@ static int ttm_bo_cleanup_refs_and_unlock(struct ttm_buffer_object *bo,
 					  bool no_wait_gpu)
 {
 	struct ttm_bo_global *glob = bo->glob;
+	struct reservation_object *resv;
 	int ret;
 
-	ret = ttm_bo_wait(bo, false, true);
+	if (unlikely(list_empty(&bo->ddestroy)))
+		resv = bo->resv;
+	else
+		resv = &bo->ttm_resv;
+
+	if (reservation_object_test_signaled_rcu(resv, true))
+		ret = 0;
+	else
+		ret = -EBUSY;
 
 	if (ret && !no_wait_gpu) {
 		long lret;
 		ww_mutex_unlock(&bo->resv->lock);
 		spin_unlock(&glob->lru_lock);
 
-		lret = reservation_object_wait_timeout_rcu(bo->resv,
-							   true,
+		lret = reservation_object_wait_timeout_rcu(resv, true,
 							   interruptible,
 							   30 * HZ);
 
-- 
2.7.4

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

Re: [PATCH 1/4] dma-buf: dma_fence_put is NULL save

[Alex Deucher]

On Wed, Jul 26, 2017 at 9:12 AM, Christian König
<deathsimple@vodafone.de> wrote:
> From: Christian König <christian.koenig@amd.com>
>
> No need to check.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Patch title: s/save/safe/
With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/dma-buf/reservation.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 9d4316d..dfcd4ef3 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -195,8 +195,7 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
>         if (old)
>                 kfree_rcu(old, rcu);
>
> -       if (old_fence)
> -               dma_fence_put(old_fence);
> +       dma_fence_put(old_fence);
>  }
>
>  /**
> @@ -258,8 +257,7 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
>                 dma_fence_put(rcu_dereference_protected(old->shared[i],
>                                                 reservation_object_held(obj)));
>
> -       if (old_fence)
> -               dma_fence_put(old_fence);
> +       dma_fence_put(old_fence);
>  }
>  EXPORT_SYMBOL(reservation_object_add_excl_fence);
>
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/ttm: individualize BO reservation obj when they are freed

[Alex Deucher]

On Wed, Jul 26, 2017 at 9:12 AM, Christian König
<deathsimple@vodafone.de> wrote:
> From: Christian König <christian.koenig@amd.com>
>
> Use the BOs reservation object when it is put on the ddelete list. This way we
> avoid delaying freeing up the BO because of new fences on the reservation object.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

You might want to mention more explicitly these uses cases that this
patch set fixes in the patch description.

Patch 2-4 are:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/ttm/ttm_bo.c | 54 ++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 46dd2d6..8f04c74 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -394,14 +394,33 @@ static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
>         ww_mutex_unlock (&bo->resv->lock);
>  }
>
> +static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo)
> +{
> +       int r;
> +
> +       if (bo->resv == &bo->ttm_resv)
> +               return 0;
> +
> +       reservation_object_init(&bo->ttm_resv);
> +       BUG_ON(reservation_object_lock(&bo->ttm_resv, NULL));
> +
> +       r = reservation_object_copy_fences(&bo->ttm_resv, bo->resv);
> +       if (r) {
> +               reservation_object_unlock(&bo->ttm_resv);
> +               reservation_object_fini(&bo->ttm_resv);
> +       }
> +
> +       return r;
> +}
> +
>  static void ttm_bo_flush_all_fences(struct ttm_buffer_object *bo)
>  {
>         struct reservation_object_list *fobj;
>         struct dma_fence *fence;
>         int i;
>
> -       fobj = reservation_object_get_list(bo->resv);
> -       fence = reservation_object_get_excl(bo->resv);
> +       fobj = reservation_object_get_list(&bo->ttm_resv);
> +       fence = reservation_object_get_excl(&bo->ttm_resv);
>         if (fence && !fence->ops->signaled)
>                 dma_fence_enable_sw_signaling(fence);
>
> @@ -430,8 +449,19 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo)
>                         ttm_bo_cleanup_memtype_use(bo);
>
>                         return;
> -               } else
> -                       ttm_bo_flush_all_fences(bo);
> +               }
> +
> +               ret = ttm_bo_individualize_resv(bo);
> +               if (ret) {
> +                       /* Last resort, if we fail to allocate memory for the
> +                        * fences block for the BO to become idle and free it.
> +                        */
> +                       spin_unlock(&glob->lru_lock);
> +                       ttm_bo_wait(bo, true, true);
> +                       ttm_bo_cleanup_memtype_use(bo);
> +                       return;
> +               }
> +               ttm_bo_flush_all_fences(bo);
>
>                 /*
>                  * Make NO_EVICT bos immediately available to
> @@ -443,6 +473,8 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo)
>                         ttm_bo_add_to_lru(bo);
>                 }
>
> +               if (bo->resv != &bo->ttm_resv)
> +                       reservation_object_unlock(&bo->ttm_resv);
>                 __ttm_bo_unreserve(bo);
>         }
>
> @@ -471,17 +503,25 @@ static int ttm_bo_cleanup_refs_and_unlock(struct ttm_buffer_object *bo,
>                                           bool no_wait_gpu)
>  {
>         struct ttm_bo_global *glob = bo->glob;
> +       struct reservation_object *resv;
>         int ret;
>
> -       ret = ttm_bo_wait(bo, false, true);
> +       if (unlikely(list_empty(&bo->ddestroy)))
> +               resv = bo->resv;
> +       else
> +               resv = &bo->ttm_resv;
> +
> +       if (reservation_object_test_signaled_rcu(resv, true))
> +               ret = 0;
> +       else
> +               ret = -EBUSY;
>
>         if (ret && !no_wait_gpu) {
>                 long lret;
>                 ww_mutex_unlock(&bo->resv->lock);
>                 spin_unlock(&glob->lru_lock);
>
> -               lret = reservation_object_wait_timeout_rcu(bo->resv,
> -                                                          true,
> +               lret = reservation_object_wait_timeout_rcu(resv, true,
>                                                            interruptible,
>                                                            30 * HZ);
>
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel