| Message ID | 20260519-host1x-tracing-v1-1-55afb8cbd186@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | gpu: host1x: trace: fix string fields in host1x traces | expand |
On Tue, 19 May 2026 12:16:43 +0200 Artur Kowalski <arturkow2000@gmail.com> wrote: > Use __assign_str and __get_str as required by tracing subsystem. Fixes > string fields being rejected by the verifier and unreadable from > userspace. Does anyone use these tracepoints? The fact that they have been broken for 5 years and nobody noticed makes me think they are useless. I rather remove them than fix them, but if someone thinks that these are still useful then by all means apply this patch. Acked-by: Steven Rostedt <rostedt@goodmis.org> -- Steve > > Tested on v6.18.21. > > Signed-off-by: Artur Kowalski <arturkow2000@gmail.com> > --- > include/trace/events/host1x.h | 50 ++++++++++++++++++++++--------------------- > 1 file changed, 26 insertions(+), 24 deletions(-) > > diff --git a/include/trace/events/host1x.h b/include/trace/events/host1x.h > index 1ba84b738e46..1b6aeb7b177b 100644 > --- a/include/trace/events/host1x.h > +++ b/include/trace/events/host1x.h > @@ -21,9 +21,11 @@ struct host1x_bo; > DECLARE_EVENT_CLASS(host1x, > TP_PROTO(const char *name), > TP_ARGS(name), > - TP_STRUCT__entry(__field(const char *, name)), > - TP_fast_assign(__entry->name = name;), > - TP_printk("name=%s", __entry->name) > + TP_STRUCT__entry(__string(name, name)), > + TP_fast_assign( > + __assign_str(name); > + ), > + TP_printk("name=%s", __get_str(name)) > ); > > DEFINE_EVENT(host1x, host1x_channel_open, > @@ -52,19 +54,19 @@ TRACE_EVENT(host1x_cdma_push, > TP_ARGS(name, op1, op2), > > TP_STRUCT__entry( > - __field(const char *, name) > + __string(name, name) > __field(u32, op1) > __field(u32, op2) > ), > > TP_fast_assign( > - __entry->name = name; > + __assign_str(name); > __entry->op1 = op1; > __entry->op2 = op2; > ), > > TP_printk("name=%s, op1=%08x, op2=%08x", > - __entry->name, __entry->op1, __entry->op2) > + __get_str(name), __entry->op1, __entry->op2) > ); > > TRACE_EVENT(host1x_cdma_push_wide, > @@ -73,7 +75,7 @@ TRACE_EVENT(host1x_cdma_push_wide, > TP_ARGS(name, op1, op2, op3, op4), > > TP_STRUCT__entry( > - __field(const char *, name) > + __string(name, name) > __field(u32, op1) > __field(u32, op2) > __field(u32, op3) > @@ -81,7 +83,7 @@ TRACE_EVENT(host1x_cdma_push_wide, > ), > > TP_fast_assign( > - __entry->name = name; > + __assign_str(name); > __entry->op1 = op1; > __entry->op2 = op2; > __entry->op3 = op3; > @@ -89,7 +91,7 @@ TRACE_EVENT(host1x_cdma_push_wide, > ), > > TP_printk("name=%s, op1=%08x, op2=%08x, op3=%08x op4=%08x", > - __entry->name, __entry->op1, __entry->op2, __entry->op3, > + __get_str(name), __entry->op1, __entry->op2, __entry->op3, > __entry->op4) > ); > > @@ -100,7 +102,7 @@ TRACE_EVENT(host1x_cdma_push_gather, > TP_ARGS(name, bo, words, offset, cmdbuf), > > TP_STRUCT__entry( > - __field(const char *, name) > + __string(name, name) > __field(struct host1x_bo *, bo) > __field(u32, words) > __field(u32, offset) > @@ -114,14 +116,14 @@ TRACE_EVENT(host1x_cdma_push_gather, > words * sizeof(u32)); > } > __entry->cmdbuf = cmdbuf; > - __entry->name = name; > + __assign_str(name); > __entry->bo = bo; > __entry->words = words; > __entry->offset = offset; > ), > > TP_printk("name=%s, bo=%p, words=%u, offset=%d, contents=[%s]", > - __entry->name, __entry->bo, > + __get_str(name), __entry->bo, > __entry->words, __entry->offset, > __print_hex(__get_dynamic_array(cmdbuf), > __entry->cmdbuf ? __entry->words * 4 : 0)) > @@ -134,7 +136,7 @@ TRACE_EVENT(host1x_channel_submit, > TP_ARGS(name, cmdbufs, relocs, syncpt_id, syncpt_incrs), > > TP_STRUCT__entry( > - __field(const char *, name) > + __string(name, name) > __field(u32, cmdbufs) > __field(u32, relocs) > __field(u32, syncpt_id) > @@ -142,7 +144,7 @@ TRACE_EVENT(host1x_channel_submit, > ), > > TP_fast_assign( > - __entry->name = name; > + __assign_str(name); > __entry->cmdbufs = cmdbufs; > __entry->relocs = relocs; > __entry->syncpt_id = syncpt_id; > @@ -151,7 +153,7 @@ TRACE_EVENT(host1x_channel_submit, > > TP_printk("name=%s, cmdbufs=%u, relocs=%u, syncpt_id=%u, " > "syncpt_incrs=%u", > - __entry->name, __entry->cmdbufs, __entry->relocs, > + __get_str(name), __entry->cmdbufs, __entry->relocs, > __entry->syncpt_id, __entry->syncpt_incrs) > ); > > @@ -161,19 +163,19 @@ TRACE_EVENT(host1x_channel_submitted, > TP_ARGS(name, syncpt_base, syncpt_max), > > TP_STRUCT__entry( > - __field(const char *, name) > + __string(name, name) > __field(u32, syncpt_base) > __field(u32, syncpt_max) > ), > > TP_fast_assign( > - __entry->name = name; > + __assign_str(name); > __entry->syncpt_base = syncpt_base; > __entry->syncpt_max = syncpt_max; > ), > > TP_printk("name=%s, syncpt_base=%d, syncpt_max=%d", > - __entry->name, __entry->syncpt_base, __entry->syncpt_max) > + __get_str(name), __entry->syncpt_base, __entry->syncpt_max) > ); > > TRACE_EVENT(host1x_channel_submit_complete, > @@ -182,19 +184,19 @@ TRACE_EVENT(host1x_channel_submit_complete, > TP_ARGS(name, count, thresh), > > TP_STRUCT__entry( > - __field(const char *, name) > + __string(name, name) > __field(int, count) > __field(u32, thresh) > ), > > TP_fast_assign( > - __entry->name = name; > + __assign_str(name); > __entry->count = count; > __entry->thresh = thresh; > ), > > TP_printk("name=%s, count=%d, thresh=%d", > - __entry->name, __entry->count, __entry->thresh) > + __get_str(name), __entry->count, __entry->thresh) > ); > > TRACE_EVENT(host1x_wait_cdma, > @@ -203,16 +205,16 @@ TRACE_EVENT(host1x_wait_cdma, > TP_ARGS(name, eventid), > > TP_STRUCT__entry( > - __field(const char *, name) > + __string(name, name) > __field(u32, eventid) > ), > > TP_fast_assign( > - __entry->name = name; > + __assign_str(name); > __entry->eventid = eventid; > ), > > - TP_printk("name=%s, event=%d", __entry->name, __entry->eventid) > + TP_printk("name=%s, event=%d", __get_str(name), __entry->eventid) > ); > > TRACE_EVENT(host1x_syncpt_load_min, > > --- > base-commit: ab5fce87a778cb780a05984a2ca448f2b41aafbf > change-id: 20260519-host1x-tracing-e2d608ec5e37 > > Best regards, > -- > Artur Kowalski <arturkow2000@gmail.com>
On Tue, May 19, 2026 at 02:10:59PM -0400, Steven Rostedt wrote: > On Tue, 19 May 2026 12:16:43 +0200 > Artur Kowalski <arturkow2000@gmail.com> wrote: > > > Use __assign_str and __get_str as required by tracing subsystem. Fixes > > string fields being rejected by the verifier and unreadable from > > userspace. > > Does anyone use these tracepoints? The fact that they have been broken > for 5 years and nobody noticed makes me think they are useless. > > I rather remove them than fix them, but if someone thinks that these > are still useful then by all means apply this patch. > > Acked-by: Steven Rostedt <rostedt@goodmis.org> I know that Mikko used them a lot early on, but this driver is pretty mature now, so we rarely need this low level of tracing. I'll defer to Mikko on whether we still need these. Thierry
On Wednesday, May 20, 2026 9:03 PM Thierry Reding wrote: > On Tue, May 19, 2026 at 02:10:59PM -0400, Steven Rostedt wrote: > > On Tue, 19 May 2026 12:16:43 +0200 > > Artur Kowalski <arturkow2000@gmail.com> wrote: > > > > > Use __assign_str and __get_str as required by tracing subsystem. Fixes > > > string fields being rejected by the verifier and unreadable from > > > userspace. > > > > Does anyone use these tracepoints? The fact that they have been broken > > for 5 years and nobody noticed makes me think they are useless. > > > > I rather remove them than fix them, but if someone thinks that these > > are still useful then by all means apply this patch. > > > > Acked-by: Steven Rostedt <rostedt@goodmis.org> > > I know that Mikko used them a lot early on, but this driver is pretty > mature now, so we rarely need this low level of tracing. I'll defer to > Mikko on whether we still need these. > > Thierry Yeah, these have been quite useful in the past when debugging why a job is failing. Without the cdma traces it can be cumbersome to find out exactly what is being sent to the hardware in some cases. My preference is for keeping them for now. Mikko
On Thu, May 21, 2026 at 01:33:24PM +0900, Mikko Perttunen wrote: > On Wednesday, May 20, 2026 9:03 PM Thierry Reding wrote: > > On Tue, May 19, 2026 at 02:10:59PM -0400, Steven Rostedt wrote: > > > On Tue, 19 May 2026 12:16:43 +0200 > > > Artur Kowalski <arturkow2000@gmail.com> wrote: > > > > > > > Use __assign_str and __get_str as required by tracing subsystem. Fixes > > > > string fields being rejected by the verifier and unreadable from > > > > userspace. > > > > > > Does anyone use these tracepoints? The fact that they have been broken > > > for 5 years and nobody noticed makes me think they are useless. > > > > > > I rather remove them than fix them, but if someone thinks that these > > > are still useful then by all means apply this patch. > > > > > > Acked-by: Steven Rostedt <rostedt@goodmis.org> > > > > I know that Mikko used them a lot early on, but this driver is pretty > > mature now, so we rarely need this low level of tracing. I'll defer to > > Mikko on whether we still need these. > > > > Thierry > > Yeah, these have been quite useful in the past when debugging why a job > is failing. Without the cdma traces it can be cumbersome to find out > exactly what is being sent to the hardware in some cases. > > My preference is for keeping them for now. Thanks, I'll pick this up then. Thierry
On Tue, May 19, 2026 at 12:16:43PM +0200, Artur Kowalski wrote: > Use __assign_str and __get_str as required by tracing subsystem. Fixes > string fields being rejected by the verifier and unreadable from > userspace. > > Tested on v6.18.21. > > Signed-off-by: Artur Kowalski <arturkow2000@gmail.com> > --- > include/trace/events/host1x.h | 50 ++++++++++++++++++++++--------------------- > 1 file changed, 26 insertions(+), 24 deletions(-) Applied, thanks. Thierry
diff --git a/include/trace/events/host1x.h b/include/trace/events/host1x.h index 1ba84b738e46..1b6aeb7b177b 100644 --- a/include/trace/events/host1x.h +++ b/include/trace/events/host1x.h @@ -21,9 +21,11 @@ struct host1x_bo; DECLARE_EVENT_CLASS(host1x, TP_PROTO(const char *name), TP_ARGS(name), - TP_STRUCT__entry(__field(const char *, name)), - TP_fast_assign(__entry->name = name;), - TP_printk("name=%s", __entry->name) + TP_STRUCT__entry(__string(name, name)), + TP_fast_assign( + __assign_str(name); + ), + TP_printk("name=%s", __get_str(name)) ); DEFINE_EVENT(host1x, host1x_channel_open, @@ -52,19 +54,19 @@ TRACE_EVENT(host1x_cdma_push, TP_ARGS(name, op1, op2), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(u32, op1) __field(u32, op2) ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->op1 = op1; __entry->op2 = op2; ), TP_printk("name=%s, op1=%08x, op2=%08x", - __entry->name, __entry->op1, __entry->op2) + __get_str(name), __entry->op1, __entry->op2) ); TRACE_EVENT(host1x_cdma_push_wide, @@ -73,7 +75,7 @@ TRACE_EVENT(host1x_cdma_push_wide, TP_ARGS(name, op1, op2, op3, op4), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(u32, op1) __field(u32, op2) __field(u32, op3) @@ -81,7 +83,7 @@ TRACE_EVENT(host1x_cdma_push_wide, ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->op1 = op1; __entry->op2 = op2; __entry->op3 = op3; @@ -89,7 +91,7 @@ TRACE_EVENT(host1x_cdma_push_wide, ), TP_printk("name=%s, op1=%08x, op2=%08x, op3=%08x op4=%08x", - __entry->name, __entry->op1, __entry->op2, __entry->op3, + __get_str(name), __entry->op1, __entry->op2, __entry->op3, __entry->op4) ); @@ -100,7 +102,7 @@ TRACE_EVENT(host1x_cdma_push_gather, TP_ARGS(name, bo, words, offset, cmdbuf), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(struct host1x_bo *, bo) __field(u32, words) __field(u32, offset) @@ -114,14 +116,14 @@ TRACE_EVENT(host1x_cdma_push_gather, words * sizeof(u32)); } __entry->cmdbuf = cmdbuf; - __entry->name = name; + __assign_str(name); __entry->bo = bo; __entry->words = words; __entry->offset = offset; ), TP_printk("name=%s, bo=%p, words=%u, offset=%d, contents=[%s]", - __entry->name, __entry->bo, + __get_str(name), __entry->bo, __entry->words, __entry->offset, __print_hex(__get_dynamic_array(cmdbuf), __entry->cmdbuf ? __entry->words * 4 : 0)) @@ -134,7 +136,7 @@ TRACE_EVENT(host1x_channel_submit, TP_ARGS(name, cmdbufs, relocs, syncpt_id, syncpt_incrs), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(u32, cmdbufs) __field(u32, relocs) __field(u32, syncpt_id) @@ -142,7 +144,7 @@ TRACE_EVENT(host1x_channel_submit, ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->cmdbufs = cmdbufs; __entry->relocs = relocs; __entry->syncpt_id = syncpt_id; @@ -151,7 +153,7 @@ TRACE_EVENT(host1x_channel_submit, TP_printk("name=%s, cmdbufs=%u, relocs=%u, syncpt_id=%u, " "syncpt_incrs=%u", - __entry->name, __entry->cmdbufs, __entry->relocs, + __get_str(name), __entry->cmdbufs, __entry->relocs, __entry->syncpt_id, __entry->syncpt_incrs) ); @@ -161,19 +163,19 @@ TRACE_EVENT(host1x_channel_submitted, TP_ARGS(name, syncpt_base, syncpt_max), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(u32, syncpt_base) __field(u32, syncpt_max) ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->syncpt_base = syncpt_base; __entry->syncpt_max = syncpt_max; ), TP_printk("name=%s, syncpt_base=%d, syncpt_max=%d", - __entry->name, __entry->syncpt_base, __entry->syncpt_max) + __get_str(name), __entry->syncpt_base, __entry->syncpt_max) ); TRACE_EVENT(host1x_channel_submit_complete, @@ -182,19 +184,19 @@ TRACE_EVENT(host1x_channel_submit_complete, TP_ARGS(name, count, thresh), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(int, count) __field(u32, thresh) ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->count = count; __entry->thresh = thresh; ), TP_printk("name=%s, count=%d, thresh=%d", - __entry->name, __entry->count, __entry->thresh) + __get_str(name), __entry->count, __entry->thresh) ); TRACE_EVENT(host1x_wait_cdma, @@ -203,16 +205,16 @@ TRACE_EVENT(host1x_wait_cdma, TP_ARGS(name, eventid), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(u32, eventid) ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->eventid = eventid; ), - TP_printk("name=%s, event=%d", __entry->name, __entry->eventid) + TP_printk("name=%s, event=%d", __get_str(name), __entry->eventid) ); TRACE_EVENT(host1x_syncpt_load_min,
Use __assign_str and __get_str as required by tracing subsystem. Fixes string fields being rejected by the verifier and unreadable from userspace. Tested on v6.18.21. Signed-off-by: Artur Kowalski <arturkow2000@gmail.com> --- include/trace/events/host1x.h | 50 ++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 24 deletions(-) --- base-commit: ab5fce87a778cb780a05984a2ca448f2b41aafbf change-id: 20260519-host1x-tracing-e2d608ec5e37 Best regards, -- Artur Kowalski <arturkow2000@gmail.com>