diff mbox

[2/6] pstore/ram: Add console messages handling

Message ID 20120517084042.GB31753@lizard
State New
Headers show

Commit Message

Anton Vorontsov May 17, 2012, 8:40 a.m. UTC 
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 |   40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

Comments

Kees Cook May 17, 2012, 4:38 p.m. UTC | #1 
On Thu, May 17, 2012 at 1:40 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>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees
Colin Cross May 17, 2012, 6:28 p.m. UTC | #2 
On Thu, May 17, 2012 at 1:40 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>

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.
Anton Vorontsov May 18, 2012, 9:49 p.m. UTC | #3 
On Thu, May 17, 2012 at 11:28:22AM -0700, Colin Cross wrote:
> On Thu, May 17, 2012 at 1:40 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>
> 
> 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!
diff mbox

Patch

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 9123cce..5d6d53e 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,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);