Re: [PATCH] tracing: fix missing osnoise tracer on max_latency

[Jackie Liu]

Hi, Steven.

在 2021/9/20 上午12:01, Steven Rostedt 写道:
> On Sat, 18 Sep 2021 13:11:18 +0800
> Jackie Liu <liu.yun@xxxxxxxxx> wrote:
>
> > From: Jackie Liu <liuyun01@xxxxxxxxxx>
> >
> > The compiler warns when the data are actually unused:
> >
> >    kernel/trace/trace.c:1712:13: error: ‘trace_create_maxlat_file’ defined but not used [-Werror=unused-function]
> >     1712 | static void trace_create_maxlat_file(struct trace_array *tr,
> >          |             ^~~~~~~~~~~~~~~~~~~~~~~~
> >
> > [Why]
> > CONFIG_HWLAT_TRACER=n, CONFIG_TRACER_MAX_TRACE=n, CONFIG_OSNOISE_TRACER=y
> > gcc report warns.
> >
> > [How]
> > Now trace_create_maxlat_file will only take effect when
> > CONFIG_HWLAT_TRACER=y or CONFIG_TRACER_MAX_TRACE=y. In fact, after
> > adding osnoise trace, it also needs to take effect.
> >
> > BTW, Fixed the conflicting defined comment information.
>
> Thanks for the report.
>
> > Fixes: bce29ac9ce0b ("trace: Add osnoise tracer")
> > Cc: Daniel Bristot de Oliveira <bristot@xxxxxxxxxx>
> > Signed-off-by: Jackie Liu <liuyun01@xxxxxxxxxx>
> > ---
> >   kernel/trace/trace.c | 9 +++------
> >   1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 7896d30d90f7..d7e3ed82fafd 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -1744,11 +1744,7 @@ void latency_fsnotify(struct trace_array *tr)
> >   	irq_work_queue(&tr->fsnotify_irqwork);
> >   }
> >   
> > -/*
> > - * (defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER)) && \
> > - *  defined(CONFIG_FSNOTIFY)
> > - */
> > -#else
> > +#else /* LATENCY_FS_NOTIFY  >>
> >   #define trace_create_maxlat_file(tr, d_tracer)				\
> >   	trace_create_file("tracing_max_latency", 0644, d_tracer,	\
>
> To clean this up even better, we should add here:
>
> #elif defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER) \
> 	|| defined(CONFIG_OSNOISE_TRACER)

This place should need to use LATENCY_FS_NOTIFY, because not only these
three Traces, we also need to pay attention to CONFIG_FSNOTIFY, at
least, we should not change the original meaning.

How about this:

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 7896d30d90f7..6a88d03c6d3b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1744,16 +1744,14 @@ void latency_fsnotify(struct trace_array *tr)
        irq_work_queue(&tr->fsnotify_irqwork);
 }

-/*
- * (defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER)) && \
- *  defined(CONFIG_FSNOTIFY)
- */
-#else
+#elif defined(LATENCY_FS_NOTIFY)

 #define trace_create_maxlat_file(tr, d_tracer)                         \
        trace_create_file("tracing_max_latency", 0644, d_tracer,        \
                          &tr->max_latency, &tracing_max_lat_fops)

+#else
+#define trace_create_maxlat_file(tr, d_tracer)  do { } while (0)
 #endif

 #ifdef CONFIG_TRACER_MAX_TRACE
@@ -9473,9 +9471,7 @@ init_tracer_tracefs(struct trace_array *tr, struct 
dentry *d_tracer)

        create_trace_options_dir(tr);

-#if defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER)
        trace_create_maxlat_file(tr, d_tracer);
-#endif

        if (ftrace_create_function_files(tr, d_tracer))
                MEM_FAIL(1, "Could not allocate function filter files");


==
What do you think? If there is no problem, I will send V2.

--
BR.
Jackie Liu

> #  define trace_create_maxlat_file(tr, d_tracer)			\
> 	trace_create_file("tracing_max_latency", 0644, d_tracer,	\
> 			  &tr->max_latency, &tracing_max_lat_fops)
>
> #else
> # define trace_create_maxlat_file(tr, d_tracer)	 do { } while (0)
> #endif
>
> > @@ -9473,7 +9469,8 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
> >   
> >   	create_trace_options_dir(tr);
> >   
> > -#if defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER)
> > +#if defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER) \
> > +	|| defined(CONFIG_OSNOISE_TRACER)
>
> And remove the #if statement complete from inside the function.
>
> -- Steve
>
> >   	trace_create_maxlat_file(tr, d_tracer);
> >   #endif
> >