Re: [PATCH v3 26/33] drm/xe: Add tracepoint for xe_gem_create_ioctl

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Tracepoint metric lock_ns incorrectly includes lock-dropped backoff sleep time due to WW mutex contention retries.
--

--- Patch [26]: [PATCH v3 26/33] drm/xe: Add tracepoint for xe_gem_create_ioctl ---
commit 08fb65ed7bb6e79b15efc1ef9667995aaebd39fe
Author: Matthew Brost <matthew.brost@intel.com>

drm/xe: Add tracepoint for xe_gem_create_ioctl

This commit adds a tracepoint to profile GEM creation in the xe driver.
It records the BO size, placement, CPU caching, ASID, total IOCTL
duration, and the time spent under the dma-resv lock to help evaluate
out-of-lock preallocation performance.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index a646a72122d40..caf5aa0bab533 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c

[ ... ]

> @@ -4100,6 +4105,10 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>  			if (err)
>  				break;
>  		}
> +
> +		if (lock_start == ktime_set(0, 0))
> +			lock_start = ktime_get();
> +

[Severity: Medium]
Will this lock_start timestamp incorrectly include lock-dropped backoff
sleep time when a WW mutex contention retry occurs?

When contention happens, drm_exec_cleanup() drops all locks and the next
iteration might sleep in dma_resv_lock_slow_interruptible() without holding
any dma-resv locks. Because of the zero-check guard here, lock_start is
retained from the first iteration.

Could this cause the lock_ns metric to encapsulate the unbounded lock-dropped
sleep time and corrupt the intended profiling metric? If vm != NULL, removing
this zero-check would exclude the sleep (which happens early in
xe_vm_drm_exec_lock). However, if vm == NULL, the sleep occurs inside
xe_bo_create_user(), so does this require capturing the start time inside
the locking functions or accumulating the actual duration per iteration?

>  		bo = xe_bo_create_user(xe, vm, args->size, args->cpu_caching,
>  				       bo_flags, have_prealloc ? &prealloc : NULL,
>  				       &exec);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711025619.2540575-1-matthew.brost@intel.com?part=26