Hi all,
Just a few patches left from the series that used to add configurable ECC size for pstore/ram backend. Most patches were merged into -next, and this is just a resend of the leftovers.
(Note that pstore/trace patches go on top of this series.)
Thanks,
--- fs/pstore/ram.c | 14 +++++++------- fs/pstore/ram_core.c | 30 ++++++++++++++---------------- include/linux/pstore_ram.h | 7 ++----- 3 files changed, 23 insertions(+), 28 deletions(-)
On Mon, Jul 09, 2012 at 04:45:59PM -0700, Anton Vorontsov wrote:
Hi all,
Just a few patches left from the series that used to add configurable ECC size for pstore/ram backend. Most patches were merged into -next, and this is just a resend of the leftovers.
Oh, I fogot: actually, it's not a plain resend, I fixed Dan Carpenter's comment (thanks!): added more text about special ecc=1 case, both as a module param description and as a comment in the code.
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;
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 | 22 ++++++++++++++-------- fs/pstore/ram_core.c | 15 ++++++++------- include/linux/pstore_ram.h | 4 ++-- 3 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 58b93fb..b39aebb 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -63,7 +63,9 @@ MODULE_PARM_DESC(dump_oops, static int ramoops_ecc; module_param_named(ecc, ramoops_ecc, int, 0600); MODULE_PARM_DESC(ramoops_ecc, - "set to 1 to enable ECC support"); + "if non-zero, the option enables ECC support and specifies " + "ECC buffer size in bytes (1 is a special value, means 16 " + "bytes ECC)");
struct ramoops_context { struct persistent_ram_zone **przs; @@ -73,7 +75,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 +290,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 +316,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 +363,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 +413,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 +480,11 @@ 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; + /* + * For backwards compatibility ramoops.ecc=1 means 16 bytes ECC + * (using 1 byte for ECC isn't much of use anyway). + */ + 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
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;
On Mon, Jul 9, 2012 at 4:45 PM, Anton Vorontsov anton.vorontsov@linaro.org wrote:
Just a few patches left from the series that used to add configurable ECC size for pstore/ram backend. Most patches were merged into -next, and this is just a resend of the leftovers.
Feel free to add my:
Acked-by: Kees Cook keescook@chromium.org
to this series. Looks good to me.
-Kees
On Tue, Jul 10, 2012 at 01:17:48PM -0700, Kees Cook wrote:
On Mon, Jul 9, 2012 at 4:45 PM, Anton Vorontsov anton.vorontsov@linaro.org wrote:
Just a few patches left from the series that used to add configurable ECC size for pstore/ram backend. Most patches were merged into -next, and this is just a resend of the leftovers.
Feel free to add my:
Acked-by: Kees Cook keescook@chromium.org
to this series. Looks good to me.
Thanks for looking at this, Kees!
linaro-kernel@lists.linaro.org