Re: [PATCH v5 1/4] drm/sched: add device name to the drm_sched_process_job event

[Pierre-Eric Pelloux-Prayer <pierre-eric@xxxxxxxxx>]

Hi,

Le 14/06/2024 à 11:08, Christian König a écrit :
> Am 14.06.24 um 10:16 schrieb Pierre-Eric Pelloux-Prayer:
> > Until the switch from kthread to workqueue, a userspace application could
> > determine the source device from the pid of the thread sending the event.
> >
> > With workqueues this is not possible anymore, so the event needs to 
> > contain
> > the dev_name() to identify the device.
> >
> > Signed-off-by: Pierre-Eric Pelloux-Prayer 
> > <pierre-eric.pelloux-prayer@xxxxxxx>
> > ---
> >   drivers/gpu/drm/scheduler/gpu_scheduler_trace.h | 10 ++++++++--
> >   1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h 
> > b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> > index c75302ca3427..1f9c5a884487 100644
> > --- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> > @@ -42,6 +42,7 @@ DECLARE_EVENT_CLASS(drm_sched_job,
> >                    __field(uint64_t, id)
> >                    __field(u32, job_count)
> >                    __field(int, hw_job_count)
> > +                 __string(dev, dev_name(sched_job->sched->dev))
> >                    ),
> >           TP_fast_assign(
> > @@ -52,6 +53,7 @@ DECLARE_EVENT_CLASS(drm_sched_job,
> >                  __entry->job_count = 
> > spsc_queue_count(&entity->job_queue);
> >                  __entry->hw_job_count = atomic_read(
> >                      &sched_job->sched->credit_count);
> > +               __assign_str(dev);
> >                  ),
> >           TP_printk("entity=%p, id=%llu, fence=%p, ring=%s, job 
> > count:%u, hw job count:%d",
> >                 __entry->entity, __entry->id,
> > @@ -64,9 +66,13 @@ DEFINE_EVENT(drm_sched_job, drm_sched_job,
> >           TP_ARGS(sched_job, entity)
> >   );
> > -DEFINE_EVENT(drm_sched_job, drm_run_job,
> > +DEFINE_EVENT_PRINT(drm_sched_job, drm_run_job,
> >           TP_PROTO(struct drm_sched_job *sched_job, struct 
> > drm_sched_entity *entity),
> > -        TP_ARGS(sched_job, entity)
> > +        TP_ARGS(sched_job, entity),
> > +        TP_printk("dev=%s, entity=%p id=%llu, fence=%p, ring=%s, job 
> > count:%u, hw job count:%d",
> > +              __get_str(dev), __entry->entity, __entry->id,
> > +              __entry->fence, __get_str(name),
> > +              __entry->job_count, __entry->hw_job_count)
>
> Why not doing that in the TP_printk() of the drm_sched_job event class?

I didn't because drm_sched_job + drm_run_job usually work in pair so the 
ring+device information will be available anyway.

But maybe you're right and it's better to add it in the event the job 
never gets scheduled.

For the other events it's not necessary IMO: the first event will always 
be drm_sched_job, so it's possible to figure out which the device/ring 
for a given job.

Regards,
Pierre-eric

> The device should now be available for all trace events of that class.
>
> Regards,
> Christian.
>
> >   );
> >   TRACE_EVENT(drm_sched_process_job,