linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Add build ID to stacktraces
@ 2021-03-01 17:47 Stephen Boyd
  2021-03-01 17:47 ` [PATCH 1/7] buildid: Add method to get running kernel's build ID Stephen Boyd
                   ` (6 more replies)
  0 siblings, 7 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-01 17:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Alexei Starovoitov, Andy Shevchenko, Baoquan He,
	Dave Young, Evan Green, Hsin-Yi Wang, Jessica Yu, Jiri Olsa,
	kexec, linux-doc, Petr Mladek, Rasmus Villemoes,
	Sergey Senozhatsky, Steven Rostedt, Vivek Goyal

This series adds the kernel's build ID to the stacktrace header printed
in oops messages, warnings, etc. and the build ID for any module that
appears in the stacktrace after the module name. The goal is to make the
stacktrace more self-contained and descriptive by including the relevant
build IDs in the kernel logs when something goes wrong. This can be used
by post processing tools like script/decode_stacktrace.sh and kernel
developers to easily locate the debug info associated with a kernel
crash and line up what line and file things started falling apart at.

This also includes a patch to make the buildid.c file use more const
arguments and consolidate logic into buildid.c from kdump. These are
left to the end as they were mostly cleanup patches. I don't know who
exactly maintains this so I guess Andrew is the best option to merge all
this code.

Here's an example lkdtm stacktrace

 WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
 Modules linked in: lkdtm rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE
 CPU: 4 PID: 3255 Comm: bash Not tainted 5.11 #3 aa23f7a1231c229de205662d5a9e0d4c580f19a1
 Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
 pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
 pc : lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
 lr : lkdtm_do_action+0x24/0x40 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
 sp : ffffffc0134fbca0
 x29: ffffffc0134fbca0 x28: ffffff92d53ba240
 x27: 0000000000000000 x26: 0000000000000000
 x25: 0000000000000000 x24: ffffffe3622352c0
 x23: 0000000000000020 x22: ffffffe362233366
 x21: ffffffe3622352e0 x20: ffffffc0134fbde0
 x19: 0000000000000008 x18: 0000000000000000
 x17: ffffff929b6536fc x16: 0000000000000000
 x15: 0000000000000000 x14: 0000000000000012
 x13: ffffffe380ed892c x12: ffffffe381d05068
 x11: 0000000000000000 x10: 0000000000000000
 x9 : 0000000000000001 x8 : ffffffe362237000
 x7 : aaaaaaaaaaaaaaaa x6 : 0000000000000000
 x5 : 0000000000000000 x4 : 0000000000000001
 x3 : 0000000000000008 x2 : ffffff93fef25a70
 x1 : ffffff93fef15788 x0 : ffffffe3622352e0
 Call trace:
  lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
  direct_entry+0x16c/0x1b4 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
  full_proxy_write+0x74/0xa4
  vfs_write+0xec/0x2e8
  ksys_write+0x84/0xf0
  __arm64_sys_write+0x24/0x30
  el0_svc_common+0xf4/0x1c0
  do_el0_svc_compat+0x28/0x3c
  el0_svc_compat+0x10/0x1c
  el0_sync_compat_handler+0xa8/0xcc
  el0_sync_compat+0x178/0x180
 ---[ end trace 3d95032303e59e68 ]---

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Evan Green <evgreen@chromium.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: <kexec@lists.infradead.org>
Cc: <linux-doc@vger.kernel.org>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Vivek Goyal <vgoyal@redhat.com>

Stephen Boyd (7):
  buildid: Add method to get running kernel's build ID
  dump_stack: Add vmlinux build ID to stack traces
  buildid: Add API to parse build ID out of buffer
  module: Parse and stash build ID on insertion
  printk: Make %pS and friends print module build ID
  buildid: Mark some arguments const
  kdump: Use vmlinux_build_id() to simplify

 Documentation/core-api/printk-formats.rst |  6 ++
 include/linux/buildid.h                   |  4 +
 include/linux/kallsyms.h                  |  6 +-
 include/linux/module.h                    |  6 +-
 kernel/crash_core.c                       | 46 ++----------
 kernel/kallsyms.c                         | 45 ++++++++----
 kernel/module.c                           | 24 +++++-
 lib/buildid.c                             | 89 +++++++++++++++++++----
 lib/dump_stack.c                          |  5 +-
 9 files changed, 157 insertions(+), 74 deletions(-)


base-commit: fe07bfda2fb9cdef8a4d4008a409bb02f35f1bd8
-- 
https://chromeos.dev


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

* [PATCH 1/7] buildid: Add method to get running kernel's build ID
  2021-03-01 17:47 [PATCH 0/7] Add build ID to stacktraces Stephen Boyd
@ 2021-03-01 17:47 ` Stephen Boyd
       [not found]   ` <CAHp75VcCjYm59-BQ0paPDCV97N5mgcq_OAdeixgUDEnAuG2qMg@mail.gmail.com>
  2021-03-01 17:47 ` [PATCH 2/7] dump_stack: Add vmlinux build ID to stack traces Stephen Boyd
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 30+ messages in thread
From: Stephen Boyd @ 2021-03-01 17:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Jiri Olsa, Alexei Starovoitov, Jessica Yu,
	Evan Green, Hsin-Yi Wang

Add vmlinux_build_id() so that callers can get a hex format string
representation of the running kernel's build ID. This will be used in
the kdump and dump_stack code so that developers can easily locate the
vmlinux for a stacktrace.

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Evan Green <evgreen@chromium.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 include/linux/buildid.h |  3 ++
 lib/buildid.c           | 65 ++++++++++++++++++++++++++++++++---------
 2 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/include/linux/buildid.h b/include/linux/buildid.h
index 40232f90db6e..dd134a96a87c 100644
--- a/include/linux/buildid.h
+++ b/include/linux/buildid.h
@@ -5,8 +5,11 @@
 #include <linux/mm_types.h>
 
 #define BUILD_ID_SIZE_MAX 20
+#define BUILD_ID_STR_SIZE_MAX (BUILD_ID_SIZE_MAX * 2 + 1)
 
 int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
 		   __u32 *size);
 
+const char *vmlinux_build_id(void);
+
 #endif
diff --git a/lib/buildid.c b/lib/buildid.c
index 6156997c3895..57daf928b133 100644
--- a/lib/buildid.c
+++ b/lib/buildid.c
@@ -2,30 +2,23 @@
 
 #include <linux/buildid.h>
 #include <linux/elf.h>
+#include <linux/kernel.h>
 #include <linux/pagemap.h>
 
 #define BUILD_ID 3
+
 /*
  * Parse build id from the note segment. This logic can be shared between
  * 32-bit and 64-bit system, because Elf32_Nhdr and Elf64_Nhdr are
  * identical.
  */
-static inline int parse_build_id(void *page_addr,
-				 unsigned char *build_id,
-				 __u32 *size,
-				 void *note_start,
-				 Elf32_Word note_size)
+static int parse_build_id_buf(unsigned char *build_id,
+			      __u32 *size,
+			      const void *note_start,
+			      Elf32_Word note_size)
 {
 	Elf32_Word note_offs = 0, new_offs;
 
-	/* check for overflow */
-	if (note_start < page_addr || note_start + note_size < note_start)
-		return -EINVAL;
-
-	/* only supports note that fits in the first page */
-	if (note_start + note_size > page_addr + PAGE_SIZE)
-		return -EINVAL;
-
 	while (note_offs + sizeof(Elf32_Nhdr) < note_size) {
 		Elf32_Nhdr *nhdr = (Elf32_Nhdr *)(note_start + note_offs);
 
@@ -49,9 +42,27 @@ static inline int parse_build_id(void *page_addr,
 			break;
 		note_offs = new_offs;
 	}
+
 	return -EINVAL;
 }
 
+static inline int parse_build_id(void *page_addr,
+				 unsigned char *build_id,
+				 __u32 *size,
+				 void *note_start,
+				 Elf32_Word note_size)
+{
+	/* check for overflow */
+	if (note_start < page_addr || note_start + note_size < note_start)
+		return -EINVAL;
+
+	/* only supports note that fits in the first page */
+	if (note_start + note_size > page_addr + PAGE_SIZE)
+		return -EINVAL;
+
+	return parse_build_id_buf(build_id, size, note_start, note_size);
+}
+
 /* Parse build ID from 32-bit ELF */
 static int get_build_id_32(void *page_addr, unsigned char *build_id,
 			   __u32 *size)
@@ -147,3 +158,31 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
 	put_page(page);
 	return ret;
 }
+
+static void build_id2hex(char *dst, const unsigned char *src, __u32 size)
+{
+	bin2hex(dst, src, size);
+	dst[2 * size] = '\0';
+}
+
+/**
+ * vmlinux_build_id - Get the running kernel's build-id in hex string format
+ *
+ * Return: Running kernel's build-id in hex string format
+ */
+const char *vmlinux_build_id(void)
+{
+	extern const void __start_notes __weak;
+	extern const void __stop_notes __weak;
+	unsigned int notes_size = &__stop_notes - &__start_notes;
+	unsigned char build_id[BUILD_ID_SIZE_MAX];
+	__u32 size;
+	static char vmlinux_build_id_buf[BUILD_ID_STR_SIZE_MAX];
+
+	if (vmlinux_build_id_buf[0] == '\0') {
+		if (!parse_build_id_buf(build_id, &size, &__start_notes, notes_size))
+			build_id2hex(vmlinux_build_id_buf, build_id, size);
+	}
+
+	return vmlinux_build_id_buf;
+}
-- 
https://chromeos.dev


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

* [PATCH 2/7] dump_stack: Add vmlinux build ID to stack traces
  2021-03-01 17:47 [PATCH 0/7] Add build ID to stacktraces Stephen Boyd
  2021-03-01 17:47 ` [PATCH 1/7] buildid: Add method to get running kernel's build ID Stephen Boyd
@ 2021-03-01 17:47 ` Stephen Boyd
  2021-03-01 17:47 ` [PATCH 3/7] buildid: Add API to parse build ID out of buffer Stephen Boyd
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-01 17:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Jiri Olsa, Alexei Starovoitov, Jessica Yu,
	Evan Green, Hsin-Yi Wang

Add the running kernel's build ID to the stacktrace information header.
This makes it simpler for developers to locate the vmlinux with full
debug info for a particular kernel stacktrace. Combined with
scripts/decode_stracktrace.sh a developer can download the correct
vmlinux from a debuginfod server and find the exact line number and file
for the functions in a stacktrace.

This is especially useful for pstore or crash debugging where the kernel
crashes and the recovery kernel may be different or the debug symbols
don't exist on the device due to space concerns. The stacktrace can be
analyzed after the crash with the matching vmlinux to understand where
in the function something went wrong.

Example stacktrace from lkdtm:

 WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm]
 Modules linked in: lkdtm rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE
 CPU: 4 PID: 3255 Comm: bash Not tainted 5.11 #3 aa23f7a1231c229de205662d5a9e0d4c580f19a1
 Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
 pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
 pc : lkdtm_WARNING+0x28/0x30 [lkdtm]
 lr : lkdtm_do_action+0x24/0x40 [lkdtm]
 sp : ffffffc0134fbca0
 x29: ffffffc0134fbca0 x28: ffffff92d53ba240
 x27: 0000000000000000 x26: 0000000000000000
 x25: 0000000000000000 x24: ffffffe3622352c0
 x23: 0000000000000020 x22: ffffffe362233366
 x21: ffffffe3622352e0 x20: ffffffc0134fbde0
 x19: 0000000000000008 x18: 0000000000000000
 x17: ffffff929b6536fc x16: 0000000000000000
 x15: 0000000000000000 x14: 0000000000000012
 x13: ffffffe380ed892c x12: ffffffe381d05068
 x11: 0000000000000000 x10: 0000000000000000
 x9 : 0000000000000001 x8 : ffffffe362237000
 x7 : aaaaaaaaaaaaaaaa x6 : 0000000000000000
 x5 : 0000000000000000 x4 : 0000000000000001
 x3 : 0000000000000008 x2 : ffffff93fef25a70
 x1 : ffffff93fef15788 x0 : ffffffe3622352e0
 Call trace:
  lkdtm_WARNING+0x28/0x30 [lkdtm]
  direct_entry+0x16c/0x1b4 [lkdtm]
  full_proxy_write+0x74/0xa4
  vfs_write+0xec/0x2e8
  ksys_write+0x84/0xf0
  __arm64_sys_write+0x24/0x30
  el0_svc_common+0xf4/0x1c0
  do_el0_svc_compat+0x28/0x3c
  el0_svc_compat+0x10/0x1c
  el0_sync_compat_handler+0xa8/0xcc
  el0_sync_compat+0x178/0x180
 ---[ end trace 3d95032303e59e68 ]---

The hex string aa23f7a1231c229de205662d5a9e0d4c580f19a1 is the build ID,
following the kernel version number.

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Evan Green <evgreen@chromium.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 lib/dump_stack.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index f5a33b6f773f..a13f3ea218d3 100644
--- a/lib/dump_stack.c
+++ b/lib/dump_stack.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/buildid.h>
 #include <linux/export.h>
 #include <linux/sched.h>
 #include <linux/sched/debug.h>
@@ -45,13 +46,13 @@ void __init dump_stack_set_arch_desc(const char *fmt, ...)
  */
 void dump_stack_print_info(const char *log_lvl)
 {
-	printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s\n",
+	printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s %s\n",
 	       log_lvl, raw_smp_processor_id(), current->pid, current->comm,
 	       kexec_crash_loaded() ? "Kdump: loaded " : "",
 	       print_tainted(),
 	       init_utsname()->release,
 	       (int)strcspn(init_utsname()->version, " "),
-	       init_utsname()->version);
+	       init_utsname()->version, vmlinux_build_id());
 
 	if (dump_stack_arch_desc_str[0] != '\0')
 		printk("%sHardware name: %s\n",
-- 
https://chromeos.dev


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

* [PATCH 3/7] buildid: Add API to parse build ID out of buffer
  2021-03-01 17:47 [PATCH 0/7] Add build ID to stacktraces Stephen Boyd
  2021-03-01 17:47 ` [PATCH 1/7] buildid: Add method to get running kernel's build ID Stephen Boyd
  2021-03-01 17:47 ` [PATCH 2/7] dump_stack: Add vmlinux build ID to stack traces Stephen Boyd
@ 2021-03-01 17:47 ` Stephen Boyd
  2021-03-01 17:47 ` [PATCH 4/7] module: Parse and stash build ID on insertion Stephen Boyd
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-01 17:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Jiri Olsa, Alexei Starovoitov, Jessica Yu,
	Evan Green, Hsin-Yi Wang

Add an API that can parse the build ID in hex string form out of a
buffer to support printing a kernel module's build ID.

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Evan Green <evgreen@chromium.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 include/linux/buildid.h |  1 +
 lib/buildid.c           | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/buildid.h b/include/linux/buildid.h
index dd134a96a87c..f5a5b920d18c 100644
--- a/include/linux/buildid.h
+++ b/include/linux/buildid.h
@@ -10,6 +10,7 @@
 int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
 		   __u32 *size);
 
+int build_id_parse_buf(const void *buf, char *build_id, u32 buf_size);
 const char *vmlinux_build_id(void);
 
 #endif
diff --git a/lib/buildid.c b/lib/buildid.c
index 57daf928b133..4e1d7c51dc5f 100644
--- a/lib/buildid.c
+++ b/lib/buildid.c
@@ -165,6 +165,26 @@ static void build_id2hex(char *dst, const unsigned char *src, __u32 size)
 	dst[2 * size] = '\0';
 }
 
+/**
+ * build_id_parse_buf - Get build ID in hex string format from elf section note
+ * @buf: Elf note section(s) to parse
+ * @build_id: Build ID in hex string format parsed from @buf
+ * @buf_size: Size of @buf in bytes
+ *
+ * Return: 0 on success, -EINVAL otherwise
+ */
+int build_id_parse_buf(const void *buf, char *build_id, u32 buf_size)
+{
+	unsigned char build_id_buf[BUILD_ID_SIZE_MAX];
+	__u32 size;
+	int ret;
+
+	ret = parse_build_id_buf(build_id_buf, &size, buf, buf_size);
+	if (!ret)
+		build_id2hex(build_id, build_id_buf, size);
+
+	return ret;
+}
 /**
  * vmlinux_build_id - Get the running kernel's build-id in hex string format
  *
-- 
https://chromeos.dev


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

* [PATCH 4/7] module: Parse and stash build ID on insertion
  2021-03-01 17:47 [PATCH 0/7] Add build ID to stacktraces Stephen Boyd
                   ` (2 preceding siblings ...)
  2021-03-01 17:47 ` [PATCH 3/7] buildid: Add API to parse build ID out of buffer Stephen Boyd
@ 2021-03-01 17:47 ` Stephen Boyd
  2021-03-01 17:47 ` [PATCH 5/7] printk: Make %pS and friends print module build ID Stephen Boyd
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-01 17:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Jiri Olsa, Alexei Starovoitov, Jessica Yu,
	Evan Green, Hsin-Yi Wang

Parse the build ID of a module when it is inserted and stash it away in
'struct module' in case we print module symbols. This will be used in a
future patch to print the module build ID when module functions are in a
stack trace.

Note: we only do this if CONFIG_KALLSYMS is enabled, because if it isn't
enabled then module names aren't printed for symbolic pointers and thus
build IDs aren't printed either.

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Evan Green <evgreen@chromium.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 include/linux/module.h |  4 ++++
 kernel/module.c        | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
index 59f094fa6f74..9d1f6a5240c1 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -11,6 +11,7 @@
 
 #include <linux/list.h>
 #include <linux/stat.h>
+#include <linux/buildid.h>
 #include <linux/compiler.h>
 #include <linux/cache.h>
 #include <linux/kmod.h>
@@ -432,6 +433,9 @@ struct module {
 
 	/* Notes attributes */
 	struct module_notes_attrs *notes_attrs;
+
+	/* Module build-id */
+	char build_id[BUILD_ID_STR_SIZE_MAX];
 #endif
 
 	/* The command line arguments (may be mangled).  People like
diff --git a/kernel/module.c b/kernel/module.c
index 30479355ab85..a7559a0de9d8 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -13,6 +13,7 @@
 #include <linux/trace_events.h>
 #include <linux/init.h>
 #include <linux/kallsyms.h>
+#include <linux/buildid.h>
 #include <linux/file.h>
 #include <linux/fs.h>
 #include <linux/sysfs.h>
@@ -2770,6 +2771,20 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
 	}
 	mod->core_kallsyms.num_symtab = ndst;
 }
+
+static void module_init_build_id(struct module *mod, const struct load_info *info)
+{
+	const Elf_Shdr *sechdr;
+	unsigned int i;
+
+	for (i = 0; i < info->hdr->e_shnum; i++) {
+		sechdr = &info->sechdrs[i];
+		if (!sect_empty(sechdr) && sechdr->sh_type == SHT_NOTE &&
+		    !build_id_parse_buf((void *)sechdr->sh_addr, mod->build_id,
+					sechdr->sh_size))
+			break;
+	}
+}
 #else
 static inline void layout_symtab(struct module *mod, struct load_info *info)
 {
@@ -2778,6 +2793,10 @@ static inline void layout_symtab(struct module *mod, struct load_info *info)
 static void add_kallsyms(struct module *mod, const struct load_info *info)
 {
 }
+
+static void module_init_build_id(struct module *mod, const struct load_info *info)
+{
+}
 #endif /* CONFIG_KALLSYMS */
 
 static void dynamic_debug_setup(struct module *mod, struct _ddebug *debug, unsigned int num)
@@ -4004,6 +4023,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
 		goto free_arch_cleanup;
 	}
 
+	module_init_build_id(mod, info);
 	dynamic_debug_setup(mod, info->debug, info->num_debug);
 
 	/* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
-- 
https://chromeos.dev


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

* [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-01 17:47 [PATCH 0/7] Add build ID to stacktraces Stephen Boyd
                   ` (3 preceding siblings ...)
  2021-03-01 17:47 ` [PATCH 4/7] module: Parse and stash build ID on insertion Stephen Boyd
@ 2021-03-01 17:47 ` Stephen Boyd
  2021-03-02  2:43   ` Steven Rostedt
                     ` (3 more replies)
  2021-03-01 17:47 ` [PATCH 6/7] buildid: Mark some arguments const Stephen Boyd
  2021-03-01 17:47 ` [PATCH 7/7] kdump: Use vmlinux_build_id() to simplify Stephen Boyd
  6 siblings, 4 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-01 17:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Jiri Olsa, Alexei Starovoitov, Jessica Yu,
	Evan Green, Hsin-Yi Wang, Petr Mladek, Steven Rostedt,
	Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc

The %pS printk format (among some others) is used to print kernel
addresses symbolically. When the kernel prints an address inside of a
module, the kernel prints the addresses' symbol name along with the
module's name that contains the address. Let's make kernel stacktraces
easier to identify on KALLSYMS builds by including the build ID of a
module when we print the address.

This is especially helpful for crash debugging with pstore or crashdump
kernels. If we have the build ID for the module in the stacktrace we can
request the debug symbols for the module from a remote debuginfod server
or parse stacktraces at a later time with decode_stacktrace.sh by
downloading the correct symbols based on the build ID. This cuts down on
the amount of time and effort needed to find the correct kernel modules
for a stacktrace by encoding that information into it.

An alternative to changing the printk format would be to update the
"Modules linked in:" line to include the build ID of each module linked
in. This can become quite long when many modules are loaded (i.e. on a
distro) so I've opted for the printk format instead. Similarly,
collecting each module build ID in a stacktrace and then printing it
after the trace would require some architecture level surgery to the
dump stack code and also provide the ID "too late" for code that is
parsing each line of the stacktrace.

Example:

 WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
 Modules linked in: lkdtm rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE
 CPU: 4 PID: 3255 Comm: bash Not tainted 5.11 #3 aa23f7a1231c229de205662d5a9e0d4c580f19a1
 Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
 pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
 pc : lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
 lr : lkdtm_do_action+0x24/0x40 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
 sp : ffffffc0134fbca0
 x29: ffffffc0134fbca0 x28: ffffff92d53ba240
 x27: 0000000000000000 x26: 0000000000000000
 x25: 0000000000000000 x24: ffffffe3622352c0
 x23: 0000000000000020 x22: ffffffe362233366
 x21: ffffffe3622352e0 x20: ffffffc0134fbde0
 x19: 0000000000000008 x18: 0000000000000000
 x17: ffffff929b6536fc x16: 0000000000000000
 x15: 0000000000000000 x14: 0000000000000012
 x13: ffffffe380ed892c x12: ffffffe381d05068
 x11: 0000000000000000 x10: 0000000000000000
 x9 : 0000000000000001 x8 : ffffffe362237000
 x7 : aaaaaaaaaaaaaaaa x6 : 0000000000000000
 x5 : 0000000000000000 x4 : 0000000000000001
 x3 : 0000000000000008 x2 : ffffff93fef25a70
 x1 : ffffff93fef15788 x0 : ffffffe3622352e0
 Call trace:
  lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
  direct_entry+0x16c/0x1b4 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
  full_proxy_write+0x74/0xa4
  vfs_write+0xec/0x2e8
  ksys_write+0x84/0xf0
  __arm64_sys_write+0x24/0x30
  el0_svc_common+0xf4/0x1c0
  do_el0_svc_compat+0x28/0x3c
  el0_svc_compat+0x10/0x1c
  el0_sync_compat_handler+0xa8/0xcc
  el0_sync_compat+0x178/0x180
 ---[ end trace 3d95032303e59e68 ]---

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Evan Green <evgreen@chromium.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: <linux-doc@vger.kernel.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---

Yet another alternative is to add a printk format like %pSB for
backtrace prints. This would require a handful of architecture updates
and I'm not sure it's worth the effort for that.

 Documentation/core-api/printk-formats.rst |  6 +++
 include/linux/kallsyms.h                  |  6 ++-
 include/linux/module.h                    |  2 +-
 kernel/kallsyms.c                         | 45 +++++++++++++++--------
 kernel/module.c                           |  4 +-
 5 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 160e710d992f..7f05962f7f68 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -114,6 +114,12 @@ used when printing stack backtraces. The specifier takes into
 consideration the effect of compiler optimisations which may occur
 when tail-calls are used and marked with the noreturn GCC attribute.
 
+If the pointer is within a module, the module name and build ID is printed after
+the symbol name.
+
+::
+	%pS	versatile_init+0x0/0x110 [module_name] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
+
 Probed Pointers from BPF / tracing
 ----------------------------------
 
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 465060acc981..c2f1660823cb 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -7,6 +7,7 @@
 #define _LINUX_KALLSYMS_H
 
 #include <linux/errno.h>
+#include <linux/buildid.h>
 #include <linux/kernel.h>
 #include <linux/stddef.h>
 #include <linux/mm.h>
@@ -15,8 +16,9 @@
 #include <asm/sections.h>
 
 #define KSYM_NAME_LEN 128
-#define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s]") + (KSYM_NAME_LEN - 1) + \
-			 2*(BITS_PER_LONG*3/10) + (MODULE_NAME_LEN - 1) + 1)
+#define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s] (%s)") + (KSYM_NAME_LEN - 1) + \
+			 2*(BITS_PER_LONG*3/10) + (MODULE_NAME_LEN - 1) + \
+			 (BUILD_ID_STR_SIZE_MAX - 1) + 1)
 
 struct cred;
 struct module;
diff --git a/include/linux/module.h b/include/linux/module.h
index 9d1f6a5240c1..c12f9215f63e 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -634,7 +634,7 @@ void *dereference_module_function_descriptor(struct module *mod, void *ptr);
 const char *module_address_lookup(unsigned long addr,
 			    unsigned long *symbolsize,
 			    unsigned long *offset,
-			    char **modname,
+			    char **modname, char **modbuildid,
 			    char *namebuf);
 int lookup_module_symbol_name(unsigned long addr, char *symname);
 int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name);
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 8043a90aa50e..a70c53764b8a 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -273,21 +273,13 @@ int kallsyms_lookup_size_offset(unsigned long addr, unsigned long *symbolsize,
 		get_symbol_pos(addr, symbolsize, offset);
 		return 1;
 	}
-	return !!module_address_lookup(addr, symbolsize, offset, NULL, namebuf) ||
+	return !!module_address_lookup(addr, symbolsize, offset, NULL, NULL, namebuf) ||
 	       !!__bpf_address_lookup(addr, symbolsize, offset, namebuf);
 }
 
-/*
- * Lookup an address
- * - modname is set to NULL if it's in the kernel.
- * - We guarantee that the returned name is valid until we reschedule even if.
- *   It resides in a module.
- * - We also guarantee that modname will be valid until rescheduled.
- */
-const char *kallsyms_lookup(unsigned long addr,
-			    unsigned long *symbolsize,
-			    unsigned long *offset,
-			    char **modname, char *namebuf)
+const char *kallsyms_lookup_buildid(unsigned long addr, unsigned long *symbolsize,
+				    unsigned long *offset, char **modname,
+				    char **modbuildid, char *namebuf)
 {
 	const char *ret;
 
@@ -303,12 +295,14 @@ const char *kallsyms_lookup(unsigned long addr,
 				       namebuf, KSYM_NAME_LEN);
 		if (modname)
 			*modname = NULL;
+		if (modbuildid)
+			*modbuildid = NULL;
 		return namebuf;
 	}
 
 	/* See if it's in a module or a BPF JITed image. */
 	ret = module_address_lookup(addr, symbolsize, offset,
-				    modname, namebuf);
+				    modname, modbuildid, namebuf);
 	if (!ret)
 		ret = bpf_address_lookup(addr, symbolsize,
 					 offset, modname, namebuf);
@@ -319,6 +313,22 @@ const char *kallsyms_lookup(unsigned long addr,
 	return ret;
 }
 
+/*
+ * Lookup an address
+ * - modname is set to NULL if it's in the kernel.
+ * - We guarantee that the returned name is valid until we reschedule even if.
+ *   It resides in a module.
+ * - We also guarantee that modname will be valid until rescheduled.
+ */
+const char *kallsyms_lookup(unsigned long addr,
+			    unsigned long *symbolsize,
+			    unsigned long *offset,
+			    char **modname, char *namebuf)
+{
+	return kallsyms_lookup_buildid(addr, symbolsize, offset, modname,
+				       NULL, namebuf);
+}
+
 int lookup_symbol_name(unsigned long addr, char *symname)
 {
 	symname[0] = '\0';
@@ -362,12 +372,14 @@ static int __sprint_symbol(char *buffer, unsigned long address,
 			   int symbol_offset, int add_offset)
 {
 	char *modname;
+	char *modbuildid;
 	const char *name;
 	unsigned long offset, size;
 	int len;
 
 	address += symbol_offset;
-	name = kallsyms_lookup(address, &size, &offset, &modname, buffer);
+	name = kallsyms_lookup_buildid(address, &size, &offset, &modname, &modbuildid,
+				       buffer);
 	if (!name)
 		return sprintf(buffer, "0x%lx", address - symbol_offset);
 
@@ -379,8 +391,11 @@ static int __sprint_symbol(char *buffer, unsigned long address,
 	if (add_offset)
 		len += sprintf(buffer + len, "+%#lx/%#lx", offset, size);
 
-	if (modname)
+	if (modname) {
 		len += sprintf(buffer + len, " [%s]", modname);
+		if (modbuildid)
+			len += sprintf(buffer + len, " (%s)", modbuildid);
+	}
 
 	return len;
 }
diff --git a/kernel/module.c b/kernel/module.c
index a7559a0de9d8..2909e62f147b 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4255,7 +4255,7 @@ void * __weak dereference_module_function_descriptor(struct module *mod,
 const char *module_address_lookup(unsigned long addr,
 			    unsigned long *size,
 			    unsigned long *offset,
-			    char **modname,
+			    char **modname, char **modbuildid,
 			    char *namebuf)
 {
 	const char *ret = NULL;
@@ -4266,6 +4266,8 @@ const char *module_address_lookup(unsigned long addr,
 	if (mod) {
 		if (modname)
 			*modname = mod->name;
+		if (modbuildid)
+			*modbuildid = mod->build_id;
 
 		ret = find_kallsyms_symbol(mod, addr, size, offset);
 	}
-- 
https://chromeos.dev


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

* [PATCH 6/7] buildid: Mark some arguments const
  2021-03-01 17:47 [PATCH 0/7] Add build ID to stacktraces Stephen Boyd
                   ` (4 preceding siblings ...)
  2021-03-01 17:47 ` [PATCH 5/7] printk: Make %pS and friends print module build ID Stephen Boyd
@ 2021-03-01 17:47 ` Stephen Boyd
  2021-03-01 17:47 ` [PATCH 7/7] kdump: Use vmlinux_build_id() to simplify Stephen Boyd
  6 siblings, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-01 17:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Jiri Olsa, Alexei Starovoitov, Jessica Yu,
	Evan Green, Hsin-Yi Wang

These arguments are never modified so they can be marked const to
indicate as such.

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Evan Green <evgreen@chromium.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 lib/buildid.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/buildid.c b/lib/buildid.c
index 4e1d7c51dc5f..71c087a6afda 100644
--- a/lib/buildid.c
+++ b/lib/buildid.c
@@ -46,10 +46,10 @@ static int parse_build_id_buf(unsigned char *build_id,
 	return -EINVAL;
 }
 
-static inline int parse_build_id(void *page_addr,
+static inline int parse_build_id(const void *page_addr,
 				 unsigned char *build_id,
 				 __u32 *size,
-				 void *note_start,
+				 const void *note_start,
 				 Elf32_Word note_size)
 {
 	/* check for overflow */
@@ -64,7 +64,7 @@ static inline int parse_build_id(void *page_addr,
 }
 
 /* Parse build ID from 32-bit ELF */
-static int get_build_id_32(void *page_addr, unsigned char *build_id,
+static int get_build_id_32(const void *page_addr, unsigned char *build_id,
 			   __u32 *size)
 {
 	Elf32_Ehdr *ehdr = (Elf32_Ehdr *)page_addr;
@@ -89,7 +89,7 @@ static int get_build_id_32(void *page_addr, unsigned char *build_id,
 }
 
 /* Parse build ID from 64-bit ELF */
-static int get_build_id_64(void *page_addr, unsigned char *build_id,
+static int get_build_id_64(const void *page_addr, unsigned char *build_id,
 			   __u32 *size)
 {
 	Elf64_Ehdr *ehdr = (Elf64_Ehdr *)page_addr;
-- 
https://chromeos.dev


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

* [PATCH 7/7] kdump: Use vmlinux_build_id() to simplify
  2021-03-01 17:47 [PATCH 0/7] Add build ID to stacktraces Stephen Boyd
                   ` (5 preceding siblings ...)
  2021-03-01 17:47 ` [PATCH 6/7] buildid: Mark some arguments const Stephen Boyd
@ 2021-03-01 17:47 ` Stephen Boyd
  2021-03-02  8:19   ` Baoquan He
  6 siblings, 1 reply; 30+ messages in thread
From: Stephen Boyd @ 2021-03-01 17:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Jiri Olsa, Alexei Starovoitov, Jessica Yu,
	Evan Green, Hsin-Yi Wang, Dave Young, Baoquan He, Vivek Goyal,
	kexec

We can use the vmlinux_build_id() helper here now instead of open coding
it. This consolidates code and possibly avoids calculating the build ID
twice in the case of a crash with a stacktrace.

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Evan Green <evgreen@chromium.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Cc: Dave Young <dyoung@redhat.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: <kexec@lists.infradead.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 kernel/crash_core.c | 46 ++++++++-------------------------------------
 1 file changed, 8 insertions(+), 38 deletions(-)

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 825284baaf46..07d3e1109a8c 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2002-2004 Eric Biederman  <ebiederm@xmission.com>
  */
 
+#include <linux/buildid.h>
 #include <linux/crash_core.h>
 #include <linux/utsname.h>
 #include <linux/vmalloc.h>
@@ -378,51 +379,20 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
 }
 EXPORT_SYMBOL(paddr_vmcoreinfo_note);
 
-#define NOTES_SIZE (&__stop_notes - &__start_notes)
-#define BUILD_ID_MAX SHA1_DIGEST_SIZE
-#define NT_GNU_BUILD_ID 3
-
-struct elf_note_section {
-	struct elf_note	n_hdr;
-	u8 n_data[];
-};
-
 /*
  * Add build ID from .notes section as generated by the GNU ld(1)
  * or LLVM lld(1) --build-id option.
  */
 static void add_build_id_vmcoreinfo(void)
 {
-	char build_id[BUILD_ID_MAX * 2 + 1];
-	int n_remain = NOTES_SIZE;
-
-	while (n_remain >= sizeof(struct elf_note)) {
-		const struct elf_note_section *note_sec =
-			&__start_notes + NOTES_SIZE - n_remain;
-		const u32 n_namesz = note_sec->n_hdr.n_namesz;
-
-		if (note_sec->n_hdr.n_type == NT_GNU_BUILD_ID &&
-		    n_namesz != 0 &&
-		    !strcmp((char *)&note_sec->n_data[0], "GNU")) {
-			if (note_sec->n_hdr.n_descsz <= BUILD_ID_MAX) {
-				const u32 n_descsz = note_sec->n_hdr.n_descsz;
-				const u8 *s = &note_sec->n_data[n_namesz];
-
-				s = PTR_ALIGN(s, 4);
-				bin2hex(build_id, s, n_descsz);
-				build_id[2 * n_descsz] = '\0';
-				VMCOREINFO_BUILD_ID(build_id);
-				return;
-			}
-			pr_warn("Build ID is too large to include in vmcoreinfo: %u > %u\n",
-				note_sec->n_hdr.n_descsz,
-				BUILD_ID_MAX);
-			return;
-		}
-		n_remain -= sizeof(struct elf_note) +
-			ALIGN(note_sec->n_hdr.n_namesz, 4) +
-			ALIGN(note_sec->n_hdr.n_descsz, 4);
+	const char *build_id = vmlinux_build_id();
+
+	if (build_id[0] == '\0') {
+		pr_warn("Build ID cannot be included in vmcoreinfo\n");
+		return;
 	}
+
+	VMCOREINFO_BUILD_ID(build_id);
 }
 
 static int __init crash_save_vmcoreinfo_init(void)
-- 
https://chromeos.dev


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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-01 17:47 ` [PATCH 5/7] printk: Make %pS and friends print module build ID Stephen Boyd
@ 2021-03-02  2:43   ` Steven Rostedt
  2021-03-02 23:24     ` Stephen Boyd
  2021-03-04 17:19     ` Matthew Wilcox
  2021-03-03  2:01   ` Steven Rostedt
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 30+ messages in thread
From: Steven Rostedt @ 2021-03-02  2:43 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Petr Mladek,
	Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc

On Mon,  1 Mar 2021 09:47:47 -0800
Stephen Boyd <swboyd@chromium.org> wrote:

> The %pS printk format (among some others) is used to print kernel
> addresses symbolically. When the kernel prints an address inside of a
> module, the kernel prints the addresses' symbol name along with the
> module's name that contains the address. Let's make kernel stacktraces
> easier to identify on KALLSYMS builds by including the build ID of a
> module when we print the address.

Please no!

This kills the output of tracing with offset, and can possibly break
scripts. I don't want to look at traces like this!

          <idle>-0       [004] ..s1   353.842577: ipv4_conntrack_in+0x0/0x10 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_hook_slow+0x40/0xb0
          <idle>-0       [004] ..s1   353.842577: nf_conntrack_in+0x0/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_hook_slow+0x40/0xb0
          <idle>-0       [004] ..s1   353.842577: get_l4proto+0x0/0x190 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_in+0x92/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
          <idle>-0       [004] ..s1   353.842577: nf_ct_get_tuple+0x0/0x240 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_in+0xec/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
          <idle>-0       [004] ..s1   353.842577: hash_conntrack_raw+0x0/0x170 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_in+0x28c/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
          <idle>-0       [004] ..s1   353.842578: __nf_conntrack_find_get.isra.0+0x0/0x2f0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_in+0x29d/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
          <idle>-0       [004] ..s1   353.842578: nf_conntrack_tcp_packet+0x0/0x1760 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_in+0x3c8/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
          <idle>-0       [004] ..s2   353.842578: nf_ct_seq_offset+0x0/0x40 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_tcp_packet+0x26d/0x1760 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
          <idle>-0       [004] ..s1   353.842578: __nf_ct_refresh_acct+0x0/0x50 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_tcp_packet+0x558/0x1760 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)

 NACK!

-- Steve

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

* Re: [PATCH 7/7] kdump: Use vmlinux_build_id() to simplify
  2021-03-01 17:47 ` [PATCH 7/7] kdump: Use vmlinux_build_id() to simplify Stephen Boyd
@ 2021-03-02  8:19   ` Baoquan He
  2021-03-02 23:21     ` Stephen Boyd
  0 siblings, 1 reply; 30+ messages in thread
From: Baoquan He @ 2021-03-02  8:19 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Andrew Morton, Jessica Yu, kexec, linux-kernel, Evan Green,
	Alexei Starovoitov, Jiri Olsa, Hsin-Yi Wang, Dave Young,
	Vivek Goyal

On 03/01/21 at 09:47am, Stephen Boyd wrote:
> We can use the vmlinux_build_id() helper here now instead of open coding
> it. This consolidates code and possibly avoids calculating the build ID
> twice in the case of a crash with a stacktrace.
> 
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Jessica Yu <jeyu@kernel.org>
> Cc: Evan Green <evgreen@chromium.org>
> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: <kexec@lists.infradead.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  kernel/crash_core.c | 46 ++++++++-------------------------------------
>  1 file changed, 8 insertions(+), 38 deletions(-)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 825284baaf46..07d3e1109a8c 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -4,6 +4,7 @@
>   * Copyright (C) 2002-2004 Eric Biederman  <ebiederm@xmission.com>
>   */
>  
> +#include <linux/buildid.h>
>  #include <linux/crash_core.h>
>  #include <linux/utsname.h>
>  #include <linux/vmalloc.h>
> @@ -378,51 +379,20 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
>  }
>  EXPORT_SYMBOL(paddr_vmcoreinfo_note);
>  
> -#define NOTES_SIZE (&__stop_notes - &__start_notes)
> -#define BUILD_ID_MAX SHA1_DIGEST_SIZE
> -#define NT_GNU_BUILD_ID 3
> -
> -struct elf_note_section {
> -	struct elf_note	n_hdr;
> -	u8 n_data[];
> -};
> -
>  /*
>   * Add build ID from .notes section as generated by the GNU ld(1)
>   * or LLVM lld(1) --build-id option.
>   */
>  static void add_build_id_vmcoreinfo(void)
>  {
> -	char build_id[BUILD_ID_MAX * 2 + 1];
> -	int n_remain = NOTES_SIZE;
> -
> -	while (n_remain >= sizeof(struct elf_note)) {
> -		const struct elf_note_section *note_sec =
> -			&__start_notes + NOTES_SIZE - n_remain;
> -		const u32 n_namesz = note_sec->n_hdr.n_namesz;
> -
> -		if (note_sec->n_hdr.n_type == NT_GNU_BUILD_ID &&
> -		    n_namesz != 0 &&
> -		    !strcmp((char *)&note_sec->n_data[0], "GNU")) {
> -			if (note_sec->n_hdr.n_descsz <= BUILD_ID_MAX) {
> -				const u32 n_descsz = note_sec->n_hdr.n_descsz;
> -				const u8 *s = &note_sec->n_data[n_namesz];
> -
> -				s = PTR_ALIGN(s, 4);
> -				bin2hex(build_id, s, n_descsz);
> -				build_id[2 * n_descsz] = '\0';
> -				VMCOREINFO_BUILD_ID(build_id);
> -				return;
> -			}
> -			pr_warn("Build ID is too large to include in vmcoreinfo: %u > %u\n",
> -				note_sec->n_hdr.n_descsz,
> -				BUILD_ID_MAX);
> -			return;
> -		}
> -		n_remain -= sizeof(struct elf_note) +
> -			ALIGN(note_sec->n_hdr.n_namesz, 4) +
> -			ALIGN(note_sec->n_hdr.n_descsz, 4);
> +	const char *build_id = vmlinux_build_id();

It's strange that I can only see the cover letter and this patch 7,
couldn't find the patch where vmlinux_build_id() is introduced in lkml.

> +
> +	if (build_id[0] == '\0') {
> +		pr_warn("Build ID cannot be included in vmcoreinfo\n");
> +		return;
>  	}
> +
> +	VMCOREINFO_BUILD_ID(build_id);
>  }
>  
>  static int __init crash_save_vmcoreinfo_init(void)
> -- 
> https://chromeos.dev
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 


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

* Re: [PATCH 7/7] kdump: Use vmlinux_build_id() to simplify
  2021-03-02  8:19   ` Baoquan He
@ 2021-03-02 23:21     ` Stephen Boyd
  0 siblings, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-02 23:21 UTC (permalink / raw)
  To: Baoquan He
  Cc: Andrew Morton, Jessica Yu, kexec, linux-kernel, Evan Green,
	Alexei Starovoitov, Jiri Olsa, Hsin-Yi Wang, Dave Young,
	Vivek Goyal

Quoting Baoquan He (2021-03-02 00:19:09)
> On 03/01/21 at 09:47am, Stephen Boyd wrote:
> > -                             note_sec->n_hdr.n_descsz,
> > -                             BUILD_ID_MAX);
> > -                     return;
> > -             }
> > -             n_remain -= sizeof(struct elf_note) +
> > -                     ALIGN(note_sec->n_hdr.n_namesz, 4) +
> > -                     ALIGN(note_sec->n_hdr.n_descsz, 4);
> > +     const char *build_id = vmlinux_build_id();
> 
> It's strange that I can only see the cover letter and this patch 7,
> couldn't find the patch where vmlinux_build_id() is introduced in lkml.
> 

Hmm not sure. Maybe spam stuff? I will Cc you on the first patch for v2.

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-02  2:43   ` Steven Rostedt
@ 2021-03-02 23:24     ` Stephen Boyd
  2021-03-04 17:19     ` Matthew Wilcox
  1 sibling, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-02 23:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Petr Mladek,
	Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc

Quoting Steven Rostedt (2021-03-01 18:43:19)
> On Mon,  1 Mar 2021 09:47:47 -0800
> Stephen Boyd <swboyd@chromium.org> wrote:
> 
> > The %pS printk format (among some others) is used to print kernel
> > addresses symbolically. When the kernel prints an address inside of a
> > module, the kernel prints the addresses' symbol name along with the
> > module's name that contains the address. Let's make kernel stacktraces
> > easier to identify on KALLSYMS builds by including the build ID of a
> > module when we print the address.
> 
> Please no!
> 
> This kills the output of tracing with offset, and can possibly break
> scripts. I don't want to look at traces like this!
> 
>           <idle>-0       [004] ..s1   353.842577: ipv4_conntrack_in+0x0/0x10 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_hook_slow+0x40/0xb0
>           <idle>-0       [004] ..s1   353.842577: nf_conntrack_in+0x0/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_hook_slow+0x40/0xb0
>           <idle>-0       [004] ..s1   353.842577: get_l4proto+0x0/0x190 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_in+0x92/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
>           <idle>-0       [004] ..s1   353.842577: nf_ct_get_tuple+0x0/0x240 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_in+0xec/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
>           <idle>-0       [004] ..s1   353.842577: hash_conntrack_raw+0x0/0x170 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_in+0x28c/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
>           <idle>-0       [004] ..s1   353.842578: __nf_conntrack_find_get.isra.0+0x0/0x2f0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_in+0x29d/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
>           <idle>-0       [004] ..s1   353.842578: nf_conntrack_tcp_packet+0x0/0x1760 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_in+0x3c8/0x5c0 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
>           <idle>-0       [004] ..s2   353.842578: nf_ct_seq_offset+0x0/0x40 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_tcp_packet+0x26d/0x1760 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
>           <idle>-0       [004] ..s1   353.842578: __nf_ct_refresh_acct+0x0/0x50 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_conntrack_tcp_packet+0x558/0x1760 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051)
> 
>  NACK!
> 

Heh, OK. Would adding a new printk format work then? From after the cut:

> Yet another alternative is to add a printk format like %pSB for
> backtrace prints. This would require a handful of architecture updates
> and I'm not sure it's worth the effort for that.
>

Or maybe take the approach of putting all the linked in module build IDs
in the "Modules linked in:" section? But as I said in the commit text
that can become quite verbose. Looking forward to your suggestions here.

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-01 17:47 ` [PATCH 5/7] printk: Make %pS and friends print module build ID Stephen Boyd
  2021-03-02  2:43   ` Steven Rostedt
@ 2021-03-03  2:01   ` Steven Rostedt
  2021-03-03  3:00     ` Stephen Boyd
  2021-03-03 10:25   ` Petr Mladek
  2021-03-04 17:00   ` Matthew Wilcox
  3 siblings, 1 reply; 30+ messages in thread
From: Steven Rostedt @ 2021-03-03  2:01 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Petr Mladek,
	Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc

On Mon,  1 Mar 2021 09:47:47 -0800
Stephen Boyd <swboyd@chromium.org> wrote:

>  WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
>  Modules linked in: lkdtm rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE
>  CPU: 4 PID: 3255 Comm: bash Not tainted 5.11 #3 aa23f7a1231c229de205662d5a9e0d4c580f19a1
>  Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
>  pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
>  pc : lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
>  lr : lkdtm_do_action+0x24/0x40 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)

Why not just change the "Modules linked in:" portion to show the build IDs
instead of every function call? Maybe make it a config option to do so?

-- Steve

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-03  2:01   ` Steven Rostedt
@ 2021-03-03  3:00     ` Stephen Boyd
  2021-03-03  8:19       ` Andy Shevchenko
  0 siblings, 1 reply; 30+ messages in thread
From: Stephen Boyd @ 2021-03-03  3:00 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Petr Mladek,
	Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc

Quoting Steven Rostedt (2021-03-02 18:01:36)
> On Mon,  1 Mar 2021 09:47:47 -0800
> Stephen Boyd <swboyd@chromium.org> wrote:
> 
> >  WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> >  Modules linked in: lkdtm rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE
> >  CPU: 4 PID: 3255 Comm: bash Not tainted 5.11 #3 aa23f7a1231c229de205662d5a9e0d4c580f19a1
> >  Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
> >  pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
> >  pc : lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> >  lr : lkdtm_do_action+0x24/0x40 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> 
> Why not just change the "Modules linked in:" portion to show the build IDs
> instead of every function call? Maybe make it a config option to do so?
> 

As I wrote in the commit text

│ An alternative to changing the printk format would be to update the                                                                                                                                             
│ "Modules linked in:" line to include the build ID of each module linked                                                                                                                                         
│ in. This can become quite long when many modules are loaded (i.e. on a                                                                                                                                          
│ distro) so I've opted for the printk format instead.

Implementing it is fairly simple compared to a new printk format. In
fact I did that initially but decided against it because it felt
unreadable and provided more information than was necessary if the
stacktrace wasn't in a module.

Example:

 Modules linked in: rfcomm 4de3e669b9fee915 algif_hash 915c61c32d476f01 algif_skcipher 53a4a330149bf071 af_alg b49d66496907f376 xt_cgroup 36fbbb65e7620df9 uinput a0ff6a06c1c53685 xt_MASQUERADE d130585f728315d2 snd_soc_sc7180 5c130cd310c81a12 venus_dec 2071e263fde9ca07 qcom_spmi_temp_alarm 41e28f2a9cc8b562 qcom_spmi_adc_tm5 7e0e48d0b6550c7a qcom_spmi_adc5 40b81a00bc2d0c1d qcom_vadc_common 09bb012dd1662dea snd_soc_rt5682_i2c c92b8935ad4a1729 venus_enc efaf3335b88287bf snd_soc_qcom_common 56d1e87c267ed02e videobuf2_dma_sg e9bc3c9e2758dfc9 snd_soc_rt5682 c4c9b31bf9a43f8b snd_soc_rl6231 908446e32436898c hci_uart 107b5fe6884df077 btqca 401a2fcc17b80a39 bluetooth 3369c881496a3cf8 venus_core a11eee3aa201ad8e ecdh_generic 883e01f98b778108 ecc 226b258cf40ad1ba v4l2_mem2mem 592f197cbad39e6b snd_soc_lpass_sc7180 42a99cb812d5e2e3 snd_soc_lpass_hdmi 95cce2160cfc58e2 snd_soc_lpass_cpu 475a4b395577affd snd_soc_lpass_platform 67517083263035ec snd_soc_max98357a 03ec1af0493d1c59
  fuse 82d170244a4d4ac6 iio_trig_sysfs b879a6228e059834 cros_ec_lid_angle a713c5f0a06a7a82 cros_ec_sensors 03f0c142ec521f67 cros_ec_sensors_core ada3f44647980056 cros_ec_sensors_ring f99590e87e998485 industrialio_triggered_buffer c697969d67f73d77 kfifo_buf e8a3aeb3069188f0 cros_ec_sensorhub 041ed1ffcefc991b ath10k_snoc 2f60802a74ff6ca7 lzo_rle 49a6228cec0c6885 ath10k_core 9407c36a2b8f8fae lzo_compress e9b4375d4bad668a ath 20c585ba6f3998f0 zram c5cdfc1d7d8a35d9 mac80211 9eaed1c76e000c02 cfg80211 872178d2dcebb722 cdc_ether 2baa214f72289402 usbnet 2bec73d0922ade28 uvcvideo eee0352cb1846ee4 r8152 47b76561b78e9b1b mii b306d150923fe614 videobuf2_vmalloc 55431ec52fa6af84 videobuf2_memops 4d43ad18fa7b1e4e videobuf2_v4l2 cdda06b9d95ab11d videobuf2_common 69cef0ca55a70a4a joydev 148399325b72d4a0

compared to 

 Modules linked in: rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE snd_soc_sc7180 venus_dec qcom_spmi_temp_alarm qcom_spmi_adc_tm5 qcom_spmi_adc5 qcom_vadc_common snd_soc_rt5682_i2c venus_enc snd_soc_qcom_common videobuf2_dma_sg snd_soc_rt5682 snd_soc_rl6231 hci_uart btqca bluetooth venus_core ecdh_generic ecc v4l2_mem2mem snd_soc_lpass_sc7180 snd_soc_lpass_hdmi snd_soc_lpass_cpu snd_soc_lpass_platform snd_soc_max98357a 
  fuse iio_trig_sysfs cros_ec_lid_angle cros_ec_sensors cros_ec_sensors_core cros_ec_sensors_ring industrialio_triggered_buffer kfifo_buf cros_ec_sensorhub ath10k_snoc lzo_rle ath10k_core lzo_compress ath zram mac80211 cfg80211 cdc_ether usbnet uvcvideo r8152 mii videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common joydev 

I suppose it could be surrounded by parenthesis or brackets and that
would be a little better? I'll try this approach again and see what
folks think.

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-03  3:00     ` Stephen Boyd
@ 2021-03-03  8:19       ` Andy Shevchenko
  2021-03-03  8:56         ` Stephen Boyd
  0 siblings, 1 reply; 30+ messages in thread
From: Andy Shevchenko @ 2021-03-03  8:19 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Steven Rostedt, Andrew Morton, linux-kernel, Jiri Olsa,
	Alexei Starovoitov, Jessica Yu, Evan Green, Hsin-Yi Wang,
	Petr Mladek, Sergey Senozhatsky, Rasmus Villemoes, linux-doc

On Tue, Mar 02, 2021 at 07:00:32PM -0800, Stephen Boyd wrote:
> Quoting Steven Rostedt (2021-03-02 18:01:36)
> > On Mon,  1 Mar 2021 09:47:47 -0800
> > Stephen Boyd <swboyd@chromium.org> wrote:
> > 
> > >  WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> > >  Modules linked in: lkdtm rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE
> > >  CPU: 4 PID: 3255 Comm: bash Not tainted 5.11 #3 aa23f7a1231c229de205662d5a9e0d4c580f19a1
> > >  Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
> > >  pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
> > >  pc : lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> > >  lr : lkdtm_do_action+0x24/0x40 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> > 
> > Why not just change the "Modules linked in:" portion to show the build IDs
> > instead of every function call? Maybe make it a config option to do so?
> > 
> 
> As I wrote in the commit text
> 
> │ An alternative to changing the printk format would be to update the                                                                                                                                             
> │ "Modules linked in:" line to include the build ID of each module linked                                                                                                                                         
> │ in. This can become quite long when many modules are loaded (i.e. on a                                                                                                                                          
> │ distro) so I've opted for the printk format instead.
> 
> Implementing it is fairly simple compared to a new printk format. In
> fact I did that initially but decided against it because it felt
> unreadable and provided more information than was necessary if the
> stacktrace wasn't in a module.
> 
> Example:
> 
>  Modules linked in: rfcomm 4de3e669b9fee915 algif_hash 915c61c32d476f01 algif_skcipher 53a4a330149bf071 af_alg b49d66496907f376 xt_cgroup 36fbbb65e7620df9 uinput a0ff6a06c1c53685 xt_MASQUERADE d130585f728315d2 snd_soc_sc7180 5c130cd310c81a12 venus_dec 2071e263fde9ca07 qcom_spmi_temp_alarm 41e28f2a9cc8b562 qcom_spmi_adc_tm5 7e0e48d0b6550c7a qcom_spmi_adc5 40b81a00bc2d0c1d qcom_vadc_common 09bb012dd1662dea snd_soc_rt5682_i2c c92b8935ad4a1729 venus_enc efaf3335b88287bf snd_soc_qcom_common 56d1e87c267ed02e videobuf2_dma_sg e9bc3c9e2758dfc9 snd_soc_rt5682 c4c9b31bf9a43f8b snd_soc_rl6231 908446e32436898c hci_uart 107b5fe6884df077 btqca 401a2fcc17b80a39 bluetooth 3369c881496a3cf8 venus_core a11eee3aa201ad8e ecdh_generic 883e01f98b778108 ecc 226b258cf40ad1ba v4l2_mem2mem 592f197cbad39e6b snd_soc_lpass_sc7180 42a99cb812d5e2e3 snd_soc_lpass_hdmi 95cce2160cfc58e2 snd_soc_lpass_cpu 475a4b395577affd snd_soc_lpass_platform 67517083263035ec snd_soc_max98357a 03ec1af0493d1c59
>   fuse 82d170244a4d4ac6 iio_trig_sysfs b879a6228e059834 cros_ec_lid_angle a713c5f0a06a7a82 cros_ec_sensors 03f0c142ec521f67 cros_ec_sensors_core ada3f44647980056 cros_ec_sensors_ring f99590e87e998485 industrialio_triggered_buffer c697969d67f73d77 kfifo_buf e8a3aeb3069188f0 cros_ec_sensorhub 041ed1ffcefc991b ath10k_snoc 2f60802a74ff6ca7 lzo_rle 49a6228cec0c6885 ath10k_core 9407c36a2b8f8fae lzo_compress e9b4375d4bad668a ath 20c585ba6f3998f0 zram c5cdfc1d7d8a35d9 mac80211 9eaed1c76e000c02 cfg80211 872178d2dcebb722 cdc_ether 2baa214f72289402 usbnet 2bec73d0922ade28 uvcvideo eee0352cb1846ee4 r8152 47b76561b78e9b1b mii b306d150923fe614 videobuf2_vmalloc 55431ec52fa6af84 videobuf2_memops 4d43ad18fa7b1e4e videobuf2_v4l2 cdda06b9d95ab11d videobuf2_common 69cef0ca55a70a4a joydev 148399325b72d4a0
> 
> compared to 
> 
>  Modules linked in: rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE snd_soc_sc7180 venus_dec qcom_spmi_temp_alarm qcom_spmi_adc_tm5 qcom_spmi_adc5 qcom_vadc_common snd_soc_rt5682_i2c venus_enc snd_soc_qcom_common videobuf2_dma_sg snd_soc_rt5682 snd_soc_rl6231 hci_uart btqca bluetooth venus_core ecdh_generic ecc v4l2_mem2mem snd_soc_lpass_sc7180 snd_soc_lpass_hdmi snd_soc_lpass_cpu snd_soc_lpass_platform snd_soc_max98357a 
>   fuse iio_trig_sysfs cros_ec_lid_angle cros_ec_sensors cros_ec_sensors_core cros_ec_sensors_ring industrialio_triggered_buffer kfifo_buf cros_ec_sensorhub ath10k_snoc lzo_rle ath10k_core lzo_compress ath zram mac80211 cfg80211 cdc_ether usbnet uvcvideo r8152 mii videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common joydev 
> I suppose it could be surrounded by parenthesis or brackets and that
> would be a little better? I'll try this approach again and see what
> folks think.

But you can do:
Modules linked: module1 [buildidX]
 module2 [buildidY]
 module3 [buildidZ]
...

i.o.w. one module per line.

* Yes, I understand that downside maybe split message, so it's just for
consideration.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-03  8:19       ` Andy Shevchenko
@ 2021-03-03  8:56         ` Stephen Boyd
  0 siblings, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-03  8:56 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Steven Rostedt, Andrew Morton, linux-kernel, Jiri Olsa,
	Alexei Starovoitov, Jessica Yu, Evan Green, Hsin-Yi Wang,
	Petr Mladek, Sergey Senozhatsky, Rasmus Villemoes, linux-doc

Quoting Andy Shevchenko (2021-03-03 00:19:05)
> On Tue, Mar 02, 2021 at 07:00:32PM -0800, Stephen Boyd wrote:
> > Quoting Steven Rostedt (2021-03-02 18:01:36)
> > > On Mon,  1 Mar 2021 09:47:47 -0800
> > > Stephen Boyd <swboyd@chromium.org> wrote:
> > > 
> > > >  WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> > > >  Modules linked in: lkdtm rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE
> > > >  CPU: 4 PID: 3255 Comm: bash Not tainted 5.11 #3 aa23f7a1231c229de205662d5a9e0d4c580f19a1
> > > >  Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
> > > >  pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
> > > >  pc : lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> > > >  lr : lkdtm_do_action+0x24/0x40 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> > > 
> > > Why not just change the "Modules linked in:" portion to show the build IDs
> > > instead of every function call? Maybe make it a config option to do so?
> > > 
> > 
> > As I wrote in the commit text
> > 
> > │ An alternative to changing the printk format would be to update the                                                                                                                                             
> > │ "Modules linked in:" line to include the build ID of each module linked                                                                                                                                         
> > │ in. This can become quite long when many modules are loaded (i.e. on a                                                                                                                                          
> > │ distro) so I've opted for the printk format instead.
> > 
> > Implementing it is fairly simple compared to a new printk format. In
> > fact I did that initially but decided against it because it felt
> > unreadable and provided more information than was necessary if the
> > stacktrace wasn't in a module.
> > 
> > Example:
> > 
> >  Modules linked in: rfcomm 4de3e669b9fee915 algif_hash 915c61c32d476f01 algif_skcipher 53a4a330149bf071 af_alg b49d66496907f376 xt_cgroup 36fbbb65e7620df9 uinput a0ff6a06c1c53685 xt_MASQUERADE d130585f728315d2 snd_soc_sc7180 5c130cd310c81a12 venus_dec 2071e263fde9ca07 qcom_spmi_temp_alarm 41e28f2a9cc8b562 qcom_spmi_adc_tm5 7e0e48d0b6550c7a qcom_spmi_adc5 40b81a00bc2d0c1d qcom_vadc_common 09bb012dd1662dea snd_soc_rt5682_i2c c92b8935ad4a1729 venus_enc efaf3335b88287bf snd_soc_qcom_common 56d1e87c267ed02e videobuf2_dma_sg e9bc3c9e2758dfc9 snd_soc_rt5682 c4c9b31bf9a43f8b snd_soc_rl6231 908446e32436898c hci_uart 107b5fe6884df077 btqca 401a2fcc17b80a39 bluetooth 3369c881496a3cf8 venus_core a11eee3aa201ad8e ecdh_generic 883e01f98b778108 ecc 226b258cf40ad1ba v4l2_mem2mem 592f197cbad39e6b snd_soc_lpass_sc7180 42a99cb812d5e2e3 snd_soc_lpass_hdmi 95cce2160cfc58e2 snd_soc_lpass_cpu 475a4b395577affd snd_soc_lpass_platform 67517083263035ec snd_soc_max98357a 03ec1af0493d1c59
> >   fuse 82d170244a4d4ac6 iio_trig_sysfs b879a6228e059834 cros_ec_lid_angle a713c5f0a06a7a82 cros_ec_sensors 03f0c142ec521f67 cros_ec_sensors_core ada3f44647980056 cros_ec_sensors_ring f99590e87e998485 industrialio_triggered_buffer c697969d67f73d77 kfifo_buf e8a3aeb3069188f0 cros_ec_sensorhub 041ed1ffcefc991b ath10k_snoc 2f60802a74ff6ca7 lzo_rle 49a6228cec0c6885 ath10k_core 9407c36a2b8f8fae lzo_compress e9b4375d4bad668a ath 20c585ba6f3998f0 zram c5cdfc1d7d8a35d9 mac80211 9eaed1c76e000c02 cfg80211 872178d2dcebb722 cdc_ether 2baa214f72289402 usbnet 2bec73d0922ade28 uvcvideo eee0352cb1846ee4 r8152 47b76561b78e9b1b mii b306d150923fe614 videobuf2_vmalloc 55431ec52fa6af84 videobuf2_memops 4d43ad18fa7b1e4e videobuf2_v4l2 cdda06b9d95ab11d videobuf2_common 69cef0ca55a70a4a joydev 148399325b72d4a0
> > 
> > compared to 
> > 
> >  Modules linked in: rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE snd_soc_sc7180 venus_dec qcom_spmi_temp_alarm qcom_spmi_adc_tm5 qcom_spmi_adc5 qcom_vadc_common snd_soc_rt5682_i2c venus_enc snd_soc_qcom_common videobuf2_dma_sg snd_soc_rt5682 snd_soc_rl6231 hci_uart btqca bluetooth venus_core ecdh_generic ecc v4l2_mem2mem snd_soc_lpass_sc7180 snd_soc_lpass_hdmi snd_soc_lpass_cpu snd_soc_lpass_platform snd_soc_max98357a 
> >   fuse iio_trig_sysfs cros_ec_lid_angle cros_ec_sensors cros_ec_sensors_core cros_ec_sensors_ring industrialio_triggered_buffer kfifo_buf cros_ec_sensorhub ath10k_snoc lzo_rle ath10k_core lzo_compress ath zram mac80211 cfg80211 cdc_ether usbnet uvcvideo r8152 mii videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common joydev 
> > I suppose it could be surrounded by parenthesis or brackets and that
> > would be a little better? I'll try this approach again and see what
> > folks think.
> 
> But you can do:
> Modules linked: module1 [buildidX]
>  module2 [buildidY]
>  module3 [buildidZ]
> ...
> 
> i.o.w. one module per line.
> 
> * Yes, I understand that downside maybe split message, so it's just for
> consideration.
> 

Yes that's another option. We could put every module on a different
line. That could be over 100 more lines of error message though. That's
probably a bad idea? Or we could print at most 5, 10 or 20 modules per
line. Given that the build id is 20 characters and then 2 brackets, 4 or
5 modules per line comes out to 120 to 150 or so characters per line.

Right now I see this:

 WARNING: CPU: 1 PID: 2124 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm]
 Modules linked in: vhost_vsock[b5de74de4890f7f9b6c6b9a75bf75cf2b7f828b9] vmw_vsock_virtio_transport_common[939626f71b43da4aab49d0490a3d86da3647f0ec] vsock[edf8bdd739f3bc46d8b9ee00fd5243e0d719aceb] vhost[d2f63a2bc2d33122a520ea624a5ed45709691edc] vhost_iotlb[4042a67c527796e0e2e19b9acd5b38c938e19bf8] lkdtm[6c2215028606bda50de823490723dc4bc5bf46f9] rfcomm[1853b40495371f7bbfdc07d27018693d0a65e32e] algif_hash[d0035cb2ccdd3e20faea6138168f5b3ee7f55027] algif_skcipher[290ded176708249e5f3fe6c20fb2645f46c38c7b] af_alg[9e1891ab46476a7f1252a23b14a8e8fd59026986] xt_cgroup[3b8d96cd8b13e760e88162b441be9ceb98500f75] uinput[3027573e498d509f71b6cb29aa7474bac9fbdba6] uvcvideo[a7676ff139a41297fcc33b80582af3ef68bbdbfa] videobuf2_vmalloc[591709b0f39979156cf3e8b90a61af75da36902a] xt_MASQUERADE[52bbd416ac57cf9c1081cb85ae0dd46fe7c256c7] venus_dec[50bf3f03f8a694acfe61f66dcdac73fde4b08384] videobuf2_dma_sg[a4d4ec21723a234278ddfe27081033ce366e82f4] hci_uart[152d461e1a2a8b5831507fc96fe18cd94391d5db]
  videobuf2_memops[3b3feac7d6440b52ac9fde356cc434f98b26f91d] btqca[0f921bc6d55432a7552dbe0677477d81bb44c6e4] bluetooth[8a98be2c3e842b9c1284775096c3d3504d5723e5] ath10k_snoc[525af7ae4d44d0f467980b67769cacaf6d9a5445] cros_ec_typec[9bc93ec5d8d14765642576b24901d5313c5db1b4] venus_core[3cea3f6cd84bdaee7bbce46f58d1eb0bf046763c] typec[e90cdc31cd39d173af46fa3b964215c8d0316534] ecdh_generic[26db794ec69dcbfbabf8dec23c7311ca529dbe6f] qcom_spmi_adc5[5fd6407c665b2af74b8b7d7a7f2ee49e6cec0bdf] ath10k_core[f54f6fd17fb433f83d1c60e32737beba72fbd151] qcom_spmi_temp_alarm[74c703822ed24645942437dda1772bc9cfdc431b] ecc[328c5244919fb66e50bd7319de74a73232d23e42] qcom_vadc_common[bc2afd1da3c27b19cfd96ea70bf23ffd710e2aad] snd_soc_rt5682_i2c[4296e093615aaf4ec303fb605bb4d870f4cf2eb4] snd_soc_rt5682[eb897b37a96eb95bae1581adf4cd0386b224cc4d] v4l2_mem2mem[b4b843c3844f4c492d0e9f89714fb7133e1d3e8c] snd_soc_sc7180[8216abc2f56916ca36e5073b776ed493f1af8e4e]
  snd_soc_qcom_common[54fbd61a2f457db385b40ae0a37c5a0719e2a8de] videobuf2_v4l2[d100d30e4b9ad480471dab30494d85d1ae2f4044] snd_soc_lpass_sc7180[aa5e3313ee630c10a1c890213c8777f6365f5ef5] ath[8f4ab51a49478f7955b3a2a95101de0de4bf0864] snd_soc_rl6231[a668e6b9790e698a5f64e3e0e45713adce578de7] videobuf2_common[bd4f81493fa674a6804ecf3244932f5a3e921d5d] snd_soc_lpass_hdmi[90a3aaff7f90ed2086b896bbdd9df09e48e3a6f6] snd_soc_lpass_cpu[c3f3eefc220abcf50857b0c202b0002ebf90a41a] snd_soc_lpass_platform[9a20405ac4e1778db7632aecf68c5cf777dc4254] mac80211[6f6acbb06c6c2c9538c37062bbfa4865708bf66b] snd_soc_max98357a[61ed67be8e6cf3c82da549b30f09c8ec1dbf7270] fuse[3b0043fe3eec1552df28ffc568bf787d75347269] iio_trig_sysfs[43a2fd24bb26966e7faa3f7532193407afb9b1ae] cros_ec_lid_angle[2e7c1c179ab8c9ee76f8abcd6e6139546743cf84] cros_ec_sensors[1bf8ac89e90b6549b9935729882722eda7b924fa] cros_ec_sensors_core[273e9b0a90fbb7066e55023a27f8b6d86371018f] cros_ec_sensors_ring[16ff4eb75deb8bcd81b1ddc5eea1eecf32c7fb14]
  industrialio_triggered_buffer[8fe79c5203ccb4f16ed9638554924a42c4a7ec9d] kfifo_buf[c1b3c4d10b52365c3887ba106b3b879c04f96f1d] cfg80211[c4084b0457e32a8408e230cdbc22432ace087c31] cros_ec_sensorhub[e1e51bc2c5324bb000c335ce427263cd17974144] lzo_rle[7c620147cbf701a30e9efa51c176394f2534a2a5] lzo_compress[4fcd02f3181b4d541032ff1f1759d482205e7c5c] zram[bec5be42fb85409cb0776b5d63c1a257b15af2ef] r8152[0a4bcdca4912d965082c8e0f27c2325c3b3e48d3] mii[0e40467221c7b7455a93ceafea794e4ce5c4d09e] joydev[0eabd30a73ec9378792dfb343ed38c2ad5a599bc]
 CPU: 1 PID: 2124 Comm: bash Not tainted 5.11 #6 5a995ea7d5d0bd2ed0ccfc90e8c36fcccc04b81b
 Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
 pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
 pc : lkdtm_WARNING+0x28/0x30 [lkdtm]
 lr : lkdtm_do_action+0x24/0x40 [lkdtm]
 sp : ffffffc013043ca0
 x29: ffffffc013043ca0 x28: ffffff987e295540 
 x27: 0000000000000000 x26: 0000000000000000 
 x25: 0000000000000000 x24: ffffffe179b7b2c0 
 x23: 0000000000000020 x22: ffffffe179b79366 
 x21: ffffffe179b7b2e0 x20: ffffffc013043de0 
 x19: 0000000000000008 x18: 0000000000000000 
 x17: ffffff9847b569fc x16: 0000000000000000 
 x15: 0000000000000000 x14: 0000000000000012 
 x13: ffffffe1d96d892c x12: ffffffe1da505068 
 x11: 0000000000000000 x10: 0000000000000000 
 x9 : 0000000000000001 x8 : ffffffe179b7d000 
 x7 : aaaaaaaaaaaaaaaa x6 : 0000000000000000 
 x5 : 0000000000000000 x4 : 0000000000000001 
 x3 : 0000000000000008 x2 : ffffff99beec5a70 
 x1 : ffffff99beeb5788 x0 : ffffffe179b7b2e0 
 Call trace:
  lkdtm_WARNING+0x28/0x30 [lkdtm]
  direct_entry+0x16c/0x1b4 [lkdtm]
  full_proxy_write+0x74/0xa4
  vfs_write+0xec/0x2e8
  ksys_write+0x84/0xf0
  __arm64_sys_write+0x24/0x30
  el0_svc_common+0xf4/0x1c0
  do_el0_svc_compat+0x28/0x3c
  el0_svc_compat+0x10/0x1c
  el0_sync_compat_handler+0xa8/0xcc
  el0_sync_compat+0x178/0x180
 ---[ end trace e1c971e4a46de18e ]---

And here's 10 per line with spacing

 WARNING: CPU: 0 PID: 2009 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm]
 Modules linked in: lkdtm [6c2215028606bda50de823490723dc4bc5bf46f9] rfcomm [1853b40495371f7bbfdc07d27018693d0a65e32e] algif_hash [d0035cb2ccdd3e20faea6138168f5b3ee7f55027] algif_skcipher [290ded176708249e5f3fe6c20fb2645f46c38c7b] af_alg [9e1891ab46476a7f1252a23b14a8e8fd59026986] xt_cgroup [3b8d96cd8b13e760e88162b441be9ceb98500f75] uinput [3027573e498d509f71b6cb29aa7474bac9fbdba6] uvcvideo [a7676ff139a41297fcc33b80582af3ef68bbdbfa] videobuf2_vmalloc [591709b0f39979156cf3e8b90a61af75da36902a] xt_MASQUERADE [52bbd416ac57cf9c1081cb85ae0dd46fe7c256c7]
  venus_dec [50bf3f03f8a694acfe61f66dcdac73fde4b08384] videobuf2_dma_sg [a4d4ec21723a234278ddfe27081033ce366e82f4] videobuf2_memops [3b3feac7d6440b52ac9fde356cc434f98b26f91d] hci_uart [152d461e1a2a8b5831507fc96fe18cd94391d5db] btqca [0f921bc6d55432a7552dbe0677477d81bb44c6e4] bluetooth [8a98be2c3e842b9c1284775096c3d3504d5723e5] venus_core [3cea3f6cd84bdaee7bbce46f58d1eb0bf046763c] ath10k_snoc [525af7ae4d44d0f467980b67769cacaf6d9a5445] ecdh_generic [26db794ec69dcbfbabf8dec23c7311ca529dbe6f] v4l2_mem2mem [b4b843c3844f4c492d0e9f89714fb7133e1d3e8c]
  videobuf2_v4l2 [d100d30e4b9ad480471dab30494d85d1ae2f4044] cros_ec_typec [9bc93ec5d8d14765642576b24901d5313c5db1b4] ath10k_core [f54f6fd17fb433f83d1c60e32737beba72fbd151] qcom_spmi_adc5 [5fd6407c665b2af74b8b7d7a7f2ee49e6cec0bdf] videobuf2_common [bd4f81493fa674a6804ecf3244932f5a3e921d5d] typec [e90cdc31cd39d173af46fa3b964215c8d0316534] snd_soc_sc7180 [8216abc2f56916ca36e5073b776ed493f1af8e4e] snd_soc_rt5682_i2c [4296e093615aaf4ec303fb605bb4d870f4cf2eb4] ecc [328c5244919fb66e50bd7319de74a73232d23e42] qcom_vadc_common [bc2afd1da3c27b19cfd96ea70bf23ffd710e2aad]
  snd_soc_rt5682 [eb897b37a96eb95bae1581adf4cd0386b224cc4d] qcom_spmi_temp_alarm [74c703822ed24645942437dda1772bc9cfdc431b] ath [8f4ab51a49478f7955b3a2a95101de0de4bf0864] snd_soc_qcom_common [54fbd61a2f457db385b40ae0a37c5a0719e2a8de] snd_soc_rl6231 [a668e6b9790e698a5f64e3e0e45713adce578de7] snd_soc_lpass_sc7180 [aa5e3313ee630c10a1c890213c8777f6365f5ef5] snd_soc_lpass_hdmi [90a3aaff7f90ed2086b896bbdd9df09e48e3a6f6] snd_soc_lpass_cpu [c3f3eefc220abcf50857b0c202b0002ebf90a41a] mac80211 [6f6acbb06c6c2c9538c37062bbfa4865708bf66b] snd_soc_lpass_platform [9a20405ac4e1778db7632aecf68c5cf777dc4254]
  snd_soc_max98357a [61ed67be8e6cf3c82da549b30f09c8ec1dbf7270] fuse [3b0043fe3eec1552df28ffc568bf787d75347269] cfg80211 [c4084b0457e32a8408e230cdbc22432ace087c31] iio_trig_sysfs [43a2fd24bb26966e7faa3f7532193407afb9b1ae] cros_ec_lid_angle [2e7c1c179ab8c9ee76f8abcd6e6139546743cf84] cros_ec_sensors [1bf8ac89e90b6549b9935729882722eda7b924fa] cros_ec_sensors_core [273e9b0a90fbb7066e55023a27f8b6d86371018f] industrialio_triggered_buffer [8fe79c5203ccb4f16ed9638554924a42c4a7ec9d] cros_ec_sensors_ring [16ff4eb75deb8bcd81b1ddc5eea1eecf32c7fb14] kfifo_buf [c1b3c4d10b52365c3887ba106b3b879c04f96f1d]
  cros_ec_sensorhub [e1e51bc2c5324bb000c335ce427263cd17974144] lzo_rle [7c620147cbf701a30e9efa51c176394f2534a2a5] lzo_compress [4fcd02f3181b4d541032ff1f1759d482205e7c5c] zram [bec5be42fb85409cb0776b5d63c1a257b15af2ef] r8152 [0a4bcdca4912d965082c8e0f27c2325c3b3e48d3] mii [0e40467221c7b7455a93ceafea794e4ce5c4d09e] [last unloaded: joydev]
 CPU: 0 PID: 2009 Comm: bash Not tainted 5.11 #8 715ebac18cc41a59b9a429ad9273467cba4680ae
 Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
 pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
 pc : lkdtm_WARNING+0x28/0x30 [lkdtm]
 lr : lkdtm_do_action+0x24/0x40 [lkdtm]
 sp : ffffffc012e9bca0
 x29: ffffffc012e9bca0 x28: ffffffbc34fbc440 
 x27: 0000000000000000 x26: 0000000000000000 
 x25: 0000000000000000 x24: ffffffdc41c382c0 
 x23: 0000000000000020 x22: ffffffdc41c36366 
 x21: ffffffdc41c382e0 x20: ffffffc012e9bde0 
 x19: 0000000000000008 x18: ffffffbc1bf65618 
 x17: ffffffbc003cc7fc x16: 0000000000000000 
 x15: 001d23f6e6650cfa x14: 0000000000000012 
 x13: ffffffdcad4d892c x12: ffffffdcadef2c94 
 x11: 0000000000000000 x10: 0000000000000000 
 x9 : 0000000000000001 x8 : ffffffdc41c3a000 
 x7 : bbbbbbbbbbbbbbbb x6 : 0000000000000020 
 x5 : ffffffc010683d40 x4 : 0000000000000001 
 x3 : 0000000000000008 x2 : ffffffbd7eea5a70 
 x1 : ffffffbd7ee95788 x0 : ffffffdc41c382e0 
 Call trace:
  lkdtm_WARNING+0x28/0x30 [lkdtm]
  direct_entry+0x16c/0x1b4 [lkdtm]
  full_proxy_write+0x74/0xa4
  vfs_write+0xec/0x2e8
  ksys_write+0x84/0xf0
  __arm64_sys_write+0x24/0x30
  el0_svc_common+0xf4/0x1c0
  do_el0_svc_compat+0x28/0x3c
  el0_svc_compat+0x10/0x1c
  el0_sync_compat_handler+0xa8/0xcc
  el0_sync_compat+0x178/0x180
 ---[ end trace 5f51a26c92780793 ]---

5 looks pretty nice

 WARNING: CPU: 2 PID: 3836 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm]
 Modules linked in: lkdtm [6c2215028606bda50de823490723dc4bc5bf46f9] vhost_vsock [b5de74de4890f7f9b6c6b9a75bf75cf2b7f828b9] vmw_vsock_virtio_transport_common [939626f71b43da4aab49d0490a3d86da3647f0ec] vsock [edf8bdd739f3bc46d8b9ee00fd5243e0d719aceb] vhost [d2f63a2bc2d33122a520ea624a5ed45709691edc]
  vhost_iotlb [4042a67c527796e0e2e19b9acd5b38c938e19bf8] rfcomm [1853b40495371f7bbfdc07d27018693d0a65e32e] algif_hash [d0035cb2ccdd3e20faea6138168f5b3ee7f55027] algif_skcipher [290ded176708249e5f3fe6c20fb2645f46c38c7b] af_alg [9e1891ab46476a7f1252a23b14a8e8fd59026986]
  uinput [3027573e498d509f71b6cb29aa7474bac9fbdba6] xt_cgroup [3b8d96cd8b13e760e88162b441be9ceb98500f75] uvcvideo [a7676ff139a41297fcc33b80582af3ef68bbdbfa] videobuf2_vmalloc [591709b0f39979156cf3e8b90a61af75da36902a] xt_MASQUERADE [52bbd416ac57cf9c1081cb85ae0dd46fe7c256c7]
  venus_dec [50bf3f03f8a694acfe61f66dcdac73fde4b08384] videobuf2_dma_sg [a4d4ec21723a234278ddfe27081033ce366e82f4] hci_uart [152d461e1a2a8b5831507fc96fe18cd94391d5db] btqca [0f921bc6d55432a7552dbe0677477d81bb44c6e4] videobuf2_memops [3b3feac7d6440b52ac9fde356cc434f98b26f91d]
  snd_soc_rt5682_i2c [4296e093615aaf4ec303fb605bb4d870f4cf2eb4] qcom_spmi_temp_alarm [74c703822ed24645942437dda1772bc9cfdc431b] snd_soc_rt5682 [eb897b37a96eb95bae1581adf4cd0386b224cc4d] snd_soc_sc7180 [8216abc2f56916ca36e5073b776ed493f1af8e4e] qcom_spmi_adc5 [5fd6407c665b2af74b8b7d7a7f2ee49e6cec0bdf]
  qcom_vadc_common [bc2afd1da3c27b19cfd96ea70bf23ffd710e2aad] bluetooth [8a98be2c3e842b9c1284775096c3d3504d5723e5] snd_soc_qcom_common [54fbd61a2f457db385b40ae0a37c5a0719e2a8de] snd_soc_rl6231 [a668e6b9790e698a5f64e3e0e45713adce578de7] ecdh_generic [26db794ec69dcbfbabf8dec23c7311ca529dbe6f]
  ecc [328c5244919fb66e50bd7319de74a73232d23e42] ath10k_snoc [525af7ae4d44d0f467980b67769cacaf6d9a5445] cros_ec_typec [9bc93ec5d8d14765642576b24901d5313c5db1b4] ath10k_core [f54f6fd17fb433f83d1c60e32737beba72fbd151] typec [e90cdc31cd39d173af46fa3b964215c8d0316534]
  venus_core [3cea3f6cd84bdaee7bbce46f58d1eb0bf046763c] v4l2_mem2mem [b4b843c3844f4c492d0e9f89714fb7133e1d3e8c] videobuf2_v4l2 [d100d30e4b9ad480471dab30494d85d1ae2f4044] videobuf2_common [bd4f81493fa674a6804ecf3244932f5a3e921d5d] snd_soc_lpass_sc7180 [aa5e3313ee630c10a1c890213c8777f6365f5ef5]
  ath [8f4ab51a49478f7955b3a2a95101de0de4bf0864] snd_soc_lpass_hdmi [90a3aaff7f90ed2086b896bbdd9df09e48e3a6f6] snd_soc_lpass_cpu [c3f3eefc220abcf50857b0c202b0002ebf90a41a] snd_soc_lpass_platform [9a20405ac4e1778db7632aecf68c5cf777dc4254] mac80211 [6f6acbb06c6c2c9538c37062bbfa4865708bf66b]
  snd_soc_max98357a [61ed67be8e6cf3c82da549b30f09c8ec1dbf7270] fuse [3b0043fe3eec1552df28ffc568bf787d75347269] iio_trig_sysfs [43a2fd24bb26966e7faa3f7532193407afb9b1ae] cfg80211 [c4084b0457e32a8408e230cdbc22432ace087c31] cros_ec_sensors_ring [16ff4eb75deb8bcd81b1ddc5eea1eecf32c7fb14]
  cros_ec_lid_angle [2e7c1c179ab8c9ee76f8abcd6e6139546743cf84] cros_ec_sensors [1bf8ac89e90b6549b9935729882722eda7b924fa] cros_ec_sensors_core [273e9b0a90fbb7066e55023a27f8b6d86371018f] industrialio_triggered_buffer [8fe79c5203ccb4f16ed9638554924a42c4a7ec9d] kfifo_buf [c1b3c4d10b52365c3887ba106b3b879c04f96f1d]
  cros_ec_sensorhub [e1e51bc2c5324bb000c335ce427263cd17974144] lzo_rle [7c620147cbf701a30e9efa51c176394f2534a2a5] lzo_compress [4fcd02f3181b4d541032ff1f1759d482205e7c5c] zram [bec5be42fb85409cb0776b5d63c1a257b15af2ef] cdc_ether [ced80b6f61771514901ed65242f51b60f2fe762b]
  usbnet [6b967ceea5e9faff6f054218e2a7fc4296313688] r8152 [0a4bcdca4912d965082c8e0f27c2325c3b3e48d3] mii [0e40467221c7b7455a93ceafea794e4ce5c4d09e] [last unloaded: joydev]
 CPU: 2 PID: 3836 Comm: bash Not tainted 5.11 #9 775145666670b360a22616ad8188bd8edf5dfd36
 Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
 pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
 pc : lkdtm_WARNING+0x28/0x30 [lkdtm]
 lr : lkdtm_do_action+0x24/0x40 [lkdtm]
 sp : ffffffc01892bca0
 x29: ffffffc01892bca0 x28: ffffff9329533340 
 x27: 0000000000000000 x26: 0000000000000000 
 x25: 0000000000000000 x24: ffffffe95a2cc2c0 
 x23: 0000000000000020 x22: ffffffe95a2ca366 
 x21: ffffffe95a2cc2e0 x20: ffffffc01892bde0 
 x19: 0000000000000008 x18: 0000000000000000 
 x17: 0000000000000000 x16: 0000000000000037 
 x15: ffffffe98d6ab134 x14: 0000000000000003 
 x13: 0000000000000004 x12: 0000000000000000 
 x11: 0000000000000000 x10: 0000000000000000 
 x9 : 0000000000000001 x8 : ffffffe95a2ce000 
 x7 : 0000000000000000 x6 : ffffffe98e3f6b54 
 x5 : 0000000000000000 x4 : 0000000000000000 
 x3 : ffffffc01892b938 x2 : ffffff947eee5a70 
 x1 : ffffff947eed5788 x0 : ffffffe95a2cc2e0 
 Call trace:
  lkdtm_WARNING+0x28/0x30 [lkdtm]
  direct_entry+0x16c/0x1b4 [lkdtm]
  full_proxy_write+0x74/0xa4
  vfs_write+0xec/0x2e8
  ksys_write+0x84/0xf0
  __arm64_sys_write+0x24/0x30
  el0_svc_common+0xf4/0x1c0
  do_el0_svc_compat+0x28/0x3c
  el0_svc_compat+0x10/0x1c
  el0_sync_compat_handler+0xa8/0xcc
  el0_sync_compat+0x178/0x180
 ---[ end trace afd0105bfc489130 ]---

 And the one per line approach

 WARNING: CPU: 1 PID: 3402 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm]
 Modules linked in: lkdtm [6c2215028606bda50de823490723dc4bc5bf46f9]
  rfcomm [1853b40495371f7bbfdc07d27018693d0a65e32e]
  algif_hash [d0035cb2ccdd3e20faea6138168f5b3ee7f55027]
  algif_skcipher [290ded176708249e5f3fe6c20fb2645f46c38c7b]
  af_alg [9e1891ab46476a7f1252a23b14a8e8fd59026986]
  xt_cgroup [3b8d96cd8b13e760e88162b441be9ceb98500f75]
  uinput [3027573e498d509f71b6cb29aa7474bac9fbdba6]
  venus_dec [50bf3f03f8a694acfe61f66dcdac73fde4b08384]
  videobuf2_dma_sg [a4d4ec21723a234278ddfe27081033ce366e82f4]
  xt_MASQUERADE [52bbd416ac57cf9c1081cb85ae0dd46fe7c256c7]
  qcom_spmi_adc5 [5fd6407c665b2af74b8b7d7a7f2ee49e6cec0bdf]
  hci_uart [152d461e1a2a8b5831507fc96fe18cd94391d5db]
  btqca [0f921bc6d55432a7552dbe0677477d81bb44c6e4]
  qcom_spmi_temp_alarm [74c703822ed24645942437dda1772bc9cfdc431b]
  qcom_vadc_common [bc2afd1da3c27b19cfd96ea70bf23ffd710e2aad]
  bluetooth [8a98be2c3e842b9c1284775096c3d3504d5723e5]
  snd_soc_sc7180 [8216abc2f56916ca36e5073b776ed493f1af8e4e]
  snd_soc_qcom_common [54fbd61a2f457db385b40ae0a37c5a0719e2a8de]
  ecdh_generic [26db794ec69dcbfbabf8dec23c7311ca529dbe6f]
  ath10k_snoc [525af7ae4d44d0f467980b67769cacaf6d9a5445]
  cros_ec_typec [9bc93ec5d8d14765642576b24901d5313c5db1b4]
  typec [e90cdc31cd39d173af46fa3b964215c8d0316534]
  snd_soc_rt5682_i2c [4296e093615aaf4ec303fb605bb4d870f4cf2eb4]
  ecc [328c5244919fb66e50bd7319de74a73232d23e42]
  ath10k_core [f54f6fd17fb433f83d1c60e32737beba72fbd151]
  snd_soc_rt5682 [eb897b37a96eb95bae1581adf4cd0386b224cc4d]
  venus_core [3cea3f6cd84bdaee7bbce46f58d1eb0bf046763c]
  snd_soc_rl6231 [a668e6b9790e698a5f64e3e0e45713adce578de7]
  ath [8f4ab51a49478f7955b3a2a95101de0de4bf0864]
  v4l2_mem2mem [b4b843c3844f4c492d0e9f89714fb7133e1d3e8c]
  snd_soc_lpass_sc7180 [aa5e3313ee630c10a1c890213c8777f6365f5ef5]
  mac80211 [6f6acbb06c6c2c9538c37062bbfa4865708bf66b]
  snd_soc_lpass_hdmi [90a3aaff7f90ed2086b896bbdd9df09e48e3a6f6]
  snd_soc_lpass_cpu [c3f3eefc220abcf50857b0c202b0002ebf90a41a]
  snd_soc_lpass_platform [9a20405ac4e1778db7632aecf68c5cf777dc4254]
  snd_soc_max98357a [61ed67be8e6cf3c82da549b30f09c8ec1dbf7270]
  fuse [3b0043fe3eec1552df28ffc568bf787d75347269]
  iio_trig_sysfs [43a2fd24bb26966e7faa3f7532193407afb9b1ae]
  cfg80211 [c4084b0457e32a8408e230cdbc22432ace087c31]
  cros_ec_lid_angle [2e7c1c179ab8c9ee76f8abcd6e6139546743cf84]
  cros_ec_sensors_ring [16ff4eb75deb8bcd81b1ddc5eea1eecf32c7fb14]
  cros_ec_sensors [1bf8ac89e90b6549b9935729882722eda7b924fa]
  cros_ec_sensors_core [273e9b0a90fbb7066e55023a27f8b6d86371018f]
  industrialio_triggered_buffer [8fe79c5203ccb4f16ed9638554924a42c4a7ec9d]
  kfifo_buf [c1b3c4d10b52365c3887ba106b3b879c04f96f1d]
  cros_ec_sensorhub [e1e51bc2c5324bb000c335ce427263cd17974144]
  lzo_rle [7c620147cbf701a30e9efa51c176394f2534a2a5]
  lzo_compress [4fcd02f3181b4d541032ff1f1759d482205e7c5c]
  zram [bec5be42fb85409cb0776b5d63c1a257b15af2ef]
  cdc_ether [ced80b6f61771514901ed65242f51b60f2fe762b]
  usbnet [6b967ceea5e9faff6f054218e2a7fc4296313688]
  r8152 [0a4bcdca4912d965082c8e0f27c2325c3b3e48d3]
  uvcvideo [a7676ff139a41297fcc33b80582af3ef68bbdbfa]
  mii [0e40467221c7b7455a93ceafea794e4ce5c4d09e]
  videobuf2_vmalloc [591709b0f39979156cf3e8b90a61af75da36902a]
  videobuf2_memops [3b3feac7d6440b52ac9fde356cc434f98b26f91d]
  videobuf2_v4l2 [d100d30e4b9ad480471dab30494d85d1ae2f4044]
  videobuf2_common [bd4f81493fa674a6804ecf3244932f5a3e921d5d]
  joydev [0eabd30a73ec9378792dfb343ed38c2ad5a599bc]
 CPU: 1 PID: 3402 Comm: bash Not tainted 5.11 #10 03359bc46737cb22e11f0e92cefc7144afedf9c3
 Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
 pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
 pc : lkdtm_WARNING+0x28/0x30 [lkdtm]
 lr : lkdtm_do_action+0x24/0x40 [lkdtm]
 sp : ffffffc0125cbca0
 x29: ffffffc0125cbca0 x28: ffffffb26a3a8040 
 x27: 0000000000000000 x26: 0000000000000000 
 x25: 0000000000000000 x24: ffffffdd7129b2c0 
 x23: 0000000000000020 x22: ffffffdd71299366 
 x21: ffffffdd7129b2e0 x20: ffffffc0125cbde0 
 x19: 0000000000000008 x18: 0000000000000000 
 x17: 0000000000000000 x16: 0000000000000037 
 x15: ffffffdd908ab0b4 x14: 0000000000000003 
 x13: 0000000000000004 x12: 0000000000000000 
 x11: 0000000000000000 x10: 0000000000000000 
 x9 : 0000000000000001 x8 : ffffffdd7129d000 
 x7 : 0000000000000000 x6 : ffffffdd915f6b54 
 x5 : 0000000000000000 x4 : 0000000000000000 
 x3 : ffffffc0125cb938 x2 : ffffffb3beec5a70 
 x1 : ffffffb3beeb5788 x0 : ffffffdd7129b2e0 
 Call trace:
  lkdtm_WARNING+0x28/0x30 [lkdtm]
  direct_entry+0x16c/0x1b4 [lkdtm]
  full_proxy_write+0x74/0xa4
  vfs_write+0xec/0x2e8
  ksys_write+0x84/0xf0
  __arm64_sys_write+0x24/0x30
  el0_svc_common+0xf4/0x1c0
  do_el0_svc_compat+0x28/0x3c
  el0_svc_compat+0x10/0x1c
  el0_sync_compat_handler+0xa8/0xcc
  el0_sync_compat+0x178/0x180
 ---[ end trace de6a15f0c875cfee ]---

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-01 17:47 ` [PATCH 5/7] printk: Make %pS and friends print module build ID Stephen Boyd
  2021-03-02  2:43   ` Steven Rostedt
  2021-03-03  2:01   ` Steven Rostedt
@ 2021-03-03 10:25   ` Petr Mladek
  2021-03-03 15:00     ` Steven Rostedt
  2021-03-04  1:17     ` Stephen Boyd
  2021-03-04 17:00   ` Matthew Wilcox
  3 siblings, 2 replies; 30+ messages in thread
From: Petr Mladek @ 2021-03-03 10:25 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Steven Rostedt,
	Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc

On Mon 2021-03-01 09:47:47, Stephen Boyd wrote:
> The %pS printk format (among some others) is used to print kernel
> addresses symbolically. When the kernel prints an address inside of a
> module, the kernel prints the addresses' symbol name along with the
> module's name that contains the address. Let's make kernel stacktraces
> easier to identify on KALLSYMS builds by including the build ID of a
> module when we print the address.
> 
> This is especially helpful for crash debugging with pstore or crashdump
> kernels. If we have the build ID for the module in the stacktrace we can
> request the debug symbols for the module from a remote debuginfod server

I have read the thread so for. IMHO, all mentioned variants complicate
the logs a lot. Either the backtrace lines are hard to parse.
Or the OOps/panic blocks gets too long when the ID is mentioned
for every loaded module. IMHO, neither proposed solution
is acceptable to be always used.

First, I think that only I some developers would actually use
this information. Many of them either know what module was
used or they do not have an easy way to get the debugging
information by the ID. So, it should be configurable
at minimum.

Second, I am not sure that it should be part of each OOps/panic blob.
One solution would be to print the ID by the module loader when
the module gets loaded. It would be mentioned earlier in the log
then.

Or we could make it available, for example, via /proc. It is a kind
of information that might be gathered later on a rebooted system.
SUSE has "supportconfig" command that allows to gather a lot
of similar information about the system. We use it when
analyzing crashdumps and kernel bugs in general.

Anyway, I consider this a very detailed information that is not
suitable for everyone. It should be availabe on request, like
for example, backtraces from all CPUs, list of tasks, memory info.

Alternative solution would be to minimize the information, for
example, by printing only the modules that appear in the backtrace.
But this might be complicated to implement.

Best Regards,
Petr

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-03 10:25   ` Petr Mladek
@ 2021-03-03 15:00     ` Steven Rostedt
  2021-03-03 16:17       ` Andy Shevchenko
  2021-03-04  1:17     ` Stephen Boyd
  1 sibling, 1 reply; 30+ messages in thread
From: Steven Rostedt @ 2021-03-03 15:00 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Stephen Boyd, Andrew Morton, linux-kernel, Jiri Olsa,
	Alexei Starovoitov, Jessica Yu, Evan Green, Hsin-Yi Wang,
	Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc

On Wed, 3 Mar 2021 11:25:58 +0100
Petr Mladek <pmladek@suse.com> wrote:

> Alternative solution would be to minimize the information, for
> example, by printing only the modules that appear in the backtrace.
> But this might be complicated to implement.

It could be a list after the backtrace perhaps, and not part of the
"modules linked in"?

But then you need a generic way of capturing those modules in the backtrace
that works for every architecture.

Honestly, I don't even know what a buildid is, and it is totally useless
information for myself. What exactly is it used for?

-- Steve

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-03 15:00     ` Steven Rostedt
@ 2021-03-03 16:17       ` Andy Shevchenko
  2021-03-04  0:38         ` Stephen Boyd
  0 siblings, 1 reply; 30+ messages in thread
From: Andy Shevchenko @ 2021-03-03 16:17 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Petr Mladek, Stephen Boyd, Andrew Morton, linux-kernel,
	Jiri Olsa, Alexei Starovoitov, Jessica Yu, Evan Green,
	Hsin-Yi Wang, Sergey Senozhatsky, Rasmus Villemoes, linux-doc

On Wed, Mar 03, 2021 at 10:00:12AM -0500, Steven Rostedt wrote:
> On Wed, 3 Mar 2021 11:25:58 +0100
> Petr Mladek <pmladek@suse.com> wrote:
> 
> > Alternative solution would be to minimize the information, for
> > example, by printing only the modules that appear in the backtrace.
> > But this might be complicated to implement.
> 
> It could be a list after the backtrace perhaps, and not part of the
> "modules linked in"?
> 
> But then you need a generic way of capturing those modules in the backtrace
> that works for every architecture.

> Honestly, I don't even know what a buildid is, and it is totally useless
> information for myself. What exactly is it used for?

Dunno Stephen's motivation, but build ID is very useful when you do tracing,
then based on ID the decoders can know what exactly was the layout of the
binary and list of (exported) functions, etc.

At least that was my (shallow) experience with perf last time I have tried it.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-03 16:17       ` Andy Shevchenko
@ 2021-03-04  0:38         ` Stephen Boyd
  2021-03-04  1:19           ` Steven Rostedt
  0 siblings, 1 reply; 30+ messages in thread
From: Stephen Boyd @ 2021-03-04  0:38 UTC (permalink / raw)
  To: Andy Shevchenko, Steven Rostedt
  Cc: Petr Mladek, Andrew Morton, linux-kernel, Jiri Olsa,
	Alexei Starovoitov, Jessica Yu, Evan Green, Hsin-Yi Wang,
	Sergey Senozhatsky, Rasmus Villemoes, linux-doc

Quoting Andy Shevchenko (2021-03-03 08:17:01)
> On Wed, Mar 03, 2021 at 10:00:12AM -0500, Steven Rostedt wrote:
> > On Wed, 3 Mar 2021 11:25:58 +0100
> > Petr Mladek <pmladek@suse.com> wrote:
> > 
> > > Alternative solution would be to minimize the information, for
> > > example, by printing only the modules that appear in the backtrace.
> > > But this might be complicated to implement.
> > 
> > It could be a list after the backtrace perhaps, and not part of the
> > "modules linked in"?
> > 
> > But then you need a generic way of capturing those modules in the backtrace
> > that works for every architecture.

Right, and doing that is sort of complicated for something that really
shouldn't need to be complicated. We're printing out information about a
crash/hang/bug and that should be fast and not too computationally
intensive so that the stacktrace can be printed before everything starts
falling apart. I'd rather not save things away while processing the
stacktrace and then print more info after the stacktrace. Seems fragile.

> 
> > Honestly, I don't even know what a buildid is, and it is totally useless
> > information for myself. What exactly is it used for?
> 
> Dunno Stephen's motivation, but build ID is very useful when you do tracing,
> then based on ID the decoders can know what exactly was the layout of the
> binary and list of (exported) functions, etc.
> 
> At least that was my (shallow) experience with perf last time I have tried it.
> 

I'm starting to feel like nobody read the commit text, or I messed up
somehow and the commit text was confusing? :(

│ This is especially helpful for crash debugging with pstore or crashdump                                                                                                                                         
│ kernels. If we have the build ID for the module in the stacktrace we can                                                                                                                                        
│ request the debug symbols for the module from a remote debuginfod server                                                                                                                                        
│ or parse stacktraces at a later time with decode_stacktrace.sh by                                                                                                                                               
│ downloading the correct symbols based on the build ID. This cuts down on                                                                                                                                        
│ the amount of time and effort needed to find the correct kernel modules                                                                                                                                         
│ for a stacktrace by encoding that information into it.  

In some distro (read: non-kernel dev) workflows the vmlinux isn't
shipped on the device and crash handling is done offline or much later.
Using the build ID[1] is a common way to identify the binary that's
running on the device. In conjunction with a debuginfod[2] server you
can download the symbols for a crash automatically if you have the build
ID information.

I can add a patch that updates decode_stacktrace.sh to show how it can
download the correct vmlinux/modules if it isn't provided on the
commandline.

If the debug symbols are on some public server then in theory we could
have some robot sitting on the mailing list that looks for stacktraces
and automatically replies with information about the line number/file
and even provides the code snippet for the code that's crashing from
that binary, because it's all stored in the full debuginfo builds.

[1] https://fedoraproject.org/wiki/RolandMcGrath/BuildID
[2] https://sourceware.org/elfutils/Debuginfod.html

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

* Re: [PATCH 1/7] buildid: Add method to get running kernel's build ID
       [not found]     ` <161472770533.1478170.3061709841985494537@swboyd.mtv.corp.google.com>
@ 2021-03-04  0:41       ` Stephen Boyd
  0 siblings, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-04  0:41 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang

Quoting Stephen Boyd (2021-03-02 15:28:25)
> (HTML mail?)
> 
> Quoting Andy Shevchenko (2021-03-01 15:33:06)
> > 
> > 
> > On Monday, March 1, 2021, Stephen Boyd <swboyd@chromium.org> wrote:
> >     @@ -147,3 +158,31 @@ int build_id_parse(struct vm_area_struct *vma,
> >     unsigned char *build_id,
> >             put_page(page);
> >             return ret;
> >      }
> >     +
> >     +static void build_id2hex(char *dst, const unsigned char *src, __u32 size)
> >     +{
> >     +       bin2hex(dst, src, size);
> >     +       dst[2 * size] = '\0';
> >     +}
> >     +
> > 
> > 
> > 
> > If you are so only printing this via printf(), just use %20phN. No need for a
> > separate function.
> > 
> 
> Makes sense. The downside is that we may have to calculate the build ID
> many times then by searching the notes section? Unless we can test the
> whole array very quickly for "not initialized" or stash another bool
> like "build_id_initialized". The kdump code would also need to take a
> printk format then. This seems better as is but let me know if you
> disagree strongly and I can change more code.

I am using memchr_inv() now to check for an all zero array. That seems
fairly cheap so I think it should be OK. We trade-off a small amount of
time for a few more bytes savings.

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-03 10:25   ` Petr Mladek
  2021-03-03 15:00     ` Steven Rostedt
@ 2021-03-04  1:17     ` Stephen Boyd
  1 sibling, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-04  1:17 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Steven Rostedt,
	Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc

Quoting Petr Mladek (2021-03-03 02:25:58)
> On Mon 2021-03-01 09:47:47, Stephen Boyd wrote:
> > The %pS printk format (among some others) is used to print kernel
> > addresses symbolically. When the kernel prints an address inside of a
> > module, the kernel prints the addresses' symbol name along with the
> > module's name that contains the address. Let's make kernel stacktraces
> > easier to identify on KALLSYMS builds by including the build ID of a
> > module when we print the address.
> > 
> > This is especially helpful for crash debugging with pstore or crashdump
> > kernels. If we have the build ID for the module in the stacktrace we can
> > request the debug symbols for the module from a remote debuginfod server
> 
> I have read the thread so for. IMHO, all mentioned variants complicate
> the logs a lot. Either the backtrace lines are hard to parse.
> Or the OOps/panic blocks gets too long when the ID is mentioned
> for every loaded module. IMHO, neither proposed solution
> is acceptable to be always used.

The modules line is already pretty hard to read once it gets beyond 8 or
10 modules. Probably it should be broken up into multiple lines just so
it can be read more easily. I find myself having to hunt in there
already even without the build ID encoded there. I've also seen folks
cut out that line in commit text and when posting to the mailing list
because it's just long and noisy already.

> 
> First, I think that only I some developers would actually use
> this information. Many of them either know what module was
> used or they do not have an easy way to get the debugging
> information by the ID. So, it should be configurable
> at minimum.

It should be configurable because it isn't used or is hard to use?
Wouldn't a config variable limit the uptake of this information and
further reinforce the fact that nobody will use it? If we always exposed
it then maybe we would get new users. I imagine that we could have a
robot search crash reports and find the "crashiest" places in the kernel
all the way down to the line level and poke kernel developers to look
and see why it crashes so much there. If the information is behind a
config then the benefit of that analysis will be limited.

> 
> Second, I am not sure that it should be part of each OOps/panic blob.
> One solution would be to print the ID by the module loader when
> the module gets loaded. It would be mentioned earlier in the log
> then.

Please no. The kernel log could wrap around before an oops/panic happens
and then the ID would be lost.

> 
> Or we could make it available, for example, via /proc. It is a kind
> of information that might be gathered later on a rebooted system.
> SUSE has "supportconfig" command that allows to gather a lot
> of similar information about the system. We use it when
> analyzing crashdumps and kernel bugs in general.

Please no. The build ID is already available via
/sys/module/<modulename>/sections/<sectionname> and /sys/kernel/notes
(for vmlinux) but that won't help for panics that reboot. If a panic
happens and a new kernel is booted then post processing on the modules
and vmlinux could all be incorrect. Furthermore, the modules will have
to be found and parsed by some userspace crash processing tool after the
reboot.

I'd really prefer to rely on the standard build ID vs. a per-distro
bespoke solution to finding the information about the binaries the
kernel was running. It's just simpler to not need to find out this sort
of information about the system when the kernel knows what binary is
running already. This is the same reason coredump_filter exists to let
coredumping code figure out the build ID of the process that crashes vs.
connecting that to some system information daemon.
 
> 
> Anyway, I consider this a very detailed information that is not
> suitable for everyone. It should be availabe on request, like
> for example, backtraces from all CPUs, list of tasks, memory info.

I suppose I can make a config option for this module printing stuff in
the "Modules linked in:" line. Then we can remove it if most distros
decide to enable it? I'm slightly disappointed that it can't just be
printed all the time for every stacktrace but if there isn't opposition
if it's all behind a config option then I will be happy to get 99% of
the code upstream.

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-04  0:38         ` Stephen Boyd
@ 2021-03-04  1:19           ` Steven Rostedt
  2021-03-04  6:25             ` Stephen Boyd
  0 siblings, 1 reply; 30+ messages in thread
From: Steven Rostedt @ 2021-03-04  1:19 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Andy Shevchenko, Petr Mladek, Andrew Morton, linux-kernel,
	Jiri Olsa, Alexei Starovoitov, Jessica Yu, Evan Green,
	Hsin-Yi Wang, Sergey Senozhatsky, Rasmus Villemoes, linux-doc

On Wed, 03 Mar 2021 16:38:28 -0800
Stephen Boyd <swboyd@chromium.org> wrote:

> I'm starting to feel like nobody read the commit text, or I messed up
> somehow and the commit text was confusing? :(
> 

I read it, I'm just unfamiliar with it. I don't use pstore, and I'm not
sure what "crashdump" is. Do you mean the kexec/kdump? in which case
you can retrieve data within the kernel quite easily.

I haven't used debuginfod (never heard of it before actually).

> │ This is especially helpful for crash debugging with pstore or crashdump                                                                                                                                         
> │ kernels. If we have the build ID for the module in the stacktrace we can                                                                                                                                        
> │ request the debug symbols for the module from a remote debuginfod server                                                                                                                                        
> │ or parse stacktraces at a later time with decode_stacktrace.sh by                                                                                                                                               
> │ downloading the correct symbols based on the build ID. This cuts down on                                                                                                                                        
> │ the amount of time and effort needed to find the correct kernel modules                                                                                                                                         
> │ for a stacktrace by encoding that information into it.  

Are you saying it's common to have modules from different builds?

> 
> In some distro (read: non-kernel dev) workflows the vmlinux isn't
> shipped on the device and crash handling is done offline or much later.
> Using the build ID[1] is a common way to identify the binary that's
> running on the device. In conjunction with a debuginfod[2] server you
> can download the symbols for a crash automatically if you have the build
> ID information.
> 
> I can add a patch that updates decode_stacktrace.sh to show how it can
> download the correct vmlinux/modules if it isn't provided on the
> commandline.

Are you just trying to match modules with the builds that they were
created with?

> 
> If the debug symbols are on some public server then in theory we could
> have some robot sitting on the mailing list that looks for stacktraces
> and automatically replies with information about the line number/file
> and even provides the code snippet for the code that's crashing from
> that binary, because it's all stored in the full debuginfo builds.

Again, I have no idea how buildids are created or what they are used
for. This is the first time I've even heard about them. I'm all for
helping other people out to make their workflow easier, if it doesn't
make a mess for everyone else.

-- Steve


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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-04  1:19           ` Steven Rostedt
@ 2021-03-04  6:25             ` Stephen Boyd
  0 siblings, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-04  6:25 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Andy Shevchenko, Petr Mladek, Andrew Morton, linux-kernel,
	Jiri Olsa, Alexei Starovoitov, Jessica Yu, Evan Green,
	Hsin-Yi Wang, Sergey Senozhatsky, Rasmus Villemoes, linux-doc

Quoting Steven Rostedt (2021-03-03 17:19:32)
> On Wed, 03 Mar 2021 16:38:28 -0800
> Stephen Boyd <swboyd@chromium.org> wrote:
> 
> > I'm starting to feel like nobody read the commit text, or I messed up
> > somehow and the commit text was confusing? :(
> > 
> 
> I read it, I'm just unfamiliar with it. I don't use pstore, and I'm not
> sure what "crashdump" is. Do you mean the kexec/kdump? in which case
> you can retrieve data within the kernel quite easily.

Right, I meant kexec/kdump. Given that it is easy to retrieve it in
kdump (presumably with some scripting?) I can remove this motivation
from the commit text.

> 
> I haven't used debuginfod (never heard of it before actually).

Got it. Hopefully the links I provided were good enough? I will provide
a link next time.

> 
> > │ This is especially helpful for crash debugging with pstore or crashdump                                                                                                                                         
> > │ kernels. If we have the build ID for the module in the stacktrace we can                                                                                                                                        
> > │ request the debug symbols for the module from a remote debuginfod server                                                                                                                                        
> > │ or parse stacktraces at a later time with decode_stacktrace.sh by                                                                                                                                               
> > │ downloading the correct symbols based on the build ID. This cuts down on                                                                                                                                        
> > │ the amount of time and effort needed to find the correct kernel modules                                                                                                                                         
> > │ for a stacktrace by encoding that information into it.  
> 
> Are you saying it's common to have modules from different builds?

No.

> 
> > 
> > In some distro (read: non-kernel dev) workflows the vmlinux isn't
> > shipped on the device and crash handling is done offline or much later.
> > Using the build ID[1] is a common way to identify the binary that's
> > running on the device. In conjunction with a debuginfod[2] server you
> > can download the symbols for a crash automatically if you have the build
> > ID information.
> > 
> > I can add a patch that updates decode_stacktrace.sh to show how it can
> > download the correct vmlinux/modules if it isn't provided on the
> > commandline.
> 
> Are you just trying to match modules with the builds that they were
> created with?

Not exactly. I don't have a mapping of modules to the kernel they're
built/used with. I could create a mapping, but then that's something
else to maintain vs. what I have right now which is just a big database
of debuginfo mapped to build IDs (i.e. a debuginfod server).

> 
> > 
> > If the debug symbols are on some public server then in theory we could
> > have some robot sitting on the mailing list that looks for stacktraces
> > and automatically replies with information about the line number/file
> > and even provides the code snippet for the code that's crashing from
> > that binary, because it's all stored in the full debuginfo builds.
> 
> Again, I have no idea how buildids are created or what they are used
> for. This is the first time I've even heard about them. I'm all for
> helping other people out to make their workflow easier, if it doesn't
> make a mess for everyone else.
> 
> 

Makes sense and sounds good. Thanks.

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-01 17:47 ` [PATCH 5/7] printk: Make %pS and friends print module build ID Stephen Boyd
                     ` (2 preceding siblings ...)
  2021-03-03 10:25   ` Petr Mladek
@ 2021-03-04 17:00   ` Matthew Wilcox
  2021-03-04 19:15     ` Stephen Boyd
  3 siblings, 1 reply; 30+ messages in thread
From: Matthew Wilcox @ 2021-03-04 17:00 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Petr Mladek,
	Steven Rostedt, Sergey Senozhatsky, Andy Shevchenko,
	Rasmus Villemoes, linux-doc

On Mon, Mar 01, 2021 at 09:47:47AM -0800, Stephen Boyd wrote:
> Example:
> 
>  WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)

Would the first 12 characters instead of all 40 make it more palatable
without reducing its utility?
And I feel it should be within the [], so maybe this:

WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm ed5019fdf5e5]


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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-02  2:43   ` Steven Rostedt
  2021-03-02 23:24     ` Stephen Boyd
@ 2021-03-04 17:19     ` Matthew Wilcox
  2021-03-04 19:11       ` Stephen Boyd
  1 sibling, 1 reply; 30+ messages in thread
From: Matthew Wilcox @ 2021-03-04 17:19 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Stephen Boyd, Andrew Morton, linux-kernel, Jiri Olsa,
	Alexei Starovoitov, Jessica Yu, Evan Green, Hsin-Yi Wang,
	Petr Mladek, Sergey Senozhatsky, Andy Shevchenko,
	Rasmus Villemoes, linux-doc

On Mon, Mar 01, 2021 at 09:43:19PM -0500, Steven Rostedt wrote:
> On Mon,  1 Mar 2021 09:47:47 -0800
> Stephen Boyd <swboyd@chromium.org> wrote:
> 
> > The %pS printk format (among some others) is used to print kernel
> > addresses symbolically. When the kernel prints an address inside of a
> > module, the kernel prints the addresses' symbol name along with the
> > module's name that contains the address. Let's make kernel stacktraces
> > easier to identify on KALLSYMS builds by including the build ID of a
> > module when we print the address.
> 
> Please no!
> 
> This kills the output of tracing with offset, and can possibly break
> scripts. I don't want to look at traces like this!
> 
>           <idle>-0       [004] ..s1   353.842577: ipv4_conntrack_in+0x0/0x10 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_hook_slow+0x40/0xb0

Would it make sense to only print the build-id if it differs from the
build-id of the kernel which has loaded it?

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-04 17:19     ` Matthew Wilcox
@ 2021-03-04 19:11       ` Stephen Boyd
  0 siblings, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-04 19:11 UTC (permalink / raw)
  To: Matthew Wilcox, Steven Rostedt
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Petr Mladek,
	Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc

Quoting Matthew Wilcox (2021-03-04 09:19:40)
> On Mon, Mar 01, 2021 at 09:43:19PM -0500, Steven Rostedt wrote:
> > On Mon,  1 Mar 2021 09:47:47 -0800
> > Stephen Boyd <swboyd@chromium.org> wrote:
> > 
> > > The %pS printk format (among some others) is used to print kernel
> > > addresses symbolically. When the kernel prints an address inside of a
> > > module, the kernel prints the addresses' symbol name along with the
> > > module's name that contains the address. Let's make kernel stacktraces
> > > easier to identify on KALLSYMS builds by including the build ID of a
> > > module when we print the address.
> > 
> > Please no!
> > 
> > This kills the output of tracing with offset, and can possibly break
> > scripts. I don't want to look at traces like this!
> > 
> >           <idle>-0       [004] ..s1   353.842577: ipv4_conntrack_in+0x0/0x10 [nf_conntrack] (3b39eb771b2566331887f671c741f90bfba0b051) <-nf_hook_slow+0x40/0xb0
> 
> Would it make sense to only print the build-id if it differs from the
> build-id of the kernel which has loaded it?

No. The build-id of the module is different from the kernel that loaded
it all the time, so it would always be printed.

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-04 17:00   ` Matthew Wilcox
@ 2021-03-04 19:15     ` Stephen Boyd
  2021-03-04 23:11       ` Rasmus Villemoes
  0 siblings, 1 reply; 30+ messages in thread
From: Stephen Boyd @ 2021-03-04 19:15 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Petr Mladek,
	Steven Rostedt, Sergey Senozhatsky, Andy Shevchenko,
	Rasmus Villemoes, linux-doc

Quoting Matthew Wilcox (2021-03-04 09:00:52)
> On Mon, Mar 01, 2021 at 09:47:47AM -0800, Stephen Boyd wrote:
> > Example:
> > 
> >  WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> 
> Would the first 12 characters instead of all 40 make it more palatable
> without reducing its utility?

I can't seem to request debuginfo from debuginfod without the full 40
characters. It's not a git sha1 hash. 

> And I feel it should be within the [], so maybe this:
> 
> WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm ed5019fdf5e5]
> 

Sure I could put the hex numbers inside the brackets. I suspect changing
%pS or updating the "Modules linked in:" line isn't going to be
palatable. I've decided to introduce another printk format %pT to print
the stacktrace and then updated the architecture code on arm64 and x86
to see how it goes. Other architecures can be updated if this is
acceptable. I'll send out a patch series in a little bit that also
updates the decode_stacktrace.sh script to parse this.

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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-04 19:15     ` Stephen Boyd
@ 2021-03-04 23:11       ` Rasmus Villemoes
  2021-03-05  4:21         ` Stephen Boyd
  0 siblings, 1 reply; 30+ messages in thread
From: Rasmus Villemoes @ 2021-03-04 23:11 UTC (permalink / raw)
  To: Stephen Boyd, Matthew Wilcox
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Petr Mladek,
	Steven Rostedt, Sergey Senozhatsky, Andy Shevchenko, linux-doc

On 04/03/2021 20.15, Stephen Boyd wrote:
> Quoting Matthew Wilcox (2021-03-04 09:00:52)
>> On Mon, Mar 01, 2021 at 09:47:47AM -0800, Stephen Boyd wrote:
>>> Example:
>>>
>>>  WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
>>
>> Would the first 12 characters instead of all 40 make it more palatable
>> without reducing its utility?
> 
> I can't seem to request debuginfo from debuginfod without the full 40
> characters. It's not a git sha1 hash. 
> 
>> And I feel it should be within the [], so maybe this:
>>
>> WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm ed5019fdf5e5]
>>
> 
> Sure I could put the hex numbers inside the brackets. I suspect changing
> %pS or updating the "Modules linked in:" line isn't going to be
> palatable. I've decided to introduce another printk format %pT to print
> the stacktrace 

Can you avoid claiming a new "top-level" %p modifier? Isn't it better to
add a new flag to '%pS', say '%pSb' to include build-id?

Rasmus


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

* Re: [PATCH 5/7] printk: Make %pS and friends print module build ID
  2021-03-04 23:11       ` Rasmus Villemoes
@ 2021-03-05  4:21         ` Stephen Boyd
  0 siblings, 0 replies; 30+ messages in thread
From: Stephen Boyd @ 2021-03-05  4:21 UTC (permalink / raw)
  To: Matthew Wilcox, Rasmus Villemoes
  Cc: Andrew Morton, linux-kernel, Jiri Olsa, Alexei Starovoitov,
	Jessica Yu, Evan Green, Hsin-Yi Wang, Petr Mladek,
	Steven Rostedt, Sergey Senozhatsky, Andy Shevchenko, linux-doc

Quoting Rasmus Villemoes (2021-03-04 15:11:47)
> On 04/03/2021 20.15, Stephen Boyd wrote:
> > Quoting Matthew Wilcox (2021-03-04 09:00:52)
> >> On Mon, Mar 01, 2021 at 09:47:47AM -0800, Stephen Boyd wrote:
> >>> Example:
> >>>
> >>>  WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm] (ed5019fdf5e53be37cb1ba7899292d7e143b259e)
> >>
> >> Would the first 12 characters instead of all 40 make it more palatable
> >> without reducing its utility?
> > 
> > I can't seem to request debuginfo from debuginfod without the full 40
> > characters. It's not a git sha1 hash. 
> > 
> >> And I feel it should be within the [], so maybe this:
> >>
> >> WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm ed5019fdf5e5]
> >>
> > 
> > Sure I could put the hex numbers inside the brackets. I suspect changing
> > %pS or updating the "Modules linked in:" line isn't going to be
> > palatable. I've decided to introduce another printk format %pT to print
> > the stacktrace 
> 
> Can you avoid claiming a new "top-level" %p modifier? Isn't it better to
> add a new flag to '%pS', say '%pSb' to include build-id?
> 

I see that %pSR is used in alpha for the stacktrace. I guess we can have
%pSb and %pSr then.

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

end of thread, other threads:[~2021-03-05  4:21 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01 17:47 [PATCH 0/7] Add build ID to stacktraces Stephen Boyd
2021-03-01 17:47 ` [PATCH 1/7] buildid: Add method to get running kernel's build ID Stephen Boyd
     [not found]   ` <CAHp75VcCjYm59-BQ0paPDCV97N5mgcq_OAdeixgUDEnAuG2qMg@mail.gmail.com>
     [not found]     ` <161472770533.1478170.3061709841985494537@swboyd.mtv.corp.google.com>
2021-03-04  0:41       ` Stephen Boyd
2021-03-01 17:47 ` [PATCH 2/7] dump_stack: Add vmlinux build ID to stack traces Stephen Boyd
2021-03-01 17:47 ` [PATCH 3/7] buildid: Add API to parse build ID out of buffer Stephen Boyd
2021-03-01 17:47 ` [PATCH 4/7] module: Parse and stash build ID on insertion Stephen Boyd
2021-03-01 17:47 ` [PATCH 5/7] printk: Make %pS and friends print module build ID Stephen Boyd
2021-03-02  2:43   ` Steven Rostedt
2021-03-02 23:24     ` Stephen Boyd
2021-03-04 17:19     ` Matthew Wilcox
2021-03-04 19:11       ` Stephen Boyd
2021-03-03  2:01   ` Steven Rostedt
2021-03-03  3:00     ` Stephen Boyd
2021-03-03  8:19       ` Andy Shevchenko
2021-03-03  8:56         ` Stephen Boyd
2021-03-03 10:25   ` Petr Mladek
2021-03-03 15:00     ` Steven Rostedt
2021-03-03 16:17       ` Andy Shevchenko
2021-03-04  0:38         ` Stephen Boyd
2021-03-04  1:19           ` Steven Rostedt
2021-03-04  6:25             ` Stephen Boyd
2021-03-04  1:17     ` Stephen Boyd
2021-03-04 17:00   ` Matthew Wilcox
2021-03-04 19:15     ` Stephen Boyd
2021-03-04 23:11       ` Rasmus Villemoes
2021-03-05  4:21         ` Stephen Boyd
2021-03-01 17:47 ` [PATCH 6/7] buildid: Mark some arguments const Stephen Boyd
2021-03-01 17:47 ` [PATCH 7/7] kdump: Use vmlinux_build_id() to simplify Stephen Boyd
2021-03-02  8:19   ` Baoquan He
2021-03-02 23:21     ` Stephen Boyd

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).