On Thu, 14 Jan 2021 at 08:50, Alexander Potapenko <gli...@google.com> wrote: > > On Wed, Jan 13, 2021 at 10:10 PM Steven Rostedt <rost...@goodmis.org> wrote: > > > > On Wed, 13 Jan 2021 10:16:54 +0100 > > Alexander Potapenko <gli...@google.com> wrote: > > > > > +DECLARE_EVENT_CLASS(error_report_template, > > > + TP_PROTO(const char *error_detector, unsigned long id), > > > > Instead of having a random string, as this should be used by a small finite > > set of subsystems, why not make the above into an enum? > > You're probably right. > I just thought it might be a good idea to minimize the effort needed > from tools' authors to add these tracepoints to the tools (see the > following two patches), and leave room for some extensibility (e.g. > passing bug type together with the tool name etc.) > > > > + TP_ARGS(error_detector, id), > > > + TP_STRUCT__entry(__field(const char *, error_detector) > > > + __field(unsigned long, id)), > > > + TP_fast_assign(__entry->error_detector = error_detector; > > > + __entry->id = id;), > > > + TP_printk("[%s] %lx", __entry->error_detector, > > > > Then the [%s] portion of this could also be just a __print_symbolic(). > > We'll need to explicitly list the enum values once again in > __print_symbolic(), right? E.g.: > > enum debugging_tool {
We need to use TRACE_DEFINE_ENUM(). > TOOL_KFENCE, For consistency I would call the enum simply "ERROR_DETECTOR" as well. (Hypothetically, there could also be an "error detector" that is not a "debugging tool".) > TOOL_KASAN, > ... > } > > TP_printk(__print_symbolic(__entry->error_detector, TOOL_KFENCE, > TOOL_KASAN, ...), It takes a list of val -> str. E.g. __print_symbolic(__entry->error_detector, { TOOL_KFENCE, "KFENCE" }, { TOOL_KASAN, "KASAN" }). Looking around the kernel, sometimes this is simplified with macros, but not sure if it's worth it. Typing the same thing 3 times is fine, given this list won't grow really fast. Thanks, -- Marco