RE: [PATCH 2/4] drm/amdgpu: add userq queue state transition tracepoints

2026-05-13 Thread Liang, Prike 
AMD General

Regards,
  Prike

> -Original Message-
> From: Pierre-Eric Pelloux-Prayer 
> Sent: Wednesday, May 13, 2026 5:14 PM
> To: Liang, Prike ; [email protected]
> Cc: Deucher, Alexander ; Koenig, Christian
> 
> Subject: Re: [PATCH 2/4] drm/amdgpu: add userq queue state transition 
> tracepoints
>
>
>
> Le 11/05/2026 à 15:54, Prike Liang a écrit :
> > Add ftrace events around user queue preempt, restore, map and unmap
> > operations to profile runtime queue state transitions.
> >
> > Signed-off-by: Prike Liang 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 25
> +++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 12 ++-
> >   2 files changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > index 5a01f63d1f32..484fbb00068b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > @@ -636,6 +636,31 @@ DEFINE_EVENT(amdgpu_userq_queue_result,
> amdgpu_userq_destroy_end,
> >  TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
> >  TP_ARGS(queue, result));
> >
> > +
> > +DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_preempt_start,
> > +TP_PROTO(struct amdgpu_usermode_queue *queue),
> > +TP_ARGS(queue));
> > +DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_restore_start,
> > +TP_PROTO(struct amdgpu_usermode_queue *queue),
> > +TP_ARGS(queue));
> > +DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_map_start,
> > +TP_PROTO(struct amdgpu_usermode_queue *queue),
> > +TP_ARGS(queue));
> > +DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_unmap_start,
> > +TP_PROTO(struct amdgpu_usermode_queue *queue),
> > +TP_ARGS(queue));
> > +DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_preempt_end,
> > +TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
> > +TP_ARGS(queue, result));
> > +DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_restore_end,
> > +TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
> > +TP_ARGS(queue, result));
> > +DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_map_end,
> > +TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
> > +TP_ARGS(queue, result));
> > +DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_unmap_end,
> > +TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
> > +TP_ARGS(queue, result));
> >   #undef AMDGPU_JOB_GET_TIMELINE_NAME
> >   #endif
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index 50c46d31fbae..83aee0810513 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -340,13 +340,16 @@ static int amdgpu_userq_preempt_helper(struct
> amdgpu_usermode_queue *queue)
> > int r;
> >
> > if (queue->state == AMDGPU_USERQ_STATE_MAPPED) {
> > +   trace_amdgpu_userq_preempt_start(queue);
> > r = userq_funcs->preempt(queue);
> > if (r) {
> > queue->state = AMDGPU_USERQ_STATE_HUNG;
> > +   trace_amdgpu_userq_preempt_end(queue, r);
> > return r;
> > } else {
> > queue->state = AMDGPU_USERQ_STATE_PREEMPTED;
> > }
> > +   trace_amdgpu_userq_preempt_end(queue, r);
>
> I prefer having only 2 trace points: trace_amdgpu_userq_state_change_start / 
> end.
> The _start event would print the current state and the _end one would print 
> the new
> state.

Yeah, we can simplify the queue state tracker this way, but it wouldn't be as 
straightforward and may require further parsing of the queue state to profile 
the queue's state transitions from the tooling library perspective.

Thanks,
Prike

> Also these events should be used eveywhere "queue->state" is modified.
>
> Pierre-Eric
>
>
> > }
> > return 0;
> >   }
> > @@ -360,12 +363,14 @@ static int amdgpu_userq_restore_helper(struct
> amdgpu_usermode_queue *queue)
> > int r = 0;
> >
> > if (queue->state == AMDGPU_USERQ_STATE_PREEMPTED) {
> > +   trace_amdgpu_userq_restore_start(queue);
> > r = userq_funcs->restore(queue);
> > if (r) {
> >

Re: [PATCH 2/4] drm/amdgpu: add userq queue state transition tracepoints

2026-05-13 Thread Pierre-Eric Pelloux-Prayer 




Le 11/05/2026 à 15:54, Prike Liang a écrit :

Add ftrace events around user queue preempt, restore, map and unmap
operations to profile runtime queue state transitions.

Signed-off-by: Prike Liang 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 25 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 12 ++-
  2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index 5a01f63d1f32..484fbb00068b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -636,6 +636,31 @@ DEFINE_EVENT(amdgpu_userq_queue_result, 
amdgpu_userq_destroy_end,
 TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
 TP_ARGS(queue, result));
  
+

+DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_preempt_start,
+TP_PROTO(struct amdgpu_usermode_queue *queue),
+TP_ARGS(queue));
+DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_restore_start,
+TP_PROTO(struct amdgpu_usermode_queue *queue),
+TP_ARGS(queue));
+DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_map_start,
+TP_PROTO(struct amdgpu_usermode_queue *queue),
+TP_ARGS(queue));
+DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_unmap_start,
+TP_PROTO(struct amdgpu_usermode_queue *queue),
+TP_ARGS(queue));
+DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_preempt_end,
+TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
+TP_ARGS(queue, result));
+DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_restore_end,
+TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
+TP_ARGS(queue, result));
+DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_map_end,
+TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
+TP_ARGS(queue, result));
+DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_unmap_end,
+TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
+TP_ARGS(queue, result));
  #undef AMDGPU_JOB_GET_TIMELINE_NAME
  #endif
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c

index 50c46d31fbae..83aee0810513 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -340,13 +340,16 @@ static int amdgpu_userq_preempt_helper(struct 
amdgpu_usermode_queue *queue)
int r;
  
  	if (queue->state == AMDGPU_USERQ_STATE_MAPPED) {

+   trace_amdgpu_userq_preempt_start(queue);
r = userq_funcs->preempt(queue);
if (r) {
queue->state = AMDGPU_USERQ_STATE_HUNG;
+   trace_amdgpu_userq_preempt_end(queue, r);
return r;
} else {
queue->state = AMDGPU_USERQ_STATE_PREEMPTED;
}
+   trace_amdgpu_userq_preempt_end(queue, r);


I prefer having only 2 trace points: trace_amdgpu_userq_state_change_start / 
end.
The _start event would print the current state and the _end one would print the 
new state.

Also these events should be used eveywhere "queue->state" is modified.

Pierre-Eric



}
return 0;
  }
@@ -360,12 +363,14 @@ static int amdgpu_userq_restore_helper(struct 
amdgpu_usermode_queue *queue)
int r = 0;
  
  	if (queue->state == AMDGPU_USERQ_STATE_PREEMPTED) {

+   trace_amdgpu_userq_restore_start(queue);
r = userq_funcs->restore(queue);
if (r) {
queue->state = AMDGPU_USERQ_STATE_HUNG;
} else {
queue->state = AMDGPU_USERQ_STATE_MAPPED;
}
+   trace_amdgpu_userq_restore_end(queue, r);
}
  
  	return r;

@@ -381,14 +386,16 @@ static int amdgpu_userq_unmap_helper(struct 
amdgpu_usermode_queue *queue)
  
  	if ((queue->state == AMDGPU_USERQ_STATE_MAPPED) ||

(queue->state == AMDGPU_USERQ_STATE_PREEMPTED)) {
-
+   trace_amdgpu_userq_unmap_start(queue);
r = userq_funcs->unmap(queue);
if (r) {
queue->state = AMDGPU_USERQ_STATE_HUNG;
+   trace_amdgpu_userq_unmap_end(queue, r);
return r;
} else {
queue->state = AMDGPU_USERQ_STATE_UNMAPPED;
}
+   trace_amdgpu_userq_unmap_end(queue, 0);
}
  
  	return 0;

@@ -403,13 +410,16 @@ static int amdgpu_userq_map_helper(struct 
amdgpu_usermode_queue *queue)
int r;
  
  	if (queue->state == AMDGPU_USERQ_STATE_UNMAPPED) {

+   trace_amdgpu_userq_map_start(queue);
r = userq_funcs->map(queue);
if (r) {
queue->state = AMDGPU_USERQ_STATE_HUNG;
+