| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <cc3444d8-a354-4332-93e7-0b1a70d3c4ac@ursulin.net>
Date: Tue, 4 Feb 2025 15:22:57 +0000
From: Tvrtko Ursulin <tursulin@...ulin.net>
To: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@....com>,
Luben Tuikov <ltuikov89@...il.com>, Matthew Brost <matthew.brost@...el.com>,
Danilo Krummrich <dakr@...nel.org>, Philipp Stanner <pstanner@...hat.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Sumit Semwal <sumit.semwal@...aro.org>,
Christian König <christian.koenig@....com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v7 4/7] drm/sched: cleanup gpu_scheduler trace events
On 31/01/2025 11:03, Pierre-Eric Pelloux-Prayer wrote:
> A fence uniquely identify a job, so this commits updates the places
> where a kernel pointer was used as an identifier by:
>
> "fence=%llu:%llu"
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@....com>
> ---
> .../gpu/drm/scheduler/gpu_scheduler_trace.h | 41 +++++++++++--------
> 1 file changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> index c4ec28540656..3cdd8d8f8021 100644
> --- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> @@ -36,28 +36,29 @@ 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(struct drm_sched_entity *, entity)
> - __field(struct dma_fence *, fence)
> - __string(name, sched_job->sched->name)
> __field(uint64_t, id)
> + __string(name, sched_job->sched->name)
> __field(u32, job_count)
> __field(int, hw_job_count)
> __string(dev, dev_name(sched_job->sched->dev))
> + __field(uint64_t, fence_context)
> + __field(uint64_t, fence_seqno)
> ),
>
> TP_fast_assign(
> - __entry->entity = entity;
> __entry->id = sched_job->id;
> - __entry->fence = &sched_job->s_fence->finished;
> __assign_str(name);
> __entry->job_count = spsc_queue_count(&entity->job_queue);
> __entry->hw_job_count = atomic_read(
> &sched_job->sched->credit_count);
> __assign_str(dev);
> + __entry->fence_context = sched_job->s_fence->finished.context;
> + __entry->fence_seqno = sched_job->s_fence->finished.seqno;
> +
> ),
> - 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),
> + TP_printk("dev=%s, id=%llu, fence=%llu:%llu, ring=%s, job count:%u, hw job count:%d",
> + __get_str(dev), __entry->id,
> + __entry->fence_context, __entry->fence_seqno, __get_str(name),
> __entry->job_count, __entry->hw_job_count)
> );
>
> @@ -75,37 +76,41 @@ TRACE_EVENT(drm_sched_process_job,
> TP_PROTO(struct drm_sched_fence *fence),
> TP_ARGS(fence),
> TP_STRUCT__entry(
> - __field(struct dma_fence *, fence)
> + __field(uint64_t, fence_context)
> + __field(uint64_t, fence_seqno)
> ),
>
> TP_fast_assign(
> - __entry->fence = &fence->finished;
> + __entry->fence_context = fence->finished.context;
> + __entry->fence_seqno = fence->finished.seqno;
> ),
> - TP_printk("fence=%p signaled", __entry->fence)
> + TP_printk("fence=%llu:%llu signaled",
> + __entry->fence_context, __entry->fence_seqno)
Any chance to rename this tracepoint while changing things around? For
me "process" is not intuitive to what stage it refers so maybe a set of
tracepoints like:
drm_sched_job_(wait_)dependenc(y|ies) - more on this in the next patch
drm_sched_job_queue
drm_sched_job_run
drm_sched_job_done
So the naming is standardised.
> );
>
> TRACE_EVENT(drm_sched_job_wait_dep,
> TP_PROTO(struct drm_sched_job *sched_job, struct dma_fence *fence),
> TP_ARGS(sched_job, fence),
> TP_STRUCT__entry(
> - __string(name, sched_job->sched->name)
> + __field(uint64_t, fence_context)
> + __field(uint64_t, fence_seqno)
> __field(uint64_t, id)
> __field(struct dma_fence *, fence)
> __field(uint64_t, ctx)
> - __field(unsigned, seqno)
> + __field(uint64_t, seqno)
> ),
>
> TP_fast_assign(
> - __assign_str(name);
> + __entry->fence_context = sched_job->s_fence->finished.context;
> + __entry->fence_seqno = sched_job->s_fence->finished.seqno;
> __entry->id = sched_job->id;
> __entry->fence = fence;
> __entry->ctx = fence->context;
> __entry->seqno = fence->seqno;
> ),
> - TP_printk("job ring=%s, id=%llu, depends fence=%p, context=%llu, seq=%u",
> - __get_str(name), __entry->id,
> - __entry->fence, __entry->ctx,
> - __entry->seqno)
> + TP_printk("fence=%llu:%llu, id=%llu, dependencies:{fence=%llu:%llu}",
Will dependencies ever become a list here? Just wondering if plural and
curlies bring anything of value, or if it would be more readable as
"fence=%llu:%llu, id=%llu, dependency=%llu:%llu".
Regards,
Tvrtko
> + __entry->fence_context, __entry->fence_seqno, __entry->id,
> + __entry->ctx, __entry->seqno)
> );
>
> #endif
Powered by blists - more mailing lists