[PATCH] drm/amdgpu: fix amdgpu trace event print string format error

Koenig, Christian Christian.Koenig at amd.com
Wed Oct 16 13:38:43 UTC 2019


Hi Kevin,

do you have other better way to do it?
Not of hand, but maybe check the trace documentation if there is any better approach.

If you can't find anything the patch is Reviewed-by: Christian König <christian.koenig at amd.com><mailto:christian.koenig at amd.com>.

Regards,
Christian.

Am 16.10.19 um 15:30 schrieb Wang, Kevin(Yang):
Hi Chris,

You said that this kind of scene also existed in other source code, these has same method.
in amdgpu_trace.h file, this usage case is exits in amdgpu driver.
likes TRACE_EVENT(amdgpu_cs_ioctl) -> timeline :
            TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, ring_name=%s, num_ibs=%u",
                      __entry->sched_job_id, __get_str(timeline), __entry->context,
                      __entry->seqno, __get_str(ring), __entry->num_ibs)
and do you have other better way to do it?
thanks.

Best Regards,
Kevin

________________________________
From: Koenig, Christian <Christian.Koenig at amd.com><mailto:Christian.Koenig at amd.com>
Sent: Wednesday, October 16, 2019 8:15 PM
To: Wang, Kevin(Yang) <Kevin1.Wang at amd.com><mailto:Kevin1.Wang at amd.com>; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> <amd-gfx at lists.freedesktop.org><mailto:amd-gfx at lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: fix amdgpu trace event print string format error

Hi Kevin,

well that copies the string into the ring buffer every time the trace event is called which is not necessary a good idea for a constant string.

Can't we avoid that somehow?

Thanks,
Christian.

Am 16.10.19 um 14:01 schrieb Wang, Kevin(Yang):
add @Koenig, Christian<mailto:Christian.Koenig at amd.com>,
could you help me review it?

Best Regards,
Kevin

________________________________
From: Wang, Kevin(Yang) <Kevin1.Wang at amd.com><mailto:Kevin1.Wang at amd.com>
Sent: Wednesday, October 16, 2019 11:06 AM
To: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> <amd-gfx at lists.freedesktop.org><mailto:amd-gfx at lists.freedesktop.org>
Cc: Wang, Kevin(Yang) <Kevin1.Wang at amd.com><mailto:Kevin1.Wang at amd.com>
Subject: [PATCH] drm/amdgpu: fix amdgpu trace event print string format error

the trace event print string format error.
(use integer type to handle string)

before:
amdgpu_test_kev-1556  [002]   138.508781: amdgpu_cs_ioctl:
sched_job=8, timeline=gfx_0.0.0, context=177, seqno=1,
ring_name=ffff94d01c207bf0, num_ibs=2

after:
amdgpu_test_kev-1506  [004]   370.703783: amdgpu_cs_ioctl:
sched_job=12, timeline=gfx_0.0.0, context=234, seqno=2,
ring_name=gfx_0.0.0, num_ibs=1

change trace event list:
1.amdgpu_cs_ioctl
2.amdgpu_sched_run_job
3.amdgpu_ib_pipe_sync

Signed-off-by: Kevin Wang <kevin1.wang at amd.com><mailto:kevin1.wang at amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index 8227ebd0f511..f940526c5889 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -170,7 +170,7 @@ TRACE_EVENT(amdgpu_cs_ioctl,
                              __field(unsigned int, context)
                              __field(unsigned int, seqno)
                              __field(struct dma_fence *, fence)
-                            __field(char *, ring_name)
+                            __string(ring, to_amdgpu_ring(job->base.sched)->name)
                              __field(u32, num_ibs)
                              ),

@@ -179,12 +179,12 @@ TRACE_EVENT(amdgpu_cs_ioctl,
                            __assign_str(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job))
                            __entry->context = job->base.s_fence->finished.context;
                            __entry->seqno = job->base.s_fence->finished.seqno;
-                          __entry->ring_name = to_amdgpu_ring(job->base.sched)->name;
+                          __assign_str(ring, to_amdgpu_ring(job->base.sched)->name)
                            __entry->num_ibs = job->num_ibs;
                            ),
             TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, ring_name=%s, num_ibs=%u",
                       __entry->sched_job_id, __get_str(timeline), __entry->context,
-                     __entry->seqno, __entry->ring_name, __entry->num_ibs)
+                     __entry->seqno, __get_str(ring), __entry->num_ibs)
 );

 TRACE_EVENT(amdgpu_sched_run_job,
@@ -195,7 +195,7 @@ TRACE_EVENT(amdgpu_sched_run_job,
                              __string(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job))
                              __field(unsigned int, context)
                              __field(unsigned int, seqno)
-                            __field(char *, ring_name)
+                            __string(ring, to_amdgpu_ring(job->base.sched)->name)
                              __field(u32, num_ibs)
                              ),

@@ -204,12 +204,12 @@ TRACE_EVENT(amdgpu_sched_run_job,
                            __assign_str(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job))
                            __entry->context = job->base.s_fence->finished.context;
                            __entry->seqno = job->base.s_fence->finished.seqno;
-                          __entry->ring_name = to_amdgpu_ring(job->base.sched)->name;
+                          __assign_str(ring, to_amdgpu_ring(job->base.sched)->name)
                            __entry->num_ibs = job->num_ibs;
                            ),
             TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, ring_name=%s, num_ibs=%u",
                       __entry->sched_job_id, __get_str(timeline), __entry->context,
-                     __entry->seqno, __entry->ring_name, __entry->num_ibs)
+                     __entry->seqno, __get_str(ring), __entry->num_ibs)
 );


@@ -473,7 +473,7 @@ TRACE_EVENT(amdgpu_ib_pipe_sync,
             TP_PROTO(struct amdgpu_job *sched_job, struct dma_fence *fence),
             TP_ARGS(sched_job, fence),
             TP_STRUCT__entry(
-                            __field(const char *,name)
+                            __string(ring, sched_job->base.sched->name);
                              __field(uint64_t, id)
                              __field(struct dma_fence *, fence)
                              __field(uint64_t, ctx)
@@ -481,14 +481,14 @@ TRACE_EVENT(amdgpu_ib_pipe_sync,
                              ),

             TP_fast_assign(
-                          __entry->name = sched_job->base.sched->name;
+                          __assign_str(ring, sched_job->base.sched->name)
                            __entry->id = sched_job->base.id;
                            __entry->fence = fence;
                            __entry->ctx = fence->context;
                            __entry->seqno = fence->seqno;
                            ),
             TP_printk("job ring=%s, id=%llu, need pipe sync to fence=%p, context=%llu, seq=%u",
-                     __entry->name, __entry->id,
+                     __get_str(ring), __entry->id,
                       __entry->fence, __entry->ctx,
                       __entry->seqno)
 );
--
2.17.1



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20191016/ce6d3684/attachment-0001.html>


More information about the amd-gfx mailing list