Re: [PATCH 5/7] drm/i915/guc: Add stall timer to non blocking CTB send function

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

 



On 7/1/2021 10:15, Matthew Brost wrote:
Implement a stall timer which fails H2G CTBs once a period of time
with no forward progress is reached to prevent deadlock.

v2:
  (Michal)
   - Improve error message in ct_deadlock()
   - Set broken when ct_deadlock() returns true
   - Return -EPIPE on ct_deadlock()
v3:
  (Michal)
   - Add ms to stall timer comment
  (Matthew)
   - Move broken check to intel_guc_ct_send()

Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
Looks plausible to me.

Reviewed-by: John Harrison <John.C.Harrison@xxxxxxxxx>

---
  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 62 ++++++++++++++++++++---
  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h |  4 ++
  2 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index fb825cc1d090..a9cb7b608520 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -4,6 +4,9 @@
   */
#include <linux/circ_buf.h>
+#include <linux/ktime.h>
+#include <linux/time64.h>
+#include <linux/timekeeping.h>
#include "i915_drv.h"
  #include "intel_guc_ct.h"
@@ -316,6 +319,7 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
  		goto err_deregister;
ct->enabled = true;
+	ct->stall_time = KTIME_MAX;
return 0; @@ -388,9 +392,6 @@ static int ct_write(struct intel_guc_ct *ct,
  	u32 *cmds = ctb->cmds;
  	unsigned int i;
- if (unlikely(ctb->broken))
-		return -EPIPE;
-
  	if (unlikely(desc->status))
  		goto corrupted;
@@ -506,6 +507,25 @@ static int wait_for_ct_request_update(struct ct_request *req, u32 *status)
  	return err;
  }
+#define GUC_CTB_TIMEOUT_MS 1500
+static inline bool ct_deadlocked(struct intel_guc_ct *ct)
+{
+	long timeout = GUC_CTB_TIMEOUT_MS;
+	bool ret = ktime_ms_delta(ktime_get(), ct->stall_time) > timeout;
+
+	if (unlikely(ret)) {
+		struct guc_ct_buffer_desc *send = ct->ctbs.send.desc;
+		struct guc_ct_buffer_desc *recv = ct->ctbs.send.desc;
+
+		CT_ERROR(ct, "Communication stalled for %lld ms, desc status=%#x,%#x\n",
+			 ktime_ms_delta(ktime_get(), ct->stall_time),
+			 send->status, recv->status);
+		ct->ctbs.send.broken = true;
+	}
+
+	return ret;
+}
+
  static inline bool h2g_has_room(struct intel_guc_ct_buffer *ctb, u32 len_dw)
  {
  	struct guc_ct_buffer_desc *desc = ctb->desc;
@@ -517,6 +537,26 @@ static inline bool h2g_has_room(struct intel_guc_ct_buffer *ctb, u32 len_dw)
  	return space >= len_dw;
  }
+static int has_room_nb(struct intel_guc_ct *ct, u32 len_dw)
+{
+	struct intel_guc_ct_buffer *ctb = &ct->ctbs.send;
+
+	lockdep_assert_held(&ct->ctbs.send.lock);
+
+	if (unlikely(!h2g_has_room(ctb, len_dw))) {
+		if (ct->stall_time == KTIME_MAX)
+			ct->stall_time = ktime_get();
+
+		if (unlikely(ct_deadlocked(ct)))
+			return -EPIPE;
+		else
+			return -EBUSY;
+	}
+
+	ct->stall_time = KTIME_MAX;
+	return 0;
+}
+
  static int ct_send_nb(struct intel_guc_ct *ct,
  		      const u32 *action,
  		      u32 len,
@@ -529,11 +569,9 @@ static int ct_send_nb(struct intel_guc_ct *ct,
spin_lock_irqsave(&ctb->lock, spin_flags); - ret = h2g_has_room(ctb, len + GUC_CTB_HDR_LEN);
-	if (unlikely(!ret)) {
-		ret = -EBUSY;
+	ret = has_room_nb(ct, len + GUC_CTB_HDR_LEN);
+	if (unlikely(ret))
  		goto out;
-	}
fence = ct_get_next_fence(ct);
  	ret = ct_write(ct, action, len, fence, flags);
@@ -576,8 +614,13 @@ static int ct_send(struct intel_guc_ct *ct,
  retry:
  	spin_lock_irqsave(&ctb->lock, flags);
  	if (unlikely(!h2g_has_room(ctb, len + GUC_CTB_HDR_LEN))) {
+		if (ct->stall_time == KTIME_MAX)
+			ct->stall_time = ktime_get();
  		spin_unlock_irqrestore(&ctb->lock, flags);
+ if (unlikely(ct_deadlocked(ct)))
+			return -EPIPE;
+
  		if (msleep_interruptible(sleep_period_ms))
  			return -EINTR;
  		sleep_period_ms = sleep_period_ms << 1;
@@ -585,6 +628,8 @@ static int ct_send(struct intel_guc_ct *ct,
  		goto retry;
  	}
+ ct->stall_time = KTIME_MAX;
+
  	fence = ct_get_next_fence(ct);
  	request.fence = fence;
  	request.status = 0;
@@ -647,6 +692,9 @@ int intel_guc_ct_send(struct intel_guc_ct *ct, const u32 *action, u32 len,
  		return -ENODEV;
  	}
+ if (unlikely(ct->ctbs.send.broken))
+		return -EPIPE;
+
  	if (flags & INTEL_GUC_CT_SEND_NB)
  		return ct_send_nb(ct, action, len, flags);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
index 5bb8bef024c8..bee03794c1eb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
@@ -9,6 +9,7 @@
  #include <linux/interrupt.h>
  #include <linux/spinlock.h>
  #include <linux/workqueue.h>
+#include <linux/ktime.h>
#include "intel_guc_fwif.h" @@ -68,6 +69,9 @@ struct intel_guc_ct {
  		struct list_head incoming; /* incoming requests */
  		struct work_struct worker; /* handler for incoming requests */
  	} requests;
+
+	/** @stall_time: time of first time a CTB submission is stalled */
+	ktime_t stall_time;
  };
void intel_guc_ct_init_early(struct intel_guc_ct *ct);




[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