linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL] tracing: Changes for 5.6
@ 2020-02-04 10:31 Steven Rostedt
  2020-02-04 11:54 ` Linus Torvalds
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2020-02-04 10:31 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tom Zanussi,
	Peter Zijlstra


Linus,

Tracing updates:

 - Added new "bootconfig".
   Looks for a file appended to initrd to add boot config options.
   This has been discussed thoroughly at Linux Plumbers.
   Very useful for adding kprobes at bootup.

 - Created dynamic event creation.
   Merges common code between creating synthetic events and
     kprobe events.

 - Rename perf "ring_buffer" structure to "perf_buffer"

 - Rename ftrace "ring_buffer" structure to "trace_buffer"
   Had to rename existing "trace_buffer" to "array_buffer"

 - Allow trace_printk() to work withing (some) tracing code.

 - Sort of tracing configs to be a little better organized

 - Various other small fixes and clean ups


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


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

Tag SHA1: e8b1f8525eeac87b9db3051f347e3defd8e2fb8b
Head SHA1: 2b90927c77c973771cc658d639724d5b247a83eb


Alex Shi (5):
      ftrace: Remove abandoned macros
      ftrace: Remove NR_TO_INIT macro
      ring-buffer: Remove abandoned macro RB_MISSED_FLAGS
      tracing: Remove unused TRACE_SEQ_BUF_USED
      trace/kprobe: Remove unused MAX_KPROBE_CMDLINE_SIZE

Colin Ian King (1):
      tools: bootconfig: Fix spelling mistake "faile" -> "failed"

Dan Carpenter (2):
      tracing/boot: Fix an IS_ERR() vs NULL bug
      tracing: Remove unneeded NULL check

Fabian Frederick (1):
      ring-bufer: kernel-doc warning fixes

Hou Pengyang (1):
      tracing: Fix comments about trace/ftrace.h

Josef Bacik (1):
      tracing: Set kernel_stack's caller size properly

Luis Henriques (1):
      tracing: Fix tracing_stat return values in error handling paths

Masami Hiramatsu (28):
      bootconfig: Add Extra Boot Config support
      bootconfig: Load boot config from the tail of initrd
      tools: bootconfig: Add bootconfig command
      tools: bootconfig: Add bootconfig test script
      proc: bootconfig: Add /proc/bootconfig to show boot config list
      init/main.c: Alloc initcall_command_line in do_initcall() and free it
      bootconfig: init: Allow admin to use bootconfig for kernel command line
      bootconfig: init: Allow admin to use bootconfig for init command line
      Documentation: bootconfig: Add a doc for extended boot config
      tracing: Apply soft-disabled and filter to tracepoints printk
      tracing: kprobes: Output kprobe event to printk buffer
      tracing: kprobes: Register to dynevent earlier stage
      tracing: Accept different type for synthetic event fields
      tracing: Add NULL trace-array check in print_synth_event()
      tracing/boot: Add boot-time tracing
      tracing/boot: Add per-event settings
      tracing/boot Add kprobe event support
      tracing/boot: Add synthetic event support
      tracing/boot: Add instance node support
      tracing/boot: Add cpu_mask option support
      tracing/boot: Add function tracer filter options
      Documentation: tracing: Add boot-time tracing document
      tracing: trigger: Replace unneeded RCU-list traversals
      bootconfig: Fix Kconfig help message for BOOT_CONFIG
      Documentation: bootconfig: Fix typos in bootconfig documentation
      Documentation: tracing: Fix typos in boot-time tracing documentation
      tracing/boot: Include required headers and sort it alphabetically
      tracing/boot: Move external function declarations to kernel/trace/trace.h

Mathieu Desnoyers (1):
      tracing: Fix sched switch start/stop refcount racy updates

Mauro Carvalho Chehab (1):
      docs: tracing: Fix a broken label

Steven Rostedt (VMware) (13):
      perf: Make struct ring_buffer less ambiguous
      tracing: Rename trace_buffer to array_buffer
      tracing: Make struct ring_buffer less ambiguous
      ring-buffer: Fix kernel doc for rb_update_event()
      tracing: Allow trace_printk() to nest in other tracing code
      tracing: Fix uninitialized buffer var on early exit to trace_vbprintk()
      tracing: Fix very unlikely race of registering two stat tracers
      tracing: Decrement trace_array when bootconfig creates an instance
      tracing: Use pr_err() instead of WARN() for memory failures
      tracing: Move all function tracing configs together
      tracing: Move tracing test module configs together
      tracing: Move mmio tracer config up with the other tracers
      tracing: Move tracing selftests to bottom of menu

Tom Zanussi (21):
      tracing: Simplify assignment parsing for hist triggers
      tracing: Add hist trigger error messages for sort specification
      tracing: Add 'hist:' to hist trigger error log error string
      tracing: Add new testcases for hist trigger parsing errors
      tracing: Add trace_array_find/_get() to find instance trace arrays
      tracing: Add trace_get/put_event_file()
      tracing: Add synth_event_delete()
      tracing: Add dynamic event command creation interface
      tracing: Add synthetic event command generation functions
      tracing: Add synth_event_trace() and related functions
      tracing: Add synth event generation test module
      tracing: Add kprobe event command generation functions
      tracing: Change trace_boot to use kprobe_event interface
      tracing: Add kprobe event command generation test module
      tracing: Documentation for in-kernel synthetic event API
      tracing: Change trace_boot to use synth_event interface
      tracing: Fix now invalid var_ref_vals assumption in trace action
      tracing: Consolidate some synth_event_trace code
      tracing: Remove check_arg() callbacks from dynevent args
      tracing: Remove useless code in dynevent_arg_pair_add()
      tracing: Use seq_buf for building dynevent_cmd string

Vasily Averin (3):
      ftrace: fpid_next() should increase position index
      tracing: eval_map_next() should always increase position index
      trigger_next should increase position index

----
 Documentation/admin-guide/bootconfig.rst           |  188 ++++
 Documentation/admin-guide/index.rst                |    1 +
 Documentation/trace/boottime-trace.rst             |  184 ++++
 Documentation/trace/events.rst                     |  515 ++++++++++
 Documentation/trace/index.rst                      |    1 +
 Documentation/trace/kprobetrace.rst                |    1 +
 MAINTAINERS                                        |    9 +
 drivers/oprofile/cpu_buffer.c                      |    2 +-
 fs/proc/Makefile                                   |    1 +
 fs/proc/bootconfig.c                               |   89 ++
 include/linux/bootconfig.h                         |  224 ++++
 include/linux/perf_event.h                         |    6 +-
 include/linux/ring_buffer.h                        |  110 +-
 include/linux/trace_events.h                       |  131 ++-
 include/trace/trace_events.h                       |   11 +-
 init/Kconfig                                       |   14 +
 init/main.c                                        |  213 +++-
 kernel/events/core.c                               |   42 +-
 kernel/events/internal.h                           |   34 +-
 kernel/events/ring_buffer.c                        |   54 +-
 kernel/trace/Kconfig                               |  360 ++++---
 kernel/trace/Makefile                              |    3 +
 kernel/trace/blktrace.c                            |    8 +-
 kernel/trace/ftrace.c                              |   22 +-
 kernel/trace/kprobe_event_gen_test.c               |  225 ++++
 kernel/trace/ring_buffer.c                         |  135 +--
 kernel/trace/ring_buffer_benchmark.c               |    2 +-
 kernel/trace/synth_event_gen_test.c                |  523 ++++++++++
 kernel/trace/trace.c                               |  453 +++++----
 kernel/trace/trace.h                               |   69 +-
 kernel/trace/trace_boot.c                          |  334 ++++++
 kernel/trace/trace_branch.c                        |    6 +-
 kernel/trace/trace_dynevent.c                      |  212 ++++
 kernel/trace/trace_dynevent.h                      |   32 +
 kernel/trace/trace_entries.h                       |    2 +-
 kernel/trace/trace_events.c                        |  106 +-
 kernel/trace/trace_events_hist.c                   | 1071 ++++++++++++++++++--
 kernel/trace/trace_events_trigger.c                |   27 +-
 kernel/trace/trace_functions.c                     |    8 +-
 kernel/trace/trace_functions_graph.c               |   14 +-
 kernel/trace/trace_hwlat.c                         |    2 +-
 kernel/trace/trace_irqsoff.c                       |    8 +-
 kernel/trace/trace_kdb.c                           |    8 +-
 kernel/trace/trace_kprobe.c                        |  238 ++++-
 kernel/trace/trace_mmiotrace.c                     |   12 +-
 kernel/trace/trace_output.c                        |    2 +-
 kernel/trace/trace_sched_switch.c                  |    4 +-
 kernel/trace/trace_sched_wakeup.c                  |   20 +-
 kernel/trace/trace_selftest.c                      |   26 +-
 kernel/trace/trace_seq.c                           |    3 -
 kernel/trace/trace_stat.c                          |   31 +-
 kernel/trace/trace_syscalls.c                      |    8 +-
 kernel/trace/trace_uprobe.c                        |    2 +-
 lib/Kconfig                                        |    3 +
 lib/Makefile                                       |    2 +
 lib/bootconfig.c                                   |  803 +++++++++++++++
 tools/Makefile                                     |   11 +-
 tools/bootconfig/.gitignore                        |    1 +
 tools/bootconfig/Makefile                          |   23 +
 tools/bootconfig/include/linux/bootconfig.h        |    7 +
 tools/bootconfig/include/linux/bug.h               |   12 +
 tools/bootconfig/include/linux/ctype.h             |    7 +
 tools/bootconfig/include/linux/errno.h             |    7 +
 tools/bootconfig/include/linux/kernel.h            |   18 +
 tools/bootconfig/include/linux/printk.h            |   17 +
 tools/bootconfig/include/linux/string.h            |   32 +
 tools/bootconfig/main.c                            |  353 +++++++
 .../samples/bad-array-space-comment.bconf          |    5 +
 tools/bootconfig/samples/bad-array.bconf           |    2 +
 tools/bootconfig/samples/bad-dotword.bconf         |    4 +
 tools/bootconfig/samples/bad-empty.bconf           |    1 +
 tools/bootconfig/samples/bad-keyerror.bconf        |    2 +
 tools/bootconfig/samples/bad-longkey.bconf         |    1 +
 tools/bootconfig/samples/bad-manywords.bconf       |    1 +
 tools/bootconfig/samples/bad-no-keyword.bconf      |    2 +
 tools/bootconfig/samples/bad-nonprintable.bconf    |    2 +
 tools/bootconfig/samples/bad-spaceword.bconf       |    2 +
 tools/bootconfig/samples/bad-tree.bconf            |    5 +
 tools/bootconfig/samples/bad-value.bconf           |    3 +
 tools/bootconfig/samples/escaped.bconf             |    3 +
 .../samples/good-array-space-comment.bconf         |    4 +
 .../samples/good-comment-after-value.bconf         |    1 +
 tools/bootconfig/samples/good-printables.bconf     |    2 +
 tools/bootconfig/samples/good-simple.bconf         |   11 +
 tools/bootconfig/samples/good-single.bconf         |    4 +
 .../samples/good-space-after-value.bconf           |    1 +
 tools/bootconfig/samples/good-tree.bconf           |   12 +
 tools/bootconfig/test-bootconfig.sh                |  105 ++
 .../test.d/trigger/trigger-hist-syntax-errors.tc   |   32 +
 89 files changed, 6464 insertions(+), 843 deletions(-)
 create mode 100644 Documentation/admin-guide/bootconfig.rst
 create mode 100644 Documentation/trace/boottime-trace.rst
 create mode 100644 fs/proc/bootconfig.c
 create mode 100644 include/linux/bootconfig.h
 create mode 100644 kernel/trace/kprobe_event_gen_test.c
 create mode 100644 kernel/trace/synth_event_gen_test.c
 create mode 100644 kernel/trace/trace_boot.c
 create mode 100644 lib/bootconfig.c
 create mode 100644 tools/bootconfig/.gitignore
 create mode 100644 tools/bootconfig/Makefile
 create mode 100644 tools/bootconfig/include/linux/bootconfig.h
 create mode 100644 tools/bootconfig/include/linux/bug.h
 create mode 100644 tools/bootconfig/include/linux/ctype.h
 create mode 100644 tools/bootconfig/include/linux/errno.h
 create mode 100644 tools/bootconfig/include/linux/kernel.h
 create mode 100644 tools/bootconfig/include/linux/printk.h
 create mode 100644 tools/bootconfig/include/linux/string.h
 create mode 100644 tools/bootconfig/main.c
 create mode 100644 tools/bootconfig/samples/bad-array-space-comment.bconf
 create mode 100644 tools/bootconfig/samples/bad-array.bconf
 create mode 100644 tools/bootconfig/samples/bad-dotword.bconf
 create mode 100644 tools/bootconfig/samples/bad-empty.bconf
 create mode 100644 tools/bootconfig/samples/bad-keyerror.bconf
 create mode 100644 tools/bootconfig/samples/bad-longkey.bconf
 create mode 100644 tools/bootconfig/samples/bad-manywords.bconf
 create mode 100644 tools/bootconfig/samples/bad-no-keyword.bconf
 create mode 100644 tools/bootconfig/samples/bad-nonprintable.bconf
 create mode 100644 tools/bootconfig/samples/bad-spaceword.bconf
 create mode 100644 tools/bootconfig/samples/bad-tree.bconf
 create mode 100644 tools/bootconfig/samples/bad-value.bconf
 create mode 100644 tools/bootconfig/samples/escaped.bconf
 create mode 100644 tools/bootconfig/samples/good-array-space-comment.bconf
 create mode 100644 tools/bootconfig/samples/good-comment-after-value.bconf
 create mode 100644 tools/bootconfig/samples/good-printables.bconf
 create mode 100644 tools/bootconfig/samples/good-simple.bconf
 create mode 100644 tools/bootconfig/samples/good-single.bconf
 create mode 100644 tools/bootconfig/samples/good-space-after-value.bconf
 create mode 100644 tools/bootconfig/samples/good-tree.bconf
 create mode 100755 tools/bootconfig/test-bootconfig.sh
 create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-syntax-errors.tc
---------------------------

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] tracing: Changes for 5.6
  2020-02-04 10:31 [GIT PULL] tracing: Changes for 5.6 Steven Rostedt
@ 2020-02-04 11:54 ` Linus Torvalds
  2020-02-04 12:28   ` Steven Rostedt
  0 siblings, 1 reply; 9+ messages in thread
From: Linus Torvalds @ 2020-02-04 11:54 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tom Zanussi,
	Peter Zijlstra

On Tue, Feb 4, 2020 at 10:32 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
>  - Added new "bootconfig".
>    Looks for a file appended to initrd to add boot config options.
>    This has been discussed thoroughly at Linux Plumbers.
>    Very useful for adding kprobes at bootup.

Is there a way to disable this from the "real" kernel command line?

If I have problems, I want to boot with a known kernel command line
(forcing text mode etc). If the bootconfig file always gets parsed,
there's no way to do that from the bootloader (grub2, whatever)..

              Linus

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] tracing: Changes for 5.6
  2020-02-04 11:54 ` Linus Torvalds
@ 2020-02-04 12:28   ` Steven Rostedt
  2020-02-04 12:43     ` [PATCH] bootconfig: Add "disable_bootconfig" cmdline option to disable bootconfig Steven Rostedt
  2020-02-04 13:19     ` [GIT PULL] tracing: Changes for 5.6 Linus Torvalds
  0 siblings, 2 replies; 9+ messages in thread
From: Steven Rostedt @ 2020-02-04 12:28 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tom Zanussi,
	Peter Zijlstra

On Tue, 4 Feb 2020 11:54:09 +0000
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Feb 4, 2020 at 10:32 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> >  - Added new "bootconfig".
> >    Looks for a file appended to initrd to add boot config options.
> >    This has been discussed thoroughly at Linux Plumbers.
> >    Very useful for adding kprobes at bootup.  
> 
> Is there a way to disable this from the "real" kernel command line?

Currently no.

> 
> If I have problems, I want to boot with a known kernel command line
> (forcing text mode etc). If the bootconfig file always gets parsed,
> there's no way to do that from the bootloader (grub2, whatever)..
> 

I totally agree as I had that exact same issue while testing this. I
wanted to not have it and found it a bit annoying I needed to modify
the initrd to remove it for just a single boot up.

Would this work?

-- Steve

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ade4e6ec23e0..c4f1417f1934 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -857,6 +857,10 @@
                 on      Perform hardened usercopy checks (default).
                 off     Disable hardened usercopy checks.
 
+	disable_bootconfig
+			Disable reading the bootconfig that may be attached
+			to the initrd.
+
 	disable_radix	[PPC]
 			Disable RADIX MMU mode on POWER9
 
diff --git a/init/main.c b/init/main.c
index dd7da62d99a5..b52636cd9c1d 100644
--- a/init/main.c
+++ b/init/main.c
@@ -336,15 +336,23 @@ u32 boot_config_checksum(unsigned char *p, u32 size)
 	return ret;
 }
 
-static void __init setup_boot_config(void)
+static void __init setup_boot_config(const char *cmdline)
 {
 	u32 size, csum;
 	char *data, *copy;
+	const char *p;
 	u32 *hdr;
 
 	if (!initrd_end)
 		return;
 
+	p = strstr(cmdline, "disable_bootconfig");
+	if (p && (p == cmdline || isspace(*(p-1))) &&
+	    (!p[18] || isspace(p[18]))) {
+		pr_info("Disabling bootconfig because 'disable_bootconfig' found on the command line\n");
+		return;
+	}
+
 	hdr = (u32 *)(initrd_end - 8);
 	size = hdr[0];
 	csum = hdr[1];
@@ -379,7 +387,7 @@ static void __init setup_boot_config(void)
 	}
 }
 #else
-#define setup_boot_config()	do { } while (0)
+#define setup_boot_config(cmdline)	do { } while (0)
 #endif
 
 /* Change NUL term back to "=", to make "param" the whole string. */
@@ -760,7 +768,7 @@ asmlinkage __visible void __init start_kernel(void)
 	pr_notice("%s", linux_banner);
 	early_security_init();
 	setup_arch(&command_line);
-	setup_boot_config();
+	setup_boot_config(command_line);
 	setup_command_line(command_line);
 	setup_nr_cpu_ids();
 	setup_per_cpu_areas();

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] bootconfig: Add "disable_bootconfig" cmdline option to disable bootconfig
  2020-02-04 12:28   ` Steven Rostedt
@ 2020-02-04 12:43     ` Steven Rostedt
  2020-02-04 13:19     ` [GIT PULL] tracing: Changes for 5.6 Linus Torvalds
  1 sibling, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2020-02-04 12:43 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tom Zanussi,
	Peter Zijlstra


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

As the bootconfig is appended to the initrd it is not as easy to modify as
the kernel command line. If there's some issue with the kernel, and the
developer wants to boot a pristine kernel, it should not be needed to modify
the initrd to remove the bootconfig for a single boot.

A "disable_bootconfig" kernel command line option solves this. If
"disable_bootconfig" is found on the kernel command line, then the
bootconfig file that may be attached to the initrd will not be parsed.

Link: https://lore.kernel.org/r/CAHk-=wjfjO+h6bQzrTf=YCZA53Y3EDyAs3Z4gEsT7icA3u_Psw@mail.gmail.com

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/admin-guide/kernel-parameters.txt |  4 ++++
 init/main.c                                     | 14 +++++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ade4e6ec23e0..c4f1417f1934 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -857,6 +857,10 @@
                 on      Perform hardened usercopy checks (default).
                 off     Disable hardened usercopy checks.
 
+	disable_bootconfig
+			Disable reading the bootconfig that may be attached
+			to the initrd.
+
 	disable_radix	[PPC]
 			Disable RADIX MMU mode on POWER9
 
diff --git a/init/main.c b/init/main.c
index dd7da62d99a5..b52636cd9c1d 100644
--- a/init/main.c
+++ b/init/main.c
@@ -336,15 +336,23 @@ u32 boot_config_checksum(unsigned char *p, u32 size)
 	return ret;
 }
 
-static void __init setup_boot_config(void)
+static void __init setup_boot_config(const char *cmdline)
 {
 	u32 size, csum;
 	char *data, *copy;
+	const char *p;
 	u32 *hdr;
 
 	if (!initrd_end)
 		return;
 
+	p = strstr(cmdline, "disable_bootconfig");
+	if (p && (p == cmdline || isspace(*(p-1))) &&
+	    (!p[18] || isspace(p[18]))) {
+		pr_info("Disabling bootconfig because 'disable_bootconfig' found on the command line\n");
+		return;
+	}
+
 	hdr = (u32 *)(initrd_end - 8);
 	size = hdr[0];
 	csum = hdr[1];
@@ -379,7 +387,7 @@ static void __init setup_boot_config(void)
 	}
 }
 #else
-#define setup_boot_config()	do { } while (0)
+#define setup_boot_config(cmdline)	do { } while (0)
 #endif
 
 /* Change NUL term back to "=", to make "param" the whole string. */
@@ -760,7 +768,7 @@ asmlinkage __visible void __init start_kernel(void)
 	pr_notice("%s", linux_banner);
 	early_security_init();
 	setup_arch(&command_line);
-	setup_boot_config();
+	setup_boot_config(command_line);
 	setup_command_line(command_line);
 	setup_nr_cpu_ids();
 	setup_per_cpu_areas();
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] tracing: Changes for 5.6
  2020-02-04 12:28   ` Steven Rostedt
  2020-02-04 12:43     ` [PATCH] bootconfig: Add "disable_bootconfig" cmdline option to disable bootconfig Steven Rostedt
@ 2020-02-04 13:19     ` Linus Torvalds
  2020-02-04 13:46       ` Steven Rostedt
  1 sibling, 1 reply; 9+ messages in thread
From: Linus Torvalds @ 2020-02-04 13:19 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tom Zanussi,
	Peter Zijlstra

On Tue, Feb 4, 2020 at 12:29 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Would this work?

Yeah, looks workable. At least this wary if you screw up your
bootconfig file somehow (or you have a kernel bug that interacts badly
with a good bootconfig file), you don't need to worry about rewriting
the initrd.

The reason _I_ care is that the initrd creation scripts tend to come
mostly pre-packaged from a distro, and editing the initrd is a big
step.

The one reaction I have is that I wonder if we should just do this the
other way around instead: instead of disabling bootconfig, have a
"enable bootconfig" model instead.

Because it strikes me that the bootconfig should be the special case
(ie "bootconfig does setup for boot-time tracing"), and that you
should explicitly say "I want you to read the extended config" on the
regular kernel command line.

So from looking at this, I do have to say that I'd have a slight
preference for simply making this be an option like
"config=bootconfig" that says "extend cmdline with the data from the
'bootconfig' file".

Would that be horribly painful for your uses?

               Linus

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] tracing: Changes for 5.6
  2020-02-04 13:19     ` [GIT PULL] tracing: Changes for 5.6 Linus Torvalds
@ 2020-02-04 13:46       ` Steven Rostedt
  2020-02-04 14:25         ` [PATCH v2] bootconfig: Only load bootconfig if "config=bootconfig" is on the kernel cmdline (was: bootconfig: Add "disable_bootconfig" cmdline option to disable bootconfig) Steven Rostedt
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2020-02-04 13:46 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tom Zanussi,
	Peter Zijlstra

On Tue, 4 Feb 2020 13:19:06 +0000
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> Because it strikes me that the bootconfig should be the special case
> (ie "bootconfig does setup for boot-time tracing"), and that you
> should explicitly say "I want you to read the extended config" on the
> regular kernel command line.
> 
> So from looking at this, I do have to say that I'd have a slight
> preference for simply making this be an option like
> "config=bootconfig" that says "extend cmdline with the data from the
> 'bootconfig' file".

OK, I like this idea. As the current approach "silently" adds the boot
config, it may be surprising to someone that the configs are changing
(if they don't know to look for /proc/bootconfig).

Having a "config=bootconfig" required for reading makes it implicit
that a bootconfig is added to the existing command line, from just
looking at /proc/cmdline, and removing an added "config=bootconfig" 

> 
> Would that be horribly painful for your uses?
> 

I have no problem with this approach. Masami, are you OK with this?

I'll send a v2 implementing this change shortly.

-- Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] bootconfig: Only load bootconfig if "config=bootconfig" is on the kernel cmdline (was: bootconfig: Add "disable_bootconfig" cmdline option to  disable bootconfig)
  2020-02-04 13:46       ` Steven Rostedt
@ 2020-02-04 14:25         ` Steven Rostedt
  2020-02-04 14:33           ` Steven Rostedt
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2020-02-04 14:25 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tom Zanussi,
	Peter Zijlstra


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

As the bootconfig is appended to the initrd it is not as easy to modify as
the kernel command line. If there's some issue with the kernel, and the
developer wants to boot a pristine kernel, it should not be needed to modify
the initrd to remove the bootconfig for a single boot.

As bootconfig is silently added (if the admin does not know where to look
they may not know it's being loaded). It should be explicitly added to the
kernel cmdline. The loading of the bootconfig is only done if
"config=bootconfig" is on the kernel command line. This will let admins know
that the kernel command line is extended.

Note, after adding printk()s for when the size is too great or the checksum
is wrong, exposed that the current method always looked for the boot config,
and if this size and checksum matched, it would parse it (as if either is
wrong a printk has been added to show this). It's better to only check this
if the boot config is asked to be looked for.

Link: https://lore.kernel.org/r/CAHk-=wjfjO+h6bQzrTf=YCZA53Y3EDyAs3Z4gEsT7icA3u_Psw@mail.gmail.com

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/admin-guide/bootconfig.rst      |  2 ++
 .../admin-guide/kernel-parameters.txt         |  6 ++++
 init/main.c                                   | 28 ++++++++++++++-----
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index 4d617693c0c8..850c06f5b909 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -123,6 +123,8 @@ To remove the config from the image, you can use -d option as below::
 
  # tools/bootconfig/bootconfig -d /boot/initrd.img-X.Y.Z
 
+Then add "config=bootconfig" on the normal kernel command line to tell
+the kernel to look for the bootconfig at the end of the initrd file.
 
 Config File Limitation
 ======================
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ade4e6ec23e0..b1972ddf597c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -608,6 +608,12 @@
 	condev=		[HW,S390] console device
 	conmode=
 
+	config=bootconfig [KNL]
+			Extended command line options can be added to an initrd
+			and this will cause the kernel to look for it.
+
+			See Documentation/admin-guide/bootconfig.rst
+
 	console=	[KNL] Output console device and options.
 
 		tty<n>	Use the virtual console device <n>.
diff --git a/init/main.c b/init/main.c
index dd7da62d99a5..de3bc0a038e0 100644
--- a/init/main.c
+++ b/init/main.c
@@ -336,28 +336,39 @@ u32 boot_config_checksum(unsigned char *p, u32 size)
 	return ret;
 }
 
-static void __init setup_boot_config(void)
+static void __init setup_boot_config(const char *cmdline)
 {
 	u32 size, csum;
 	char *data, *copy;
+	const char *p;
 	u32 *hdr;
 
-	if (!initrd_end)
+	p = strstr(cmdline, "config=bootconfig");
+	if (!p || (p != cmdline && !isspace(*(p-1))) ||
+	    (p[17] && !isspace(p[17])))
 		return;
 
+	if (!initrd_end)
+		goto not_found;
+
 	hdr = (u32 *)(initrd_end - 8);
 	size = hdr[0];
 	csum = hdr[1];
 
-	if (size >= XBC_DATA_MAX)
+	if (size >= XBC_DATA_MAX) {
+		pr_err("bootconfig size %d greater than max size %d\n",
+			size, XBC_DATA_MAX);
 		return;
+	}
 
 	data = ((void *)hdr) - size;
 	if ((unsigned long)data < initrd_start)
-		return;
+		goto not_found;
 
-	if (boot_config_checksum((unsigned char *)data, size) != csum)
+	if (boot_config_checksum((unsigned char *)data, size) != csum) {
+		pr_err("bootconfig checksum failed\n");
 		return;
+	}
 
 	copy = memblock_alloc(size + 1, SMP_CACHE_BYTES);
 	if (!copy) {
@@ -377,9 +388,12 @@ static void __init setup_boot_config(void)
 		/* Also, "init." keys are init arguments */
 		extra_init_args = xbc_make_cmdline("init");
 	}
+	return;
+not_found:
+	pr_err("config=bootconfig on command line, but no bootconfig found\n");
 }
 #else
-#define setup_boot_config()	do { } while (0)
+#define setup_boot_config(cmdline)	do { } while (0)
 #endif
 
 /* Change NUL term back to "=", to make "param" the whole string. */
@@ -760,7 +774,7 @@ asmlinkage __visible void __init start_kernel(void)
 	pr_notice("%s", linux_banner);
 	early_security_init();
 	setup_arch(&command_line);
-	setup_boot_config();
+	setup_boot_config(command_line);
 	setup_command_line(command_line);
 	setup_nr_cpu_ids();
 	setup_per_cpu_areas();
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] bootconfig: Only load bootconfig if "config=bootconfig" is on the kernel cmdline (was: bootconfig: Add "disable_bootconfig" cmdline option to  disable bootconfig)
  2020-02-04 14:25         ` [PATCH v2] bootconfig: Only load bootconfig if "config=bootconfig" is on the kernel cmdline (was: bootconfig: Add "disable_bootconfig" cmdline option to disable bootconfig) Steven Rostedt
@ 2020-02-04 14:33           ` Steven Rostedt
  2020-02-04 15:43             ` Linus Torvalds
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2020-02-04 14:33 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tom Zanussi,
	Peter Zijlstra

On Tue, 4 Feb 2020 09:25:28 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> As bootconfig is silently added (if the admin does not know where to look
> they may not know it's being loaded). It should be explicitly added to the
> kernel cmdline. The loading of the bootconfig is only done if
> "config=bootconfig" is on the kernel command line. This will let admins know
> that the kernel command line is extended.

I wonder if we need the "config=" part. Would "bootconfig" be
sufficient, or is it better to keep it for documentation purposes.

-- Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] bootconfig: Only load bootconfig if "config=bootconfig" is on the kernel cmdline (was: bootconfig: Add "disable_bootconfig" cmdline option to disable bootconfig)
  2020-02-04 14:33           ` Steven Rostedt
@ 2020-02-04 15:43             ` Linus Torvalds
  0 siblings, 0 replies; 9+ messages in thread
From: Linus Torvalds @ 2020-02-04 15:43 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tom Zanussi,
	Peter Zijlstra

On Tue, Feb 4, 2020 at 2:33 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> I wonder if we need the "config=" part. Would "bootconfig" be
> sufficient, or is it better to keep it for documentation purposes.

I suspect "bootconfig" is sufficient, if it's easier to parse.

            Linus

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-02-04 15:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-04 10:31 [GIT PULL] tracing: Changes for 5.6 Steven Rostedt
2020-02-04 11:54 ` Linus Torvalds
2020-02-04 12:28   ` Steven Rostedt
2020-02-04 12:43     ` [PATCH] bootconfig: Add "disable_bootconfig" cmdline option to disable bootconfig Steven Rostedt
2020-02-04 13:19     ` [GIT PULL] tracing: Changes for 5.6 Linus Torvalds
2020-02-04 13:46       ` Steven Rostedt
2020-02-04 14:25         ` [PATCH v2] bootconfig: Only load bootconfig if "config=bootconfig" is on the kernel cmdline (was: bootconfig: Add "disable_bootconfig" cmdline option to disable bootconfig) Steven Rostedt
2020-02-04 14:33           ` Steven Rostedt
2020-02-04 15:43             ` Linus Torvalds

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).