| Message ID | 20211207004542.24298-1-umesh.nerlige.ramappa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/pmu: Fix wakeref leak in PMU busyness during reset | expand |
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 <umesh.nerlige.ramappa@intel.com> > --- > 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 >
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 <umesh.nerlige.ramappa@intel.com> >> --- >> 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 >>
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;
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 <umesh.nerlige.ramappa@intel.com> --- drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)