[PATCH 2/2] drm/amdgpu: trace fence details in amdgpu_sched_run_job

[Christian König]

Am 24.02.2017 um 19:20 schrieb Andres Rodriguez:
> 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 at gmail.com>
> ---
>   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/amdgpu_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->base.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->base.s_fence->finished))

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.

> +			   __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",

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.

Anyway that should be done in an extra patch, so this one is 
Reviewed-by: Christian König <christian.koenig at amd.com>.

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)
>   );
>   
>