Re: [Intel-gfx] [PATCH] drm/i915/guc: Cancel GuC engine busyness worker synchronously
On Fri, 26 Aug 2022 17:21:35 -0700, Umesh Nerlige Ramappa wrote:
>
> The worker is canceled in gt_park path, but earlier it was assumed that
> gt_park path cannot sleep and the cancel is asynchronous. This caused a
> race with suspend flow where the worker runs after suspend and causes an
> unclaimed register access warning. Cancel the worker synchronously since
> the gt_park is indeed allowed to sleep.
Indeed, __gt_park already calls cancel_work_sync and synchronize_irq which
can sleep:
Reviewed-by: Ashutosh Dixit
> v2: Fix author name and sign-off mismatch
>
> Signed-off-by: Umesh Nerlige Ramappa
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4419
> Fixes: 77cdd054dd2c ("drm/i915/pmu: Connect engine busyness stats from GuC to
> pmu")
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 0d56b615bf78..e6275380b253 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1438,7 +1438,12 @@ void intel_guc_busyness_park(struct intel_gt *gt)
> if (!guc_submission_initialized(guc))
> return;
>
> - cancel_delayed_work(>timestamp.work);
> + /*
> + * There is a race with suspend flow where the worker runs after suspend
> + * and causes an unclaimed register access warning. Cancel the worker
> + * synchronously here.
> + */
> + cancel_delayed_work_sync(>timestamp.work);
>
> /*
>* Before parking, we should sample engine busyness stats if we need to.
> --
> 2.25.1
>
[Intel-gfx] [PATCH] drm/i915/guc: Cancel GuC engine busyness worker synchronously
The worker is canceled in gt_park path, but earlier it was assumed that
gt_park path cannot sleep and the cancel is asynchronous. This caused a
race with suspend flow where the worker runs after suspend and causes an
unclaimed register access warning. Cancel the worker synchronously since
the gt_park is indeed allowed to sleep.
v2: Fix author name and sign-off mismatch
Signed-off-by: Umesh Nerlige Ramappa
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4419
Fixes: 77cdd054dd2c ("drm/i915/pmu: Connect engine busyness stats from GuC to
pmu")
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 0d56b615bf78..e6275380b253 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1438,7 +1438,12 @@ void intel_guc_busyness_park(struct intel_gt *gt)
if (!guc_submission_initialized(guc))
return;
- cancel_delayed_work(>timestamp.work);
+ /*
+* There is a race with suspend flow where the worker runs after suspend
+* and causes an unclaimed register access warning. Cancel the worker
+* synchronously here.
+*/
+ cancel_delayed_work_sync(>timestamp.work);
/*
* Before parking, we should sample engine busyness stats if we need to.
--
2.25.1
[Intel-gfx] [PATCH] drm/i915/guc: Cancel GuC engine busyness worker synchronously
The worker is canceled in gt_park path, but earlier it was assumed that gt_park path cannot sleep and the cancel is asynchronous. This caused a race with suspend flow where the worker runs after suspend and causes an unclaimed register access warning. Cancel the worker synchronously since the gt_park is indeed allowed to sleep. Signed-off-by: Umesh Nerlige Ramappa --- drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 0d56b615bf78..e6275380b253 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -1438,7 +1438,12 @@ void intel_guc_busyness_park(struct intel_gt *gt) if (!guc_submission_initialized(guc)) return; - cancel_delayed_work(>timestamp.work); + /* +* There is a race with suspend flow where the worker runs after suspend +* and causes an unclaimed register access warning. Cancel the worker +* synchronously here. +*/ + cancel_delayed_work_sync(>timestamp.work); /* * Before parking, we should sample engine busyness stats if we need to. -- 2.25.1
Re: [Intel-gfx] [PATCH] drm/i915/guc: Cancel GuC engine busyness worker synchronously
On 7/26/2022 13:51, Nerlige Ramappa, Umesh wrote:
The worker is canceled in gt_park path, but earlier it was assumed that
gt_park path cannot sleep and the cancel is asynchronous. This caused a
race with suspend flow where the worker runs after suspend and causes an
unclaimed register access warning. Cancel the worker synchronously since
the gt_park is indeed allowed to sleep.
Signed-off-by: Umesh Nerlige Ramappa
Fixes: 77cdd054dd2c ("drm/i915/pmu: Connect engine busyness stats from GuC to
pmu")
Reviewed-by: John Harrison
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 76916aed897a..0b7a5ecb640a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1438,7 +1438,12 @@ void intel_guc_busyness_park(struct intel_gt *gt)
if (!guc_submission_initialized(guc))
return;
- cancel_delayed_work(>timestamp.work);
+ /*
+* There is a race with suspend flow where the worker runs after suspend
+* and causes an unclaimed register access warning. Cancel the worker
+* synchronously here.
+*/
+ cancel_delayed_work_sync(>timestamp.work);
/*
* Before parking, we should sample engine busyness stats if we need to.
[Intel-gfx] [PATCH] drm/i915/guc: Cancel GuC engine busyness worker synchronously
The worker is canceled in gt_park path, but earlier it was assumed that
gt_park path cannot sleep and the cancel is asynchronous. This caused a
race with suspend flow where the worker runs after suspend and causes an
unclaimed register access warning. Cancel the worker synchronously since
the gt_park is indeed allowed to sleep.
Signed-off-by: Umesh Nerlige Ramappa
Fixes: 77cdd054dd2c ("drm/i915/pmu: Connect engine busyness stats from GuC to
pmu")
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 76916aed897a..0b7a5ecb640a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1438,7 +1438,12 @@ void intel_guc_busyness_park(struct intel_gt *gt)
if (!guc_submission_initialized(guc))
return;
- cancel_delayed_work(>timestamp.work);
+ /*
+* There is a race with suspend flow where the worker runs after suspend
+* and causes an unclaimed register access warning. Cancel the worker
+* synchronously here.
+*/
+ cancel_delayed_work_sync(>timestamp.work);
/*
* Before parking, we should sample engine busyness stats if we need to.
--
2.36.1
