Skip to contentSkip to conversation

Merge ramoops and persistent_ram, generic pstore RAM backend

PATCH Superseded Anton Vorontsov 5 participants 17 messages May 16, 2012 – May 17, 2012

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover Merge ramoops and persistent_ram, generic pstore RAM backend

Hi all,

Here is v2 of the previous patch set. The series do not include patches that were already merged.

I believe I addressed all the previous comments, plus now the series include another small cleanup patch.

Here's some background for the series:

There are currently two competing debug facilities to store kernel messages in a persistent storage: a generic pstore and Google's persistent_ram. Not so long ago (https://lkml.org/lkml/2012/3/8/252),
it was decided that we should fix this situation.

Recently ramoops has switched to pstore, which basically means that it became a RAM backend for the pstore framework.

persistent_ram+ram_console and ramoops+pstore have almost the same features, except:

1. Ramoops doesn't support ECC. Having ECC is useful when a hardware reset was used to bring the machine back to life (i.e. a watchdog triggered). In such cases, RAM may be somewhat corrupt, but usually it is restorable.

2. Pstore doesn't support logging kernel messages in run-time, it only dumps dmesg when kernel oopses/panics. This makes pstore useless for debugging hangs caused by HW issues or improper use of HW (e.g. weird device inserted -> driver tried to write a reserved bits -> SoC hanged. In that case we don't get any messages in the pstore.

These patches solve the first issue, plus move things to their proper places. Patches that will fix the second issue will be sent shortly.

---

 Documentation/ramoops.txt                |    8 +-
 drivers/char/Kconfig                     |    9 -
 drivers/char/Makefile                    |    1 -
 drivers/char/ramoops.c                   |  362 --------------------
 drivers/staging/android/Kconfig          |   10 +-
 drivers/staging/android/Makefile         |    1 -
 drivers/staging/android/persistent_ram.c |  530 -----------------------------
 drivers/staging/android/persistent_ram.h |   84 -----
 drivers/staging/android/ram_console.c    |    2 +-
 fs/pstore/Kconfig                        |   17 +
 fs/pstore/Makefile                       |    3 +
 fs/pstore/ram.c                          |  383 +++++++++++++++++++++
 fs/pstore/ram_core.c                     |  532 ++++++++++++++++++++++++++++++
 include/linux/pstore_ram.h               |   98 ++++++
 include/linux/ramoops.h                  |   17 -
 15 files changed, 1042 insertions(+), 1015 deletions(-)
Signature · 2 lines
Anton Vorontsov
Email: [email protected]
#1 lore raw

PATCH v2 1/6 persistent_ram: Remove never used struct persistent_ram_zone members

Remove 'old_log_footer_size' and 'early' members of struct persistent_ram_zone, as these were never used at all.

---
 drivers/staging/android/persistent_ram.h |    2 --
 1 file changed, 2 deletions(-)
drivers/staging/android/persistent_ram.h +0 -2
index d3b2b45..d23850c 100644
@@ -59,8 +59,6 @@ struct persistent_ram_zone {
 
 	char *old_log;
 	size_t old_log_size;
-	size_t old_log_footer_size;
-	bool early;
 };
 
 int persistent_ram_early_init(struct persistent_ram *ram);
Signature · 1 line
1.7.9.2
#2 lore raw

PATCH v2 2/6 ramoops: Move to fs/pstore/ram.c

In reply to Anton Vorontsov

Since ramoops was converted to pstore, it has nothing to do with character devices nowadays. Instead, today it is just a RAM backend for pstore.

The patch just moves things around. There are a few changes were needed because of the move:

1. Kconfig and Makefiles fixups, of course.

2. In pstore/ram.c we have to play a bit with MODULE_PARAM_PREFIX, this is needed to keep user experience the same as with ramoops driver (i.e. so that ramoops.foo kernel command line arguments would still work).

---
 Documentation/ramoops.txt  |    2 +-
 drivers/char/Kconfig       |    9 --
 drivers/char/Makefile      |    1 -
 drivers/char/ramoops.c     |  362 --------------------------------------------
 fs/pstore/Kconfig          |   14 ++
 fs/pstore/Makefile         |    3 +
 fs/pstore/ram.c            |  362 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pstore_ram.h |   17 +++
 include/linux/ramoops.h    |   17 ---
 9 files changed, 397 insertions(+), 390 deletions(-)
 delete mode 100644 drivers/char/ramoops.c
 create mode 100644 fs/pstore/ram.c
 create mode 100644 include/linux/pstore_ram.h
 delete mode 100644 include/linux/ramoops.h
Documentation/ramoops.txt +1 -1
index a0b9d8e..470d2c4 100644
@@ -38,7 +38,7 @@ Setting the ramoops parameters can be done in 2 different manners:
  2. Use a platform device and set the platform data. The parameters can then
  be set through that platform data. An example of doing that is:
 
-#include <linux/ramoops.h>
+#include <linux/pstore_ram.h>
 [...]
 
 static struct ramoops_platform_data ramoops_data = {
drivers/char/Kconfig +0 -9
index fab778d4..ea6f632 100644
@@ -585,15 +585,6 @@ config DEVPORT
 
 source "drivers/s390/char/Kconfig"
 
-config RAMOOPS
-	tristate "Log panic/oops to a RAM buffer"
-	depends on HAS_IOMEM
-	depends on PSTORE
-	default n
-	help
-	  This enables panic and oops messages to be logged to a circular
-	  buffer in RAM where it can be read back at some later point.
-
 config MSM_SMD_PKT
 	bool "Enable device interface for some SMD packet ports"
 	default n
drivers/char/Makefile +0 -1
index 0dc5d7c..d0b27a3 100644
@@ -58,7 +58,6 @@ obj-$(CONFIG_HANGCHECK_TIMER)	+= hangcheck-timer.o
 obj-$(CONFIG_TCG_TPM)		+= tpm/
 
 obj-$(CONFIG_PS3_FLASH)		+= ps3flash.o
-obj-$(CONFIG_RAMOOPS)		+= ramoops.o
 
 obj-$(CONFIG_JS_RTC)		+= js-rtc.o
 js-rtc-y = rtc.o
drivers/char/ramoops.c deleted +0 -362

362 lines not on this page · raw message

fs/pstore/Kconfig +14 -0
index 8007ae7..b75ee51 100644
@@ -11,3 +11,17 @@ config PSTORE
 	   (e.g. ACPI_APEI on X86) which will select this for you.
 	   If you don't have a platform persistent store driver,
 	   say N.
+
+config PSTORE_RAM
+	tristate "Log panic/oops to a RAM buffer"
+	depends on HAS_IOMEM
+	depends on PSTORE
+	default n
+	help
+	  This enables panic and oops messages to be logged to a circular
+	  buffer in RAM where it can be read back at some later point.
+
+	  Note that for historical reasons, the module will be named
+	  "ramoops.ko".
+
+	  For more information, see Documentation/ramoops.txt.
fs/pstore/Makefile +3 -0
index 760f4bc..2ab3d0d 100644
@@ -5,3 +5,6 @@
 obj-y += pstore.o
 
 pstore-objs += inode.o platform.o
+
+ramoops-objs += ram.o
+obj-$(CONFIG_PSTORE_RAM)	+= ramoops.o
fs/pstore/ram.c new file +362 -0

362 lines not on this page · raw message

include/linux/pstore_ram.h new file +17 -0
new file mode 100644
index 0000000..fa4cb02
@@ -0,0 +1,17 @@
+#ifndef __LINUX_PSTORE_RAM_H__
+#define __LINUX_PSTORE_RAM_H__
+
+/*
+ * Ramoops platform data
+ * @mem_size	memory size for ramoops
+ * @mem_address	physical memory address to contain ramoops
+ */
+
+struct ramoops_platform_data {
+	unsigned long	mem_size;
+	unsigned long	mem_address;
+	unsigned long	record_size;
+	int		dump_oops;
+};
+
+#endif
include/linux/ramoops.h deleted +0 -17
deleted file mode 100644
index 484fef8..0000000
@@ -1,17 +0,0 @@
-#ifndef __RAMOOPS_H
-#define __RAMOOPS_H
-
-/*
- * Ramoops platform data
- * @mem_size	memory size for ramoops
- * @mem_address	physical memory address to contain ramoops
- */
-
-struct ramoops_platform_data {
-	unsigned long	mem_size;
-	unsigned long	mem_address;
-	unsigned long	record_size;
-	int		dump_oops;
-};
-
-#endif
Signature · 1 line
1.7.9.2
#7 lore raw

PATCH v2 3/6 persistent_ram: Prepare for modular builds

In reply to Anton Vorontsov

This is a transition patch to keep things bisectable, just moves some routines under '#ifndef MODULE'. The code inside the #ifndef will go away soon, but so far we must support pstore and ram_console.

So, we are about to use persistent_ram with pstore, with the ability to compile persistent_ram routines as modules. Some parts of persistent_ram uses memblock_reserve() routine, which is should be only used built-in code, and thus it is not exported.

These persistent_ram bits are only used by Android's ram_console, which is always built-in.

Without this patch, we won't able to compile persistent_ram as a module:

ERROR: "memblock_reserve" [fs/pstore/ramoops.ko] undefined! make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

(As alternative, we could export memblock_reserve, but the thing
is: we won't need it later.)

---
 drivers/staging/android/persistent_ram.c |   46 ++++++++++++++++--------------
 1 file changed, 24 insertions(+), 22 deletions(-)
drivers/staging/android/persistent_ram.c +24 -22
index 63481da..4b46eaa 100644
@@ -384,28 +384,6 @@ static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
 	return 0;
 }
 
-static int __init persistent_ram_buffer_init(const char *name,
-		struct persistent_ram_zone *prz)
-{
-	int i;
-	struct persistent_ram *ram;
-	struct persistent_ram_descriptor *desc;
-	phys_addr_t start;
-
-	list_for_each_entry(ram, &persistent_ram_list, node) {
-		start = ram->start;
-		for (i = 0; i < ram->num_descs; i++) {
-			desc = &ram->descs[i];
-			if (!strcmp(desc->name, name))
-				return persistent_ram_buffer_map(start,
-						desc->size, prz);
-			start += desc->size;
-		}
-	}
-
-	return -EINVAL;
-}
-
 static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool ecc)
 {
 	int ret;
@@ -478,6 +456,29 @@ err:
 	return ERR_PTR(ret);
 }
 
+#ifndef MODULE
+static int __init persistent_ram_buffer_init(const char *name,
+		struct persistent_ram_zone *prz)
+{
+	int i;
+	struct persistent_ram *ram;
+	struct persistent_ram_descriptor *desc;
+	phys_addr_t start;
+
+	list_for_each_entry(ram, &persistent_ram_list, node) {
+		start = ram->start;
+		for (i = 0; i < ram->num_descs; i++) {
+			desc = &ram->descs[i];
+			if (!strcmp(desc->name, name))
+				return persistent_ram_buffer_map(start,
+						desc->size, prz);
+			start += desc->size;
+		}
+	}
+
+	return -EINVAL;
+}
+
 static  __init
 struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
 {
@@ -528,3 +529,4 @@ int __init persistent_ram_early_init(struct persistent_ram *ram)
 
 	return 0;
 }
+#endif
Signature · 1 line
1.7.9.2
#8 lore raw

PATCH v2 4/6 persistent_ram: Move to fs/pstore/ram_core.c

In reply to Anton Vorontsov

This is a first step for adding ECC support for pstore RAM backend: we will use the persistent_ram routines, kindly provided by Google.

Basically, persistent_ram is a set of helper routines to deal with the [optionally] ECC-protected persistent ram regions.

A bit of Makefile, Kconfig and header files adjustments were needed because of the move.

---
 drivers/staging/android/Kconfig          |   10 +-
 drivers/staging/android/Makefile         |    1 -
 drivers/staging/android/persistent_ram.c |  532 ------------------------------
 drivers/staging/android/persistent_ram.h |   82 -----
 drivers/staging/android/ram_console.c    |    2 +-
 fs/pstore/Kconfig                        |    7 +-
 fs/pstore/Makefile                       |    2 +-
 fs/pstore/ram_core.c                     |  532 ++++++++++++++++++++++++++++++
 include/linux/pstore_ram.h               |   80 +++++
 9 files changed, 620 insertions(+), 628 deletions(-)
 delete mode 100644 drivers/staging/android/persistent_ram.c
 delete mode 100644 drivers/staging/android/persistent_ram.h
 create mode 100644 fs/pstore/ram_core.c
drivers/staging/android/Kconfig +1 -9
index 42f0133..4bfcceb 100644
@@ -25,17 +25,9 @@ config ANDROID_LOGGER
 	tristate "Android log driver"
 	default n
 
-config ANDROID_PERSISTENT_RAM
-	bool
-	depends on HAVE_MEMBLOCK
-	select REED_SOLOMON
-	select REED_SOLOMON_ENC8
-	select REED_SOLOMON_DEC8
-
 config ANDROID_RAM_CONSOLE
 	bool "Android RAM buffer console"
-	depends on !S390 && !UML && HAVE_MEMBLOCK
-	select ANDROID_PERSISTENT_RAM
+	depends on !S390 && !UML && HAVE_MEMBLOCK && PSTORE_RAM
 	default n
 
 config ANDROID_TIMED_OUTPUT
drivers/staging/android/Makefile +0 -1
index 2de5c10..98711e2 100644
@@ -1,7 +1,6 @@
 obj-$(CONFIG_ANDROID_BINDER_IPC)	+= binder.o
 obj-$(CONFIG_ASHMEM)			+= ashmem.o
 obj-$(CONFIG_ANDROID_LOGGER)		+= logger.o
-obj-$(CONFIG_ANDROID_PERSISTENT_RAM)	+= persistent_ram.o
 obj-$(CONFIG_ANDROID_RAM_CONSOLE)	+= ram_console.o
 obj-$(CONFIG_ANDROID_TIMED_OUTPUT)	+= timed_output.o
 obj-$(CONFIG_ANDROID_TIMED_GPIO)	+= timed_gpio.o
drivers/staging/android/persistent_ram.c deleted +0 -532

532 lines not on this page · raw message

drivers/staging/android/persistent_ram.h deleted +0 -82

82 lines not on this page · raw message

drivers/staging/android/ram_console.c +1 -1
index ce140ff..82323bb 100644
@@ -21,7 +21,7 @@
 #include <linux/string.h>
 #include <linux/uaccess.h>
 #include <linux/io.h>
-#include "persistent_ram.h"
+#include <linux/pstore_ram.h>
 #include "ram_console.h"
 
 static struct persistent_ram_zone *ram_console_zone;
fs/pstore/Kconfig +5 -2
index b75ee51..23ade26 100644
@@ -14,9 +14,12 @@ config PSTORE
 
 config PSTORE_RAM
 	tristate "Log panic/oops to a RAM buffer"
-	depends on HAS_IOMEM
 	depends on PSTORE
-	default n
+	depends on HAS_IOMEM
+	depends on HAVE_MEMBLOCK
+	select REED_SOLOMON
+	select REED_SOLOMON_ENC8
+	select REED_SOLOMON_DEC8
 	help
 	  This enables panic and oops messages to be logged to a circular
 	  buffer in RAM where it can be read back at some later point.
fs/pstore/Makefile +1 -1
index 2ab3d0d..278a44e 100644
@@ -6,5 +6,5 @@ obj-y += pstore.o
 
 pstore-objs += inode.o platform.o
 
-ramoops-objs += ram.o
+ramoops-objs += ram.o ram_core.o
 obj-$(CONFIG_PSTORE_RAM)	+= ramoops.o
fs/pstore/ram_core.c new file +532 -0

532 lines not on this page · raw message

include/linux/pstore_ram.h +80 -0

86 lines not on this page · raw message

Signature · 1 line
1.7.9.2
#14 lore raw

PATCH v2 5/6 pstore/ram: Switch to persistent_ram routines

In reply to Anton Vorontsov

The patch switches pstore RAM backend to use persistent_ram routines, one step closer to the ECC support.

---
 fs/pstore/ram.c |  106 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 59 insertions(+), 47 deletions(-)
fs/pstore/ram.c +59 -47
index e443c9c..62b13ed 100644
@@ -57,7 +57,7 @@ MODULE_PARM_DESC(dump_oops,
 		"set to 1 to dump oopses, 0 to only dump panics (default 1)");
 
 struct ramoops_context {
-	void *virt_addr;
+	struct persistent_ram_zone **przs;
 	phys_addr_t phys_addr;
 	unsigned long size;
 	size_t record_size;
@@ -85,39 +85,56 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
 				   struct pstore_info *psi)
 {
 	ssize_t size;
-	char *rambuf;
 	struct ramoops_context *cxt = psi->data;
+	struct persistent_ram_zone *prz;
 
 	if (cxt->read_count >= cxt->max_count)
 		return -EINVAL;
+
 	*id = cxt->read_count++;
+	prz = cxt->przs[*id];
+
 	/* Only supports dmesg output so far. */
 	*type = PSTORE_TYPE_DMESG;
 	/* TODO(kees): Bogus time for the moment. */
 	time->tv_sec = 0;
 	time->tv_nsec = 0;
 
-	rambuf = cxt->virt_addr + (*id * cxt->record_size);
-	size = strnlen(rambuf, cxt->record_size);
+	size = persistent_ram_old_size(prz);
 	*buf = kmalloc(size, GFP_KERNEL);
 	if (*buf == NULL)
 		return -ENOMEM;
-	memcpy(*buf, rambuf, size);
+	memcpy(*buf, persistent_ram_old(prz), size);
 
 	return size;
 }
 
+static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz)
+{
+	char *hdr;
+	struct timeval timestamp;
+	size_t len;
+
+	do_gettimeofday(&timestamp);
+	hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lu.%lu\n",
+		(long)timestamp.tv_sec, (long)timestamp.tv_usec);
+	WARN_ON_ONCE(!hdr);
+	len = hdr ? strlen(hdr) : 0;
+	persistent_ram_write(prz, hdr, len);
+	kfree(hdr);
+
+	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)
 {
-	char *buf;
-	size_t res;
-	struct timeval timestamp;
 	struct ramoops_context *cxt = psi->data;
-	size_t available = cxt->record_size;
+	struct persistent_ram_zone *prz = cxt->przs[cxt->count];
+	size_t hlen;
 
 	/* Currently ramoops is designed to only store dmesg dumps. */
 	if (type != PSTORE_TYPE_DMESG)
@@ -142,22 +159,10 @@ static int ramoops_pstore_write(enum pstore_type_id type,
 	if (part != 1)
 		return -ENOSPC;
 
-	buf = cxt->virt_addr + (cxt->count * cxt->record_size);
-
-	res = sprintf(buf, "%s", RAMOOPS_KERNMSG_HDR);
-	buf += res;
-	available -= res;
-
-	do_gettimeofday(&timestamp);
-	res = sprintf(buf, "%lu.%lu\n", (long)timestamp.tv_sec, (long)timestamp.tv_usec);
-	buf += res;
-	available -= res;
-
-	if (size > available)
-		size = available;
-
-	memcpy(buf, cxt->pstore.buf, size);
-	memset(buf + size, '\0', available - size);
+	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);
 
 	cxt->count = (cxt->count + 1) % cxt->max_count;
 
@@ -167,14 +172,12 @@ static int ramoops_pstore_write(enum pstore_type_id type,
 static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
 				struct pstore_info *psi)
 {
-	char *buf;
 	struct ramoops_context *cxt = psi->data;
 
 	if (id >= cxt->max_count)
 		return -EINVAL;
 
-	buf = cxt->virt_addr + (id * cxt->record_size);
-	memset(buf, '\0', cxt->record_size);
+	persistent_ram_free_old(cxt->przs[id]);
 
 	return 0;
 }
@@ -192,9 +195,11 @@ static struct ramoops_context oops_cxt = {
 
 static int __init ramoops_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct ramoops_platform_data *pdata = pdev->dev.platform_data;
 	struct ramoops_context *cxt = &oops_cxt;
 	int err = -EINVAL;
+	int i;
 
 	/* Only a single ramoops area allowed at a time, so fail extra
 	 * probes.
@@ -232,8 +237,28 @@ static int __init ramoops_probe(struct platform_device *pdev)
 	cxt->record_size = pdata->record_size;
 	cxt->dump_oops = pdata->dump_oops;
 
+	cxt->przs = kzalloc(sizeof(*cxt->przs) * cxt->max_count, GFP_KERNEL);
+	if (!cxt->przs) {
+		err = -ENOMEM;
+		dev_err(dev, "failed to initialize a prz array\n");
+		goto fail_out;
+	}
+
+	for (i = 0; i < cxt->max_count; i++) {
+		size_t sz = cxt->record_size;
+		phys_addr_t start = cxt->phys_addr + sz * i;
+
+		cxt->przs[i] = persistent_ram_new(start, sz, 0);
+		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",
+				sz, (unsigned long long)start, err);
+			goto fail_przs;
+		}
+	}
+
 	cxt->pstore.data = cxt;
-	cxt->pstore.bufsize = cxt->record_size;
+	cxt->pstore.bufsize = cxt->przs[0]->buffer_size;
 	cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
 	spin_lock_init(&cxt->pstore.buf_lock);
 	if (!cxt->pstore.buf) {
@@ -241,23 +266,10 @@ static int __init ramoops_probe(struct platform_device *pdev)
 		goto fail_clear;
 	}
 
-	if (!request_mem_region(cxt->phys_addr, cxt->size, "ramoops")) {
-		pr_err("request mem region (0x%lx@0x%llx) failed\n",
-			cxt->size, (unsigned long long)cxt->phys_addr);
-		err = -EINVAL;
-		goto fail_buf;
-	}
-
-	cxt->virt_addr = ioremap(cxt->phys_addr,  cxt->size);
-	if (!cxt->virt_addr) {
-		pr_err("ioremap failed\n");
-		goto fail_mem_region;
-	}
-
 	err = pstore_register(&cxt->pstore);
 	if (err) {
 		pr_err("registering with pstore failed\n");
-		goto fail_iounmap;
+		goto fail_buf;
 	}
 
 	/*
@@ -275,15 +287,15 @@ static int __init ramoops_probe(struct platform_device *pdev)
 
 	return 0;
 
-fail_iounmap:
-	iounmap(cxt->virt_addr);
-fail_mem_region:
-	release_mem_region(cxt->phys_addr, cxt->size);
 fail_buf:
 	kfree(cxt->pstore.buf);
 fail_clear:
 	cxt->pstore.bufsize = 0;
 	cxt->max_count = 0;
+fail_przs:
+	for (i = 0; cxt->przs[i]; i++)
+		persistent_ram_free(cxt->przs[i]);
+	kfree(cxt->przs);
 fail_out:
 	return err;
 }
Signature · 1 line
1.7.9.2
#15 lore raw

PATCH v2 6/6 pstore/ram: Add ECC support

In reply to Anton Vorontsov

This is now straightforward: just introduce a module parameter and pass the needed value to persistent_ram_new().

---
 Documentation/ramoops.txt  |    6 ++++++
 fs/pstore/ram.c            |   15 ++++++++++++---
 include/linux/pstore_ram.h |    1 +
 3 files changed, 19 insertions(+), 3 deletions(-)
Documentation/ramoops.txt +6 -0
index 470d2c4..4ba7db2 100644
@@ -30,6 +30,11 @@ variable while setting 0 in that variable dumps only the panics.
 The module uses a counter to record multiple dumps but the counter gets reset
 on restart (i.e. new dumps after the restart will overwrite old ones).
 
+Ramoops also supports software ECC protection of persistent memory regions.
+This might be useful when a hardware reset was used to bring the machine back
+to life (i.e. a watchdog triggered). In such cases, RAM may be somewhat
+corrupt, but usually it is restorable.
+
 2. Setting the parameters
 
 Setting the ramoops parameters can be done in 2 different manners:
@@ -46,6 +51,7 @@ static struct ramoops_platform_data ramoops_data = {
         .mem_address            = <...>,
         .record_size            = <...>,
         .dump_oops              = <...>,
+        .ecc                    = <...>,
 };
 
 static struct platform_device ramoops_dev = {
fs/pstore/ram.c +12 -3
index 62b13ed..9123cce 100644
@@ -56,12 +56,18 @@ module_param(dump_oops, int, 0600);
 MODULE_PARM_DESC(dump_oops,
 		"set to 1 to dump oopses, 0 to only dump panics (default 1)");
 
+static int ramoops_ecc;
+module_param_named(ecc, ramoops_ecc, int, 0600);
+MODULE_PARM_DESC(ramoops_ecc,
+		"set to 1 to enable ECC support");
+
 struct ramoops_context {
 	struct persistent_ram_zone **przs;
 	phys_addr_t phys_addr;
 	unsigned long size;
 	size_t record_size;
 	int dump_oops;
+	bool ecc;
 	unsigned int count;
 	unsigned int max_count;
 	unsigned int read_count;
@@ -236,6 +242,7 @@ static int __init ramoops_probe(struct platform_device *pdev)
 	cxt->phys_addr = pdata->mem_address;
 	cxt->record_size = pdata->record_size;
 	cxt->dump_oops = pdata->dump_oops;
+	cxt->ecc = pdata->ecc;
 
 	cxt->przs = kzalloc(sizeof(*cxt->przs) * cxt->max_count, GFP_KERNEL);
 	if (!cxt->przs) {
@@ -248,7 +255,7 @@ static int __init ramoops_probe(struct platform_device *pdev)
 		size_t sz = cxt->record_size;
 		phys_addr_t start = cxt->phys_addr + sz * i;
 
-		cxt->przs[i] = persistent_ram_new(start, sz, 0);
+		cxt->przs[i] = persistent_ram_new(start, sz, cxt->ecc);
 		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",
@@ -281,9 +288,10 @@ static int __init ramoops_probe(struct platform_device *pdev)
 	record_size = pdata->record_size;
 	dump_oops = pdata->dump_oops;
 
-	pr_info("attached 0x%lx@0x%llx (%ux0x%zx)\n",
+	pr_info("attached 0x%lx@0x%llx (%ux0x%zx), ecc: %s\n",
 		cxt->size, (unsigned long long)cxt->phys_addr,
-		cxt->max_count, cxt->record_size);
+		cxt->max_count, cxt->record_size,
+		ramoops_ecc ? "on" : "off");
 
 	return 0;
 
@@ -347,6 +355,7 @@ static int __init ramoops_init(void)
 		dummy_data->mem_address = mem_address;
 		dummy_data->record_size = record_size;
 		dummy_data->dump_oops = dump_oops;
+		dummy_data->ecc = ramoops_ecc;
 		dummy = platform_create_bundle(&ramoops_driver, ramoops_probe,
 			NULL, 0, dummy_data,
 			sizeof(struct ramoops_platform_data));
include/linux/pstore_ram.h +1 -0
index ffe24a5..7ed7fd4 100644
@@ -92,6 +92,7 @@ struct ramoops_platform_data {
 	unsigned long	mem_address;
 	unsigned long	record_size;
 	int		dump_oops;
+	bool		ecc;
 };
 
 #endif
Signature · 1 line
1.7.9.2
Acked-by on v2 2/6
#3 lore raw

Re: [PATCH 2/6] ramoops: Move to fs/pstore/ram.c

In reply to Anton Vorontsov
Quoted 8 lines · Anton Vorontsov

Since ramoops was converted to pstore, it has nothing to do with character devices nowadays. Instead, today it is just a RAM backend for pstore.

The patch just moves things around. There are a few changes were needed because of the move:

1. Kconfig and Makefiles fixups, of course.

Quoted 7 lines

2. In pstore/ram.c we have to play a bit with MODULE_PARAM_PREFIX, this is needed to keep user experience the same as with ramoops driver (i.e. so that ramoops.foo kernel command line arguments would still work).

Signature · 2 lines
Kees Cook
Chrome OS Security
#4 lore raw

Re: [PATCH 2/6] ramoops: Move to fs/pstore/ram.c

In reply to Anton Vorontsov
Quoted 16 lines · Anton Vorontsov

Since ramoops was converted to pstore, it has nothing to do with character devices nowadays. Instead, today it is just a RAM backend for pstore.

The patch just moves things around. There are a few changes were needed because of the move:

1. Kconfig and Makefiles fixups, of course.

2. In pstore/ram.c we have to play a bit with MODULE_PARAM_PREFIX, this is needed to keep user experience the same as with ramoops driver (i.e. so that ramoops.foo kernel command line arguments would still work).

Quoted 15 lines
---
 Documentation/ramoops.txt  |    2 +-
 drivers/char/Kconfig       |    9 --
 drivers/char/Makefile      |    1 -
 drivers/char/ramoops.c     |  362 --------------------------------------------
 fs/pstore/Kconfig          |   14 ++
 fs/pstore/Makefile         |    3 +
 fs/pstore/ram.c            |  362 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pstore_ram.h |   17 +++
 include/linux/ramoops.h    |   17 ---
 9 files changed, 397 insertions(+), 390 deletions(-)
 delete mode 100644 drivers/char/ramoops.c
 create mode 100644 fs/pstore/ram.c
 create mode 100644 include/linux/pstore_ram.h
 delete mode 100644 include/linux/ramoops.h

This patch fails to apply to my staging-next tree due to problems with drivers/char/Kconfig and drivers/char/ramoops.c. What tree are you making this patch against?

drivers/char/Kconfig config DEVPORT · line 591

Quoted 10 lines
drivers/char/Kconfig +0 -4
@@ -585,15 +585,6 @@ config DEVPORT
 
 source "drivers/s390/char/Kconfig"
 
-config RAMOOPS
-	tristate "Log panic/oops to a RAM buffer"
-	depends on HAS_IOMEM
-	depends on PSTORE

This "depends on PSTORE" line is not in my staging-next tree, where did it come from?

I'm guessing that because of this, is why the other conflict happens, right?

Any ideas on what I can do to resolve it? As you are moving a file, I want to make sure I move the correct version of it :)

thanks,

greg k-h

#16 lore raw
In reply to Anton Vorontsov
Quoted 7 lines · Anton Vorontsov

Hi all,

Here is v2 of the previous patch set. The series do not include patches that were already merged.

I believe I addressed all the previous comments, plus now the series include another small cleanup patch.

I've applied patches 1 and 3 of this series to my staging-next tree, the others would not apply due to the failure in patch 2. Care to fix that up and resend so that I can apply them?

thanks,

greg k-h

#5 lore raw

Re: [PATCH 2/6] ramoops: Move to fs/pstore/ram.c

In reply to Greg Kroah-Hartman

On Wed, May 16, 2012 at 06:17:51AM -0700, Greg Kroah-Hartman wrote:
[...]

Quoted 6 lines
Quoted 6 lines
drivers/char/Kconfig +0 -0
@@ -585,15 +585,6 @@ config DEVPORT
 
 source "drivers/s390/char/Kconfig"
 
Quoted 7 lines

drivers/char/Kconfig config DEVPORT · line 591

Quoted 4 lines
drivers/char/Kconfig +0 -4
-config RAMOOPS
-	tristate "Log panic/oops to a RAM buffer"
-	depends on HAS_IOMEM
-	depends on PSTORE

This "depends on PSTORE" line is not in my staging-next tree, where did it come from?

The thing is that ramoops switched to pstore just recently, and the patch is in -mm tree.

(Just for the record, I use linux-next-20120504, HEAD at commit 44449d2b34aca3b1)

Quoted 2 lines

I'm guessing that because of this, is why the other conflict happens, right?

Yep.

Quoted 2 lines

Any ideas on what I can do to resolve it? As you are moving a file, I want to make sure I move the correct version of it :)

There's a cross-tree dependency with -mm tree, as it appears.

So, I'm not sure which is better:

1. Push the whole pile of patches via -mm; or

2. You apply the two patches from -mm tree before my series:
"ramoops: use pstore interface"
linux-next commit 60aad9fc1f04ca19e800c5984ad84f405d133a11
and
"ramoops: fix printk format warnings" linux-next commit f2a28afdb529c43a7eb21fd0c991cab5676ef011

http://userweb.kernel.org/~akpm/mmotm/ seem to not respond at the moment, so I guess the easiest way to grab the patches would be to cherry-pick the two commits above from the linux-next repository. That way Andrew would automatically drop them from his tree.

Thanks!

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

Re: [PATCH 2/6] ramoops: Move to fs/pstore/ram.c

Quoted 43 lines · Anton Vorontsov

On Wed, May 16, 2012 at 06:17:51AM -0700, Greg Kroah-Hartman wrote:
[...]

Quoted 6 lines
Quoted 6 lines
drivers/char/Kconfig +0 -0
@@ -585,15 +585,6 @@ config DEVPORT
 
 source "drivers/s390/char/Kconfig"
 
Quoted 7 lines

drivers/char/Kconfig config DEVPORT · line 591

Quoted 4 lines
drivers/char/Kconfig +0 -4
-config RAMOOPS
-	tristate "Log panic/oops to a RAM buffer"
-	depends on HAS_IOMEM
-	depends on PSTORE

This "depends on PSTORE" line is not in my staging-next tree, where did it come from?

The thing is that ramoops switched to pstore just recently, and the patch is in -mm tree.

(Just for the record, I use linux-next-20120504, HEAD at commit 44449d2b34aca3b1)

Quoted 2 lines

I'm guessing that because of this, is why the other conflict happens, right?

Yep.

Quoted 2 lines

Any ideas on what I can do to resolve it? As you are moving a file, I want to make sure I move the correct version of it :)

There's a cross-tree dependency with -mm tree, as it appears.

So, I'm not sure which is better:

1. Push the whole pile of patches via -mm; or

2. You apply the two patches from -mm tree before my series:
"ramoops: use pstore interface"
linux-next commit 60aad9fc1f04ca19e800c5984ad84f405d133a11
and
"ramoops: fix printk format warnings" linux-next commit f2a28afdb529c43a7eb21fd0c991cab5676ef011

Quoted 5 lines

http://userweb.kernel.org/~akpm/mmotm/ seem to not respond at the moment, so I guess the easiest way to grab the patches would be to cherry-pick the two commits above from the linux-next repository. That way Andrew would automatically drop them from his tree.

Ok, I'll pick from linux-next, that seems to be the easiest way. I'll do that later this morning and let you know how it goes...

greg k-h

#9 lore raw

Re: [PATCH 4/6] staging: android: persistent_ram: Move to fs/pstore/ram_core.c

In reply to Anton Vorontsov
Quoted 11 lines · Anton Vorontsov

This is a first step for adding ECC support for pstore RAM backend: we will use the persistent_ram routines, kindly provided by Google.

Basically, persistent_ram is a set of helper routines to deal with the [optionally] ECC-protected persistent ram regions.

A bit of Makefile, Kconfig and header files adjustments were needed because of the move.

Quoted 14 lines
---
 drivers/staging/android/Kconfig          |   10 +-
 drivers/staging/android/Makefile         |    1 -
 drivers/staging/android/persistent_ram.c |  532 ------------------------------
 drivers/staging/android/persistent_ram.h |   82 -----
 drivers/staging/android/ram_console.c    |    2 +-
 fs/pstore/Kconfig                        |    7 +-
 fs/pstore/Makefile                       |    2 +-
 fs/pstore/ram_core.c                     |  532 ++++++++++++++++++++++++++++++
 include/linux/pstore_ram.h               |   80 +++++
 9 files changed, 620 insertions(+), 628 deletions(-)
 delete mode 100644 drivers/staging/android/persistent_ram.c
 delete mode 100644 drivers/staging/android/persistent_ram.h
 create mode 100644 fs/pstore/ram_core.c

After applying this patch, and building things, I get the following errors:

drivers/built-in.o: In function `ram_console_write':
ram_console.c:(.text+0x1690b9): undefined reference to persistent_ram_write' drivers/built-in.o: In function ram_console_read_old':
ram_console.c:(.text+0x1690f7): undefined reference to persistent_ram_old_size' ram_console.c:(.text+0x169103): undefined reference to persistent_ram_old' ram_console.c:(.text+0x169136): undefined reference to persistent_ram_ecc_string' ram_console.c:(.text+0x169177): undefined reference to persistent_ram_ecc_string' drivers/built-in.o: In function `ram_console_probe':
ram_console.c:(.init.text+0xc9ea): undefined reference to persistent_ram_init_ringbuffer' drivers/built-in.o: In function ram_console_late_init':
ram_console.c:(.init.text+0xca6c): undefined reference to persistent_ram_old_size' ram_console.c:(.init.text+0xcaa2): undefined reference to persistent_ram_free_old' ram_console.c:(.init.text+0xcab4): undefined reference to persistent_ram_old_size' ram_console.c:(.init.text+0xcac3): undefined reference to persistent_ram_ecc_string'
make: *** [.tmp_vmlinux1] Error 1

What went wrong?

I've now applied the first 3 patches in this series to my tree, plus the 2 others from the linux-next tree, so care to just fix up these last three and resend them?

thanks,

greg k-h

#10 lore raw

Re: [PATCH 4/6] staging: android: persistent_ram: Move to fs/pstore/ram_core.c

Quoted 3 lines · Greg Kroah-Hartman

After applying this patch, and building things, I get the following errors:

Greg,

Quoted 5 lines

What went wrong?

I've now applied the first 3 patches in this series to my tree, plus the 2 others from the linux-next tree, so care to just fix up these last three and resend them?

Picking [PATCH 5/6] staging/android: Remove ram_console driver should fix this problem. This patches removes ram_console.c from the staging area.

-- Shuah

#11 lore raw

Re: [PATCH 4/6] staging: android: persistent_ram: Move to fs/pstore/ram_core.c

Quoted 10 lines · Shuah Khan
Quoted 3 lines · Greg Kroah-Hartman

After applying this patch, and building things, I get the following errors:

Greg,

Quoted 2 lines

What went wrong?

Quoted 7 lines
Quoted 3 lines

I've now applied the first 3 patches in this series to my tree, plus the 2 others from the linux-next tree, so care to just fix up these last three and resend them?

Picking [PATCH 5/6] staging/android: Remove ram_console driver should fix this problem. This patches removes ram_console.c from the staging area.

That's nice, but you CAN NOT BREAK THE BUILD at any step of a patch series progress.

So please rework this to prevent this from happening, as it is, I can not accept these.

greg k-h

#13 lore raw

Re: [PATCH 4/6] persistent_ram: Move to fs/pstore/ram_core.c

In reply to Anton Vorontsov
Quoted 6 lines · Anton Vorontsov

This is a first step for adding ECC support for pstore RAM backend: we will use the persistent_ram routines, kindly provided by Google.

Basically, persistent_ram is a set of helper routines to deal with the [optionally] ECC-protected persistent ram regions.

...

fs/pstore/ram_core.c line 187

Quoted 12 lines
fs/pstore/ram_core.c new file +12 -0
+static int persistent_ram_init_ecc(struct persistent_ram_zone *prz,
+       size_t buffer_size)
+{
+       int numerr;
+       struct persistent_ram_buffer *buffer = prz->buffer;
+       int ecc_blocks;
+
+       if (!prz->ecc)
+               return 0;
+
+       prz->ecc_block_size = 128;
+       prz->ecc_size = 16;

I realize you are just moving code here, but 16 ecc bytes for per 128 bytes of data can only correct 8 bad bytes, which is not always sufficient. At some point this will need to be made configurable again.

Signature · 1 line
Arve Hjønnevåg
#12 lore raw

Re: [PATCH 4/6] staging: android: persistent_ram: Move to fs/pstore/ram_core.c

In reply to Greg Kroah-Hartman

On Wed, May 16, 2012 at 08:05:50AM -0700, Greg Kroah-Hartman wrote:
[...]

Quoted 19 lines

After applying this patch, and building things, I get the following errors:

drivers/built-in.o: In function `ram_console_write':
ram_console.c:(.text+0x1690b9): undefined reference to persistent_ram_write' drivers/built-in.o: In function ram_console_read_old':
ram_console.c:(.text+0x1690f7): undefined reference to persistent_ram_old_size' ram_console.c:(.text+0x169103): undefined reference to persistent_ram_old' ram_console.c:(.text+0x169136): undefined reference to persistent_ram_ecc_string' ram_console.c:(.text+0x169177): undefined reference to persistent_ram_ecc_string' drivers/built-in.o: In function `ram_console_probe':
ram_console.c:(.init.text+0xc9ea): undefined reference to persistent_ram_init_ringbuffer' drivers/built-in.o: In function ram_console_late_init':
ram_console.c:(.init.text+0xca6c): undefined reference to persistent_ram_old_size' ram_console.c:(.init.text+0xcaa2): undefined reference to persistent_ram_free_old' ram_console.c:(.init.text+0xcab4): undefined reference to persistent_ram_old_size' ram_console.c:(.init.text+0xcac3): undefined reference to persistent_ram_ecc_string'
make: *** [.tmp_vmlinux1] Error 1

Quoted 1 line

What went wrong?

I believe that error caused by two missing bytes: '=' and 'y'. :-) RAM_CONSOLE is a bool, so it should be enabled only when PSTORE_RAM is also =y. (For some reason I keep foregetting that bool's 'depends on' may be satisfied by tristates' =m option, which easily lead to the errors above.)

Thanks for catching this!

I've applied the following hunk to this patch and will resend the left over patches soon:

drivers/staging/android/Kconfig +1 -1
index 4bfcceb..0e16b59 100644
@@ -27,7 +27,7 @@ config ANDROID_LOGGER
 
 config ANDROID_RAM_CONSOLE
 	bool "Android RAM buffer console"
-	depends on !S390 && !UML && HAVE_MEMBLOCK && PSTORE_RAM
+	depends on !S390 && !UML && HAVE_MEMBLOCK && PSTORE_RAM=y
 	default n
 
 config ANDROID_TIMED_OUTPUT
Signature · 2 lines
Anton Vorontsov
Email: [email protected]