Re: [PATCH v9 10/20] drm/xe/vsec: Support possible hotplug exit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cleanup: mix of goto and scoped_guard() in xe_pmt_telem_read()
--

commit 5940c0b6f7d9cab3fae443cef41e2d74cd3f39e9
Author: Michael J. Ruhl <michael.j.ruhl@intel.com>

drm/xe/vsec: Support possible hotplug exit

This commit introduces DRM hotplug verification to the VSEC callback API.
It verifies that the device is valid using drm_dev_enter() and handles
cleanup on exit with drm_dev_exit().

> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index ca5d02dce8189..c20cad9a4f1f2 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
[ ... ]
> @@ -171,13 +178,17 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  	u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
>  	u32 mem_region;
>  	u32 offset;
> -	int ret;
> +	int ret = 0;
> +	int idx;
>  
>  	xe_assert(xe, !IS_SRIOV_VF(xe));
>  
> +	if (!drm_dev_enter(&xe->drm, &idx))
> +		return -ENODEV;
> +
>  	ret = xe_guid_decode(guid, &mem_region, &offset);
>  	if (ret)
> -		return ret;
> +		goto dev_exit;
>  
>  	telem_addr += offset + user_offset;
>  
> @@ -187,11 +198,14 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  		xe_pm_runtime_get(xe);
>  		break;
>  	case TELEMETRY:
> -		if (!xe_pm_runtime_get_if_active(xe))
> -			return -ENODATA;
> +		if (!xe_pm_runtime_get_if_active(xe)) {
> +			ret = -ENODATA;
> +			goto dev_exit;
> +		}
>  		break;
>  	case WATCHER:
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto dev_exit;
>  	}
>  
>  	scoped_guard(mutex, &xe->pmt.lock) {

[Severity: Low]
Does mixing goto-based error handling and scoped_guard() in the same
function create an issue with the cleanup guidelines?

According to include/linux/cleanup.h, the expectation is that usage of
goto and cleanup helpers should never be mixed in the same function, as it
creates confusing ownership semantics. 

Since xe_pmt_telem_read() now uses both a dev_exit goto label and
scoped_guard() for mutex management, could the drm_dev_exit() sequence be 
converted to use a scope-based cleanup instead, or could the locking avoid
using scoped_guard() to keep the paradigms consistent?

> @@ -203,7 +217,10 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  
>  	xe_pm_runtime_put(xe);
>  
> -	return count;
> +dev_exit:
> +	drm_dev_exit(idx);
> +
> +	return ret == 0 ? count : ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916201139.2374531-22-michael.j.ruhl@intel.com?part=10