Re: [PATCH v2 3/4] Revert "tracing: Add "(fault)" name injection to kernel probes"

[Google]

On Fri, 7 Jul 2023 10:14:20 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> On Fri,  7 Jul 2023 22:41:12 +0900
> "Masami Hiramatsu (Google)" <mhiramat@xxxxxxxxxx> wrote:
> 
> > diff --git a/kernel/trace/trace_probe_kernel.h b/kernel/trace/trace_probe_kernel.h
> > index c4e1d4c03a85..6deae2ce34f8 100644
> > --- a/kernel/trace/trace_probe_kernel.h
> > +++ b/kernel/trace/trace_probe_kernel.h
> > @@ -2,8 +2,6 @@
> >  #ifndef __TRACE_PROBE_KERNEL_H_
> >  #define __TRACE_PROBE_KERNEL_H_
> >  
> > -#define FAULT_STRING "(fault)"
> > -
> 
> Instead of deleting this, why not use it in both places? After applying
> your patch, we have:
> 
>  $ git grep '(fault)' kernel/trace
> kernel/trace/trace_events_synth.c:                      strcpy(str_field, "(fault)");
> kernel/trace/trace_probe.c:             trace_seq_puts(s, "(fault)");
> 
> We could make that consistent with:
> 
> kernel/trace/trace_events_synth.c:                      strcpy(str_field, FAULT_STRING);
> kernel/trace/trace_probe.c:             trace_seq_puts(s, FAULT_STRING);

Indeed. So can I tweak the revert patch more for this?

I would like to move the definition to trace.h.

Thanks,

> 
> -- Steve
> 


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>