| Message ID | 20120516125634.GB20475@lizard |
|---|---|
| State | New |
| Headers | show |
On Wed, 2012-05-16 at 05:56 -0700, Anton Vorontsov wrote: > 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. What about the saving /proc/last_kmsg? Are you not bringing that feature from ramconsole? Sorry if I missed it in reading the patches. -- Shuah
On Wed, May 16, 2012 at 5:56 AM, Anton Vorontsov <anton.vorontsov@linaro.org> wrote: > 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 <anton.vorontsov@linaro.org> > --- > fs/pstore/ram.c | 39 ++++++++++++++++++++++++++++++--------- > 1 file changed, 30 insertions(+), 9 deletions(-) > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > index 9123cce..54213eb 100644 > --- a/fs/pstore/ram.c > +++ b/fs/pstore/ram.c > [...] > @@ -142,7 +148,12 @@ 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); > + } > + > if (type != PSTORE_TYPE_DMESG) > return -EINVAL; Doesn't this mean that type == PSTORE_TYPE_CONSOLE will write to the ram, but then fail with -EINVAL? -Kees
On Wed, May 16, 2012 at 09:30:52AM -0600, Shuah Khan wrote: > On Wed, 2012-05-16 at 05:56 -0700, Anton Vorontsov wrote: > > 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. > > What about the saving /proc/last_kmsg? Are you not bringing that feature > from ramconsole? Sorry if I missed it in reading the patches. Nope, sorry. The /proc/last_kms ABI is just a duplication of pstore's console logging interface, and since last_kmsg thing is in drivers/staging/, there's no point in keeping it, unless there's a really good reason. Thanks!
On Wed, May 16, 2012 at 09:45:35AM -0700, Kees Cook wrote: > On Wed, May 16, 2012 at 5:56 AM, Anton Vorontsov > <anton.vorontsov@linaro.org> wrote: > > 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 <anton.vorontsov@linaro.org> > > --- > > fs/pstore/ram.c | 39 ++++++++++++++++++++++++++++++--------- > > 1 file changed, 30 insertions(+), 9 deletions(-) > > > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > > index 9123cce..54213eb 100644 > > --- a/fs/pstore/ram.c > > +++ b/fs/pstore/ram.c > > [...] > > @@ -142,7 +148,12 @@ 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); > > + } > > + > > if (type != PSTORE_TYPE_DMESG) > > return -EINVAL; > > Doesn't this mean that type == PSTORE_TYPE_CONSOLE will write to the > ram, but then fail with -EINVAL? Right you are, there should be 'return 0' for TYPE_CONSOLE. It is harmless tho (and unnoticeable, since we can't check return value of the ->write() calback and print error, since this would recurse), but I'll surely amend that. Thanks!
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 9123cce..54213eb 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -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,12 @@ 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); + } + if (type != PSTORE_TYPE_DMESG) return -EINVAL; @@ -170,7 +181,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 +275,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);
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 <anton.vorontsov@linaro.org> --- fs/pstore/ram.c | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-)