[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 >