[PATCH 0/7] pstore/ram: Early registration and configurable ECC buffers size

[PATCH 0/7] pstore/ram: Early registration and configurable ECC buffers size

[Anton Vorontsov]

Hi all,

Here are some more updates for the pstore/ram: it is implementation
of Colin Cross's and Arve Hjønnevåg's suggestions, i.e. early probing
and configurable ECC size. The last one needed quite a bit of cleanups
in the core code, though.

Thanks,

---
 fs/pstore/ram.c            |   77 ++++++++++++++++++++++----------------------
 fs/pstore/ram_core.c       |   65 ++++++++++++++++++++-----------------
 include/linux/pstore_ram.h |   11 +++----
 3 files changed, 79 insertions(+), 74 deletions(-)

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

[PATCH 1/7] pstore/ram: Probe as early as possible

[Anton Vorontsov]

Registering the platform driver before module_init allows us 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;
and because we use driver_register, now can't use create_bundle() (since
it will try to register the same driver once again), so we have to switch
to platform_device_register_data().

Also, some __init -> __devinit changes were needed.

Overall, the registration logic is now much clearer, since we have only
one driver registration point, and just an optional dummy device, which
is created from the module parameters.

Suggested-by: Colin Cross <ccross@android.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 fs/pstore/ram.c            |   63 ++++++++++++++++++++++----------------------
 fs/pstore/ram_core.c       |    9 ++++---
 include/linux/pstore_ram.h |    6 ++---
 3 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index c7acf94..0b36e91 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -330,7 +330,7 @@ static int ramoops_init_prz(struct device *dev, struct ramoops_context *cxt,
 	return 0;
 }
 
-static int __init ramoops_probe(struct platform_device *pdev)
+static int __devinit ramoops_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct ramoops_platform_data *pdata = pdev->dev.platform_data;
@@ -452,6 +452,7 @@ static int __exit ramoops_remove(struct platform_device *pdev)
 }
 
 static struct platform_driver ramoops_driver = {
+	.probe		= ramoops_probe,
 	.remove		= __exit_p(ramoops_remove),
 	.driver		= {
 		.name	= "ramoops",
@@ -459,46 +460,46 @@ static struct platform_driver ramoops_driver = {
 	},
 };
 
-static int __init ramoops_init(void)
+static void ramoops_register_dummy(void)
 {
-	int ret;
-	ret = platform_driver_probe(&ramoops_driver, ramoops_probe);
-	if (ret == -ENODEV) {
-		/*
-		 * If we didn't find a platform device, we use module parameters
-		 * building platform data on the fly.
-		 */
-		pr_info("platform device not found, using module parameters\n");
-		dummy_data = kzalloc(sizeof(struct ramoops_platform_data),
-				     GFP_KERNEL);
-		if (!dummy_data)
-			return -ENOMEM;
-		dummy_data->mem_size = mem_size;
-		dummy_data->mem_address = mem_address;
-		dummy_data->record_size = record_size;
-		dummy_data->console_size = ramoops_console_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));
-
-		if (IS_ERR(dummy))
-			ret = PTR_ERR(dummy);
-		else
-			ret = 0;
+	if (!mem_size)
+		return;
+
+	pr_info("using module parameters\n");
+
+	dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL);
+	if (!dummy_data) {
+		pr_info("could not allocate pdata\n");
+		return;
 	}
 
-	return ret;
+	dummy_data->mem_size = mem_size;
+	dummy_data->mem_address = mem_address;
+	dummy_data->record_size = record_size;
+	dummy_data->console_size = ramoops_console_size;
+	dummy_data->dump_oops = dump_oops;
+	dummy_data->ecc = ramoops_ecc;
+
+	dummy = platform_device_register_data(NULL, "ramoops", -1,
+			dummy_data, sizeof(struct ramoops_platform_data));
+	if (IS_ERR(dummy)) {
+		pr_info("could not create platform device: %ld\n",
+			PTR_ERR(dummy));
+	}
+}
+
+static int __init ramoops_init(void)
+{
+	ramoops_register_dummy();
+	return platform_driver_register(&ramoops_driver);
 }
+postcore_initcall(ramoops_init);
 
 static void __exit ramoops_exit(void)
 {
 	platform_driver_unregister(&ramoops_driver);
 	kfree(dummy_data);
 }
-
-module_init(ramoops_init);
 module_exit(ramoops_exit);
 
 MODULE_LICENSE("GPL");
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 0fd8161..2653185 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -390,7 +390,8 @@ static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
 	return 0;
 }
 
-static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool ecc)
+static int __devinit persistent_ram_post_init(struct persistent_ram_zone *prz,
+					      bool ecc)
 {
 	int ret;
 
@@ -436,9 +437,9 @@ void persistent_ram_free(struct persistent_ram_zone *prz)
 	kfree(prz);
 }
 
-struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
-						       size_t size,
-						       bool ecc)
+struct persistent_ram_zone * __devinit persistent_ram_new(phys_addr_t start,
+							  size_t size,
+							  bool ecc)
 {
 	struct persistent_ram_zone *prz;
 	int ret = -ENOMEM;
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index 2470bb5..e681af9 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -48,9 +48,9 @@ struct persistent_ram_zone {
 	size_t old_log_size;
 };
 
-struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
-						       size_t size,
-						       bool ecc);
+struct persistent_ram_zone * __devinit persistent_ram_new(phys_addr_t start,
+							  size_t size,
+							  bool ecc);
 void persistent_ram_free(struct persistent_ram_zone *prz);
 void persistent_ram_zap(struct persistent_ram_zone *prz);
 
-- 
1.7.10.4

[PATCH 2/7] pstore/ram: Fix error handling during przs allocation

[Anton Vorontsov]

persistent_ram_new() returns ERR_PTR() value on errors, so during
freeing of the przs we should check for both NULL and IS_ERR() entries,
otherwise bad things will happen.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 fs/pstore/ram.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 0b36e91..58b93fb 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -260,7 +260,7 @@ static void ramoops_free_przs(struct ramoops_context *cxt)
 	if (!cxt->przs)
 		return;
 
-	for (i = 0; cxt->przs[i]; i++)
+	for (i = 0; !IS_ERR_OR_NULL(cxt->przs[i]); i++)
 		persistent_ram_free(cxt->przs[i]);
 	kfree(cxt->przs);
 }
-- 
1.7.10.4

[PATCH 3/7] pstore/ram_core: Proper checking for post_init errors (e.g. improper ECC size)

[Anton Vorontsov]

We will implement variable-sized ECC buffers soon, so post_init routine
might fail much more likely, so we'd better check for its errors.

To make error handling simple, modify persistent_ram_free() to it be safe
at all times.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 fs/pstore/ram_core.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 2653185..f62ebf2 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -427,11 +427,17 @@ static int __devinit persistent_ram_post_init(struct persistent_ram_zone *prz,
 
 void persistent_ram_free(struct persistent_ram_zone *prz)
 {
-	if (pfn_valid(prz->paddr >> PAGE_SHIFT)) {
-		vunmap(prz->vaddr);
-	} else {
-		iounmap(prz->vaddr);
-		release_mem_region(prz->paddr, prz->size);
+	if (!prz)
+		return;
+
+	if (prz->vaddr) {
+		if (pfn_valid(prz->paddr >> PAGE_SHIFT)) {
+			vunmap(prz->vaddr);
+		} else {
+			iounmap(prz->vaddr);
+			release_mem_region(prz->paddr, prz->size);
+		}
+		prz->vaddr = NULL;
 	}
 	persistent_ram_free_old(prz);
 	kfree(prz);
@@ -454,10 +460,12 @@ struct persistent_ram_zone * __devinit persistent_ram_new(phys_addr_t start,
 	if (ret)
 		goto err;
 
-	persistent_ram_post_init(prz, ecc);
+	ret = persistent_ram_post_init(prz, ecc);
+	if (ret)
+		goto err;
 
 	return prz;
 err:
-	kfree(prz);
+	persistent_ram_free(prz);
 	return ERR_PTR(ret);
 }
-- 
1.7.10.4

[PATCH 4/7] pstore/ram_core: Better ECC size checking

[Anton Vorontsov]

- Instead of exploiting unsigned overflows (which doesn't work for all
  sizes), use straightforward checking for ECC total size not exceeding
  initial buffer size;

- Printing overflowed buffer_size is not informative. Instead, print
  ecc_size and buffer_size;

- No need for buffer_size argument in persistent_ram_init_ecc(),
  we can address prz->buffer_size directly.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 fs/pstore/ram_core.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index f62ebf2..a5a7b13 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -171,12 +171,12 @@ static void persistent_ram_ecc_old(struct persistent_ram_zone *prz)
 	}
 }
 
-static int persistent_ram_init_ecc(struct persistent_ram_zone *prz,
-	size_t buffer_size)
+static int persistent_ram_init_ecc(struct persistent_ram_zone *prz)
 {
 	int numerr;
 	struct persistent_ram_buffer *buffer = prz->buffer;
 	int ecc_blocks;
+	size_t ecc_total;
 
 	if (!prz->ecc)
 		return 0;
@@ -187,14 +187,14 @@ static int persistent_ram_init_ecc(struct persistent_ram_zone *prz,
 	prz->ecc_poly = 0x11d;
 
 	ecc_blocks = DIV_ROUND_UP(prz->buffer_size, prz->ecc_block_size);
-	prz->buffer_size -= (ecc_blocks + 1) * prz->ecc_size;
-
-	if (prz->buffer_size > buffer_size) {
-		pr_err("persistent_ram: invalid size %zu, non-ecc datasize %zu\n",
-		       buffer_size, prz->buffer_size);
+	ecc_total = (ecc_blocks + 1) * prz->ecc_size;
+	if (ecc_total >= prz->buffer_size) {
+		pr_err("%s: invalid ecc_size %u (total %zu, buffer size %zu)\n",
+		       __func__, prz->ecc_size, ecc_total, prz->buffer_size);
 		return -EINVAL;
 	}
 
+	prz->buffer_size -= ecc_total;
 	prz->par_buffer = buffer->data + prz->buffer_size;
 	prz->par_header = prz->par_buffer + ecc_blocks * prz->ecc_size;
 
@@ -397,7 +397,7 @@ static int __devinit persistent_ram_post_init(struct persistent_ram_zone *prz,
 
 	prz->ecc = ecc;
 
-	ret = persistent_ram_init_ecc(prz, prz->buffer_size);
+	ret = persistent_ram_init_ecc(prz);
 	if (ret)
 		return ret;
 
-- 
1.7.10.4

[PATCH 5/7] pstore/ram_core: Get rid of prz->ecc_symsize and prz->ecc_poly

[Anton Vorontsov]

The struct members were never used anywhere outside of
persistent_ram_init_ecc(), so there's actually no need for them
to be in the struct.

If we ever want to make polynomial or symbol size configurable,
it would make more sense to just pass initialized rs_decoder
to the persistent_ram init functions.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 fs/pstore/ram_core.c       |    7 +++----
 include/linux/pstore_ram.h |    2 --
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index a5a7b13..3f4d6e6 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -177,14 +177,14 @@ static int persistent_ram_init_ecc(struct persistent_ram_zone *prz)
 	struct persistent_ram_buffer *buffer = prz->buffer;
 	int ecc_blocks;
 	size_t ecc_total;
+	int ecc_symsize = 8;
+	int ecc_poly = 0x11d;
 
 	if (!prz->ecc)
 		return 0;
 
 	prz->ecc_block_size = 128;
 	prz->ecc_size = 16;
-	prz->ecc_symsize = 8;
-	prz->ecc_poly = 0x11d;
 
 	ecc_blocks = DIV_ROUND_UP(prz->buffer_size, prz->ecc_block_size);
 	ecc_total = (ecc_blocks + 1) * prz->ecc_size;
@@ -202,8 +202,7 @@ static int persistent_ram_init_ecc(struct persistent_ram_zone *prz)
 	 * first consecutive root is 0
 	 * primitive element to generate roots = 1
 	 */
-	prz->rs_decoder = init_rs(prz->ecc_symsize, prz->ecc_poly, 0, 1,
-				  prz->ecc_size);
+	prz->rs_decoder = init_rs(ecc_symsize, ecc_poly, 0, 1, prz->ecc_size);
 	if (prz->rs_decoder == NULL) {
 		pr_info("persistent_ram: init_rs failed\n");
 		return -EINVAL;
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index e681af9..a0975c0 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -41,8 +41,6 @@ struct persistent_ram_zone {
 	int bad_blocks;
 	int ecc_block_size;
 	int ecc_size;
-	int ecc_symsize;
-	int ecc_poly;
 
 	char *old_log;
 	size_t old_log_size;
-- 
1.7.10.4

[PATCH 6/7] pstore/ram: Make ECC size configurable

[Anton Vorontsov]

This is now pretty straightforward: instead of using bool, just pass
an integer. For backwards compatibility ramoops.ecc=1 means 16 bytes
ECC (using 1 byte for ECC isn't much of use anyway).

Suggested-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 fs/pstore/ram.c            |   14 +++++++-------
 fs/pstore/ram_core.c       |   15 ++++++++-------
 include/linux/pstore_ram.h |    4 ++--
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 58b93fb..78ecefc 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -73,7 +73,7 @@ struct ramoops_context {
 	size_t record_size;
 	size_t console_size;
 	int dump_oops;
-	bool ecc;
+	int ecc_size;
 	unsigned int max_dump_cnt;
 	unsigned int dump_write_cnt;
 	unsigned int dump_read_cnt;
@@ -288,7 +288,7 @@ static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt,
 	for (i = 0; i < cxt->max_dump_cnt; i++) {
 		size_t sz = cxt->record_size;
 
-		cxt->przs[i] = persistent_ram_new(*paddr, sz, cxt->ecc);
+		cxt->przs[i] = persistent_ram_new(*paddr, sz, cxt->ecc_size);
 		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",
@@ -314,7 +314,7 @@ static int ramoops_init_prz(struct device *dev, struct ramoops_context *cxt,
 	if (*paddr + sz > *paddr + cxt->size)
 		return -ENOMEM;
 
-	*prz = persistent_ram_new(*paddr, sz, cxt->ecc);
+	*prz = persistent_ram_new(*paddr, sz, cxt->ecc_size);
 	if (IS_ERR(*prz)) {
 		int err = PTR_ERR(*prz);
 
@@ -361,7 +361,7 @@ static int __devinit ramoops_probe(struct platform_device *pdev)
 	cxt->record_size = pdata->record_size;
 	cxt->console_size = pdata->console_size;
 	cxt->dump_oops = pdata->dump_oops;
-	cxt->ecc = pdata->ecc;
+	cxt->ecc_size = pdata->ecc_size;
 
 	paddr = cxt->phys_addr;
 
@@ -411,9 +411,9 @@ static int __devinit ramoops_probe(struct platform_device *pdev)
 	record_size = pdata->record_size;
 	dump_oops = pdata->dump_oops;
 
-	pr_info("attached 0x%lx@0x%llx, ecc: %s\n",
+	pr_info("attached 0x%lx@0x%llx, ecc: %d\n",
 		cxt->size, (unsigned long long)cxt->phys_addr,
-		ramoops_ecc ? "on" : "off");
+		cxt->ecc_size);
 
 	return 0;
 
@@ -478,7 +478,7 @@ static void ramoops_register_dummy(void)
 	dummy_data->record_size = record_size;
 	dummy_data->console_size = ramoops_console_size;
 	dummy_data->dump_oops = dump_oops;
-	dummy_data->ecc = ramoops_ecc;
+	dummy_data->ecc_size = ramoops_ecc == 1 ? 16 : ramoops_ecc;
 
 	dummy = platform_device_register_data(NULL, "ramoops", -1,
 			dummy_data, sizeof(struct ramoops_platform_data));
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 3f4d6e6..7e5a2a9 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -171,7 +171,8 @@ static void persistent_ram_ecc_old(struct persistent_ram_zone *prz)
 	}
 }
 
-static int persistent_ram_init_ecc(struct persistent_ram_zone *prz)
+static int persistent_ram_init_ecc(struct persistent_ram_zone *prz,
+				   int ecc_size)
 {
 	int numerr;
 	struct persistent_ram_buffer *buffer = prz->buffer;
@@ -184,7 +185,7 @@ static int persistent_ram_init_ecc(struct persistent_ram_zone *prz)
 		return 0;
 
 	prz->ecc_block_size = 128;
-	prz->ecc_size = 16;
+	prz->ecc_size = ecc_size;
 
 	ecc_blocks = DIV_ROUND_UP(prz->buffer_size, prz->ecc_block_size);
 	ecc_total = (ecc_blocks + 1) * prz->ecc_size;
@@ -390,13 +391,13 @@ static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
 }
 
 static int __devinit persistent_ram_post_init(struct persistent_ram_zone *prz,
-					      bool ecc)
+					      int ecc_size)
 {
 	int ret;
 
-	prz->ecc = ecc;
+	prz->ecc = ecc_size;
 
-	ret = persistent_ram_init_ecc(prz);
+	ret = persistent_ram_init_ecc(prz, ecc_size);
 	if (ret)
 		return ret;
 
@@ -444,7 +445,7 @@ void persistent_ram_free(struct persistent_ram_zone *prz)
 
 struct persistent_ram_zone * __devinit persistent_ram_new(phys_addr_t start,
 							  size_t size,
-							  bool ecc)
+							  int ecc_size)
 {
 	struct persistent_ram_zone *prz;
 	int ret = -ENOMEM;
@@ -459,7 +460,7 @@ struct persistent_ram_zone * __devinit persistent_ram_new(phys_addr_t start,
 	if (ret)
 		goto err;
 
-	ret = persistent_ram_post_init(prz, ecc);
+	ret = persistent_ram_post_init(prz, ecc_size);
 	if (ret)
 		goto err;
 
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index a0975c0..94b79f1 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -48,7 +48,7 @@ struct persistent_ram_zone {
 
 struct persistent_ram_zone * __devinit persistent_ram_new(phys_addr_t start,
 							  size_t size,
-							  bool ecc);
+							  int ecc_size);
 void persistent_ram_free(struct persistent_ram_zone *prz);
 void persistent_ram_zap(struct persistent_ram_zone *prz);
 
@@ -74,7 +74,7 @@ struct ramoops_platform_data {
 	unsigned long	record_size;
 	unsigned long	console_size;
 	int		dump_oops;
-	bool		ecc;
+	int		ecc_size;
 };
 
 #endif
-- 
1.7.10.4

[PATCH 7/7] pstore/ram_core: Get rid of prz->ecc enable/disable flag

[Anton Vorontsov]

Nowadays we can use prz->ecc_size as a flag, no need for the special
member in the prz struct.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 fs/pstore/ram_core.c       |   10 ++++------
 include/linux/pstore_ram.h |    1 -
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 7e5a2a9..4dabbb8 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -114,7 +114,7 @@ static void notrace persistent_ram_update_ecc(struct persistent_ram_zone *prz,
 	int ecc_size = prz->ecc_size;
 	int size = prz->ecc_block_size;
 
-	if (!prz->ecc)
+	if (!prz->ecc_size)
 		return;
 
 	block = buffer->data + (start & ~(ecc_block_size - 1));
@@ -133,7 +133,7 @@ static void persistent_ram_update_header_ecc(struct persistent_ram_zone *prz)
 {
 	struct persistent_ram_buffer *buffer = prz->buffer;
 
-	if (!prz->ecc)
+	if (!prz->ecc_size)
 		return;
 
 	persistent_ram_encode_rs8(prz, (uint8_t *)buffer, sizeof(*buffer),
@@ -146,7 +146,7 @@ static void persistent_ram_ecc_old(struct persistent_ram_zone *prz)
 	uint8_t *block;
 	uint8_t *par;
 
-	if (!prz->ecc)
+	if (!prz->ecc_size)
 		return;
 
 	block = buffer->data;
@@ -181,7 +181,7 @@ static int persistent_ram_init_ecc(struct persistent_ram_zone *prz,
 	int ecc_symsize = 8;
 	int ecc_poly = 0x11d;
 
-	if (!prz->ecc)
+	if (!ecc_size)
 		return 0;
 
 	prz->ecc_block_size = 128;
@@ -395,8 +395,6 @@ static int __devinit persistent_ram_post_init(struct persistent_ram_zone *prz,
 {
 	int ret;
 
-	prz->ecc = ecc_size;
-
 	ret = persistent_ram_init_ecc(prz, ecc_size);
 	if (ret)
 		return ret;
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index 94b79f1..dcf805f 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -33,7 +33,6 @@ struct persistent_ram_zone {
 	size_t buffer_size;
 
 	/* ECC correction */
-	bool ecc;
 	char *par_buffer;
 	char *par_header;
 	struct rs_control *rs_decoder;
-- 
1.7.10.4

Re: [PATCH 6/7] pstore/ram: Make ECC size configurable

[Dan Carpenter]

On Mon, Jun 18, 2012 at 07:15:55PM -0700, Anton Vorontsov wrote:
> @@ -478,7 +478,7 @@ static void ramoops_register_dummy(void)
>  	dummy_data->record_size = record_size;
>  	dummy_data->console_size = ramoops_console_size;
>  	dummy_data->dump_oops = dump_oops;
> -	dummy_data->ecc = ramoops_ecc;
> +	dummy_data->ecc_size = ramoops_ecc == 1 ? 16 : ramoops_ecc;
>  

I know it's in the change log, but there should probably be a
comment here as well that 1 and 16 are magic backwards compatability
numbers.

>  	dummy = platform_device_register_data(NULL, "ramoops", -1,
>  			dummy_data, sizeof(struct ramoops_platform_data));

regards,
dan carpenter

Re: [PATCH 1/7] pstore/ram: Probe as early as possible

[Kees Cook]

On Mon, Jun 18, 2012 at 7:15 PM, Anton Vorontsov
<anton.vorontsov@linaro.org> wrote:
> Registering the platform driver before module_init allows us 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;
> and because we use driver_register, now can't use create_bundle() (since
> it will try to register the same driver once again), so we have to switch
> to platform_device_register_data().
>
> Also, some __init -> __devinit changes were needed.
>
> Overall, the registration logic is now much clearer, since we have only
> one driver registration point, and just an optional dummy device, which
> is created from the module parameters.
>
> Suggested-by: Colin Cross <ccross@android.com>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>

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

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 2/7] pstore/ram: Fix error handling during przs allocation

[Kees Cook]

On Mon, Jun 18, 2012 at 7:15 PM, Anton Vorontsov
<anton.vorontsov@linaro.org> wrote:
> persistent_ram_new() returns ERR_PTR() value on errors, so during
> freeing of the przs we should check for both NULL and IS_ERR() entries,
> otherwise bad things will happen.
>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>

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

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 3/7] pstore/ram_core: Proper checking for post_init errors (e.g. improper ECC size)

[Kees Cook]

On Mon, Jun 18, 2012 at 7:15 PM, Anton Vorontsov
<anton.vorontsov@linaro.org> wrote:
> We will implement variable-sized ECC buffers soon, so post_init routine
> might fail much more likely, so we'd better check for its errors.
>
> To make error handling simple, modify persistent_ram_free() to it be safe
> at all times.
>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>

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

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 4/7] pstore/ram_core: Better ECC size checking

[Kees Cook]

On Mon, Jun 18, 2012 at 7:15 PM, Anton Vorontsov
<anton.vorontsov@linaro.org> wrote:
> - Instead of exploiting unsigned overflows (which doesn't work for all
>  sizes), use straightforward checking for ECC total size not exceeding
>  initial buffer size;
>
> - Printing overflowed buffer_size is not informative. Instead, print
>  ecc_size and buffer_size;
>
> - No need for buffer_size argument in persistent_ram_init_ecc(),
>  we can address prz->buffer_size directly.
>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>

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

-- 
Kees Cook
Chrome OS Security