Re: [PATCH 08/20] drm/xe/eudebug: Introduce exec_queue events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] drm/xe/eudebug: discover_client() counts non-debuggable queues
  in eq_count
--

commit 926a26106604e590716ee8ab1001dab4ed5f0566
Author: Dominik Grzegorzek <dominik.grzegorzek@intel.com>

drm/xe/eudebug: Introduce exec_queue events

This commit adds eudebug resource creation and destruction events for
debuggable render and compute exec queues, both during live ioctls and
when discovering an existing client's resources.

> diff --git a/drivers/gpu/drm/xe/xe_eudebug.c b/drivers/gpu/drm/xe/xe_eudebug.c
> index e0ef34588d3d4..0f8b0452f9bae 100644
> --- a/drivers/gpu/drm/xe/xe_eudebug.c
> +++ b/drivers/gpu/drm/xe/xe_eudebug.c
[ ... ]
> +void xe_eudebug_exec_queue_create(struct xe_file *xef, struct xe_exec_queue *q)
> +{
> +	struct xe_eudebug *d;
> +	int err;
> +
> +	if (!xe_engine_supports_eudebug(q->class))
> +		return;
> +
> +	d = xe_eudebug_get(xef);
> +	if (!d)
> +		return;
> +
> +	err = exec_queue_create_event(d, xef, q);
> +	if (err)
> +		xe_eudebug_disconnect(d, err);
> +
> +	xe_eudebug_put(d);
> +}
> +
> +void xe_eudebug_exec_queue_destroy(struct xe_file *xef, struct xe_exec_queue *q)
> +{
> +	struct xe_eudebug *d;
> +	int err;
> +
> +	if (!xe_engine_supports_eudebug(q->class))
> +		return;
> +
> +	d = xe_eudebug_get(xef);
> +	if (!d)
> +		return;
> +
> +	err = exec_queue_destroy_event(d, xef, q);
> +	if (err)
> +		xe_eudebug_disconnect(d, err);
> +
> +	xe_eudebug_put(d);
> +}
> +
>  /*
>   * All the relevant ioctls that manipulate the xe_file resources eudebug
>   * cares about are gated behind down_read(&xef->eudebug.ioctl_lock). The
> @@ -807,9 +1069,10 @@ void xe_eudebug_vm_destroy(struct xe_file *xef, struct xe_vm *vm)
>   */
>  static int discover_client(struct xe_eudebug *d, struct xe_file *xef)
>  {
> +	struct xe_exec_queue *q;
>  	struct xe_vm *vm;
>  	unsigned long i;
> -	unsigned int vm_count = 0;
> +	unsigned int vm_count = 0, eq_count = 0;
>  	int err = 0;
>  
>  	lockdep_assert_held_write(&xef->eudebug.ioctl_lock);
> @@ -828,10 +1091,21 @@ static int discover_client(struct xe_eudebug *d, struct xe_file *xef)
>  		vm_count++;
>  	}
>  
> +	xa_for_each(&xef->exec_queue.xa, i, q) {
> +		if (!xe_engine_supports_eudebug(q->class))
> +			continue;
> +
> +		err = exec_queue_create_event(d, xef, q);
> +		if (err)
> +			goto out;
> +
> +		eq_count++;
> +	}
> +
>  out:
> -	if (vm_count)
> -		eu_dbg(d, "Discovery found %u vms",
> -		       vm_count);
> +	if (vm_count || eq_count)
> +		eu_dbg(d, "Discovery found %u vms, %u exec_queues",
> +		       vm_count, eq_count);

[Severity: Low]
Does discover_client() count non-debuggable render and compute queues in
eq_count?

exec_queue_create_event() returns 0 without emitting an event when
!xe_exec_queue_is_debuggable(q), so any render or compute queue created
without DRM_XE_EXEC_QUEUE_SET_PROPERTY_EUDEBUG still increments eq_count.
Checking xe_exec_queue_is_debuggable(q) in discover_client(), as well as
at the top of xe_eudebug_exec_queue_create() and
xe_eudebug_exec_queue_destroy(), would also avoid taking and dropping the
debugger reference for non-debuggable queues.  Finally, the kernel-doc for
struct drm_xe_eudebug_event_exec_queue in include/uapi/drm/xe_drm_eudebug.h
could mention that only queues with DRM_XE_EXEC_QUEUE_SET_PROPERTY_EUDEBUG
are reported, matching struct drm_xe_eudebug_event_vm.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109135549.4128280-1-mika.kuoppala@linux.intel.com?part=8