[PATCH] drm/amdgpu: trace amdgpu_job fence details

[Christian König]

Am 23.02.2017 um 00:47 schrieb Andres Rodriguez:
> This trace is intended to provide the required information to associate
> the completion of an amdgpu_job with its corresponding dma_fence_*
> tracepoints.
>
> Signed-off-by: Andres Rodriguez <andresx7 at gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c   |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 22 ++++++++++++++++++++++
>   2 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 86a1242..84a04e4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -177,6 +177,8 @@ static struct dma_fence *amdgpu_job_run(struct amd_sched_job *sched_job)
>   	/* if gpu reset, hw fence will be replaced here */
>   	dma_fence_put(job->fence);
>   	job->fence = dma_fence_get(fence);
> +	trace_amdgpu_job_attach_fence(job);
> +
>   	amdgpu_job_free_resources(job);
>   	return fence;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> index a18ae1e..0a61ed9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> @@ -147,6 +147,28 @@ TRACE_EVENT(amdgpu_sched_run_job,
>   		      __entry->fence, __entry->ring_name, __entry->num_ibs)
>   );
>   
> +TRACE_EVENT(amdgpu_job_attach_fence,
> +	    TP_PROTO(struct amdgpu_job *job),
> +	    TP_ARGS(job),
> +	    TP_STRUCT__entry(
> +			     __field(struct amdgpu_device *, adev)
> +			     __field(struct amd_sched_job *, sched_job)

Neither add adev nor sched_job pointer to your trace point.

The adev pointer is completely meaningless in the logs and the scheduler 
job might already be freed when the printk is called.

Just checked the source and a couple of other trace points get this 
horrible wrong as well, so that isn't your fault. Going to put it on my 
todo to fix those.

Regards,
Christian.

> +			     __string(timeline, job->fence->ops->get_timeline_name(job->fence))
> +			     __field(unsigned int, context)
> +			     __field(unsigned int, seqno)
> +			     ),
> +
> +	    TP_fast_assign(
> +			   __entry->adev = job->adev;
> +			   __entry->sched_job = &job->base;
> +			   __assign_str(timeline, job->fence->ops->get_timeline_name(job->fence))
> +			   __entry->context = job->fence->context;
> +			   __entry->seqno = job->fence->seqno;
> +			   ),
> +	    TP_printk("adev=%p, sched_job=%p, timeline:%s, context:%u, seqno:%u",
> +		      __entry->adev, __entry->sched_job,
> +		      __get_str(timeline), __entry->context, __entry->seqno)
> +);
>   
>   TRACE_EVENT(amdgpu_vm_grab_id,
>   	    TP_PROTO(struct amdgpu_vm *vm, int ring, struct amdgpu_job *job),