Merge ramoops and persistent_ram, generic pstore RAM backend
Subsystems (3)
v3 cover Merge ramoops and persistent_ram, generic pstore RAM backend
Hi all,
Here comes v3:
- Rebased on current staging-next tree, so only 3 patches left;
- Fixed ram_console dependency in Kconfig (issue noticed by Greg KH).
And the boilerplate, 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.
---
Documentation/ramoops.txt | 6 + 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.c | 119 ++++--- fs/pstore/ram_core.c | 532 ++++++++++++++++++++++++++++++ include/linux/pstore_ram.h | 81 +++++ 11 files changed, 697 insertions(+), 677 deletions(-)
Signature · 2 lines
Anton Vorontsov Email: [email protected]
PATCH v3 1/3 persistent_ram: Move to fs/pstore/ram_core.c
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
@@ -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=y default n config ANDROID_TIMED_OUTPUT
drivers/staging/android/Makefile +0 -1
@@ -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
@@ -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
@@ -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
@@ -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
PATCH v3 2/3 pstore/ram: Switch to persistent_ram routines
In reply to Anton VorontsovThe 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
@@ -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 (×tamp); + 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 (×tamp); - 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
Re: [PATCH 2/3] pstore/ram: Switch to persistent_ram routines
Quoted 101 lines · Anton Vorontsov
The patch switches pstore RAM backend to use
persistent_ramroutines, one step closer to the ECC support.Signed-off-by Anton Vorontsov <[email protected]>Acked-by Marco Stornelli <[email protected]>---
fs/pstore/ram.c | 106 +++++++++++++++++++++++++++++++------------------------1 file changed, 59 insertions(+), 47 deletions(-)fs/pstore/ram.c +26
-19@@ -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(×tamp); + 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(×tamp); - res = sprintf(buf, "%lu.%lu\n", (long)timestamp.tv_sec, (long)timestamp.tv_usec); - buf += res; - available -= res;
fs/pstore/ram.c static int ramoops_pstore_write(enum pstore_type_id type, · line 165
Quoted 10 lines
fs/pstore/ram.c +4
-6- - 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);
This still needs to wipe out the remaining bytes in the buffer (the second memset above).
Quoted 5 lines
fs/pstore/ram.c +0
-0cxt->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)
fs/pstore/ram.c static int ramoops_pstore_write(enum pstore_type_id type, · line 180
Quoted 10 lines
fs/pstore/ram.c +1
-3{ - 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]);
Same here -- erasing the buffer means wiping it with NULL bytes.
Otherwise, it looks good to me.
-Kees
Signature · 2 lines
Kees Cook Chrome OS Security
Re: [PATCH 2/3] pstore/ram: Switch to persistent_ram routines
On Thu, May 17, 2012 at 09:34:19AM -0700, Kees Cook wrote:
[...]
Quoted 9 lines
Quoted 6 lines
-
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);This still needs to wipe out the remaining bytes in the buffer (the second memset above).
[...]
Quoted 5 lines
Quoted 3 lines
- buf =
cxt->virt_addr+ (id *cxt->record_size);
-memset(buf, '\0', cxt->record_size);
+persistent_ram_free_old(cxt->przs[id]);Same here -- erasing the buffer means wiping it with NULL bytes.
Well, with persistent_ram we don't need to actually erase buffers (with persistent_ram we might even hold binary data). But yes, we'd better reset size pointer, otherwise the unlinked buffer will show up on the next reboot. Thanks for noticing!
Signature · 2 lines
Anton Vorontsov Email: [email protected]
PATCH v3 3/3 pstore/ram: Add ECC support
In reply to Anton VorontsovThis 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
@@ -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
@@ -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
@@ -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
Greg Kroah-Hartman
maintainerQuoted 6 lines · Anton Vorontsov
Hi all,
Here comes v3:
- Rebased on current staging-next tree, so only 3 patches left;
- Fixedram_consoledependency inKconfig(issue noticed by Greg KH).
Nice, thanks for reworking this, now applied.
greg k-h