Re: [PATCH v5 4/4] drm/xe/forcewake: enable configurable delayed forcewake release

On 8/12/2026 5:18 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The function `query_engine_cycles` mixes goto-based error handling with scope-based cleanup macros.
> --
>
> --- Patch [4]: [PATCH v5 4/4] drm/xe/forcewake: enable configurable delayed forcewake release ---
> commit 7d3e5746e331fb4da88407a96b40b25e06abf763
> Author: Zongyao Bai <[email protected]>
>
> drm/xe/forcewake: enable configurable delayed forcewake release
>
> This commit introduces a configurable delayed forcewake release for
> engine-cycle queries to avoid repeated wake and ACK cycles during bursts.
> It adds a configfs attribute to control the hold interval before releasing
> an idle forcewake domain.
>
>> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
>> --- a/drivers/gpu/drm/xe/xe_query.c
>> +++ b/drivers/gpu/drm/xe/xe_query.c
>> @@ -168,7 +168,7 @@ query_engine_cycles(struct xe_device *xe,
>>   
>>   	fw_domain = xe_hw_engine_to_fw_domain(hwe);
>>   
>> -	xe_with_force_wake(fw_ref, gt_to_fw(gt), fw_domain) {
>> +	xe_with_force_wake_delay(fw_ref, gt_to_fw(gt), fw_domain) {
>>   		if (!xe_force_wake_ref_has_domain(fw_ref.domains, fw_domain)) {
>>   			err = -EIO;
>>   			goto out;
> [Severity: Low]
> Does this code mix goto-based error handling with scope-based cleanup macros?
>
> According to the cleanup subsystem guidelines, the expectation is that usage of
> goto and cleanup helpers is never mixed in the same function.
>
> The function query_engine_cycles() uses goto out; for manual drm_dev_exit()
> cleanup while simultaneously using the xe_with_force_wake_delay() macro, which
> internally uses the scope-based CLASS() cleanup macro. While jumping out of the
> loop block correctly invokes the scope destructor here without causing a
> double-free or resource leak, it violates the subsystem structural guidelines
> against mixing these cleanup styles.

Yes, I agree this is code style issue,
this function contains both a goto out and the 
xe_with_force_wake_delay() scope macro.
And this issue is introduced by path [PATCH v5 1/4], but not in this patch.

It may need split query_engine_cycles to two to avoid goto.
Since this appears to be more of a coding style preference,
I'd like to wait for consensus from other reviewers before updating it.