[PATCH v3] drm/i915/gt: Use poll_timeout_us in place of sliding sleep window

[PATCH v3] drm/i915/gt: Use poll_timeout_us in place of sliding sleep window

[Jonathan Cavitt]

The functions intel_guc_send_busy_loop and ct_send can theoretically
loop forever.  In the former case, intel_guc_send_busy_loop can iterate
forever if intel_guc_send_nb repeatedly returns -EBUSY.  In the latter
case, ct_send can loop forever if the guc-to-host or host-to-guc buffers
get stuck in a full state.

Rework the functions to use the poll_timeout_us family of functions
instead of calculating sleep_period_ms repeatedly.  In both cases now,
if the loop condition is not met after 10 minutes, the function will
report it as a failure.

This also resolves a static analysis issue involving sleep_period_ms
overflowing after several shift-left-logical calls.

v2:
- Reduce default sleep/udelay duration (jcavitt)

v3:
- Use atomic in ct_send (jcavitt)

Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Cc: Andi Shyti <andi.shyti@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.h    | 32 +++++++++++------------
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 19 +++++---------
 2 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 053780f562c1..13be4f7e7ab7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -7,6 +7,7 @@
 #define _INTEL_GUC_H_
 
 #include <linux/delay.h>
+#include <linux/iopoll.h>
 #include <linux/iosys-map.h>
 #include <linux/xarray.h>
 
@@ -360,8 +361,7 @@ static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
 					   u32 g2h_len_dw,
 					   bool loop)
 {
-	int err;
-	unsigned int sleep_period_ms = 1;
+	int err, timedout;
 	bool not_atomic = !in_atomic() && !irqs_disabled();
 
 	/*
@@ -374,20 +374,20 @@ static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
 	/* No sleeping with spin locks, just busy loop */
 	might_sleep_if(loop && not_atomic);
 
-retry:
-	err = intel_guc_send_nb(guc, action, len, g2h_len_dw);
-	if (unlikely(err == -EBUSY && loop)) {
-		if (likely(not_atomic)) {
-			if (msleep_interruptible(sleep_period_ms))
-				return -EINTR;
-			sleep_period_ms = sleep_period_ms << 1;
-		} else {
-			cpu_relax();
-		}
-		goto retry;
-	}
-
-	return err;
+	if (!loop)
+		return intel_guc_send_nb(guc, action, len, g2h_len_dw);
+
+	if (not_atomic)
+		timedout = poll_timeout_us(err = intel_guc_send_nb(guc, action,
+								   len, g2h_len_dw),
+					   err != -EBUSY, USEC_PER_MSEC,
+					   600 * USEC_PER_SEC, false);
+	else
+		timedout = poll_timeout_us_atomic(err = intel_guc_send_nb(guc, action,
+									  len, g2h_len_dw),
+						  err != -EBUSY, USEC_PER_MSEC,
+						  600 * USEC_PER_SEC, false);
+	return timedout ?: err;
 }
 
 /* Only call this from the interrupt handler code */
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 1c455d84bf9d..dc4a5486b42c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -716,7 +716,6 @@ static int ct_send(struct intel_guc_ct *ct,
 	struct intel_guc_ct_buffer *ctb = &ct->ctbs.send;
 	struct ct_request request;
 	unsigned long flags;
-	unsigned int sleep_period_ms = 1;
 	bool send_again;
 	u32 fence;
 	int err;
@@ -736,22 +735,18 @@ static int ct_send(struct intel_guc_ct *ct,
 	 * rare. Reserving the maximum size in the G2H credits as we don't know
 	 * how big the response is going to be.
 	 */
-retry:
 	spin_lock_irqsave(&ctb->lock, flags);
-	if (unlikely(!h2g_has_room(ct, len + GUC_CTB_HDR_LEN) ||
-		     !g2h_has_room(ct, GUC_CTB_HXG_MSG_MAX_LEN))) {
+	err = poll_timeout_us_atomic(err = 0,
+				     !h2g_has_room(ct, len + GUC_CTB_HDR_LEN) ||
+				     !g2h_has_room(ct, GUC_CTB_HXG_MSG_MAX_LEN),
+				     USEC_PER_MSEC, 600 * USEC_PER_SEC, false);
+
+	if (err) {
 		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;
-
-		goto retry;
+		return unlikely(ct_deadlocked(ct)) ? -EPIPE : err;
 	}
 
 	ct->stall_time = KTIME_MAX;
-- 
2.53.0

✗ i915.CI.BAT: failure for drm/i915/gt: Use poll_timeout_us in place of sliding sleep window (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6885 bytes --]

== Series Details ==

Series: drm/i915/gt: Use poll_timeout_us in place of sliding sleep window (rev3)
URL   : https://patchwork.freedesktop.org/series/169454/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_18736 -> Patchwork_169454v3
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_169454v3 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_169454v3, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/index.html

Participating hosts (41 -> 40)
------------------------------

  Additional (1): bat-adls-6 
  Missing    (2): bat-dg2-13 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_169454v3:

### CI changes ###

#### Possible regressions ####

  * boot:
    - bat-dg2-8:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-dg2-8/boot.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-dg2-8/boot.html
    - bat-arls-6:         [PASS][3] -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-arls-6/boot.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-arls-6/boot.html
    - bat-mtlp-8:         [PASS][5] -> [FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-mtlp-8/boot.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-mtlp-8/boot.html
    - bat-arlh-2:         [PASS][7] -> [FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-arlh-2/boot.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-arlh-2/boot.html
    - bat-atsm-1:         [PASS][9] -> [FAIL][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-atsm-1/boot.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-atsm-1/boot.html
    - bat-adlp-9:         [PASS][11] -> [FAIL][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-adlp-9/boot.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-adlp-9/boot.html
    - bat-rpls-4:         [PASS][13] -> [FAIL][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-rpls-4/boot.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-rpls-4/boot.html

  

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - bat-adlp-6:         [PASS][15] -> [ABORT][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-adlp-6/igt@i915_module_load@load.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-adlp-6/igt@i915_module_load@load.html
    - bat-arlh-3:         [PASS][17] -> [ABORT][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-arlh-3/igt@i915_module_load@load.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-arlh-3/igt@i915_module_load@load.html
    - bat-dg1-7:          [PASS][19] -> [INCOMPLETE][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-dg1-7/igt@i915_module_load@load.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-dg1-7/igt@i915_module_load@load.html
    - bat-twl-2:          [PASS][21] -> [INCOMPLETE][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-twl-2/igt@i915_module_load@load.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-twl-2/igt@i915_module_load@load.html
    - bat-twl-1:          [PASS][23] -> [INCOMPLETE][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-twl-1/igt@i915_module_load@load.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-twl-1/igt@i915_module_load@load.html
    - bat-dg2-14:         [PASS][25] -> [INCOMPLETE][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-dg2-14/igt@i915_module_load@load.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-dg2-14/igt@i915_module_load@load.html
    - bat-arls-5:         [PASS][27] -> [ABORT][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-arls-5/igt@i915_module_load@load.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-arls-5/igt@i915_module_load@load.html
    - bat-rplp-1:         [PASS][29] -> [INCOMPLETE][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-rplp-1/igt@i915_module_load@load.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-rplp-1/igt@i915_module_load@load.html
    - fi-cfl-guc:         [PASS][31] -> [INCOMPLETE][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/fi-cfl-guc/igt@i915_module_load@load.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/fi-cfl-guc/igt@i915_module_load@load.html
    - bat-mtlp-9:         [PASS][33] -> [ABORT][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-mtlp-9/igt@i915_module_load@load.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-mtlp-9/igt@i915_module_load@load.html
    - bat-dg2-9:          [PASS][35] -> [INCOMPLETE][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-dg2-9/igt@i915_module_load@load.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-dg2-9/igt@i915_module_load@load.html
    - bat-adlp-11:        [PASS][37] -> [ABORT][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-adlp-11/igt@i915_module_load@load.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-adlp-11/igt@i915_module_load@load.html
    - bat-dg1-6:          [PASS][39] -> [INCOMPLETE][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18736/bat-dg1-6/igt@i915_module_load@load.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-dg1-6/igt@i915_module_load@load.html
    - bat-adls-6:         NOTRUN -> [INCOMPLETE][41]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/bat-adls-6/igt@i915_module_load@load.html

  


Build changes
-------------

  * Linux: CI_DRM_18736 -> Patchwork_169454v3

  CI-20190529: 20190529
  CI_DRM_18736: 7100870965845da8c31005c07fd1b390bbe96b20 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8988: 8988
  Patchwork_169454v3: 7100870965845da8c31005c07fd1b390bbe96b20 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_169454v3/index.html

[-- Attachment #2: Type: text/html, Size: 7608 bytes --]

Re: [PATCH v3] drm/i915/gt: Use poll_timeout_us in place of sliding sleep window

[Krzysztof Karas]

Hi Jonathan,

On 2026-06-30 at 23:25:11 +0800, Jonathan Cavitt wrote:
> The functions intel_guc_send_busy_loop and ct_send can theoretically
> loop forever.  In the former case, intel_guc_send_busy_loop can iterate
> forever if intel_guc_send_nb repeatedly returns -EBUSY.  In the latter
> case, ct_send can loop forever if the guc-to-host or host-to-guc buffers
> get stuck in a full state.
> 
> Rework the functions to use the poll_timeout_us family of functions
> instead of calculating sleep_period_ms repeatedly.  In both cases now,
> if the loop condition is not met after 10 minutes, the function will
> report it as a failure.
> 
> This also resolves a static analysis issue involving sleep_period_ms
> overflowing after several shift-left-logical calls.
> 
> v2:
> - Reduce default sleep/udelay duration (jcavitt)
> 
> v3:
> - Use atomic in ct_send (jcavitt)
> 
> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Cc: Andi Shyti <andi.shyti@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc.h    | 32 +++++++++++------------
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 19 +++++---------
>  2 files changed, 23 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index 053780f562c1..13be4f7e7ab7 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -7,6 +7,7 @@
>  #define _INTEL_GUC_H_
>  
>  #include <linux/delay.h>
> +#include <linux/iopoll.h>
>  #include <linux/iosys-map.h>
>  #include <linux/xarray.h>
>  
> @@ -360,8 +361,7 @@ static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
>  					   u32 g2h_len_dw,
>  					   bool loop)
>  {
> -	int err;
> -	unsigned int sleep_period_ms = 1;
> +	int err, timedout;
>  	bool not_atomic = !in_atomic() && !irqs_disabled();
>  
>  	/*
> @@ -374,20 +374,20 @@ static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
>  	/* No sleeping with spin locks, just busy loop */
>  	might_sleep_if(loop && not_atomic);
>  
> -retry:
> -	err = intel_guc_send_nb(guc, action, len, g2h_len_dw);
> -	if (unlikely(err == -EBUSY && loop)) {
> -		if (likely(not_atomic)) {
> -			if (msleep_interruptible(sleep_period_ms))
> -				return -EINTR;
> -			sleep_period_ms = sleep_period_ms << 1;
> -		} else {
> -			cpu_relax();
> -		}
> -		goto retry;
> -	}
> -
> -	return err;
> +	if (!loop)
> +		return intel_guc_send_nb(guc, action, len, g2h_len_dw);
> +
> +	if (not_atomic)
> +		timedout = poll_timeout_us(err = intel_guc_send_nb(guc, action,
> +								   len, g2h_len_dw),
> +					   err != -EBUSY, USEC_PER_MSEC,
> +					   600 * USEC_PER_SEC, false);
Was there a reason for choosing 10 minutes or wat that more of a
"should be long enough" decision?
Also, since you use this magic number in multiple places, it
might be beneficial to put it behind a #define symbol that is
easily trackable in git history.

-- 
Best Regards,
Krzysztof

RE: [PATCH v3] drm/i915/gt: Use poll_timeout_us in place of sliding sleep window

[Cavitt, Jonathan]

-----Original Message-----
From: Karas, Krzysztof <krzysztof.karas@intel.com> 
Sent: Wednesday, July 1, 2026 1:36 AM
To: Cavitt, Jonathan <jonathan.cavitt@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; jani.nikula@linux.intel.com; Shyti, Andi <andi.shyti@intel.com>
Subject: Re: [PATCH v3] drm/i915/gt: Use poll_timeout_us in place of sliding sleep window
> 
> Hi Jonathan,
> 
> On 2026-06-30 at 23:25:11 +0800, Jonathan Cavitt wrote:
> > The functions intel_guc_send_busy_loop and ct_send can theoretically
> > loop forever.  In the former case, intel_guc_send_busy_loop can iterate
> > forever if intel_guc_send_nb repeatedly returns -EBUSY.  In the latter
> > case, ct_send can loop forever if the guc-to-host or host-to-guc buffers
> > get stuck in a full state.
> > 
> > Rework the functions to use the poll_timeout_us family of functions
> > instead of calculating sleep_period_ms repeatedly.  In both cases now,
> > if the loop condition is not met after 10 minutes, the function will
> > report it as a failure.
> > 
> > This also resolves a static analysis issue involving sleep_period_ms
> > overflowing after several shift-left-logical calls.
> > 
> > v2:
> > - Reduce default sleep/udelay duration (jcavitt)
> > 
> > v3:
> > - Use atomic in ct_send (jcavitt)
> > 
> > Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > Cc: Andi Shyti <andi.shyti@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/uc/intel_guc.h    | 32 +++++++++++------------
> >  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 19 +++++---------
> >  2 files changed, 23 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > index 053780f562c1..13be4f7e7ab7 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > @@ -7,6 +7,7 @@
> >  #define _INTEL_GUC_H_
> >  
> >  #include <linux/delay.h>
> > +#include <linux/iopoll.h>
> >  #include <linux/iosys-map.h>
> >  #include <linux/xarray.h>
> >  
> > @@ -360,8 +361,7 @@ static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
> >  					   u32 g2h_len_dw,
> >  					   bool loop)
> >  {
> > -	int err;
> > -	unsigned int sleep_period_ms = 1;
> > +	int err, timedout;
> >  	bool not_atomic = !in_atomic() && !irqs_disabled();
> >  
> >  	/*
> > @@ -374,20 +374,20 @@ static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
> >  	/* No sleeping with spin locks, just busy loop */
> >  	might_sleep_if(loop && not_atomic);
> >  
> > -retry:
> > -	err = intel_guc_send_nb(guc, action, len, g2h_len_dw);
> > -	if (unlikely(err == -EBUSY && loop)) {
> > -		if (likely(not_atomic)) {
> > -			if (msleep_interruptible(sleep_period_ms))
> > -				return -EINTR;
> > -			sleep_period_ms = sleep_period_ms << 1;
> > -		} else {
> > -			cpu_relax();
> > -		}
> > -		goto retry;
> > -	}
> > -
> > -	return err;
> > +	if (!loop)
> > +		return intel_guc_send_nb(guc, action, len, g2h_len_dw);
> > +
> > +	if (not_atomic)
> > +		timedout = poll_timeout_us(err = intel_guc_send_nb(guc, action,
> > +								   len, g2h_len_dw),
> > +					   err != -EBUSY, USEC_PER_MSEC,
> > +					   600 * USEC_PER_SEC, false);
> Was there a reason for choosing 10 minutes or wat that more of a
> "should be long enough" decision?

Yes.

> Also, since you use this magic number in multiple places, it
> might be beneficial to put it behind a #define symbol that is
> easily trackable in git history.

I'll do that as soon as I figure out why poll_timeout_us is causing the
driver to fail loading.  Or, alternatively, I'll apply this revision note
to https://patchwork.freedesktop.org/series/162517/ once it receives
better than lukewarm approval.

I should also note that all this effort is being put towards preventing
an integer overflow that isn't even particularly likely to happen in the
first place.  By all accounts, it might be better to just close the report
internally and be done with it.
-Jonathan Cavitt

> 
> -- 
> Best Regards,
> Krzysztof
> 

Re: [PATCH v3] drm/i915/gt: Use poll_timeout_us in place of sliding sleep window

[Krzysztof Karas]

On 2026-07-01 at 14:10:40 +0000, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Karas, Krzysztof <krzysztof.karas@intel.com> 
> Sent: Wednesday, July 1, 2026 1:36 AM
> To: Cavitt, Jonathan <jonathan.cavitt@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; jani.nikula@linux.intel.com; Shyti, Andi <andi.shyti@intel.com>
> Subject: Re: [PATCH v3] drm/i915/gt: Use poll_timeout_us in place of sliding sleep window
> > 
> > Hi Jonathan,
> > 
> > On 2026-06-30 at 23:25:11 +0800, Jonathan Cavitt wrote:
> > > The functions intel_guc_send_busy_loop and ct_send can theoretically
> > > loop forever.  In the former case, intel_guc_send_busy_loop can iterate
> > > forever if intel_guc_send_nb repeatedly returns -EBUSY.  In the latter
> > > case, ct_send can loop forever if the guc-to-host or host-to-guc buffers
> > > get stuck in a full state.
> > > 
> > > Rework the functions to use the poll_timeout_us family of functions
> > > instead of calculating sleep_period_ms repeatedly.  In both cases now,
> > > if the loop condition is not met after 10 minutes, the function will
> > > report it as a failure.
> > > 
> > > This also resolves a static analysis issue involving sleep_period_ms
> > > overflowing after several shift-left-logical calls.
> > > 
> > > v2:
> > > - Reduce default sleep/udelay duration (jcavitt)
> > > 
> > > v3:
> > > - Use atomic in ct_send (jcavitt)
> > > 
> > > Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > > Cc: Andi Shyti <andi.shyti@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gt/uc/intel_guc.h    | 32 +++++++++++------------
> > >  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 19 +++++---------
> > >  2 files changed, 23 insertions(+), 28 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > > index 053780f562c1..13be4f7e7ab7 100644
> > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > > @@ -7,6 +7,7 @@
> > >  #define _INTEL_GUC_H_
> > >  
> > >  #include <linux/delay.h>
> > > +#include <linux/iopoll.h>
> > >  #include <linux/iosys-map.h>
> > >  #include <linux/xarray.h>
> > >  
> > > @@ -360,8 +361,7 @@ static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
> > >  					   u32 g2h_len_dw,
> > >  					   bool loop)
> > >  {
> > > -	int err;
> > > -	unsigned int sleep_period_ms = 1;
> > > +	int err, timedout;
> > >  	bool not_atomic = !in_atomic() && !irqs_disabled();
> > >  
> > >  	/*
> > > @@ -374,20 +374,20 @@ static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
> > >  	/* No sleeping with spin locks, just busy loop */
> > >  	might_sleep_if(loop && not_atomic);
> > >  
> > > -retry:
> > > -	err = intel_guc_send_nb(guc, action, len, g2h_len_dw);
> > > -	if (unlikely(err == -EBUSY && loop)) {
> > > -		if (likely(not_atomic)) {
> > > -			if (msleep_interruptible(sleep_period_ms))
> > > -				return -EINTR;
> > > -			sleep_period_ms = sleep_period_ms << 1;
> > > -		} else {
> > > -			cpu_relax();
> > > -		}
> > > -		goto retry;
> > > -	}
> > > -
> > > -	return err;
> > > +	if (!loop)
> > > +		return intel_guc_send_nb(guc, action, len, g2h_len_dw);
> > > +
> > > +	if (not_atomic)
> > > +		timedout = poll_timeout_us(err = intel_guc_send_nb(guc, action,
> > > +								   len, g2h_len_dw),
> > > +					   err != -EBUSY, USEC_PER_MSEC,
> > > +					   600 * USEC_PER_SEC, false);
> > Was there a reason for choosing 10 minutes or wat that more of a
> > "should be long enough" decision?
> 
> Yes.
Okay, thanks for clarifying.

> 
> > Also, since you use this magic number in multiple places, it
> > might be beneficial to put it behind a #define symbol that is
> > easily trackable in git history.
> 
> I'll do that as soon as I figure out why poll_timeout_us is causing the
> driver to fail loading.  Or, alternatively, I'll apply this revision note
> to https://patchwork.freedesktop.org/series/162517/ once it receives
> better than lukewarm approval.
> 
> I should also note that all this effort is being put towards preventing
> an integer overflow that isn't even particularly likely to happen in the
> first place.  By all accounts, it might be better to just close the report
> internally and be done with it.
> -Jonathan Cavitt

If you decide to make another version, I'll be happy to
review it ;)

> 
> > 
> > -- 
> > Best Regards,
> > Krzysztof
> > 

-- 
Best Regards,
Krzysztof

Re: [PATCH v3] drm/i915/gt: Use poll_timeout_us in place of sliding sleep window

[Andi Shyti]

Hi Jonathan,

...

> 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 1c455d84bf9d..dc4a5486b42c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> @@ -716,7 +716,6 @@ static int ct_send(struct intel_guc_ct *ct,
>  	struct intel_guc_ct_buffer *ctb = &ct->ctbs.send;
>  	struct ct_request request;
>  	unsigned long flags;
> -	unsigned int sleep_period_ms = 1;
>  	bool send_again;
>  	u32 fence;
>  	int err;
> @@ -736,22 +735,18 @@ static int ct_send(struct intel_guc_ct *ct,
>  	 * rare. Reserving the maximum size in the G2H credits as we don't know
>  	 * how big the response is going to be.
>  	 */
> -retry:
>  	spin_lock_irqsave(&ctb->lock, flags);
> -	if (unlikely(!h2g_has_room(ct, len + GUC_CTB_HDR_LEN) ||
> -		     !g2h_has_room(ct, GUC_CTB_HXG_MSG_MAX_LEN))) {
> +	err = poll_timeout_us_atomic(err = 0,
> +				     !h2g_has_room(ct, len + GUC_CTB_HDR_LEN) ||
> +				     !g2h_has_room(ct, GUC_CTB_HXG_MSG_MAX_LEN),
> +				     USEC_PER_MSEC, 600 * USEC_PER_SEC, false);

This changes the original flow quite a bit.

Before, the code checks whether there is room in the buffers. If
there is not, it drops the lock, checks for deadlock, sleeps, and
retries.

With this change, we keep polling for room until timeout, and
only then check for deadlock. On top of that, the polling is done
while holding the spinlock and with interrupts disabled.

I do not think this is a good tradeoff.

If we want to use poll_timeout_us_atomic(), I think the better
approach would be to move the current retry logic into a small
helper or callback replacing the condition inside
poll_timeout_use_atomic() that:

- takes the lock
- checks whether there is room
- drops the lock
- checks for deadlock
- returns the condition to the poll_timeout_timeout_us_atomic()

That would keep the old logic intact.

Andi