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

[Alex Deucher]

On Sat, Feb 25, 2017 at 4:39 AM, Christian König
<deathsimple at vodafone.de> wrote:
> 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>.

Applied.  thanks!

Alex


>
> 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)
>>   );
>>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx