Re: [PATCH] dma-buf/fence-array: get signaled state when signaling is disabled

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

 



Am 21.09.2016 um 13:36 schrieb Gustavo Padovan:
From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>

If the fences in the fence_array signal on the fence_array does not have
signalling enabled num_pending will not be updated accordingly.

So when signaling is disabled check the signal of every fence with
fence_is_signaled() and then compare with num_pending to learn if the
fence_array was signalled or not.

If we want to keep the poll_does_not_wait optimization I think we need
something like this. It keeps the same behaviour if signalling is enabled
but tries to calculated the state otherwise.

Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>

First of all the patch is horrible wrong because fence_array_signaled() is called without any locks held. So you can run into a race condition between checking the fences here and enable signaling.

Additional to that I'm not sure if that is such a good idea or not, cause fence_array_signaled() should be light weight and without calling enable_signaling there is not guarantee that fences will ever signal.

Regards,
Christian.

---
  drivers/dma-buf/fence-array.c | 19 ++++++++++++++++++-
  1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/fence-array.c b/drivers/dma-buf/fence-array.c
index f1989fc..1eec271 100644
--- a/drivers/dma-buf/fence-array.c
+++ b/drivers/dma-buf/fence-array.c
@@ -75,8 +75,25 @@ static bool fence_array_enable_signaling(struct fence *fence)
  static bool fence_array_signaled(struct fence *fence)
  {
  	struct fence_array *array = to_fence_array(fence);
+	int i, num_pending;
+
+	num_pending = atomic_read(&array->num_pending);
+
+	/*
+	 * Before signaling is enabled, num_pending is static (set during array
+	 * construction as a count of all fences or set to 1 if signal_on_any
+	 * flag is passed. To ensure forward progress, i.e. a while
+	 * (!fence_is_signaled()) ; busy-loop eventually proceeds, we need to
+	 * check the current status of our fences.
+	 */
+	if (!test_bit(FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)) {
+		for (i = 0 ; i < array->num_fences; ++i) {
+			if (fence_is_signaled(array->fences[i]))
+				num_pending--;
+		}
+	}
- return atomic_read(&array->num_pending) <= 0;
+	return num_pending <= 0;
  }
static void fence_array_release(struct fence *fence)


_______________________________________________
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