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

["Koenig, Christian" <Christian.Koenig@xxxxxxx>]

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@xxxxxxx>.

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@xxxxxxx>
Sent: Wednesday, October 16, 2019 8:15 PM
To: Wang, Kevin(Yang) <Kevin1.Wang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
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,

could you help me review it?

Best Regards,
Kevin

---

From: Wang, Kevin(Yang) <Kevin1.Wang@xxxxxxx>
Sent: Wednesday, October 16, 2019 11:06 AM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Cc: Wang, Kevin(Yang) <Kevin1.Wang@xxxxxxx>
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@xxxxxxx>
---
 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

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx