* [PATCH] tracing: fix missing osnoise tracer on max_latency
@ 2021-09-18 5:11 Jackie Liu
2021-09-19 16:01 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Jackie Liu @ 2021-09-18 5:11 UTC (permalink / raw)
To: rostedt, mingo; +Cc: linux-kernel, bristot, liu.yun
From: Jackie Liu <liuyun01@kylinos.cn>
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.
Fixes: bce29ac9ce0b ("trace: Add osnoise tracer")
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
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, \
@@ -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)
trace_create_maxlat_file(tr, d_tracer);
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] tracing: fix missing osnoise tracer on max_latency 2021-09-18 5:11 [PATCH] tracing: fix missing osnoise tracer on max_latency Jackie Liu @ 2021-09-19 16:01 ` Steven Rostedt 2021-09-22 2:26 ` Jackie Liu 0 siblings, 1 reply; 5+ messages in thread From: Steven Rostedt @ 2021-09-19 16:01 UTC (permalink / raw) To: Jackie Liu; +Cc: mingo, linux-kernel, bristot On Sat, 18 Sep 2021 13:11:18 +0800 Jackie Liu <liu.yun@linux.dev> wrote: > From: Jackie Liu <liuyun01@kylinos.cn> > > 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@redhat.com> > Signed-off-by: Jackie Liu <liuyun01@kylinos.cn> > --- > 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) # 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: fix missing osnoise tracer on max_latency 2021-09-19 16:01 ` Steven Rostedt @ 2021-09-22 2:26 ` Jackie Liu 2021-09-22 2:40 ` Steven Rostedt 0 siblings, 1 reply; 5+ messages in thread From: Jackie Liu @ 2021-09-22 2:26 UTC (permalink / raw) To: Steven Rostedt; +Cc: mingo, linux-kernel, bristot Hi, Steven. 在 2021/9/20 上午12:01, Steven Rostedt 写道: > On Sat, 18 Sep 2021 13:11:18 +0800 > Jackie Liu <liu.yun@linux.dev> wrote: > >> From: Jackie Liu <liuyun01@kylinos.cn> >> >> 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@redhat.com> >> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn> >> --- >> 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 >> > ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: fix missing osnoise tracer on max_latency 2021-09-22 2:26 ` Jackie Liu @ 2021-09-22 2:40 ` Steven Rostedt 2021-09-22 2:45 ` Jackie Liu 0 siblings, 1 reply; 5+ messages in thread From: Steven Rostedt @ 2021-09-22 2:40 UTC (permalink / raw) To: Jackie Liu; +Cc: mingo, linux-kernel, bristot On Wed, 22 Sep 2021 10:26:24 +0800 Jackie Liu <liu.yun@linux.dev> wrote: > >> 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) Um, but isn't the #if before the #else: #ifdef LATENCY_FS_NOTIFY ? Then, here we have: #ifdef LATENCY_FS_NOTIFY [..] #elif defined(LATENCY_FS_NOTIFY) // this will never be called. That doesn't make any sense. -- Steve > > #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. > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: fix missing osnoise tracer on max_latency 2021-09-22 2:40 ` Steven Rostedt @ 2021-09-22 2:45 ` Jackie Liu 0 siblings, 0 replies; 5+ messages in thread From: Jackie Liu @ 2021-09-22 2:45 UTC (permalink / raw) To: Steven Rostedt; +Cc: mingo, linux-kernel, bristot 在 2021/9/22 上午10:40, Steven Rostedt 写道: > On Wed, 22 Sep 2021 10:26:24 +0800 > Jackie Liu <liu.yun@linux.dev> wrote: > >>>> 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) > > Um, but isn't the #if before the #else: > > #ifdef LATENCY_FS_NOTIFY > > ? > > Then, here we have: > > > #ifdef LATENCY_FS_NOTIFY > > [..] > > #elif defined(LATENCY_FS_NOTIFY) > > // this will never be called. > > That doesn't make any sense. > > -- Steve Thanks Steve, I see. :) -- 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 >> >> #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. >> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-22 2:45 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-18 5:11 [PATCH] tracing: fix missing osnoise tracer on max_latency Jackie Liu 2021-09-19 16:01 ` Steven Rostedt 2021-09-22 2:26 ` Jackie Liu 2021-09-22 2:40 ` Steven Rostedt 2021-09-22 2:45 ` Jackie Liu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.