[GIT PULL] tracing: Fixes and clean ups for v5.14

[GIT PULL] tracing: Fixes and clean ups for v5.14

[Steven Rostedt]

Linus,

Fixes and clean ups to tracing:

- Fix warning for use of "main" variable in osnoise tracer

- Fix header alignment when PREEMPT_RT is enabled for osnoise tracer

- Inject "stop" event to see where osnoise stopped the trace

- Define DYNAMIC_FTRACE_WITH_ARGS as some code had an #ifdef for it

- Fix erroneous message for bootconfig cmdline parameter

- Fix crash caused by not found variable in histograms


Please pull the latest trace-v5.14-rc5 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v5.14-rc5

Tag SHA1: a580741e941446efd9bc43d74e34fca7b9e3bd2c
Head SHA1: c5bcfacca8ad9c00aafdbbc5145b30cb31a22fb3


Daniel Bristot de Oliveira (4):
      trace/osnoise: Rename main variable to tracer_main
      trace/osnoise: Add a header with PREEMPT_RT additional fields
      trace/timerlat: Add a header with PREEMPT_RT additional fields
      trace/osnoise: Print a stop tracing message

Lukas Bulwahn (1):
      tracing: define needed config DYNAMIC_FTRACE_WITH_ARGS

Masami Hiramatsu (1):
      init: Suppress wrong warning for bootconfig cmdline parameter

Steven Rostedt (VMware) (1):
      tracing / histogram: Fix NULL pointer dereference on strcmp() on NULL event name

----
 init/main.c                      |  9 ++++--
 kernel/trace/Kconfig             |  5 ++++
 kernel/trace/trace_events_hist.c |  2 ++
 kernel/trace/trace_osnoise.c     | 62 +++++++++++++++++++++++++++++++++++++---
 4 files changed, 72 insertions(+), 6 deletions(-)
---------------------------
diff --git a/init/main.c b/init/main.c
index f5b8246e8aa1..8d97aba78c3a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -397,6 +397,12 @@ static int __init bootconfig_params(char *param, char *val,
 	return 0;
 }
 
+static int __init warn_bootconfig(char *str)
+{
+	/* The 'bootconfig' has been handled by bootconfig_params(). */
+	return 0;
+}
+
 static void __init setup_boot_config(void)
 {
 	static char tmp_cmdline[COMMAND_LINE_SIZE] __initdata;
@@ -475,9 +481,8 @@ static int __init warn_bootconfig(char *str)
 	pr_warn("WARNING: 'bootconfig' found on the kernel command line but CONFIG_BOOT_CONFIG is not set.\n");
 	return 0;
 }
-early_param("bootconfig", warn_bootconfig);
-
 #endif
+early_param("bootconfig", warn_bootconfig);
 
 /* Change NUL term back to "=", to make "param" the whole string. */
 static void __init repair_env_string(char *param, char *val)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index d567b1717c4c..3ee23f4d437f 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -219,6 +219,11 @@ config DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	depends on DYNAMIC_FTRACE_WITH_REGS
 	depends on HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 
+config DYNAMIC_FTRACE_WITH_ARGS
+	def_bool y
+	depends on DYNAMIC_FTRACE
+	depends on HAVE_DYNAMIC_FTRACE_WITH_ARGS
+
 config FUNCTION_PROFILER
 	bool "Kernel function profiler"
 	depends on FUNCTION_TRACER
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 949ef09dc537..a48aa2a2875b 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -3430,6 +3430,8 @@ trace_action_create_field_var(struct hist_trigger_data *hist_data,
 			event = data->match_data.event;
 		}
 
+		if (!event)
+			goto free;
 		/*
 		 * At this point, we're looking at a field on another
 		 * event.  Because we can't modify a hist trigger on
diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index a7e3c24dee13..b576e96d9741 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -253,10 +253,40 @@ static struct osnoise_data {
  */
 static bool osnoise_busy;
 
+#ifdef CONFIG_PREEMPT_RT
 /*
  * Print the osnoise header info.
  */
 static void print_osnoise_headers(struct seq_file *s)
+{
+	if (osnoise_data.tainted)
+		seq_puts(s, "# osnoise is tainted!\n");
+
+	seq_puts(s, "#                                _-------=> irqs-off\n");
+	seq_puts(s, "#                               / _------=> need-resched\n");
+	seq_puts(s, "#                              | / _-----=> need-resched-lazy\n");
+	seq_puts(s, "#                              || / _----=> hardirq/softirq\n");
+	seq_puts(s, "#                              ||| / _---=> preempt-depth\n");
+	seq_puts(s, "#                              |||| / _--=> preempt-lazy-depth\n");
+	seq_puts(s, "#                              ||||| / _-=> migrate-disable\n");
+
+	seq_puts(s, "#                              |||||| /          ");
+	seq_puts(s, "                                     MAX\n");
+
+	seq_puts(s, "#                              ||||| /                         ");
+	seq_puts(s, "                    SINGLE      Interference counters:\n");
+
+	seq_puts(s, "#                              |||||||               RUNTIME   ");
+	seq_puts(s, "   NOISE  %% OF CPU  NOISE    +-----------------------------+\n");
+
+	seq_puts(s, "#           TASK-PID      CPU# |||||||   TIMESTAMP    IN US    ");
+	seq_puts(s, "   IN US  AVAILABLE  IN US     HW    NMI    IRQ   SIRQ THREAD\n");
+
+	seq_puts(s, "#              | |         |   |||||||      |           |      ");
+	seq_puts(s, "       |    |            |      |      |      |      |      |\n");
+}
+#else /* CONFIG_PREEMPT_RT */
+static void print_osnoise_headers(struct seq_file *s)
 {
 	if (osnoise_data.tainted)
 		seq_puts(s, "# osnoise is tainted!\n");
@@ -279,6 +309,7 @@ static void print_osnoise_headers(struct seq_file *s)
 	seq_puts(s, "#              | |         |   ||||      |           |      ");
 	seq_puts(s, "       |    |            |      |      |      |      |      |\n");
 }
+#endif /* CONFIG_PREEMPT_RT */
 
 /*
  * osnoise_taint - report an osnoise error.
@@ -323,6 +354,24 @@ static void trace_osnoise_sample(struct osnoise_sample *sample)
 /*
  * Print the timerlat header info.
  */
+#ifdef CONFIG_PREEMPT_RT
+static void print_timerlat_headers(struct seq_file *s)
+{
+	seq_puts(s, "#                                _-------=> irqs-off\n");
+	seq_puts(s, "#                               / _------=> need-resched\n");
+	seq_puts(s, "#                              | / _-----=> need-resched-lazy\n");
+	seq_puts(s, "#                              || / _----=> hardirq/softirq\n");
+	seq_puts(s, "#                              ||| / _---=> preempt-depth\n");
+	seq_puts(s, "#                              |||| / _--=> preempt-lazy-depth\n");
+	seq_puts(s, "#                              ||||| / _-=> migrate-disable\n");
+	seq_puts(s, "#                              |||||| /\n");
+	seq_puts(s, "#                              |||||||             ACTIVATION\n");
+	seq_puts(s, "#           TASK-PID      CPU# |||||||   TIMESTAMP    ID     ");
+	seq_puts(s, "       CONTEXT                LATENCY\n");
+	seq_puts(s, "#              | |         |   |||||||      |         |      ");
+	seq_puts(s, "            |                       |\n");
+}
+#else /* CONFIG_PREEMPT_RT */
 static void print_timerlat_headers(struct seq_file *s)
 {
 	seq_puts(s, "#                                _-----=> irqs-off\n");
@@ -336,6 +385,7 @@ static void print_timerlat_headers(struct seq_file *s)
 	seq_puts(s, "#              | |         |   ||||      |         |      ");
 	seq_puts(s, "            |                       |\n");
 }
+#endif /* CONFIG_PREEMPT_RT */
 
 /*
  * Record an timerlat_sample into the tracer buffer.
@@ -1025,9 +1075,13 @@ diff_osn_sample_stats(struct osnoise_variables *osn_var, struct osnoise_sample *
 /*
  * osnoise_stop_tracing - Stop tracing and the tracer.
  */
-static void osnoise_stop_tracing(void)
+static __always_inline void osnoise_stop_tracing(void)
 {
 	struct trace_array *tr = osnoise_trace;
+
+	trace_array_printk_buf(tr->array_buffer.buffer, _THIS_IP_,
+			"stop tracing hit on cpu %d\n", smp_processor_id());
+
 	tracer_tracing_off(tr);
 }
 
@@ -1458,20 +1512,20 @@ static void stop_per_cpu_kthreads(void)
 static int start_kthread(unsigned int cpu)
 {
 	struct task_struct *kthread;
-	void *main = osnoise_main;
+	void *tracer_main = osnoise_main;
 	char comm[24];
 
 #ifdef CONFIG_TIMERLAT_TRACER
 	if (osnoise_data.timerlat_tracer) {
 		snprintf(comm, 24, "timerlat/%d", cpu);
-		main = timerlat_main;
+		tracer_main = timerlat_main;
 	} else {
 		snprintf(comm, 24, "osnoise/%d", cpu);
 	}
 #else
 	snprintf(comm, 24, "osnoise/%d", cpu);
 #endif
-	kthread = kthread_create_on_cpu(main, NULL, cpu, comm);
+	kthread = kthread_create_on_cpu(tracer_main, NULL, cpu, comm);
 
 	if (IS_ERR(kthread)) {
 		pr_err(BANNER "could not start sampling thread\n");

Re: [GIT PULL] tracing: Fixes and clean ups for v5.14

[Linus Torvalds]

On Thu, Aug 12, 2021 at 5:29 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> - Fix warning for use of "main" variable in osnoise tracer

Honestly, I refuse to pull this kind of inexplicable garbage.

The commit message talks about "some compilers/analyzers" without any
explanation, and the "Link:" thing doesn't link to anything useful
either.

Those compilers/analyzers are clearly complete and utter crap, and my
reaction is that I want a *lot* more explanation why we should care
about other peoples crap.

I do NOT want to see people caring about warnings that make no sense,
certainly not without extensive explanations about why we should care
about broken tools.

Fix the tool. Add a "--Wno-warn-about-stupid-issues" flag to the tool
usage. Don't make pointless changes to the kernel to work around other
peoples garbage.

                    Linus

Re: [GIT PULL] tracing: Fixes and clean ups for v5.14

[Steven Rostedt]

On Thu, 12 Aug 2021 07:18:37 -1000
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, Aug 12, 2021 at 5:29 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > - Fix warning for use of "main" variable in osnoise tracer  
> 
> Honestly, I refuse to pull this kind of inexplicable garbage.

That was my take too, when this was first addressed:

  https://lore.kernel.org/lkml/20210730204050.46975ae2@oasis.local.home/

> 
> The commit message talks about "some compilers/analyzers" without any
> explanation, and the "Link:" thing doesn't link to anything useful
> either.

I should have added the above link. :-/

Where the link in this commit was simply added by my scripts.

> 
> Those compilers/analyzers are clearly complete and utter crap, and my
> reaction is that I want a *lot* more explanation why we should care
> about other peoples crap.

I totally agree.

> 
> I do NOT want to see people caring about warnings that make no sense,
> certainly not without extensive explanations about why we should care
> about broken tools.
> 
> Fix the tool. Add a "--Wno-warn-about-stupid-issues" flag to the tool
> usage. Don't make pointless changes to the kernel to work around other
> peoples garbage.

:-) :-) :-)

I'm totally happy with this approach. I took the lazy method, and let
them change the variable.

But I'm 100% on your side of the issue. I HATE these stupid warnings.

I swear, these warnings are the gcc equivalent of spelling fixes to the
kernel. "Oh, I can get a patch into gcc via adding a new useless
warning!"

I'll drop the patch, and send you a v2.

-- Steve

Re: [GIT PULL] tracing: Fixes and clean ups for v5.14

[Linus Torvalds]

On Thu, Aug 12, 2021 at 7:33 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> That was my take too, when this was first addressed:
>
>   https://lore.kernel.org/lkml/20210730204050.46975ae2@oasis.local.home/
>
> >
> > The commit message talks about "some compilers/analyzers" without any
> > explanation, and the "Link:" thing doesn't link to anything useful
> > either.
>
> I should have added the above link. :-/

Even the above link doesn't explain it to me.

What is it that gives this crazy garbage warning?

It looks like Randy Dunlap compiles with the '-Wmain' flag, and then
complains about it hurting.

It's a classic case of "Doctor, doctor, it hurts when I hit myself in
the head with an ice pick".

The solution is not to take some Ibuprofen.

The solution is DON'T DO THAT THEN.

People who build with W=2 or some other crazy thing need to understand
that the bad warnings it exposes are not kernel problems, they are
THEIR problems.

               Linus

Re: [GIT PULL] tracing: Fixes and clean ups for v5.14

[Randy Dunlap]

On 8/12/21 10:54 AM, Linus Torvalds wrote:
> On Thu, Aug 12, 2021 at 7:33 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>>
>> That was my take too, when this was first addressed:
>>
>>    https://lore.kernel.org/lkml/20210730204050.46975ae2@oasis.local.home/
>>
>>>
>>> The commit message talks about "some compilers/analyzers" without any
>>> explanation, and the "Link:" thing doesn't link to anything useful
>>> either.

Yes, Steven wanted to use -Wno-main or whatever it is called from
the start.

>> I should have added the above link. :-/
> 
> Even the above link doesn't explain it to me.
> 
> What is it that gives this crazy garbage warning?
>
Some tool called gcc.

> It looks like Randy Dunlap compiles with the '-Wmain' flag, and then
> complains about it hurting.

I just used some default settings. I didn't choose to use -Wmain.

> It's a classic case of "Doctor, doctor, it hurts when I hit myself in
> the head with an ice pick".
> 
> The solution is not to take some Ibuprofen.
> 
> The solution is DON'T DO THAT THEN.
> 
> People who build with W=2 or some other crazy thing need to understand
> that the bad warnings it exposes are not kernel problems, they are
> THEIR problems.

Nah, it's a tooling problem.

-- 
~Randy

Re: [GIT PULL] tracing: Fixes and clean ups for v5.14

[Linus Torvalds]

On Thu, Aug 12, 2021 at 8:04 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> I just used some default settings. I didn't choose to use -Wmain.

What broken distro, what broken gcc version?

We can certainly add a -Wno-main for this case. We already do that for
a lot of other idiotic warnings like -Wno-pointer-sign.

But when we do so, I want the exact tool and distro version named and
shamed. Because I sure don't see that warning, and from what I can
tell, most other people don't see it either.

So it's almost certainly your distro that has configured the gcc
install incorrectly - or some new gcc version that makes new insane
defaults. The commit message should talk about those kinds of details,
exactly so that people like me get an explanation for why we'd need
that odd '-Wno-main' flag.

Maybe even the line in the Makefile should have it. Like that
-Wno-pointer-sign thing does:

  # disable pointer signed / unsigned warnings in gcc 4.0
  KBUILD_CFLAGS += -Wno-pointer-sign

just because unexplained random compiler flags are a bad thing (the
same way unexplained random code changes due to them are bad)

                Linus

Re: [GIT PULL] tracing: Fixes and clean ups for v5.14

[Randy Dunlap]

On 8/12/21 11:17 AM, Linus Torvalds wrote:
> On Thu, Aug 12, 2021 at 8:04 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> I just used some default settings. I didn't choose to use -Wmain.
> 
> What broken distro, what broken gcc version?

openSUSE 15.3
gcc (SUSE Linux) 7.5.0

> We can certainly add a -Wno-main for this case. We already do that for
> a lot of other idiotic warnings like -Wno-pointer-sign.

That's what my first patch did, but Steven didn't like it.

> But when we do so, I want the exact tool and distro version named and
> shamed. Because I sure don't see that warning, and from what I can
> tell, most other people don't see it either.
> 
> So it's almost certainly your distro that has configured the gcc
> install incorrectly - or some new gcc version that makes new insane
> defaults. The commit message should talk about those kinds of details,
> exactly so that people like me get an explanation for why we'd need
> that odd '-Wno-main' flag.
> 
> Maybe even the line in the Makefile should have it. Like that
> -Wno-pointer-sign thing does:
> 
>    # disable pointer signed / unsigned warnings in gcc 4.0
>    KBUILD_CFLAGS += -Wno-pointer-sign
> 
> just because unexplained random compiler flags are a bad thing (the
> same way unexplained random code changes due to them are bad)


-- 
~Randy

Re: [GIT PULL] tracing: Fixes and clean ups for v5.14

[Randy Dunlap]

On 8/12/21 11:57 AM, Randy Dunlap wrote:
> On 8/12/21 11:17 AM, Linus Torvalds wrote:
>> On Thu, Aug 12, 2021 at 8:04 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>>>
>>> I just used some default settings. I didn't choose to use -Wmain.
>>
>> What broken distro, what broken gcc version?
> 
> openSUSE 15.3
> gcc (SUSE Linux) 7.5.0
> 
>> We can certainly add a -Wno-main for this case. We already do that for
>> a lot of other idiotic warnings like -Wno-pointer-sign.
> 
> That's what my first patch did, but Steven didn't like it.
> 
>> But when we do so, I want the exact tool and distro version named and
>> shamed. Because I sure don't see that warning, and from what I can
>> tell, most other people don't see it either.
>>
>> So it's almost certainly your distro that has configured the gcc
>> install incorrectly - or some new gcc version that makes new insane
>> defaults. The commit message should talk about those kinds of details,
>> exactly so that people like me get an explanation for why we'd need
>> that odd '-Wno-main' flag.
>>
>> Maybe even the line in the Makefile should have it. Like that
>> -Wno-pointer-sign thing does:
>>
>>    # disable pointer signed / unsigned warnings in gcc 4.0
>>    KBUILD_CFLAGS += -Wno-pointer-sign
>>
>> just because unexplained random compiler flags are a bad thing (the
>> same way unexplained random code changes due to them are bad)

I probably misread that a bit. To be clear, my patch only set -Wno-main
for this one source file, not for the entire kernel tree, whereas Steven
wanted it to be set to the full kernel tree.


-- 
~Randy

Re: [GIT PULL] tracing: Fixes and clean ups for v5.14

[Linus Torvalds]

On Thu, Aug 12, 2021 at 10:47 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> I probably misread that a bit. To be clear, my patch only set -Wno-main
> for this one source file, not for the entire kernel tree, whereas Steven
> wanted it to be set to the full kernel tree.

Ack, let's just set it globally, there's no need for this to be some
surgical strike thing.

                Linus