Merge ram_console into pstore
Subsystems (3)
v2 cover Merge ram_console into pstore
Hi all,
In v2:
- Updated documentation per Colin Cross' comments;
- Corrected return value in ramoops_pstore_write() (noticed by Kees Cook);
- Fixed large writes handling in pstore_console_write(), i.e. when log_buf write is larger than pstore bufsize. Also Noticed by Kees Cook.
These patches depend on the the following series:
http://thread.gmane.org/gmane.linux.kernel/1298642
"[PATCH v3 0/3] Merge ramoops and persistent_ram, generic pstore RAM backend"
And a rationale for the series:
Currently 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 reserved bits -> SoC hanged. In that case we don't get any messages in the pstore.
This series add a new message type for pstore, i.e. PSTORE_TYPE_CONSOLE, plus make pstore/ram.c handle the new messages.
The old ram_console driver is removed. This might probably cause some pain for out-of-tree code, as it would need to be adjusted... but "no pain, no gain"? :-) Though, if there's some serious resistance, we can probably postpone the last two patches.
Thanks!
Signature · 2 lines
Anton Vorontsov Email: [email protected]
PATCH v2 1/6 pstore: Add console log messages support
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.
Therefore, let's add a runtime logging support: PSTORE_TYPE_CONSOLE.
--- fs/pstore/Kconfig | 7 +++++++ fs/pstore/inode.c | 3 +++ fs/pstore/platform.c | 33 +++++++++++++++++++++++++++++++++ include/linux/pstore.h | 1 + 4 files changed, 44 insertions(+)
fs/pstore/Kconfig +7 -0
@@ -12,6 +12,13 @@ config PSTORE If you don't have a platform persistent store driver, say N. +config PSTORE_CONSOLE + bool "Log kernel console messages" + depends on PSTORE + help + When the option is enabled, pstore will log all kernel + messages, even if no oops or panic happened. + config PSTORE_RAM tristate "Log panic/oops to a RAM buffer" depends on PSTORE
fs/pstore/inode.c +3 -0
@@ -212,6 +212,9 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id,case PSTORE_TYPE_DMESG :sprintf (name,"dmesg-%s-%lld" , psname, id);break ; +case PSTORE_TYPE_CONSOLE : +sprintf (name,"console-%s" , psname); +break ;case PSTORE_TYPE_MCE :sprintf (name,"mce-%s-%lld" , psname, id);break ;
fs/pstore/platform.c +33 -0
@@ -1,6 +1,7 @@/* * Persistent Storage - platform driver interface parts. * +* Copyright (C) 2007-2008 Google, Inc. * Copyright (C) 2010 Intel Corporation <[email protected]> * * This program is free software; you can redistribute it and/or modify
@@ -22,6 +23,7 @@#include <linux/errno.h> #include <linux/init.h> #include <linux/kmsg_dump.h> +#include <linux/console.h> #include <linux/module.h> #include <linux/pstore.h> #include <linux/string.h>
@@ -156,6 +158,36 @@ static struct kmsg_dumper pstore_dumper = { .dump = pstore_dump, }; +#ifdef CONFIG_PSTORE_CONSOLE +static void pstore_console_write (struct console *con,const char *s,unsigned c) +{ +const char *e = s + c; + +while (s < e) { +if (c > psinfo->bufsize) + c = psinfo->bufsize; +memcpy (psinfo->buf, s, c); + psinfo->write (PSTORE_TYPE_CONSOLE ,0 ,NULL ,0 , c, psinfo); + s += c; + c = e - s; + } +} + +static struct console pstore_console = { + .name ="pstore" , + .write = pstore_console_write, + .flags =CON_PRINTBUFFER |CON_ENABLED |CON_ANYTIME , + .index =-1 , +}; + +static void pstore_register_console (void ) +{ +register_console (&pstore_console); +} +#else +static void pstore_register_console (void ) {} +#endif +/* * platform specific persistent storage driver registers with * us here. If pstore is already mounted, call the platform
@@ -193,6 +225,7 @@ int pstore_register(struct pstore_info *psi)pstore_get_records (0 );kmsg_dump_register (&pstore_dumper); +pstore_register_console (); pstore_timer.expires = jiffies +PSTORE_INTERVAL ;add_timer (&pstore_timer);
include/linux/pstore.h +1 -0
@@ -29,6 +29,7 @@enum pstore_type_id {PSTORE_TYPE_DMESG =0 ,PSTORE_TYPE_MCE =1 , +PSTORE_TYPE_CONSOLE =2 ,PSTORE_TYPE_UNKNOWN =255 };
Signature · 1 line
1.7.9.2
Re: [PATCH 1/6] pstore: Add console log messages support
Quoted 9 lines · Anton Vorontsov
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.
Therefore, let's add a runtime logging support:
PSTORE_TYPE_CONSOLE.Signed-off-by Anton Vorontsov <[email protected]>
Signature · 2 lines
Kees Cook Chrome OS Security
PATCH v2 2/6 pstore/ram: Add console messages handling
In reply to Anton VorontsovThis is all straightforward: we just use the last region for console logging. If there's just one region, we fall-back to the old behaviour: just a oops/dumps logging.
--- fs/pstore/ram.c | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-)
fs/pstore/ram.c +31 -9
@@ -63,6 +63,7 @@ MODULE_PARM_DESC(ramoops_ecc,struct ramoops_context {struct persistent_ram_zone **przs; +struct persistent_ram_zone *cprz;phys_addr_t phys_addr;unsigned long size;size_t record_size;
@@ -70,6 +71,7 @@ struct ramoops_context {bool ecc;unsigned int count;unsigned int max_count; +unsigned int max_dump_count;unsigned int read_count;struct pstore_info pstore; };
@@ -92,16 +94,20 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, {ssize_t size;struct ramoops_context *cxt = psi->data; -struct persistent_ram_zone *prz; - -if (cxt->read_count >= cxt->max_count) -return -EINVAL ; +struct persistent_ram_zone *prz = cxt->przs[cxt->read_count]; *id = cxt->read_count++; - prz = cxt->przs[*id]; -/* Only supports dmesg output so far. */ - *type =PSTORE_TYPE_DMESG ; +if (*id < cxt->max_dump_count &&persistent_ram_old_size (prz)) { + *type =PSTORE_TYPE_DMESG ; + }else if (*id < cxt->max_count) { + *type =PSTORE_TYPE_CONSOLE ; + prz = cxt->cprz; + cxt->read_count = cxt->max_count; + }else { +return 0 ; + } +/* TODO(kees): Bogus time for the moment. */ time->tv_sec =0 ; time->tv_nsec =0 ;
@@ -142,7 +148,13 @@ static int ramoops_pstore_write(enum pstore_type_id type,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_CONSOLE ) { +if (!cxt->cprz) +return -ENOMEM ; +persistent_ram_write (cxt->cprz, cxt->pstore.buf, size); +return 0 ; + } +if (type !=PSTORE_TYPE_DMESG )return -EINVAL ;
@@ -170,7 +182,7 @@ static int ramoops_pstore_write(enum pstore_type_id type, size = prz->buffer_size - hlen;persistent_ram_write (prz, cxt->pstore.buf, size); - cxt->count = (cxt->count +1 ) % cxt->max_count; + cxt->count = (cxt->count +1 ) % cxt->max_dump_count;return 0 ; }
@@ -264,6 +276,16 @@ static int __init ramoops_probe(struct platform_device *pdev) } } +/* +* The last zone is for TYPE_CONSOLE, unless we have only +* one, in which case we use it for oops/panic logging. +*/ + cxt->max_dump_count = cxt->max_count; +if (cxt->max_count >1 ) { + cxt->max_dump_count--; + cxt->cprz = cxt->przs[cxt->max_count -1 ]; + } + cxt->pstore.data = cxt; cxt->pstore.bufsize = cxt->przs[0 ]->buffer_size; cxt->pstore.buf =kmalloc (cxt->pstore.bufsize,GFP_KERNEL );
Signature · 1 line
1.7.9.2
Re: [PATCH 2/6] pstore/ram: Add console messages handling
Quoted 5 lines · Anton Vorontsov
This is all straightforward: we just use the last region for console logging. If there's just one region, we fall-back to the old behaviour: just a oops/dumps logging.
Signed-off-by Anton Vorontsov <[email protected]>
-Kees
Signature · 2 lines
Kees Cook Chrome OS Security
Re: [PATCH 2/6] pstore/ram: Add console messages handling
In reply to Anton VorontsovQuoted 5 lines · Anton Vorontsov
This is all straightforward: we just use the last region for console logging. If there's just one region, we fall-back to the old behaviour: just a oops/dumps logging.
Signed-off-by Anton Vorontsov <[email protected]>
I don't think using the last region is the right way to handle this. We often use very large regions for ramconsole, up to 2MB. Using a single region would require a 2MB region for console and a 2MB region for oops, which is far too large.
Re: [PATCH 2/6] pstore/ram: Add console messages handling
Quoted 6 lines · Colin Cross
Quoted 4 lines · Anton Vorontsov
This is all straightforward: we just use the last region for console logging. If there's just one region, we fall-back to the old behaviour: just a oops/dumps logging.
Quoted 6 lines
Quoted 1 line
Signed-off-by Anton Vorontsov <[email protected]>I don't think using the last region is the right way to handle this. We often use very large regions for ramconsole, up to 2MB. Using a single region would require a 2MB region for console and a 2MB region for oops, which is far too large.
Yeah, I agree. I planned to take care of it later, as this would involve touching more code. But OK, let's do this right away.
Thanks!
Signature · 2 lines
Anton Vorontsov Email: [email protected]
PATCH v2 3/6 pstore/ram_core: Silence some printks
In reply to Anton VorontsovSince we use multiple regions, the messages are somewhat annoying. We do print total mapped memory already, so no need to print the information for each region in the library routines.
--- fs/pstore/ram_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
fs/pstore/ram_core.c +2 -2
@@ -401,13 +401,13 @@ static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool" size %zu, start %zu\n" , buffer_size(prz), buffer_start(prz)); else { -pr_info ("persistent_ram: found existing buffer," +pr_debug ("persistent_ram: found existing buffer," " size %zu, start %zu\n" ,buffer_size (prz),buffer_start (prz));persistent_ram_save_old (prz); } }else { -pr_info ("persistent_ram: no valid data in buffer" +pr_debug ("persistent_ram: no valid data in buffer" " (sig = 0x%08x)\n" , prz->buffer->sig); }
Signature · 1 line
1.7.9.2
Re: [PATCH 3/6] pstore/ram_core: Silence some printks
Quoted 5 lines · Anton Vorontsov
Since we use multiple regions, the messages are somewhat annoying. We do print total mapped memory already, so no need to print the information for each region in the library routines.
Signed-off-by Anton Vorontsov <[email protected]>
-Kees
Signature · 2 lines
Kees Cook Chrome OS Security
PATCH v2 4/6 pstore/ram: Add some more documentation and examples
In reply to Anton Vorontsov--- Documentation/ramoops.txt | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Documentation/ramoops.txt +14 -0
@@ -40,6 +40,12 @@ corrupt, but usually it is restorable. Setting the ramoops parameters can be done in 2 different manners: 1. Use the module parameters (which have the names of the variables described as before). + For quick debugging, you can also reserve parts of memory during boot + and then use the reserved memory for ramoops. For example, assuming a machine + with > 128 MB of memory, the following kernel command line will tell the + kernel to use only the first 128 MB of memory, and place ECC-protected ramoops + region at 128 MB boundary: + "mem=128M ramoops.mem_address=0x8000000 ramoops.ecc=1" 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:
@@ -70,6 +76,14 @@ if (ret) { return ret; } +You can specify either RAM memory or peripheral devices' memory. However, when +specifying RAM, be sure to reserve the memory by issuing memblock_reserve() +very early in the architecture code, e.g.: + +#include <linux/memblock.h> + +memblock_reserve(ramoops_data.mem_address, ramoops_data.mem_size); + 3. Dump format The data dump begins with a header, currently defined as "====" followed by a
Signature · 1 line
1.7.9.2
PATCH v2 5/6 staging/android: Remove ram_console driver
In reply to Anton VorontsovAll the functionality is now supported by pstore and pstore_ram drivers.
--- drivers/staging/android/Kconfig | 5 - drivers/staging/android/Makefile | 1 - drivers/staging/android/ram_console.c | 179 --------------------------------- 3 files changed, 185 deletions(-) delete mode 100644 drivers/staging/android/ram_console.c
drivers/staging/android/Kconfig +0 -5
@@ -25,11 +25,6 @@ config ANDROID_LOGGER tristate "Android log driver" default n -config ANDROID_RAM_CONSOLE - bool "Android RAM buffer console" - depends on !S390 && !UML && HAVE_MEMBLOCK && PSTORE_RAM=y - default n - config ANDROID_TIMED_OUTPUT bool "Timed output class driver" default y
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_RAM_CONSOLE) += ram_console.o obj-$(CONFIG_ANDROID_TIMED_OUTPUT) += timed_output.o obj-$(CONFIG_ANDROID_TIMED_GPIO) += timed_gpio.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER) += lowmemorykiller.o
drivers/staging/android/ram_console.c deleted +0 -179
179 lines not on this page · raw message
Signature · 1 line
1.7.9.2
PATCH v2 6/6 pstore/ram_core: Remove now unused code
In reply to Anton VorontsovThe code tried to maintain the global list of persistent ram zones, which isn't a great idea overall, plus since Android's ram_console is no longer there, we can remove some unused functions.
--- fs/pstore/ram_core.c | 77 -------------------------------------------- include/linux/pstore_ram.h | 19 ----------- 2 files changed, 96 deletions(-)
fs/pstore/ram_core.c +0 -77
@@ -35,8 +35,6 @@ struct persistent_ram_buffer {#define PERSISTENT_RAM_SIG (0x43474244)/* DBGC */ -static __initdata LIST_HEAD (persistent_ram_list); -static inline size_t buffer_size (struct persistent_ram_zone *prz) {return atomic_read (&prz->buffer->size);
@@ -455,78 +453,3 @@ err:kfree (prz);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) -{ -struct persistent_ram_zone *prz; -int ret = -ENOMEM ; - - prz =kzalloc (sizeof (struct persistent_ram_zone ),GFP_KERNEL ); -if (!prz) { -pr_err ("persistent_ram: failed to allocate persistent ram zone\n" ); - goto err; - } - - ret =persistent_ram_buffer_init (dev_name (dev), prz); -if (ret) { -pr_err ("persistent_ram: failed to initialize buffer\n" ); - goto err; - } - -persistent_ram_post_init (prz, ecc); - -return prz; -err: -kfree (prz); -return ERR_PTR (ret); -} - -struct persistent_ram_zone *__init -persistent_ram_init_ringbuffer (struct device *dev,bool ecc) -{ -return __persistent_ram_init (dev, ecc); -} - -int __init persistent_ram_early_init (struct persistent_ram *ram) -{ -int ret; - - ret =memblock_reserve (ram->start, ram->size); -if (ret) { -pr_err ("Failed to reserve persistent memory from %08lx-%08lx\n" , - (long )ram->start, (long )(ram->start + ram->size -1 )); -return ret; - } - -list_add_tail (&ram->node, &persistent_ram_list); - -pr_info ("Initialized persistent memory from %08lx-%08lx\n" , - (long )ram->start, (long )(ram->start + ram->size -1 )); - -return 0 ; -} -#endif
include/linux/pstore_ram.h +0 -19
@@ -25,21 +25,6 @@struct persistent_ram_buffer ; -struct persistent_ram_descriptor { -const char *name; -phys_addr_t size; -}; - -struct persistent_ram { -phys_addr_t start; -phys_addr_t size; - -int num_descs; -struct persistent_ram_descriptor *descs; - -struct list_head node; -}; -struct persistent_ram_zone {phys_addr_t paddr;size_t size;
@@ -63,14 +48,10 @@ struct persistent_ram_zone {size_t old_log_size; }; -int persistent_ram_early_init (struct persistent_ram *ram); -struct persistent_ram_zone *__init persistent_ram_new (phys_addr_t start,size_t size,bool ecc);void persistent_ram_free (struct persistent_ram_zone *prz); -struct persistent_ram_zone *persistent_ram_init_ringbuffer (struct device *dev, -bool ecc);int persistent_ram_write (struct persistent_ram_zone *prz,const void *s,unsigned int count);
Signature · 1 line
1.7.9.2
Re: [PATCH 6/6] pstore/ram_core: Remove now unused code
Quoted 5 lines · Anton Vorontsov
The code tried to maintain the global list of persistent ram zones, which isn't a great idea overall, plus since Android's
ram_consoleis no longer there, we can remove some unused functions.Signed-off-by Anton Vorontsov <[email protected]>
-Kees
Signature · 2 lines
Kees Cook Chrome OS Security
Quoted 29 lines · Anton Vorontsov
Hi all,
In v2:
- Updated documentation per Colin Cross' comments;
- Corrected return value inramoops_pstore_write()(noticed by Kees Cook);
- Fixed large writes handling inpstore_console_write(), i.e. whenlog_bufwrite is larger than pstore bufsize. Also Noticed by Kees Cook.These patches depend on the the following series:
http://thread.gmane.org/gmane.linux.kernel/1298642
"[PATCH v3 0/3] Merge ramoops andpersistent_ram, generic pstore RAM backend"And a rationale for the series:
Currently 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 reserved bits -> SoC hanged. In that case we don't get any messages in the pstore.
This series add a new message type for pstore, i.e.
PSTORE_TYPE_CONSOLE, plus makepstore/ram.chandle the new messages.The old
ram_consoledriver is removed. This might probably cause some pain for out-of-tree code, as it would need to be adjusted... but "no pain, no gain"? :-) Though, if there's some serious resistance, we can probably postpone the last two patches.
Quoted 5 lines
Thanks!
--
Anton Vorontsov
Email: [email protected]
Other than my comment on logging console into a single record,
There is one feature that ram_console had but lost when I added persistent_ram, which would be nice to get back: registering the platform driver before module_init, to allow it to log oopses that happen during device probing. This requires changing module_init to postcore_initcall, and switching from platform_driver_probe to platform_driver_register because the platform device is not registered when the platform driver is registered. Depending on what functions are __init, it may cause a cascading change from __init to __devinit as well.
On Thu, May 17, 2012 at 11:42:29AM -0700, Colin Cross wrote:
[...]
Quoted 2 lines
Other than my comment on logging console into a single record,
Acked-by Colin Cross <[email protected]>
Thanks!
Quoted 9 lines
There is one feature that
ram_consolehad but lost when I addedpersistent_ram, which would be nice to get back: registering the platform driver beforemodule_init, to allow it to log oopses that happen during device probing. This requires changingmodule_inittopostcore_initcall, and switching fromplatform_driver_probetoplatform_driver_registerbecause the platform device is not registered when the platform driver is registered. Depending on what functions are__init, it may cause a cascading change from__initto__devinitas well.
Yep, I'll look into it.
Thanks,
Signature · 2 lines
Anton Vorontsov Email: [email protected]