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 <[email protected]> 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/[email protected]?part=13
