Skip to contentSkip to conversation

Function tracing support for pstore

PATCH Superseded Anton Vorontsov 2 participants 10 messages Jun 26, 2012 – Jul 9, 2012

Subsystems (3)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v3 cover Function tracing support for pstore

Hi all,

In v3:
- Make traces versioned, as suggested by Steven, Tony and Colin. (The version tag is stored in the PRZ signature, see the last patch for the implementation details).
- Add Steven's Ack on the first patch.

In v2:
- Do not introduce a separate 'persistent' tracer, but introduce an option to the existing 'function' tracer.

Rationale for this patch set:

With this support kernel can save functions call chain log into a persistent ram buffer that can be decoded and dumped after reboot through pstore filesystem. It can be used to determine what function was last called before a hang or an unexpected reset (caused by, for example, a buggy driver that abuses HW).

Here's a "nano howto", to get the idea:

 # mount -t debugfs debugfs /sys/kernel/debug/
 # cd /sys/kernel/debug/tracing
 # echo function > current_tracer
 # echo 1 > options/func_pstore
 # reboot -f
 [...]
 # mount -t pstore pstore /mnt/
 # tail /mnt/ftrace-ramoops
 0 ffffffff8101ea64  ffffffff8101bcda  native_apic_mem_read <- disconnect_bsp_APIC+0x6a/0xc0
 0 ffffffff8101ea44  ffffffff8101bcf6  native_apic_mem_write <- disconnect_bsp_APIC+0x86/0xc0
 0 ffffffff81020084  ffffffff8101a4b5  hpet_disable <- native_machine_shutdown+0x75/0x90
 0 ffffffff81005f94  ffffffff8101a4bb  iommu_shutdown_noop <- native_machine_shutdown+0x7b/0x90
 0 ffffffff8101a6a1  ffffffff8101a437  native_machine_emergency_restart <- native_machine_restart+0x37/0x40
 0 ffffffff811f9876  ffffffff8101a73a  acpi_reboot <- native_machine_emergency_restart+0xaa/0x1e0
 0 ffffffff8101a514  ffffffff8101a772  mach_reboot_fixups <- native_machine_emergency_restart+0xe2/0x1e0
 0 ffffffff811d9c54  ffffffff8101a7a0  __const_udelay <- native_machine_emergency_restart+0x110/0x1e0
 0 ffffffff811d9c34  ffffffff811d9c80  __delay <- __const_udelay+0x30/0x40
 0 ffffffff811d9d14  ffffffff811d9c3f  delay_tsc <- __delay+0xf/0x20

Mostly the code comes from trace_persistent.c driver found in the Android git tree, written by Colin Cross <[email protected]> (according to sign-off history). I reworked the driver a little bit, and ported it to pstore subsystem.

--

 Documentation/ramoops.txt      |   25 +++++++++
 fs/pstore/Kconfig              |   13 +++++
 fs/pstore/Makefile             |    1 +
 fs/pstore/ftrace.c             |   35 +++++++++++++
 fs/pstore/inode.c              |  111 ++++++++++++++++++++++++++++++++++++++--
 fs/pstore/internal.h           |   43 ++++++++++++++++
 fs/pstore/platform.c           |   12 ++++-
 fs/pstore/ram.c                |   65 +++++++++++++++++------
 fs/pstore/ram_core.c           |   12 +++--
 include/linux/pstore.h         |   13 +++++
 include/linux/pstore_ram.h     |    3 +-
 kernel/trace/trace.c           |    7 +--
 kernel/trace/trace_functions.c |   25 +++++++--
 13 files changed, 330 insertions(+), 35 deletions(-)
Signature · 2 lines
Anton Vorontsov
Email: [email protected]
#1 lore raw

PATCH v3 1/7 tracing: Fix initialization failure path in tracing_set_tracer()

If tracer->init() fails, current code will leave current_tracer pointing to an unusable tracer, which at best makes 'current_tracer' report inaccurate value.

Fix the issue by pointing current_tracer to nop tracer, and only update current_tracer with the new one after all the initialization succeeds.

---
 kernel/trace/trace.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
kernel/trace/trace.c +4 -3
index a7fa070..7647d33 100644
@@ -3172,10 +3172,10 @@ static int tracing_set_tracer(const char *buf)
 	}
 	destroy_trace_option_files(topts);
 
-	current_trace = t;
+	current_trace = &nop_trace;
 
-	topts = create_trace_option_files(current_trace);
-	if (current_trace->use_max_tr) {
+	topts = create_trace_option_files(t);
+	if (t->use_max_tr) {
 		int cpu;
 		/* we need to make per cpu buffer sizes equivalent */
 		for_each_tracing_cpu(cpu) {
@@ -3195,6 +3195,7 @@ static int tracing_set_tracer(const char *buf)
 			goto out;
 	}
 
+	current_trace = t;
 	trace_branch_enable(tr);
  out:
 	mutex_unlock(&trace_types_lock);
Signature · 1 line
1.7.10.4
#2 lore raw

PATCH v3 2/7 pstore: Introduce write_buf backend callback

In reply to Anton Vorontsov

For function tracing we need to stop using pstore.buf directly, since in a tracing callback we can't use spinlocks, and thus we can't safely use the global buffer.

With write_buf callback, backends no longer need to access pstore.buf directly, and thus we can pass any buffers (e.g. allocated on stack).

---
 fs/pstore/platform.c   |   10 ++++++++++
 include/linux/pstore.h |    4 ++++
 2 files changed, 14 insertions(+)
fs/pstore/platform.c +10 -0
index 6b3ff04..ef5ca8a 100644
@@ -188,6 +188,14 @@ static void pstore_register_console(void)
 static void pstore_register_console(void) {}
 #endif
 
+static int pstore_write_compat(enum pstore_type_id type,
+			       enum kmsg_dump_reason reason,
+			       u64 *id, unsigned int part,
+			       size_t size, struct pstore_info *psi)
+{
+	return psi->write_buf(type, reason, id, part, psinfo->buf, size, psi);
+}
+
 /*
  * platform specific persistent storage driver registers with
  * us here. If pstore is already mounted, call the platform
@@ -212,6 +220,8 @@ int pstore_register(struct pstore_info *psi)
 		return -EINVAL;
 	}
 
+	if (!psi->write)
+		psi->write = pstore_write_compat;
 	psinfo = psi;
 	mutex_init(&psinfo->read_mutex);
 	spin_unlock(&pstore_lock);
include/linux/pstore.h +4 -0
index 1bd014b..b107484 100644
@@ -48,6 +48,10 @@ struct pstore_info {
 	int		(*write)(enum pstore_type_id type,
 			enum kmsg_dump_reason reason, u64 *id,
 			unsigned int part, size_t size, struct pstore_info *psi);
+	int		(*write_buf)(enum pstore_type_id type,
+			enum kmsg_dump_reason reason, u64 *id,
+			unsigned int part, const char *buf, size_t size,
+			struct pstore_info *psi);
 	int		(*erase)(enum pstore_type_id type, u64 id,
 			struct pstore_info *psi);
 	void		*data;
Signature · 1 line
1.7.10.4
#3 lore raw

PATCH v3 3/7 pstore: Add persistent function tracing

In reply to Anton Vorontsov

With this support kernel can save function call chain log into a persistent ram buffer that can be decoded and dumped after reboot through pstore filesystem. It can be used to determine what function was last called before a reset or panic.

We store the log in a binary format and then decode it at read time.

p.s.
Mostly the code comes from trace_persistent.c driver found in the Android git tree, written by Colin Cross <[email protected]> (according to sign-off history). I reworked the driver a little bit, and ported it to pstore.

---
 fs/pstore/Kconfig      |   12 ++++++
 fs/pstore/Makefile     |    1 +
 fs/pstore/ftrace.c     |   35 +++++++++++++++
 fs/pstore/inode.c      |  111 ++++++++++++++++++++++++++++++++++++++++++++++--
 fs/pstore/internal.h   |   43 +++++++++++++++++++
 fs/pstore/platform.c   |    2 +-
 include/linux/pstore.h |    9 ++++
 7 files changed, 208 insertions(+), 5 deletions(-)
 create mode 100644 fs/pstore/ftrace.c
fs/pstore/Kconfig +12 -0
index d044de6..d39bb5c 100644
@@ -19,6 +19,18 @@ config PSTORE_CONSOLE
 	  When the option is enabled, pstore will log all kernel
 	  messages, even if no oops or panic happened.
 
+config PSTORE_FTRACE
+	bool "Persistent function tracer"
+	depends on PSTORE
+	depends on FUNCTION_TRACER
+	help
+	  With this option kernel traces function calls into a persistent
+	  ram buffer that can be decoded and dumped after reboot through
+	  pstore filesystem. It can be used to determine what function
+	  was last called before a reset or panic.
+
+	  If unsure, say N.
+
 config PSTORE_RAM
 	tristate "Log panic/oops to a RAM buffer"
 	depends on PSTORE
fs/pstore/Makefile +1 -0
index 278a44e..4c9095c 100644
@@ -5,6 +5,7 @@
 obj-y += pstore.o
 
 pstore-objs += inode.o platform.o
+obj-$(CONFIG_PSTORE_FTRACE)	+= ftrace.o
 
 ramoops-objs += ram.o ram_core.o
 obj-$(CONFIG_PSTORE_RAM)	+= ramoops.o
fs/pstore/ftrace.c new file +35 -0
new file mode 100644
index 0000000..a130d48
@@ -0,0 +1,35 @@
+/*
+ * Copyright 2012  Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/compiler.h>
+#include <linux/irqflags.h>
+#include <linux/percpu.h>
+#include <linux/smp.h>
+#include <linux/atomic.h>
+#include <asm/barrier.h>
+#include "internal.h"
+
+void notrace pstore_ftrace_call(unsigned long ip, unsigned long parent_ip)
+{
+	struct pstore_ftrace_record rec = {};
+
+	if (unlikely(oops_in_progress))
+		return;
+
+	rec.ip = ip;
+	rec.parent_ip = parent_ip;
+	pstore_ftrace_encode_cpu(&rec, raw_smp_processor_id());
+	psinfo->write_buf(PSTORE_TYPE_FTRACE, 0, NULL, 0, (void *)&rec,
+			  sizeof(rec), psinfo);
+}
fs/pstore/inode.c +107 -4

137 lines not on this page · raw message

fs/pstore/internal.h +43 -0

49 lines not on this page · raw message

fs/pstore/platform.c +1 -1
index ef5ca8a..29996e8 100644
@@ -61,7 +61,7 @@ static DECLARE_WORK(pstore_work, pstore_dowork);
  * calls to pstore_register()
  */
 static DEFINE_SPINLOCK(pstore_lock);
-static struct pstore_info *psinfo;
+struct pstore_info *psinfo;
 
 static char *backend;
 
include/linux/pstore.h +9 -0
index b107484..120443b 100644
@@ -30,6 +30,7 @@ enum pstore_type_id {
 	PSTORE_TYPE_DMESG	= 0,
 	PSTORE_TYPE_MCE		= 1,
 	PSTORE_TYPE_CONSOLE	= 2,
+	PSTORE_TYPE_FTRACE	= 3,
 	PSTORE_TYPE_UNKNOWN	= 255
 };
 
@@ -57,6 +58,14 @@ struct pstore_info {
 	void		*data;
 };
 
+
+#ifdef CONFIG_PSTORE_FTRACE
+extern void pstore_ftrace_call(unsigned long ip, unsigned long parent_ip);
+#else
+static inline void pstore_ftrace_call(unsigned long ip, unsigned long parent_ip)
+{ }
+#endif
+
 #ifdef CONFIG_PSTORE
 extern int pstore_register(struct pstore_info *);
 #else
Signature · 1 line
1.7.10.4
#4 lore raw

PATCH v3 4/7 tracing/function: Introduce persistent trace option

In reply to Anton Vorontsov

This patch introduces 'func_ptrace' option, now available in /sys/kernel/debug/tracing/options when function tracer is selected.

The patch also adds some tiny code that calls back to pstore to record the trace. The callback is no-op when PSTORE=n.

---
 kernel/trace/trace_functions.c |   25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)
kernel/trace/trace_functions.c +20 -5
index c7b0c6a..269627c 100644
@@ -13,6 +13,7 @@
 #include <linux/debugfs.h>
 #include <linux/uaccess.h>
 #include <linux/ftrace.h>
+#include <linux/pstore.h>
 #include <linux/fs.h>
 
 #include "trace.h"
@@ -74,6 +75,14 @@ function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip)
 	preempt_enable_notrace();
 }
 
+/* Our two options */
+enum {
+	TRACE_FUNC_OPT_STACK	= 0x1,
+	TRACE_FUNC_OPT_PSTORE	= 0x2,
+};
+
+static struct tracer_flags func_flags;
+
 static void
 function_trace_call(unsigned long ip, unsigned long parent_ip)
 {
@@ -97,6 +106,12 @@ function_trace_call(unsigned long ip, unsigned long parent_ip)
 	disabled = atomic_inc_return(&data->disabled);
 
 	if (likely(disabled == 1)) {
+		/*
+		 * So far tracing doesn't support multiple buffers, so
+		 * we make an explicit call for now.
+		 */
+		if (unlikely(func_flags.val & TRACE_FUNC_OPT_PSTORE))
+			pstore_ftrace_call(ip, parent_ip);
 		pc = preempt_count();
 		trace_function(tr, ip, parent_ip, flags, pc);
 	}
@@ -158,15 +173,13 @@ static struct ftrace_ops trace_stack_ops __read_mostly =
 	.flags = FTRACE_OPS_FL_GLOBAL,
 };
 
-/* Our two options */
-enum {
-	TRACE_FUNC_OPT_STACK = 0x1,
-};
-
 static struct tracer_opt func_opts[] = {
 #ifdef CONFIG_STACKTRACE
 	{ TRACER_OPT(func_stack_trace, TRACE_FUNC_OPT_STACK) },
 #endif
+#ifdef CONFIG_PSTORE_FTRACE
+	{ TRACER_OPT(func_pstore, TRACE_FUNC_OPT_PSTORE) },
+#endif
 	{ } /* Always set a last empty entry */
 };
 
@@ -219,6 +232,8 @@ static int func_set_flag(u32 old_flags, u32 bit, int set)
 
 		return 0;
 	}
+	if (bit == TRACE_FUNC_OPT_PSTORE)
+		return 0;
 
 	return -EINVAL;
 }
Signature · 1 line
1.7.10.4
maintainer
#5 lore raw

Re: [PATCH 4/7] tracing/function: Introduce persistent trace option

kernel/trace/trace_functions.c static int func_set_flag(u32 old_flags, u32 bit, int set) · line 236

Quoted 7 lines · Anton Vorontsov
kernel/trace/trace_functions.c +2 -0
@@ -219,6 +232,8 @@ static int func_set_flag(u32 old_flags, u32 bit, int set)
 
 		return 0;
 	}
+	if (bit == TRACE_FUNC_OPT_PSTORE)
+		return 0;

Perhaps this should be a switch() statement, or at least put a
} else if () {

As it can't be bit == TRACE_FUNC_OPT_STACK and bit == TRACE_FUNC_OPT_PSTORE.

-- Steve

Quoted 3 lines
kernel/trace/trace_functions.c +0 -0
 	return -EINVAL;
 }
#6 lore raw

Re: [PATCH 4/7] tracing/function: Introduce persistent trace option

Quoted 11 lines · Steven Rostedt

kernel/trace/trace_functions.c static int func_set_flag(u32 old_flags, u32 bit, int set) · line 236

Quoted 6 lines · Anton Vorontsov
kernel/trace/trace_functions.c +2 -0
@@ -219,6 +232,8 @@ static int func_set_flag(u32 old_flags, u32 bit, int set)
 
 		return 0;
 	}
+	if (bit == TRACE_FUNC_OPT_PSTORE)
+		return 0;

Perhaps this should be a switch() statement, or at least put a
} else if () {

Quoted 2 lines

As it can't be bit == TRACE_FUNC_OPT_STACK and bit == TRACE_FUNC_OPT_PSTORE.

Yup, thanks for the idea. I'll start using 'else if' at first, and then will convert it to a switch statement via a separate patch, that way I'll not clobber the functional patch with cosmetic changes.

Thanks,

Signature · 2 lines
Anton Vorontsov
Email: [email protected]
#7 lore raw

PATCH v3 5/7 pstore/ram: Convert to write_buf callback

In reply to Anton Vorontsov

Don't use pstore.buf directly, instead convert the code to write_buf callback which passes a pointer to a buffer as an argument.

---
 fs/pstore/ram.c |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)
fs/pstore/ram.c +9 -8
index 78ecefc..ee58344 100644
@@ -168,11 +168,12 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz)
 	return len;
 }
 
-static int ramoops_pstore_write(enum pstore_type_id type,
-				enum kmsg_dump_reason reason,
-				u64 *id,
-				unsigned int part,
-				size_t size, struct pstore_info *psi)
+
+static int ramoops_pstore_write_buf(enum pstore_type_id type,
+				    enum kmsg_dump_reason reason,
+				    u64 *id, unsigned int part,
+				    const char *buf, size_t size,
+				    struct pstore_info *psi)
 {
 	struct ramoops_context *cxt = psi->data;
 	struct persistent_ram_zone *prz = cxt->przs[cxt->dump_write_cnt];
@@ -181,7 +182,7 @@ static int ramoops_pstore_write(enum pstore_type_id type,
 	if (type == PSTORE_TYPE_CONSOLE) {
 		if (!cxt->cprz)
 			return -ENOMEM;
-		persistent_ram_write(cxt->cprz, cxt->pstore.buf, size);
+		persistent_ram_write(cxt->cprz, buf, size);
 		return 0;
 	}
 
@@ -210,7 +211,7 @@ static int ramoops_pstore_write(enum pstore_type_id type,
 	hlen = ramoops_write_kmsg_hdr(prz);
 	if (size + hlen > prz->buffer_size)
 		size = prz->buffer_size - hlen;
-	persistent_ram_write(prz, cxt->pstore.buf, size);
+	persistent_ram_write(prz, buf, size);
 
 	cxt->dump_write_cnt = (cxt->dump_write_cnt + 1) % cxt->max_dump_cnt;
 
@@ -248,7 +249,7 @@ static struct ramoops_context oops_cxt = {
 		.name	= "ramoops",
 		.open	= ramoops_pstore_open,
 		.read	= ramoops_pstore_read,
-		.write	= ramoops_pstore_write,
+		.write_buf	= ramoops_pstore_write_buf,
 		.erase	= ramoops_pstore_erase,
 	},
 };
Signature · 1 line
1.7.10.4
#8 lore raw

PATCH v3 6/7 pstore/ram: Add ftrace messages handling

In reply to Anton Vorontsov

The ftrace log size is configurable via ramoops.ftrace_size module option, and the log itself is available via <pstore-mount>/ftrace-ramoops file.

---
 Documentation/ramoops.txt  |   25 +++++++++++++++++++++++++
 fs/pstore/ram.c            |   37 +++++++++++++++++++++++++++++++++----
 include/linux/pstore_ram.h |    1 +
 3 files changed, 59 insertions(+), 4 deletions(-)
Documentation/ramoops.txt +25 -0
index 59a74a8..197ad59 100644
@@ -94,3 +94,28 @@ timestamp and a new line. The dump then continues with the actual data.
 The dump data can be read from the pstore filesystem. The format for these
 files is "dmesg-ramoops-N", where N is the record number in memory. To delete
 a stored record from RAM, simply unlink the respective pstore file.
+
+5. Persistent function tracing
+
+Persistent function tracing might be useful for debugging software or hardware
+related hangs. The functions call chain log is stored in a "ftrace-ramoops"
+file. Here is an example of usage:
+
+ # mount -t debugfs debugfs /sys/kernel/debug/
+ # cd /sys/kernel/debug/tracing
+ # echo function > current_tracer
+ # echo 1 > options/func_pstore
+ # reboot -f
+ [...]
+ # mount -t pstore pstore /mnt/
+ # tail /mnt/ftrace-ramoops
+ 0 ffffffff8101ea64  ffffffff8101bcda  native_apic_mem_read <- disconnect_bsp_APIC+0x6a/0xc0
+ 0 ffffffff8101ea44  ffffffff8101bcf6  native_apic_mem_write <- disconnect_bsp_APIC+0x86/0xc0
+ 0 ffffffff81020084  ffffffff8101a4b5  hpet_disable <- native_machine_shutdown+0x75/0x90
+ 0 ffffffff81005f94  ffffffff8101a4bb  iommu_shutdown_noop <- native_machine_shutdown+0x7b/0x90
+ 0 ffffffff8101a6a1  ffffffff8101a437  native_machine_emergency_restart <- native_machine_restart+0x37/0x40
+ 0 ffffffff811f9876  ffffffff8101a73a  acpi_reboot <- native_machine_emergency_restart+0xaa/0x1e0
+ 0 ffffffff8101a514  ffffffff8101a772  mach_reboot_fixups <- native_machine_emergency_restart+0xe2/0x1e0
+ 0 ffffffff811d9c54  ffffffff8101a7a0  __const_udelay <- native_machine_emergency_restart+0x110/0x1e0
+ 0 ffffffff811d9c34  ffffffff811d9c80  __delay <- __const_udelay+0x30/0x40
+ 0 ffffffff811d9d14  ffffffff811d9c3f  delay_tsc <- __delay+0xf/0x20
fs/pstore/ram.c +33 -4
index ee58344..2cc029f 100644
@@ -45,6 +45,10 @@ static ulong ramoops_console_size = MIN_MEM_SIZE;
 module_param_named(console_size, ramoops_console_size, ulong, 0400);
 MODULE_PARM_DESC(console_size, "size of kernel console log");
 
+static ulong ramoops_ftrace_size = MIN_MEM_SIZE;
+module_param_named(ftrace_size, ramoops_ftrace_size, ulong, 0400);
+MODULE_PARM_DESC(ftrace_size, "size of ftrace log");
+
 static ulong mem_address;
 module_param(mem_address, ulong, 0400);
 MODULE_PARM_DESC(mem_address,
@@ -68,16 +72,19 @@ MODULE_PARM_DESC(ramoops_ecc,
 struct ramoops_context {
 	struct persistent_ram_zone **przs;
 	struct persistent_ram_zone *cprz;
+	struct persistent_ram_zone *fprz;
 	phys_addr_t phys_addr;
 	unsigned long size;
 	size_t record_size;
 	size_t console_size;
+	size_t ftrace_size;
 	int dump_oops;
 	int ecc_size;
 	unsigned int max_dump_cnt;
 	unsigned int dump_write_cnt;
 	unsigned int dump_read_cnt;
 	unsigned int console_read_cnt;
+	unsigned int ftrace_read_cnt;
 	struct pstore_info pstore;
 };
 
@@ -136,6 +143,9 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
 		prz = ramoops_get_next_prz(&cxt->cprz, &cxt->console_read_cnt,
 					   1, id, type, PSTORE_TYPE_CONSOLE, 0);
 	if (!prz)
+		prz = ramoops_get_next_prz(&cxt->fprz, &cxt->ftrace_read_cnt,
+					   1, id, type, PSTORE_TYPE_FTRACE, 0);
+	if (!prz)
 		return 0;
 
 	/* TODO(kees): Bogus time for the moment. */
@@ -184,6 +194,11 @@ static int ramoops_pstore_write_buf(enum pstore_type_id type,
 			return -ENOMEM;
 		persistent_ram_write(cxt->cprz, buf, size);
 		return 0;
+	} else if (type == PSTORE_TYPE_FTRACE) {
+		if (!cxt->fprz)
+			return -ENOMEM;
+		persistent_ram_write(cxt->fprz, buf, size);
+		return 0;
 	}
 
 	if (type != PSTORE_TYPE_DMESG)
@@ -233,6 +248,9 @@ static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
 	case PSTORE_TYPE_CONSOLE:
 		prz = cxt->cprz;
 		break;
+	case PSTORE_TYPE_FTRACE:
+		prz = cxt->fprz;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -346,7 +364,8 @@ static int __devinit ramoops_probe(struct platform_device *pdev)
 	if (cxt->max_dump_cnt)
 		goto fail_out;
 
-	if (!pdata->mem_size || (!pdata->record_size && !pdata->console_size)) {
+	if (!pdata->mem_size || (!pdata->record_size && !pdata->console_size &&
+			!pdata->ftrace_size)) {
 		pr_err("The memory size and the record/console size must be "
 			"non-zero\n");
 		goto fail_out;
@@ -355,18 +374,20 @@ static int __devinit ramoops_probe(struct platform_device *pdev)
 	pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
 	pdata->record_size = rounddown_pow_of_two(pdata->record_size);
 	pdata->console_size = rounddown_pow_of_two(pdata->console_size);
+	pdata->ftrace_size = rounddown_pow_of_two(pdata->ftrace_size);
 
 	cxt->dump_read_cnt = 0;
 	cxt->size = pdata->mem_size;
 	cxt->phys_addr = pdata->mem_address;
 	cxt->record_size = pdata->record_size;
 	cxt->console_size = pdata->console_size;
+	cxt->ftrace_size = pdata->ftrace_size;
 	cxt->dump_oops = pdata->dump_oops;
 	cxt->ecc_size = pdata->ecc_size;
 
 	paddr = cxt->phys_addr;
 
-	dump_mem_sz = cxt->size - cxt->console_size;
+	dump_mem_sz = cxt->size - cxt->console_size - cxt->ftrace_size;
 	err = ramoops_init_przs(dev, cxt, &paddr, dump_mem_sz);
 	if (err)
 		goto fail_out;
@@ -375,9 +396,14 @@ static int __devinit ramoops_probe(struct platform_device *pdev)
 	if (err)
 		goto fail_init_cprz;
 
-	if (!cxt->przs && !cxt->cprz) {
+	err = ramoops_init_prz(dev, cxt, &cxt->fprz, &paddr, cxt->ftrace_size);
+	if (err)
+		goto fail_init_fprz;
+
+	if (!cxt->przs && !cxt->cprz && !cxt->fprz) {
 		pr_err("memory size too small, minimum is %lu\n",
-			cxt->console_size + cxt->record_size);
+			cxt->console_size + cxt->record_size +
+			cxt->ftrace_size);
 		goto fail_cnt;
 	}
 
@@ -424,6 +450,8 @@ fail_clear:
 	cxt->pstore.bufsize = 0;
 	cxt->max_dump_cnt = 0;
 fail_cnt:
+	kfree(cxt->fprz);
+fail_init_fprz:
 	kfree(cxt->cprz);
 fail_init_cprz:
 	ramoops_free_przs(cxt);
@@ -478,6 +506,7 @@ static void ramoops_register_dummy(void)
 	dummy_data->mem_address = mem_address;
 	dummy_data->record_size = record_size;
 	dummy_data->console_size = ramoops_console_size;
+	dummy_data->ftrace_size = ramoops_ftrace_size;
 	dummy_data->dump_oops = dump_oops;
 	dummy_data->ecc_size = ramoops_ecc == 1 ? 16 : ramoops_ecc;
 
include/linux/pstore_ram.h +1 -0
index dcf805f..af848e1 100644
@@ -72,6 +72,7 @@ struct ramoops_platform_data {
 	unsigned long	mem_address;
 	unsigned long	record_size;
 	unsigned long	console_size;
+	unsigned long	ftrace_size;
 	int		dump_oops;
 	int		ecc_size;
 };
Signature · 1 line
1.7.10.4
#9 lore raw

PATCH v3 7/7 pstore/ram: Make tracing log versioned

In reply to Anton Vorontsov

Decoding the binary trace w/ a different kernel might be troublesome since we convert addresses to symbols. For kernels with minimal changes, the mappings would probably match, but it's not guaranteed at all. (But still we could convert the addresses by hand, since we do print raw addresses.)

If we use modules, the symbols could be loaded at different addresses from the previously booted kernel, and so this would also fail, but there's nothing we can do about it.

Also, the binary data format that pstore/ram is using in its ringbuffer may change between the kernels, so here we too must ensure that we're running the same kernel.

So, there are two questions really:

1. How to compute the unique kernel tag;
2. Where to store it.

In this patch we're just passing linux_banner through CRC32. This way we are protecting from the kernel version mismatch, making sure that we're running the same image. We could also add CRC of a symbol table (as suggested by Tony Luck), but for now let's not be that strict.

And as for storing, we are using a small trick here. Instead of allocating a dedicated buffer for the tag (i.e. another prz), or hacking ram_core routines to "reserve" some control data in the buffer, we are just encoding the tag into the buffer signature (and XOR'ing it with the actual signature value, so that buffers not needing a tag can just pass zero, which will result into the plain old PRZ signature).

---
 fs/pstore/Kconfig          |    1 +
 fs/pstore/ram.c            |   13 ++++++++-----
 fs/pstore/ram_core.c       |   12 +++++++-----
 include/linux/pstore_ram.h |    2 +-
 4 files changed, 17 insertions(+), 11 deletions(-)
fs/pstore/Kconfig +1 -0
index d39bb5c..a123754 100644
@@ -36,6 +36,7 @@ config PSTORE_RAM
 	depends on PSTORE
 	depends on HAS_IOMEM
 	depends on HAVE_MEMBLOCK
+	select CRC32
 	select REED_SOLOMON
 	select REED_SOLOMON_ENC8
 	select REED_SOLOMON_DEC8
fs/pstore/ram.c +8 -5
index 2cc029f..3716ca9 100644
@@ -31,6 +31,7 @@
 #include <linux/ioport.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/crc32.h>
 #include <linux/pstore_ram.h>
 
 #define RAMOOPS_KERNMSG_HDR "===="
@@ -307,7 +308,7 @@ static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt,
 	for (i = 0; i < cxt->max_dump_cnt; i++) {
 		size_t sz = cxt->record_size;
 
-		cxt->przs[i] = persistent_ram_new(*paddr, sz, cxt->ecc_size);
+		cxt->przs[i] = persistent_ram_new(*paddr, sz, 0, cxt->ecc_size);
 		if (IS_ERR(cxt->przs[i])) {
 			err = PTR_ERR(cxt->przs[i]);
 			dev_err(dev, "failed to request mem region (0x%zx@0x%llx): %d\n",
@@ -325,7 +326,7 @@ fail_prz:
 
 static int ramoops_init_prz(struct device *dev, struct ramoops_context *cxt,
 			    struct persistent_ram_zone **prz,
-			    phys_addr_t *paddr, size_t sz)
+			    phys_addr_t *paddr, size_t sz, u32 sig)
 {
 	if (!sz)
 		return 0;
@@ -333,7 +334,7 @@ static int ramoops_init_prz(struct device *dev, struct ramoops_context *cxt,
 	if (*paddr + sz > *paddr + cxt->size)
 		return -ENOMEM;
 
-	*prz = persistent_ram_new(*paddr, sz, cxt->ecc_size);
+	*prz = persistent_ram_new(*paddr, sz, sig, cxt->ecc_size);
 	if (IS_ERR(*prz)) {
 		int err = PTR_ERR(*prz);
 
@@ -392,11 +393,13 @@ static int __devinit ramoops_probe(struct platform_device *pdev)
 	if (err)
 		goto fail_out;
 
-	err = ramoops_init_prz(dev, cxt, &cxt->cprz, &paddr, cxt->console_size);
+	err = ramoops_init_prz(dev, cxt, &cxt->cprz, &paddr,
+			       cxt->console_size, 0);
 	if (err)
 		goto fail_init_cprz;
 
-	err = ramoops_init_prz(dev, cxt, &cxt->fprz, &paddr, cxt->ftrace_size);
+	err = ramoops_init_prz(dev, cxt, &cxt->fprz, &paddr, cxt->ftrace_size,
+			       crc32_le(0, linux_banner, strlen(linux_banner)));
 	if (err)
 		goto fail_init_fprz;
 
fs/pstore/ram_core.c +7 -5
index 4dabbb8..eecd2a8 100644
@@ -391,7 +391,7 @@ static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
 }
 
 static int __devinit persistent_ram_post_init(struct persistent_ram_zone *prz,
-					      int ecc_size)
+					      u32 sig, int ecc_size)
 {
 	int ret;
 
@@ -399,7 +399,9 @@ static int __devinit persistent_ram_post_init(struct persistent_ram_zone *prz,
 	if (ret)
 		return ret;
 
-	if (prz->buffer->sig == PERSISTENT_RAM_SIG) {
+	sig ^= PERSISTENT_RAM_SIG;
+
+	if (prz->buffer->sig == sig) {
 		if (buffer_size(prz) > prz->buffer_size ||
 		    buffer_start(prz) > buffer_size(prz))
 			pr_info("persistent_ram: found existing invalid buffer,"
@@ -417,7 +419,7 @@ static int __devinit persistent_ram_post_init(struct persistent_ram_zone *prz,
 			" (sig = 0x%08x)\n", prz->buffer->sig);
 	}
 
-	prz->buffer->sig = PERSISTENT_RAM_SIG;
+	prz->buffer->sig = sig;
 	persistent_ram_zap(prz);
 
 	return 0;
@@ -442,7 +444,7 @@ void persistent_ram_free(struct persistent_ram_zone *prz)
 }
 
 struct persistent_ram_zone * __devinit persistent_ram_new(phys_addr_t start,
-							  size_t size,
+							  size_t size, u32 sig,
 							  int ecc_size)
 {
 	struct persistent_ram_zone *prz;
@@ -458,7 +460,7 @@ struct persistent_ram_zone * __devinit persistent_ram_new(phys_addr_t start,
 	if (ret)
 		goto err;
 
-	ret = persistent_ram_post_init(prz, ecc_size);
+	ret = persistent_ram_post_init(prz, sig, ecc_size);
 	if (ret)
 		goto err;
 
include/linux/pstore_ram.h +1 -1
index af848e1..1d779c4 100644
@@ -46,7 +46,7 @@ struct persistent_ram_zone {
 };
 
 struct persistent_ram_zone * __devinit persistent_ram_new(phys_addr_t start,
-							  size_t size,
+							  size_t size, u32 sig,
 							  int ecc_size);
 void persistent_ram_free(struct persistent_ram_zone *prz);
 void persistent_ram_zap(struct persistent_ram_zone *prz);
Signature · 1 line
1.7.10.4