Re: [PATCH 13/13] drm/xe: Block BO VM access when device I/O is unavailable

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The function `xe_bo_vm_access` mixes goto-based error handling with scope-based cleanup helpers, violating kernel cleanup guidelines.
--

commit ab2c90ef1f693fb22833ef442c6ad80ec3443ca6
Author: Arvind Yadav <arvind.yadav@intel.com>

drm/xe: Block BO VM access when device I/O is unavailable

This commit modifies xe_bo_vm_access() to return -EIO when device I/O is
blocked. It uses drm_dev_enter() to protect against concurrent unplug and
protects the block with a userfault SRCU lock.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 1eabece56f6c6..717d318c7072d 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -2245,9 +2245,25 @@ static int xe_bo_vm_access(struct vm_area_struct *vma, unsigned long addr,
>  	struct ttm_buffer_object *ttm_bo = vma->vm_private_data;
>  	struct xe_bo *bo = ttm_to_xe_bo(ttm_bo);
>  	struct xe_device *xe = xe_bo_device(bo);
> +	int idx, srcu_idx, ret = -EIO;
>  
> -	guard(xe_pm_runtime)(xe);
> -	return ttm_bo_vm_access(vma, addr, buf, len, write);
> +	srcu_idx = srcu_read_lock(&xe->mem_access.vram_userfault.srcu);
> +
> +	if (xe_device_io_blocked(xe) || !drm_dev_enter(&xe->drm, &idx))
> +		goto out_srcu;
> +
> +	/*
> +	 * Keep the drm_dev_enter() protection until the runtime PM
> +	 * reference has been released.
> +	 */
> +	scoped_guard(xe_pm_runtime, xe)
> +		ret = ttm_bo_vm_access(vma, addr, buf, len, write);

[Severity: Low]
Does this mix goto-based error handling with scope-based cleanup helpers in
xe_bo_vm_access()?

According to the cleanup guidelines in include/linux/cleanup.h, the usage
of goto and scope-based cleanup helpers should never be mixed in the same
function. 

Could this create confusing ownership semantics or risk subtle ordering bugs
on scope exit? To maintain consistency, would it be better to convert all
resources in this function to scope-based cleanup, or stick entirely to
goto-based cleanup?

> +
> +	drm_dev_exit(idx);
> +
> +out_srcu:
> +	srcu_read_unlock(&xe->mem_access.vram_userfault.srcu, srcu_idx);
> +	return ret;
>  }
>  
>  /**

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827101801.1247654-1-arvind.yadav@intel.com?part=13