diff mbox series

drm/i915/selftests: Use clear_and_wake_up_bit() for the per-engine reset bitlocks

Message ID 20211104125844.707783-1-thomas.hellstrom@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/selftests: Use clear_and_wake_up_bit() for the per-engine reset bitlocks | expand

Commit Message

Thomas Hellstrom Nov. 4, 2021, 12:58 p.m. UTC 
Some selftests assume that nothing will attempt to grab these bitlocks
while they are held by the selftests. With GuC, for example, that is
not true because the hanging workloads may cause the GuC code to attempt
to grab them for a global reset, and that may cause it to end up
sleeping on the bit never waking up. Regardless whether that will be
the final solution for GuC, use clear_and_wake_up_bit() pending a more
thorough investigation on how this should be handled moving forward.

Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 8 ++++----
 drivers/gpu/drm/i915/selftests/igt_reset.c   | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Matthew Brost Nov. 4, 2021, 4 p.m. UTC | #1 
On Thu, Nov 04, 2021 at 01:58:44PM +0100, Thomas Hellström wrote:
> Some selftests assume that nothing will attempt to grab these bitlocks
> while they are held by the selftests. With GuC, for example, that is
> not true because the hanging workloads may cause the GuC code to attempt
> to grab them for a global reset, and that may cause it to end up
> sleeping on the bit never waking up. Regardless whether that will be
> the final solution for GuC, use clear_and_wake_up_bit() pending a more
> thorough investigation on how this should be handled moving forward.
> 
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

This series will also fix the CI crash:
https://patchwork.freedesktop.org/series/96406/

Regardless of the above series this one looks correct and needed.

With that:
Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 8 ++++----
>  drivers/gpu/drm/i915/selftests/igt_reset.c   | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> index 7e2d99dd012d..8590419be4c6 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> @@ -528,7 +528,7 @@ static int igt_reset_nop_engine(void *arg)
>  				break;
>  			}
>  		} while (time_before(jiffies, end_time));
> -		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
> +		clear_and_wake_up_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
>  		st_engine_heartbeat_enable(engine);
>  
>  		pr_info("%s(%s): %d resets\n", __func__, engine->name, count);
> @@ -679,7 +679,7 @@ static int igt_reset_fail_engine(void *arg)
>  out:
>  		pr_info("%s(%s): %d resets\n", __func__, engine->name, count);
>  skip:
> -		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
> +		clear_and_wake_up_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
>  		st_engine_heartbeat_enable(engine);
>  		intel_context_put(ce);
>  
> @@ -824,7 +824,7 @@ static int __igt_reset_engine(struct intel_gt *gt, bool active)
>  			if (err)
>  				break;
>  		} while (time_before(jiffies, end_time));
> -		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
> +		clear_and_wake_up_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
>  		st_engine_heartbeat_enable(engine);
>  		pr_info("%s: Completed %lu %s resets\n",
>  			engine->name, count, active ? "active" : "idle");
> @@ -1165,7 +1165,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
>  			if (err)
>  				break;
>  		} while (time_before(jiffies, end_time));
> -		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
> +		clear_and_wake_up_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
>  		st_engine_heartbeat_enable_no_pm(engine);
>  
>  		pr_info("i915_reset_engine(%s:%s): %lu resets\n",
> diff --git a/drivers/gpu/drm/i915/selftests/igt_reset.c b/drivers/gpu/drm/i915/selftests/igt_reset.c
> index 9f8590b868a9..a2838c65f8a5 100644
> --- a/drivers/gpu/drm/i915/selftests/igt_reset.c
> +++ b/drivers/gpu/drm/i915/selftests/igt_reset.c
> @@ -36,7 +36,7 @@ void igt_global_reset_unlock(struct intel_gt *gt)
>  	enum intel_engine_id id;
>  
>  	for_each_engine(engine, gt, id)
> -		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
> +		clear_and_wake_up_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
>  
>  	clear_bit(I915_RESET_BACKOFF, &gt->reset.flags);
>  	wake_up_all(&gt->reset.queue);
> -- 
> 2.31.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 7e2d99dd012d..8590419be4c6 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -528,7 +528,7 @@  static int igt_reset_nop_engine(void *arg)
 				break;
 			}
 		} while (time_before(jiffies, end_time));
-		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
+		clear_and_wake_up_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
 		st_engine_heartbeat_enable(engine);
 
 		pr_info("%s(%s): %d resets\n", __func__, engine->name, count);
@@ -679,7 +679,7 @@  static int igt_reset_fail_engine(void *arg)
 out:
 		pr_info("%s(%s): %d resets\n", __func__, engine->name, count);
 skip:
-		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
+		clear_and_wake_up_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
 		st_engine_heartbeat_enable(engine);
 		intel_context_put(ce);
 
@@ -824,7 +824,7 @@  static int __igt_reset_engine(struct intel_gt *gt, bool active)
 			if (err)
 				break;
 		} while (time_before(jiffies, end_time));
-		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
+		clear_and_wake_up_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
 		st_engine_heartbeat_enable(engine);
 		pr_info("%s: Completed %lu %s resets\n",
 			engine->name, count, active ? "active" : "idle");
@@ -1165,7 +1165,7 @@  static int __igt_reset_engines(struct intel_gt *gt,
 			if (err)
 				break;
 		} while (time_before(jiffies, end_time));
-		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
+		clear_and_wake_up_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
 		st_engine_heartbeat_enable_no_pm(engine);
 
 		pr_info("i915_reset_engine(%s:%s): %lu resets\n",
diff --git a/drivers/gpu/drm/i915/selftests/igt_reset.c b/drivers/gpu/drm/i915/selftests/igt_reset.c
index 9f8590b868a9..a2838c65f8a5 100644
--- a/drivers/gpu/drm/i915/selftests/igt_reset.c
+++ b/drivers/gpu/drm/i915/selftests/igt_reset.c
@@ -36,7 +36,7 @@  void igt_global_reset_unlock(struct intel_gt *gt)
 	enum intel_engine_id id;
 
 	for_each_engine(engine, gt, id)
-		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
+		clear_and_wake_up_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
 
 	clear_bit(I915_RESET_BACKOFF, &gt->reset.flags);
 	wake_up_all(&gt->reset.queue);