On Feb 25, 2017 4:40 AM, "Christian König" <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> wrote:
Am 24.02.2017 um 19:20 schrieb Andres Rodriguez:Not 100% sure, but we might be able to use a char * field here instead of the extra overhead of embedding a string, the timeline names are never freed IIRC.
This information is intended to provide the required data to associate
amdgpu tracepoints with their corresponding dma_fence_* events.
Signed-off-by: Andres Rodriguez <andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/a mdgpu_trace.h
index 01623d1..cc9a31d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -130,6 +130,9 @@ TRACE_EVENT(amdgpu_sched_run_job,
__field(struct amd_sched_job *, sched_job)
__field(struct amdgpu_ib *, ib)
__field(struct dma_fence *, fence)
+ __string(timeline, job->base.s_fence->finished.ops->get_timeline_name(&job->bas e.s_fence->finished))
+ __field(unsigned int, context)
+ __field(unsigned int, seqno)
__field(char *, ring_name)
__field(u32, num_ibs)
),
@@ -139,12 +142,16 @@ TRACE_EVENT(amdgpu_sched_run_job,
__entry->sched_job = &job->base;
__entry->ib = job->ibs;
__entry->fence = &job->base.s_fence->finished;
+ __assign_str(timeline, job->base.s_fence->finished.ops->get_timeline_name(&job->bas e.s_fence->finished))
I'll give this a try.If you have time for another patch please drop the pinters here as well. Scheduler job, IBs and fence are all heavily reallocated (sometimes even with a slab allocator). So the pointers are completely meaningless. The adev pointer is superseded by the timeline name and context numbers.
+ __entry->context = job->base.s_fence->finished.context;
+ __entry->seqno = job->base.s_fence->finished.seqno;
__entry->ring_name = job->ring->name;
__entry->num_ibs = job->num_ibs;
),
- TP_printk("adev=%p, sched_job=%p, first ib=%p, sched fence=%p, ring name=%s, num_ibs=%u",
+ TP_printk("adev=%p, sched_job=%p, first ib=%p, sched fence=%p, timeline=%s, context=%u, seqno=%u, ring name=%s, num_ibs=%u",
The pointers do provide some useful data. But you need to make sure you read it in between an allocation event and a fence signal event. It is extremely terrible for reading from a trace text file.
How do you feel about if we replaced the job pointer with a job id, and replaced the fence pointers with the fence data?
Anyway that should be done in an extra patch, so this one is Reviewed-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>.
Regards,
Christian.
__entry->adev, __entry->sched_job, __entry->ib,
- __entry->fence, __entry->ring_name, __entry->num_ibs)
+ __entry->fence, __get_str(timeline), __entry->context, __entry->seqno,
+ __entry->ring_name, __entry->num_ibs)
);