[Intel-gfx] [PATCH] drm/i915/pmu: Fix wakeref leak in PMU busyness during reset
GuC PMU busyness gets gt wakeref if awake, but fails to release the
wakeref if a reset is in progress. Release the wakeref if it was
acquried successfully.
Signed-off-by: Umesh Nerlige Ramappa
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
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 1f9d4fde421f..a243304a2db1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1181,6 +1181,7 @@ static ktime_t guc_engine_busyness(struct intel_engine_cs
*engine, ktime_t *now)
struct intel_gt *gt = engine->gt;
struct intel_guc *guc = >->uc.guc;
u64 total, gt_stamp_saved;
+ intel_wakeref_t wakeref;
unsigned long flags;
u32 reset_count;
bool in_reset;
@@ -1206,18 +1207,21 @@ static ktime_t guc_engine_busyness(struct
intel_engine_cs *engine, ktime_t *now)
* start_gt_clk is derived from GuC state. To get a consistent
* view of activity, we query the GuC state only if gt is awake.
*/
- if (intel_gt_pm_get_if_awake(gt) && !in_reset) {
+ wakeref = intel_gt_pm_get_if_awake(gt);
+ if (wakeref && !in_reset) {
stats_saved = *stats;
gt_stamp_saved = guc->timestamp.gt_stamp;
guc_update_engine_gt_clks(engine);
guc_update_pm_timestamp(guc, engine, now);
- intel_gt_pm_put_async(gt);
if (i915_reset_count(gpu_error) != reset_count) {
*stats = stats_saved;
guc->timestamp.gt_stamp = gt_stamp_saved;
}
}
+ if (wakeref)
+ intel_gt_pm_put_async(gt);
+
total = intel_gt_clock_interval_to_ns(gt, stats->total_gt_clks);
if (stats->running) {
u64 clk = guc->timestamp.gt_stamp - stats->start_gt_clk;
--
2.20.1
[Intel-gfx] [PATCH] drm/i915/pmu: Fix wakeref leak in PMU busyness during reset
GuC PMU busyness gets gt wakeref if awake, but fails to release the
wakeref if a reset is in progress. Release the wakeref if it was
acquried successfully.
v2: Simplify the fix (Ashutosh)
Signed-off-by: Umesh Nerlige Ramappa
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +-
1 file changed, 1 insertion(+), 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 1f9d4fde421f..9739da6f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1206,7 +1206,7 @@ static ktime_t guc_engine_busyness(struct intel_engine_cs
*engine, ktime_t *now)
* start_gt_clk is derived from GuC state. To get a consistent
* view of activity, we query the GuC state only if gt is awake.
*/
- if (intel_gt_pm_get_if_awake(gt) && !in_reset) {
+ if (!in_reset && intel_gt_pm_get_if_awake(gt)) {
stats_saved = *stats;
gt_stamp_saved = guc->timestamp.gt_stamp;
guc_update_engine_gt_clks(engine);
--
2.20.1
Re: [Intel-gfx] [PATCH] drm/i915/pmu: Fix wakeref leak in PMU busyness during reset
On Mon, 06 Dec 2021 16:45:42 -0800, Umesh Nerlige Ramappa wrote:
>
> GuC PMU busyness gets gt wakeref if awake, but fails to release the
> wakeref if a reset is in progress. Release the wakeref if it was
> acquried successfully.
>
> Signed-off-by: Umesh Nerlige Ramappa
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> 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 1f9d4fde421f..a243304a2db1 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1181,6 +1181,7 @@ static ktime_t guc_engine_busyness(struct
> intel_engine_cs *engine, ktime_t *now)
> struct intel_gt *gt = engine->gt;
> struct intel_guc *guc = >->uc.guc;
> u64 total, gt_stamp_saved;
> + intel_wakeref_t wakeref;
Should be bool.
> unsigned long flags;
> u32 reset_count;
> bool in_reset;
> @@ -1206,18 +1207,21 @@ static ktime_t guc_engine_busyness(struct
> intel_engine_cs *engine, ktime_t *now)
>* start_gt_clk is derived from GuC state. To get a consistent
>* view of activity, we query the GuC state only if gt is awake.
>*/
> - if (intel_gt_pm_get_if_awake(gt) && !in_reset) {
Also how about just switch them around:
if (!in_reset && intel_gt_pm_get_if_awake(gt))
Then you don't need any other changes I think.
> + wakeref = intel_gt_pm_get_if_awake(gt);
> + if (wakeref && !in_reset) {
> stats_saved = *stats;
> gt_stamp_saved = guc->timestamp.gt_stamp;
> guc_update_engine_gt_clks(engine);
> guc_update_pm_timestamp(guc, engine, now);
> - intel_gt_pm_put_async(gt);
> if (i915_reset_count(gpu_error) != reset_count) {
> *stats = stats_saved;
> guc->timestamp.gt_stamp = gt_stamp_saved;
> }
> }
>
> + if (wakeref)
> + intel_gt_pm_put_async(gt);
> +
> total = intel_gt_clock_interval_to_ns(gt, stats->total_gt_clks);
> if (stats->running) {
> u64 clk = guc->timestamp.gt_stamp - stats->start_gt_clk;
> --
> 2.20.1
>
Re: [Intel-gfx] [PATCH] drm/i915/pmu: Fix wakeref leak in PMU busyness during reset
On Mon, Dec 06, 2021 at 05:30:43PM -0800, Dixit, Ashutosh wrote:
On Mon, 06 Dec 2021 16:45:42 -0800, Umesh Nerlige Ramappa wrote:
GuC PMU busyness gets gt wakeref if awake, but fails to release the
wakeref if a reset is in progress. Release the wakeref if it was
acquried successfully.
Signed-off-by: Umesh Nerlige Ramappa
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
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 1f9d4fde421f..a243304a2db1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1181,6 +1181,7 @@ static ktime_t guc_engine_busyness(struct intel_engine_cs
*engine, ktime_t *now)
struct intel_gt *gt = engine->gt;
struct intel_guc *guc = >->uc.guc;
u64 total, gt_stamp_saved;
+ intel_wakeref_t wakeref;
Should be bool.
unsigned long flags;
u32 reset_count;
bool in_reset;
@@ -1206,18 +1207,21 @@ static ktime_t guc_engine_busyness(struct
intel_engine_cs *engine, ktime_t *now)
* start_gt_clk is derived from GuC state. To get a consistent
* view of activity, we query the GuC state only if gt is awake.
*/
- if (intel_gt_pm_get_if_awake(gt) && !in_reset) {
Also how about just switch them around:
if (!in_reset && intel_gt_pm_get_if_awake(gt))
Then you don't need any other changes I think.
:) that looks much simpler, I posted this in v2.
Thanks
Umesh
+ wakeref = intel_gt_pm_get_if_awake(gt);
+ if (wakeref && !in_reset) {
stats_saved = *stats;
gt_stamp_saved = guc->timestamp.gt_stamp;
guc_update_engine_gt_clks(engine);
guc_update_pm_timestamp(guc, engine, now);
- intel_gt_pm_put_async(gt);
if (i915_reset_count(gpu_error) != reset_count) {
*stats = stats_saved;
guc->timestamp.gt_stamp = gt_stamp_saved;
}
}
+ if (wakeref)
+ intel_gt_pm_put_async(gt);
+
total = intel_gt_clock_interval_to_ns(gt, stats->total_gt_clks);
if (stats->running) {
u64 clk = guc->timestamp.gt_stamp - stats->start_gt_clk;
--
2.20.1
Re: [Intel-gfx] [PATCH] drm/i915/pmu: Fix wakeref leak in PMU busyness during reset
On Mon, 06 Dec 2021 18:02:39 -0800, Umesh Nerlige Ramappa wrote:
>
> GuC PMU busyness gets gt wakeref if awake, but fails to release the
> wakeref if a reset is in progress. Release the wakeref if it was
> acquried successfully.
>
> v2: Simplify the fix (Ashutosh)
Reviewed-by: Ashutosh Dixit
> Signed-off-by: Umesh Nerlige Ramappa
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +-
> 1 file changed, 1 insertion(+), 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 1f9d4fde421f..9739da6f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1206,7 +1206,7 @@ static ktime_t guc_engine_busyness(struct
> intel_engine_cs *engine, ktime_t *now)
>* start_gt_clk is derived from GuC state. To get a consistent
>* view of activity, we query the GuC state only if gt is awake.
>*/
> - if (intel_gt_pm_get_if_awake(gt) && !in_reset) {
> + if (!in_reset && intel_gt_pm_get_if_awake(gt)) {
> stats_saved = *stats;
> gt_stamp_saved = guc->timestamp.gt_stamp;
> guc_update_engine_gt_clks(engine);
> --
> 2.20.1
>
