From: "Christian König" <christian.koenig@amd.com>
To: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>,
Alex Deucher <alexander.deucher@amd.com>,
David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
Matthew Brost <matthew.brost@intel.com>,
Danilo Krummrich <dakr@kernel.org>,
Philipp Stanner <phasta@kernel.org>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>
Cc: "Tvrtko Ursulin" <tursulin@igalia.com>,
"Christian König" <ckoenig.leichtzumerken@gmail.com>,
amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 09/10] drm: get rid of drm_sched_job::id
Date: Thu, 20 Mar 2025 11:09:57 +0100 [thread overview]
Message-ID: <b23bb5ea-3e19-4e70-89c2-0bee658cac72@amd.com> (raw)
In-Reply-To: <20250320095818.40622-10-pierre-eric.pelloux-prayer@amd.com>
Am 20.03.25 um 10:58 schrieb Pierre-Eric Pelloux-Prayer:
> Its only purpose was for trace events, but jobs can already be
> uniquely identified using their fence.
>
> The downside of using the fence is that it's only available
> after 'drm_sched_job_arm' was called which is true for all trace
> events that used job.id so they can safely switch to using it.
>
> Suggested-by: Tvrtko Ursulin <tursulin@igalia.com>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 18 ++++++------------
> .../gpu/drm/scheduler/gpu_scheduler_trace.h | 18 ++++++------------
> drivers/gpu/drm/scheduler/sched_main.c | 1 -
> include/drm/gpu_scheduler.h | 3 ---
> 4 files changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> index 383fce40d4dd..a4f394d827bc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> @@ -167,7 +167,6 @@ TRACE_EVENT(amdgpu_cs_ioctl,
> TP_PROTO(struct amdgpu_job *job),
> TP_ARGS(job),
> TP_STRUCT__entry(
> - __field(uint64_t, sched_job_id)
> __string(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job))
> __field(unsigned int, context)
> __field(unsigned int, seqno)
> @@ -177,15 +176,14 @@ TRACE_EVENT(amdgpu_cs_ioctl,
> ),
>
> TP_fast_assign(
> - __entry->sched_job_id = job->base.id;
> __assign_str(timeline);
> __entry->context = job->base.s_fence->finished.context;
> __entry->seqno = job->base.s_fence->finished.seqno;
> __assign_str(ring);
> __entry->num_ibs = job->num_ibs;
> ),
> - TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, ring_name=%s, num_ibs=%u",
> - __entry->sched_job_id, __get_str(timeline), __entry->context,
> + TP_printk("timeline=%s, context=%u, seqno=%u, ring_name=%s, num_ibs=%u",
> + __get_str(timeline), __entry->context,
> __entry->seqno, __get_str(ring), __entry->num_ibs)
> );
>
> @@ -193,7 +191,6 @@ TRACE_EVENT(amdgpu_sched_run_job,
> TP_PROTO(struct amdgpu_job *job),
> TP_ARGS(job),
> TP_STRUCT__entry(
> - __field(uint64_t, sched_job_id)
> __string(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job))
> __field(unsigned int, context)
> __field(unsigned int, seqno)
> @@ -202,15 +199,14 @@ TRACE_EVENT(amdgpu_sched_run_job,
> ),
>
> TP_fast_assign(
> - __entry->sched_job_id = job->base.id;
> __assign_str(timeline);
> __entry->context = job->base.s_fence->finished.context;
> __entry->seqno = job->base.s_fence->finished.seqno;
> __assign_str(ring);
> __entry->num_ibs = job->num_ibs;
> ),
> - TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, ring_name=%s, num_ibs=%u",
> - __entry->sched_job_id, __get_str(timeline), __entry->context,
> + TP_printk("timeline=%s, context=%u, seqno=%u, ring_name=%s, num_ibs=%u",
> + __get_str(timeline), __entry->context,
> __entry->seqno, __get_str(ring), __entry->num_ibs)
> );
>
> @@ -519,7 +515,6 @@ TRACE_EVENT(amdgpu_ib_pipe_sync,
> TP_ARGS(sched_job, fence),
> TP_STRUCT__entry(
> __string(ring, sched_job->base.sched->name)
> - __field(uint64_t, id)
> __field(struct dma_fence *, fence)
> __field(uint64_t, ctx)
> __field(unsigned, seqno)
> @@ -527,13 +522,12 @@ TRACE_EVENT(amdgpu_ib_pipe_sync,
>
> TP_fast_assign(
> __assign_str(ring);
> - __entry->id = sched_job->base.id;
> __entry->fence = fence;
> __entry->ctx = fence->context;
> __entry->seqno = fence->seqno;
> ),
> - TP_printk("job ring=%s, id=%llu, need pipe sync to fence=%p, context=%llu, seq=%u",
> - __get_str(ring), __entry->id,
> + TP_printk("job ring=%s need pipe sync to fence=%p, context=%llu, seq=%u",
> + __get_str(ring),
> __entry->fence, __entry->ctx,
> __entry->seqno)
> );
> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> index 3c7f6a39cf91..ad03240b2f03 100644
> --- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> @@ -58,7 +58,6 @@ DECLARE_EVENT_CLASS(drm_sched_job,
> TP_PROTO(struct drm_sched_job *sched_job, struct drm_sched_entity *entity),
> TP_ARGS(sched_job, entity),
> TP_STRUCT__entry(
> - __field(uint64_t, id)
> __string(name, sched_job->sched->name)
> __field(u32, job_count)
> __field(int, hw_job_count)
> @@ -69,7 +68,6 @@ DECLARE_EVENT_CLASS(drm_sched_job,
> ),
>
> TP_fast_assign(
> - __entry->id = sched_job->id;
> __assign_str(name);
> __entry->job_count = spsc_queue_count(&entity->job_queue);
> __entry->hw_job_count = atomic_read(
> @@ -79,8 +77,8 @@ DECLARE_EVENT_CLASS(drm_sched_job,
> __entry->fence_seqno = sched_job->s_fence->finished.seqno;
> __entry->client_id = sched_job->s_fence->drm_client_id;
> ),
> - TP_printk("dev=%s, id=%llu, fence=%llu:%llu, ring=%s, job count:%u, hw job count:%d, client_id:%llu",
> - __get_str(dev), __entry->id,
> + TP_printk("dev=%s, fence=%llu:%llu, ring=%s, job count:%u, hw job count:%d, client_id:%llu",
> + __get_str(dev),
> __entry->fence_context, __entry->fence_seqno, __get_str(name),
> __entry->job_count, __entry->hw_job_count, __entry->client_id)
> );
> @@ -117,7 +115,6 @@ TRACE_EVENT(drm_sched_job_add_dep,
> TP_STRUCT__entry(
> __field(u64, fence_context)
> __field(u64, fence_seqno)
> - __field(u64, id)
> __field(u64, ctx)
> __field(u64, seqno)
> ),
> @@ -125,12 +122,11 @@ TRACE_EVENT(drm_sched_job_add_dep,
> TP_fast_assign(
> __entry->fence_context = sched_job->s_fence->finished.context;
> __entry->fence_seqno = sched_job->s_fence->finished.seqno;
> - __entry->id = sched_job->id;
> __entry->ctx = fence->context;
> __entry->seqno = fence->seqno;
> ),
> - TP_printk("fence=%llu:%llu, id=%llu depends on fence=%llu:%llu",
> - __entry->fence_context, __entry->fence_seqno, __entry->id,
> + TP_printk("fence=%llu:%llu depends on fence=%llu:%llu",
> + __entry->fence_context, __entry->fence_seqno,
> __entry->ctx, __entry->seqno)
> );
>
> @@ -140,7 +136,6 @@ TRACE_EVENT(drm_sched_job_unschedulable,
> TP_STRUCT__entry(
> __field(u64, fence_context)
> __field(u64, fence_seqno)
> - __field(uint64_t, id)
> __field(u64, ctx)
> __field(u64, seqno)
> ),
> @@ -148,12 +143,11 @@ TRACE_EVENT(drm_sched_job_unschedulable,
> TP_fast_assign(
> __entry->fence_context = sched_job->s_fence->finished.context;
> __entry->fence_seqno = sched_job->s_fence->finished.seqno;
> - __entry->id = sched_job->id;
> __entry->ctx = fence->context;
> __entry->seqno = fence->seqno;
> ),
> - TP_printk("fence=%llu:%llu, id=%llu depends on unsignalled fence=%llu:%llu",
> - __entry->fence_context, __entry->fence_seqno, __entry->id,
> + TP_printk("fence=%llu:%llu depends on unsignalled fence=%llu:%llu",
> + __entry->fence_context, __entry->fence_seqno,
> __entry->ctx, __entry->seqno)
> );
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> index 85c2111e5500..85e0ba850746 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -848,7 +848,6 @@ void drm_sched_job_arm(struct drm_sched_job *job)
>
> job->sched = sched;
> job->s_priority = entity->priority;
> - job->id = atomic64_inc_return(&sched->job_id_count);
>
> drm_sched_fence_init(job->s_fence, job->entity);
> }
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 032214a49138..ddc24512c281 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -326,7 +326,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
> * @finish_cb: the callback for the finished fence.
> * @credits: the number of credits this job contributes to the scheduler
> * @work: Helper to reschedule job kill to different context.
> - * @id: a unique id assigned to each job scheduled on the scheduler.
> * @karma: increment on every hang caused by this job. If this exceeds the hang
> * limit of the scheduler then the job is marked guilty and will not
> * be scheduled further.
> @@ -339,8 +338,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
> * to schedule the job.
> */
> struct drm_sched_job {
> - u64 id;
> -
> /**
> * @submit_ts:
> *
next prev parent reply other threads:[~2025-03-20 10:10 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-20 9:57 [PATCH v8 00/10] Improve gpu_scheduler trace events + UAPI Pierre-Eric Pelloux-Prayer
2025-03-20 9:57 ` [PATCH v8 01/10] drm/debugfs: output client_id in in drm_clients_info Pierre-Eric Pelloux-Prayer
2025-03-20 9:57 ` [PATCH v8 02/10] drm/sched: store the drm client_id in drm_sched_fence Pierre-Eric Pelloux-Prayer
2025-03-20 9:57 ` [PATCH v8 03/10] drm/sched: add device name to the drm_sched_process_job event Pierre-Eric Pelloux-Prayer
2025-03-20 9:58 ` [PATCH v8 04/10] drm/sched: cleanup gpu_scheduler trace events Pierre-Eric Pelloux-Prayer
2025-03-20 10:46 ` Tvrtko Ursulin
2025-03-20 9:58 ` [PATCH v8 05/10] drm/sched: trace dependencies for gpu jobs Pierre-Eric Pelloux-Prayer
2025-03-20 10:47 ` Tvrtko Ursulin
2025-03-20 9:58 ` [PATCH v8 06/10] drm/sched: add the drm_client_id to the drm_sched_run/exec_job events Pierre-Eric Pelloux-Prayer
2025-04-09 7:56 ` Philipp Stanner
2025-03-20 9:58 ` [PATCH v8 07/10] drm/sched: cleanup event names Pierre-Eric Pelloux-Prayer
2025-03-20 10:49 ` Tvrtko Ursulin
2025-03-20 9:58 ` [PATCH v8 08/10] drm/doc: document some tracepoints as uAPI Pierre-Eric Pelloux-Prayer
2025-03-20 9:58 ` [PATCH v8 09/10] drm: get rid of drm_sched_job::id Pierre-Eric Pelloux-Prayer
2025-03-20 10:09 ` Christian König [this message]
2025-03-20 10:55 ` Tvrtko Ursulin
2025-03-20 9:58 ` [PATCH v8 10/10] drm/amdgpu: update trace format to match gpu_scheduler_trace Pierre-Eric Pelloux-Prayer
2025-03-20 10:09 ` Christian König
2025-04-09 7:52 ` [PATCH v8 00/10] Improve gpu_scheduler trace events + UAPI Pierre-Eric Pelloux-Prayer
2025-04-09 8:03 ` Philipp Stanner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b23bb5ea-3e19-4e70-89c2-0bee658cac72@amd.com \
--to=christian.koenig@amd.com \
--cc=airlied@gmail.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=dakr@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=matthew.brost@intel.com \
--cc=mripard@kernel.org \
--cc=phasta@kernel.org \
--cc=pierre-eric.pelloux-prayer@amd.com \
--cc=simona@ffwll.ch \
--cc=tursulin@igalia.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox