[PATCH 1/2] dma-buf/fence: add fence_wait_any_timeout function v2

[PATCH 1/2] dma-buf/fence: add fence_wait_any_timeout function v2

[Christian König]

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

Waiting for the first fence in an array of fences to signal.

This is useful for device driver specific resource managers
and also Vulkan needs something similar.

v2: more parameter checks, handling for timeout==0,
    remove NULL entry support, better callback removal.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/dma-buf/fence.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fence.h   |  3 +-
 2 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
index 50ef8bd..7b05dbe 100644
--- a/drivers/dma-buf/fence.c
+++ b/drivers/dma-buf/fence.c
@@ -397,6 +397,104 @@ out:
 }
 EXPORT_SYMBOL(fence_default_wait);
 
+static bool
+fence_test_signaled_any(struct fence **fences, uint32_t count)
+{
+	int i;
+
+	for (i = 0; i < count; ++i) {
+		struct fence *fence = fences[i];
+		if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+			return true;
+	}
+	return false;
+}
+
+/**
+ * fence_wait_any_timeout - sleep until any fence gets signaled
+ * or until timeout elapses
+ * @fences:	[in]	array of fences to wait on
+ * @count:	[in]	number of fences to wait on
+ * @intr:	[in]	if true, do an interruptible wait
+ * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
+ *
+ * Returns -EINVAL on custom fence wait implementation, -ERESTARTSYS if
+ * interrupted, 0 if the wait timed out, or the remaining timeout in jiffies
+ * on success.
+ *
+ * Synchronous waits for the first fence in the array to be signaled. The
+ * caller needs to hold a reference to all fences in the array, otherwise a
+ * fence might be freed before return, resulting in undefined behavior.
+ */
+signed long
+fence_wait_any_timeout(struct fence **fences, uint32_t count,
+		       bool intr, signed long timeout)
+{
+	struct default_wait_cb *cb;
+	signed long ret = timeout;
+	unsigned i;
+
+	if (WARN_ON(!fences || !count || timeout < 0))
+		return -EINVAL;
+
+	if (timeout == 0) {
+		for (i = 0; i < count; ++i)
+			if (fence_is_signaled(fences[i]))
+				return 1;
+
+		return 0;
+	}
+
+	cb = kcalloc(count, sizeof(struct default_wait_cb), GFP_KERNEL);
+	if (cb == NULL) {
+		ret = -ENOMEM;
+		goto err_free_cb;
+	}
+
+	for (i = 0; i < count; ++i) {
+		struct fence *fence = fences[i];
+
+		if (fence->ops->wait != fence_default_wait) {
+			ret = -EINVAL;
+			goto fence_rm_cb;
+		}
+
+		cb[i].task = current;
+		if (fence_add_callback(fence, &cb[i].base,
+				       fence_default_wait_cb)) {
+			/* This fence is already signaled */
+			goto fence_rm_cb;
+		}
+	}
+
+	while (ret > 0) {
+		if (intr)
+			set_current_state(TASK_INTERRUPTIBLE);
+		else
+			set_current_state(TASK_UNINTERRUPTIBLE);
+
+		if (fence_test_signaled_any(fences, count))
+			break;
+
+		ret = schedule_timeout(ret);
+
+		if (ret > 0 && intr && signal_pending(current))
+			ret = -ERESTARTSYS;
+	}
+
+	__set_current_state(TASK_RUNNING);
+
+fence_rm_cb:
+	while (i-- > 0)
+		fence_remove_callback(fences[i], &cb[i].base);
+
+err_free_cb:
+	kfree(cb);
+
+	return ret;
+}
+EXPORT_SYMBOL(fence_wait_any_timeout);
+
 /**
  * fence_init - Initialize a custom fence.
  * @fence:	[in]	the fence to initialize
diff --git a/include/linux/fence.h b/include/linux/fence.h
index ddc0f26..bb52201 100644
--- a/include/linux/fence.h
+++ b/include/linux/fence.h
@@ -321,7 +321,8 @@ static inline struct fence *fence_later(struct fence *f1, struct fence *f2)
 }
 
 signed long fence_wait_timeout(struct fence *, bool intr, signed long timeout);
-
+signed long fence_wait_any_timeout(struct fence **fences, uint32_t count,
+				   bool intr, signed long timeout);
 
 /**
  * fence_wait - sleep until the fence gets signaled
-- 
1.9.1

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

[PATCH 2/2] drm/amdgpu: switch to common fence_wait_any_timeout v2

[Christian König]

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

No need to duplicate the functionality any more.

v2: fix handling if no fence is available.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com> (v1)
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  4 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 98 -------------------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c    | 20 ++++---
 3 files changed, 13 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a9c0def..dd7d2ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -447,10 +447,6 @@ int amdgpu_fence_wait_next(struct amdgpu_ring *ring);
 int amdgpu_fence_wait_empty(struct amdgpu_ring *ring);
 unsigned amdgpu_fence_count_emitted(struct amdgpu_ring *ring);
 
-signed long amdgpu_fence_wait_any(struct fence **array,
-				  uint32_t count,
-				  bool intr,
-				  signed long t);
 struct amdgpu_fence *amdgpu_fence_ref(struct amdgpu_fence *fence);
 void amdgpu_fence_unref(struct amdgpu_fence **fence);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 663caa9..c4bb282 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -822,104 +822,6 @@ static const char *amdgpu_fence_get_timeline_name(struct fence *f)
 	return (const char *)fence->ring->name;
 }
 
-static bool amdgpu_test_signaled_any(struct fence **fences, uint32_t count)
-{
-	int idx;
-	struct fence *fence;
-
-	for (idx = 0; idx < count; ++idx) {
-		fence = fences[idx];
-		if (fence) {
-			if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))
-				return true;
-		}
-	}
-	return false;
-}
-
-struct amdgpu_wait_cb {
-	struct fence_cb base;
-	struct task_struct *task;
-};
-
-static void amdgpu_fence_wait_cb(struct fence *fence, struct fence_cb *cb)
-{
-	struct amdgpu_wait_cb *wait =
-		container_of(cb, struct amdgpu_wait_cb, base);
-	wake_up_process(wait->task);
-}
-
-/**
- * Wait the fence array with timeout
- *
- * @array:    the fence array with amdgpu fence pointer
- * @count:    the number of the fence array
- * @intr:     when sleep, set the current task interruptable or not
- * @t:        timeout to wait
- *
- * It will return when any fence is signaled or timeout.
- */
-signed long amdgpu_fence_wait_any(struct fence **array, uint32_t count,
-				  bool intr, signed long t)
-{
-	struct amdgpu_wait_cb *cb;
-	struct fence *fence;
-	unsigned idx;
-
-	BUG_ON(!array);
-
-	cb = kcalloc(count, sizeof(struct amdgpu_wait_cb), GFP_KERNEL);
-	if (cb == NULL) {
-		t = -ENOMEM;
-		goto err_free_cb;
-	}
-
-	for (idx = 0; idx < count; ++idx) {
-		fence = array[idx];
-		if (fence) {
-			cb[idx].task = current;
-			if (fence_add_callback(fence,
-					&cb[idx].base, amdgpu_fence_wait_cb)) {
-				/* The fence is already signaled */
-				goto fence_rm_cb;
-			}
-		}
-	}
-
-	while (t > 0) {
-		if (intr)
-			set_current_state(TASK_INTERRUPTIBLE);
-		else
-			set_current_state(TASK_UNINTERRUPTIBLE);
-
-		/*
-		 * amdgpu_test_signaled_any must be called after
-		 * set_current_state to prevent a race with wake_up_process
-		 */
-		if (amdgpu_test_signaled_any(array, count))
-			break;
-
-		t = schedule_timeout(t);
-
-		if (t > 0 && intr && signal_pending(current))
-			t = -ERESTARTSYS;
-	}
-
-	__set_current_state(TASK_RUNNING);
-
-fence_rm_cb:
-	for (idx = 0; idx < count; ++idx) {
-		fence = array[idx];
-		if (fence && cb[idx].base.func)
-			fence_remove_callback(fence, &cb[idx].base);
-	}
-
-err_free_cb:
-	kfree(cb);
-
-	return t;
-}
-
 const struct fence_ops amdgpu_fence_ops = {
 	.get_driver_name = amdgpu_fence_get_driver_name,
 	.get_timeline_name = amdgpu_fence_get_timeline_name,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
index 5cb27d5..3f48759 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
@@ -337,6 +337,7 @@ int amdgpu_sa_bo_new(struct amdgpu_device *adev,
 {
 	struct fence *fences[AMDGPU_MAX_RINGS];
 	unsigned tries[AMDGPU_MAX_RINGS];
+	unsigned count;
 	int i, r;
 	signed long t;
 
@@ -371,13 +372,18 @@ int amdgpu_sa_bo_new(struct amdgpu_device *adev,
 			/* see if we can skip over some allocations */
 		} while (amdgpu_sa_bo_next_hole(sa_manager, fences, tries));
 
-		spin_unlock(&sa_manager->wq.lock);
-		t = amdgpu_fence_wait_any(fences, AMDGPU_MAX_RINGS,
-					  false, MAX_SCHEDULE_TIMEOUT);
-		r = (t > 0) ? 0 : t;
-		spin_lock(&sa_manager->wq.lock);
-		/* if we have nothing to wait for block */
-		if (r == -ENOENT) {
+		for (i = 0, count = 0; i < AMDGPU_MAX_RINGS; ++i)
+			if (fences[i])
+				fences[count++] = fences[i];
+
+		if (count) {
+			spin_unlock(&sa_manager->wq.lock);
+			t = fence_wait_any_timeout(fences, count, false,
+						   MAX_SCHEDULE_TIMEOUT);
+			r = (t > 0) ? 0 : t;
+			spin_lock(&sa_manager->wq.lock);
+		} else {
+			/* if we have nothing to wait for block */
 			r = wait_event_interruptible_locked(
 				sa_manager->wq,
 				amdgpu_sa_event(sa_manager, size, align)
-- 
1.9.1

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

Re: [PATCH 1/2] dma-buf/fence: add fence_wait_any_timeout function v2

[Christian König]

Ping!

Any more comments or can I get an rb on this? Sorry for the hurry, but I 
want to get this our of my feet.

Regards,
Christian.

Silence is golden. Except when you have kids or wait for code review, 
then silence is suspicious.

On 27.10.2015 17:04, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> Waiting for the first fence in an array of fences to signal.
>
> This is useful for device driver specific resource managers
> and also Vulkan needs something similar.
>
> v2: more parameter checks, handling for timeout==0,
>      remove NULL entry support, better callback removal.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/dma-buf/fence.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/fence.h   |  3 +-
>   2 files changed, 100 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
> index 50ef8bd..7b05dbe 100644
> --- a/drivers/dma-buf/fence.c
> +++ b/drivers/dma-buf/fence.c
> @@ -397,6 +397,104 @@ out:
>   }
>   EXPORT_SYMBOL(fence_default_wait);
>   
> +static bool
> +fence_test_signaled_any(struct fence **fences, uint32_t count)
> +{
> +	int i;
> +
> +	for (i = 0; i < count; ++i) {
> +		struct fence *fence = fences[i];
> +		if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +			return true;
> +	}
> +	return false;
> +}
> +
> +/**
> + * fence_wait_any_timeout - sleep until any fence gets signaled
> + * or until timeout elapses
> + * @fences:	[in]	array of fences to wait on
> + * @count:	[in]	number of fences to wait on
> + * @intr:	[in]	if true, do an interruptible wait
> + * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
> + *
> + * Returns -EINVAL on custom fence wait implementation, -ERESTARTSYS if
> + * interrupted, 0 if the wait timed out, or the remaining timeout in jiffies
> + * on success.
> + *
> + * Synchronous waits for the first fence in the array to be signaled. The
> + * caller needs to hold a reference to all fences in the array, otherwise a
> + * fence might be freed before return, resulting in undefined behavior.
> + */
> +signed long
> +fence_wait_any_timeout(struct fence **fences, uint32_t count,
> +		       bool intr, signed long timeout)
> +{
> +	struct default_wait_cb *cb;
> +	signed long ret = timeout;
> +	unsigned i;
> +
> +	if (WARN_ON(!fences || !count || timeout < 0))
> +		return -EINVAL;
> +
> +	if (timeout == 0) {
> +		for (i = 0; i < count; ++i)
> +			if (fence_is_signaled(fences[i]))
> +				return 1;
> +
> +		return 0;
> +	}
> +
> +	cb = kcalloc(count, sizeof(struct default_wait_cb), GFP_KERNEL);
> +	if (cb == NULL) {
> +		ret = -ENOMEM;
> +		goto err_free_cb;
> +	}
> +
> +	for (i = 0; i < count; ++i) {
> +		struct fence *fence = fences[i];
> +
> +		if (fence->ops->wait != fence_default_wait) {
> +			ret = -EINVAL;
> +			goto fence_rm_cb;
> +		}
> +
> +		cb[i].task = current;
> +		if (fence_add_callback(fence, &cb[i].base,
> +				       fence_default_wait_cb)) {
> +			/* This fence is already signaled */
> +			goto fence_rm_cb;
> +		}
> +	}
> +
> +	while (ret > 0) {
> +		if (intr)
> +			set_current_state(TASK_INTERRUPTIBLE);
> +		else
> +			set_current_state(TASK_UNINTERRUPTIBLE);
> +
> +		if (fence_test_signaled_any(fences, count))
> +			break;
> +
> +		ret = schedule_timeout(ret);
> +
> +		if (ret > 0 && intr && signal_pending(current))
> +			ret = -ERESTARTSYS;
> +	}
> +
> +	__set_current_state(TASK_RUNNING);
> +
> +fence_rm_cb:
> +	while (i-- > 0)
> +		fence_remove_callback(fences[i], &cb[i].base);
> +
> +err_free_cb:
> +	kfree(cb);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(fence_wait_any_timeout);
> +
>   /**
>    * fence_init - Initialize a custom fence.
>    * @fence:	[in]	the fence to initialize
> diff --git a/include/linux/fence.h b/include/linux/fence.h
> index ddc0f26..bb52201 100644
> --- a/include/linux/fence.h
> +++ b/include/linux/fence.h
> @@ -321,7 +321,8 @@ static inline struct fence *fence_later(struct fence *f1, struct fence *f2)
>   }
>   
>   signed long fence_wait_timeout(struct fence *, bool intr, signed long timeout);
> -
> +signed long fence_wait_any_timeout(struct fence **fences, uint32_t count,
> +				   bool intr, signed long timeout);
>   
>   /**
>    * fence_wait - sleep until the fence gets signaled

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

Re: [PATCH 1/2] dma-buf/fence: add fence_wait_any_timeout function v2

[Maarten Lankhorst]

Op 29-10-15 om 10:28 schreef Christian König:
> Ping!
>
> Any more comments or can I get an rb on this? Sorry for the hurry, but I want to get this our of my feet.
>
> Regards,
> Christian.
>
> Silence is golden. Except when you have kids or wait for code review, then silence is suspicious.
Looks good to me.

R-B
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel