| Message ID | 20210507095333.1080910-4-tz.stoyanov@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Changes to trace-cmd logs | expand |
On Fri, 7 May 2021 12:53:28 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > Removed the "weak" definition of the library log functions. Setting > the desired log level can be used to silence the library logs, instead > of overwriting the log functions. That's not the purpose of the weak definition. It is so that a GUI (like kernelshark) can turn it into a pop up if need be or show in a status window. It was not for quieting the function. Please do not remove the weak symbols. -- Steve
On Fri, May 14, 2021 at 12:23 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 7 May 2021 12:53:28 +0300 > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > > > Removed the "weak" definition of the library log functions. Setting > > the desired log level can be used to silence the library logs, instead > > of overwriting the log functions. > > That's not the purpose of the weak definition. It is so that a GUI (like > kernelshark) can turn it into a pop up if need be or show in a status > window. It was not for quieting the function. The description is not correct. All these log functions use tep_vprint(), which is implemented as weak. This function has information for the library and log severity and should be used by kernelshark for that purpose. That's why I removed weak library specific functions. > > Please do not remove the weak symbols. > > -- Steve
On Fri, 14 May 2021 05:51:24 +0300 Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote: > On Fri, May 14, 2021 at 12:23 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > On Fri, 7 May 2021 12:53:28 +0300 > > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > > > > > Removed the "weak" definition of the library log functions. Setting > > > the desired log level can be used to silence the library logs, instead > > > of overwriting the log functions. > > > > That's not the purpose of the weak definition. It is so that a GUI (like > > kernelshark) can turn it into a pop up if need be or show in a status > > window. It was not for quieting the function. > > The description is not correct. All these log functions use > tep_vprint(), which is implemented as weak. This function has > information for the library and log severity and should be used by > kernelshark for that purpose. That's why I removed weak library > specific functions. Yes, tep_vprintk() could offer this as well, but there's still no reason to not keep them as weak. Adding log levels is still unrelated to removing the weak attribute. I still do not see the rationale for removing the weak attribute of these functions. It gives more flexibility for the users, without any downsides. Or is there a downside that you see for keeping them weak? -- Steve
diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h index 7305487c..6984db86 100644 --- a/include/trace-cmd/trace-cmd.h +++ b/include/trace-cmd/trace-cmd.h @@ -43,4 +43,6 @@ int tracecmd_buffer_instances(struct tracecmd_input *handle); const char *tracecmd_buffer_instance_name(struct tracecmd_input *handle, int indx); struct tracecmd_input *tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx); +void tracecmd_set_loglevel(enum tep_loglevel level); + #endif /* _TRACE_CMD_H */ diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h index cd868f60..76179148 100644 --- a/lib/trace-cmd/include/trace-cmd-local.h +++ b/lib/trace-cmd/include/trace-cmd-local.h @@ -9,7 +9,6 @@ #include <byteswap.h> #include "trace-cmd-private.h" -/* Can be overridden */ void tracecmd_warning(const char *fmt, ...); void tracecmd_fatal(const char *fmt, ...); void tracecmd_info(const char *fmt, ...); diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c index 049fe049..b3f1b075 100644 --- a/lib/trace-cmd/trace-util.c +++ b/lib/trace-cmd/trace-util.c @@ -30,7 +30,7 @@ #define PROC_STACK_FILE "/proc/sys/kernel/stack_tracer_enabled" static bool debug; - +static int log_level = TEP_LOG_CRITICAL; static FILE *logfp; const static struct { @@ -355,10 +355,24 @@ trace_load_plugins(struct tep_handle *tep, int flags) return list; } -void __weak tracecmd_warning(const char *fmt, ...) +/** + * tracecmd_set_loglevel - set log level of the library + * @level: desired level of the library messages + */ +void tracecmd_set_loglevel(enum tep_loglevel level) +{ + log_level = level; + tracefs_set_loglevel(level); + tep_set_loglevel(level); +} + +void tracecmd_warning(const char *fmt, ...) { va_list ap; + if (log_level < TEP_LOG_WARNING) + return; + va_start(ap, fmt); tep_vprint("libtracecmd", TEP_LOG_WARNING, true, fmt, ap); va_end(ap); @@ -368,17 +382,23 @@ void tracecmd_info(const char *fmt, ...) { va_list ap; + if (log_level < TEP_LOG_INFO) + return; + va_start(ap, fmt); tep_vprint("libtracecmd", TEP_LOG_INFO, false, fmt, ap); va_end(ap); } -void __weak tracecmd_fatal(const char *fmt, ...) +void tracecmd_fatal(const char *fmt, ...) { int ret; va_list ap; + if (log_level < TEP_LOG_CRITICAL) + return; + va_start(ap, fmt); ret = tep_vprint("libtracecmd", TEP_LOG_CRITICAL, true, fmt, ap); va_end(ap);
Add levels to library logs and introduce a new API to set the desired log severity: tracecmd_set_loglevel() When a new trace-cmd library log level is set, propagate it to tracefs and traceevent libraries as well. Removed the "weak" definition of the library log functions. Setting the desired log level can be used to silence the library logs, instead of overwriting the log functions. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- include/trace-cmd/trace-cmd.h | 2 ++ lib/trace-cmd/include/trace-cmd-local.h | 1 - lib/trace-cmd/trace-util.c | 26 ++++++++++++++++++++++--- 3 files changed, 25 insertions(+), 4 deletions(-)