[PATCH 2/2] drm/xe: Do not access xe file when updating exec queue run_ticks

[Lucas De Marchi]

On Fri, May 24, 2024 at 04:47:44PM GMT, Umesh Nerlige Ramappa wrote:
>The current code is running into a use after free case where xe file is
>closed before the exec queue run_ticks can be updated. This is occurring
>in the xe_file_close path. To fix that, do not access xe file when
>updating the exec queue run_ticks. Instead store the exec queue run_ticks
>locally in the exec queue object and accumulate it when the user dumps
>the drm client stats. We know that the xe file is valid when user is
>dumping the run_ticks for the drm client, so this effectively
>removes the dependency on xe file object in
>xe_exec_queue_update_run_ticks().
>
>v2:
>- Fix the accumulation of q->run_ticks delta into xe file run_ticks
>- s/runtime/run_ticks/ (Rodrigo)
>
>Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1908
>Fixes: 6109f24f87d7 ("drm/xe: Add helper to accumulate exec queue runtime")
>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>---
> drivers/gpu/drm/xe/xe_drm_client.c       | 5 ++++-
> drivers/gpu/drm/xe/xe_exec_queue.c       | 5 +----
> drivers/gpu/drm/xe/xe_exec_queue_types.h | 4 ++++
> 3 files changed, 9 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c
>index 1dcae0e6d66d..4a19b771e3a0 100644
>--- a/drivers/gpu/drm/xe/xe_drm_client.c
>+++ b/drivers/gpu/drm/xe/xe_drm_client.c
>@@ -251,8 +251,11 @@ static void show_run_ticks(struct drm_printer *p, struct drm_file *file)
>
> 	/* Accumulate all the exec queues from this client */
> 	mutex_lock(&xef->exec_queue.lock);
>-	xa_for_each(&xef->exec_queue.xa, i, q)
>+	xa_for_each(&xef->exec_queue.xa, i, q) {
> 		xe_exec_queue_update_run_ticks(q);
>+		xef->run_ticks[q->class] += q->run_ticks - q->old_run_ticks;
>+		q->old_run_ticks = q->run_ticks;

these 2 lines are kind of a layer violation... we shouldn't be setting
something in xe_exec_queue from xe_drm_client. Since we can't return the
delta from xe_exec_queue_update_run_ticks() since it's called from
places other than the query by userspace, we should probably move
these 2 lines code to a function like....
xe_exec_queue_get_delta_run_ticks() or s/get/flush/ or something else.

other than that, lgtm. Thanks for the fix.


Lucas De Marchi