[Intel-gfx] [PATCH] drm/i915/pmu: Fix wakeref leak in PMU busyness during reset

[Umesh Nerlige Ramappa]

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 at 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 = &gt->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
>>