[PATCH 0/2] tracing: Fix ftrace_boot_snapshot command line

[PATCH 0/2] tracing: Fix ftrace_boot_snapshot command line

[Steven Rostedt]

While debugging some boot up code, I found that the snapshots were
constantly triggering at boot up, even though there was no boot
snapshot specified. Looking into it, I found there were too bugs.

1) It would trigger a snapshot on any instance if one was created
   from the kernel command line.

2) The error handling would only affect the top level instance.
   So the fact that a snapshot was done on a instance that didn't
   allocate a buffer triggered a warning written into the top level
   buffer, and worse yet, disabled the top level buffer.

This addresses both of the above bugs.

Steven Rostedt (Google) (2):
      tracing: Have tracing_snapshot_instance_cond() write errors to the appropriate instance
      tracing: Fix ftrace_boot_snapshot command line logic

----
 kernel/trace/trace.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

[PATCH 1/2] tracing: Have tracing_snapshot_instance_cond() write errors to the appropriate instance

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

If a trace instance has a failure with its snapshot code, the error
message is to be written to that instance's buffer. But currently, the
message is written to the top level buffer. Worse yet, it may also disable
the top level buffer and not the instance that had the issue.

Cc: stable@vger.kernel.org
Fixes: 2824f50332486 ("tracing: Make the snapshot trigger work with instances")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 937e9676dfd4..ed1d1093f5e9 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1149,22 +1149,22 @@ static void tracing_snapshot_instance_cond(struct trace_array *tr,
 	unsigned long flags;
 
 	if (in_nmi()) {
-		internal_trace_puts("*** SNAPSHOT CALLED FROM NMI CONTEXT ***\n");
-		internal_trace_puts("*** snapshot is being ignored        ***\n");
+		trace_array_puts(tr, "*** SNAPSHOT CALLED FROM NMI CONTEXT ***\n");
+		trace_array_puts(tr, "*** snapshot is being ignored        ***\n");
 		return;
 	}
 
 	if (!tr->allocated_snapshot) {
-		internal_trace_puts("*** SNAPSHOT NOT ALLOCATED ***\n");
-		internal_trace_puts("*** stopping trace here!   ***\n");
-		tracing_off();
+		trace_array_puts(tr, "*** SNAPSHOT NOT ALLOCATED ***\n");
+		trace_array_puts(tr, "*** stopping trace here!   ***\n");
+		tracer_tracing_off(tr);
 		return;
 	}
 
 	/* Note, snapshot can not be used when the tracer uses it */
 	if (tracer->use_max_tr) {
-		internal_trace_puts("*** LATENCY TRACER ACTIVE ***\n");
-		internal_trace_puts("*** Can not use snapshot (sorry) ***\n");
+		trace_array_puts(tr, "*** LATENCY TRACER ACTIVE ***\n");
+		trace_array_puts(tr, "*** Can not use snapshot (sorry) ***\n");
 		return;
 	}
 
-- 
2.39.2

[PATCH 2/2] tracing: Fix ftrace_boot_snapshot command line logic

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The kernel command line ftrace_boot_snapshot by itself is supposed to
trigger a snapshot at the end of boot up of the main top level trace
buffer. A ftrace_boot_snapshot=foo will do the same for an instance called
foo that was created by trace_instance=foo,...

The logic was broken where if ftrace_boot_snapshot was by itself, it would
trigger a snapshot for all instances that had tracing enabled, regardless
if it asked for a snapshot or not.

When a snapshot is requested for a buffer, the buffer's
tr->allocated_snapshot is set to true. Use that to know if a trace buffer
wants a snapshot at boot up or not.

Since the top level buffer is part of the ftrace_trace_arrays list,
there's no reason to treat it differently than the other buffers. Just
iterate the list if ftrace_boot_snapshot was specified.

Cc: stable@vger.kernel.org
Fixes: 9c1c251d670bc ("tracing: Allow boot instances to have snapshot buffers")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index ed1d1093f5e9..8ae51f1dea8e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -10395,16 +10395,15 @@ void __init ftrace_boot_snapshot(void)
 {
 	struct trace_array *tr;
 
-	if (snapshot_at_boot) {
-		tracing_snapshot();
-		internal_trace_puts("** Boot snapshot taken **\n");
-	}
+	if (!snapshot_at_boot)
+		return;
 
 	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
-		if (tr == &global_trace)
+		if (!tr->allocated_snapshot)
 			continue;
-		trace_array_puts(tr, "** Boot snapshot taken **\n");
+
 		tracing_snapshot_instance(tr);
+		trace_array_puts(tr, "** Boot snapshot taken **\n");
 	}
 }
 
-- 
2.39.2

Re: [PATCH 2/2] tracing: Fix ftrace_boot_snapshot command line logic

[kernel test robot]

Hi Steven,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v6.3-rc5 next-20230404]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Steven-Rostedt/tracing-Have-tracing_snapshot_instance_cond-write-errors-to-the-appropriate-instance/20230405-070406
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20230404230308.501833715%40goodmis.org
patch subject: [PATCH 2/2] tracing: Fix ftrace_boot_snapshot command line logic
config: arc-buildonly-randconfig-r003-20230403 (https://download.01.org/0day-ci/archive/20230405/202304051001.dL3Fvaxd-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ac5e916ec8d65b5e0c13719694efd3084105f416
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Steven-Rostedt/tracing-Have-tracing_snapshot_instance_cond-write-errors-to-the-appropriate-instance/20230405-070406
        git checkout ac5e916ec8d65b5e0c13719694efd3084105f416
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash kernel/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304051001.dL3Fvaxd-lkp@intel.com/

All errors (new ones prefixed by >>):

   kernel/trace/trace.c: In function 'ftrace_boot_snapshot':
>> kernel/trace/trace.c:10402:24: error: 'struct trace_array' has no member named 'allocated_snapshot'
   10402 |                 if (!tr->allocated_snapshot)
         |                        ^~


vim +10402 kernel/trace/trace.c

 10393	
 10394	void __init ftrace_boot_snapshot(void)
 10395	{
 10396		struct trace_array *tr;
 10397	
 10398		if (!snapshot_at_boot)
 10399			return;
 10400	
 10401		list_for_each_entry(tr, &ftrace_trace_arrays, list) {
 10402			if (!tr->allocated_snapshot)
 10403				continue;
 10404	
 10405			tracing_snapshot_instance(tr);
 10406			trace_array_puts(tr, "** Boot snapshot taken **\n");
 10407		}
 10408	}
 10409	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests