Patchwork [v3] mtd: mtd_oobtest: generate consitent data for verification

login
register
mail settings
Submitter Akinobu Mita
Date March 7, 2014, 3:24 p.m.
Message ID <1394205850-5366-1-git-send-email-akinobu.mita@gmail.com>
Download mbox | patch
Permalink /patch/328014/
State Accepted
Commit be54f8f1c76890f4b6163715aed5a3d0a7309dc2
Headers show

Comments

Akinobu Mita - March 7, 2014, 3:24 p.m.
mtd_oobtest writes OOB, read it back and verify.  The verification is
not correctly done if oobsize is not multiple of 4.  Although the data
to be written and the data to be compared are generated by several
prandom_byte_state() calls starting with the same seed, these two are
generated with the different size and different number of calls.

Due to the implementation of prandom_byte_state() if the size on each
call is not multiple of 4, the resulting data is not always same.

This fixes it by just calling prandom_byte_state() once and using
correct range instead of calling it multiple times for each.

Reported-by: George Cherian <george.cherian@ti.com>
Reported-by: Lothar Waßmann <LW@KARO-electronics.de>
Cc: George Cherian <george.cherian@ti.com>
Cc: Lothar Waßmann <LW@KARO-electronics.de>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: linux-mtd@lists.infradead.org
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
v3: bracket-up the new complicated math for clarity requested by Lee Jones

 drivers/mtd/tests/oobtest.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
Lothar Waßmann - March 10, 2014, 11:13 a.m.
Hi,

Akinobu Mita wrote:
> mtd_oobtest writes OOB, read it back and verify.  The verification is
> not correctly done if oobsize is not multiple of 4.  Although the data
> to be written and the data to be compared are generated by several
> prandom_byte_state() calls starting with the same seed, these two are
> generated with the different size and different number of calls.
> 
> Due to the implementation of prandom_byte_state() if the size on each
> call is not multiple of 4, the resulting data is not always same.
> 
> This fixes it by just calling prandom_byte_state() once and using
> correct range instead of calling it multiple times for each.
> 
With this patch the oobtest works on our i.MX53 platform.

You can add my:
Tested-by: <LW@KARO-electronics.de>


Lothar Waßmann
Brian Norris - April 18, 2014, 5:24 a.m.
On Mon, Mar 10, 2014 at 12:13:04PM +0100, Lothar Waßmann wrote:
> Akinobu Mita wrote:
> > mtd_oobtest writes OOB, read it back and verify.  The verification is
> > not correctly done if oobsize is not multiple of 4.  Although the data
> > to be written and the data to be compared are generated by several
> > prandom_byte_state() calls starting with the same seed, these two are
> > generated with the different size and different number of calls.
> > 
> > Due to the implementation of prandom_byte_state() if the size on each
> > call is not multiple of 4, the resulting data is not always same.
> > 
> > This fixes it by just calling prandom_byte_state() once and using
> > correct range instead of calling it multiple times for each.
> > 
> With this patch the oobtest works on our i.MX53 platform.
> 
> You can add my:
> Tested-by: <LW@KARO-electronics.de>

Pushed to l2-mtd.git/master. Thanks!

Brian

Patch

diff --git a/drivers/mtd/tests/oobtest.c b/drivers/mtd/tests/oobtest.c
index 2e9e2d1..f19ab1a 100644
--- a/drivers/mtd/tests/oobtest.c
+++ b/drivers/mtd/tests/oobtest.c
@@ -69,8 +69,8 @@  static int write_eraseblock(int ebnum)
 	int err = 0;
 	loff_t addr = ebnum * mtd->erasesize;
 
+	prandom_bytes_state(&rnd_state, writebuf, use_len_max * pgcnt);
 	for (i = 0; i < pgcnt; ++i, addr += mtd->writesize) {
-		prandom_bytes_state(&rnd_state, writebuf, use_len);
 		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
@@ -78,7 +78,7 @@  static int write_eraseblock(int ebnum)
 		ops.oobretlen = 0;
 		ops.ooboffs   = use_offset;
 		ops.datbuf    = NULL;
-		ops.oobbuf    = writebuf;
+		ops.oobbuf    = writebuf + (use_len_max * i) + use_offset;
 		err = mtd_write_oob(mtd, addr, &ops);
 		if (err || ops.oobretlen != use_len) {
 			pr_err("error: writeoob failed at %#llx\n",
@@ -122,8 +122,8 @@  static int verify_eraseblock(int ebnum)
 	int err = 0;
 	loff_t addr = ebnum * mtd->erasesize;
 
+	prandom_bytes_state(&rnd_state, writebuf, use_len_max * pgcnt);
 	for (i = 0; i < pgcnt; ++i, addr += mtd->writesize) {
-		prandom_bytes_state(&rnd_state, writebuf, use_len);
 		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
@@ -139,7 +139,8 @@  static int verify_eraseblock(int ebnum)
 			errcnt += 1;
 			return err ? err : -1;
 		}
-		if (memcmp(readbuf, writebuf, use_len)) {
+		if (memcmp(readbuf, writebuf + (use_len_max * i) + use_offset,
+			   use_len)) {
 			pr_err("error: verify failed at %#llx\n",
 			       (long long)addr);
 			errcnt += 1;
@@ -166,7 +167,9 @@  static int verify_eraseblock(int ebnum)
 				errcnt += 1;
 				return err ? err : -1;
 			}
-			if (memcmp(readbuf + use_offset, writebuf, use_len)) {
+			if (memcmp(readbuf + use_offset,
+				   writebuf + (use_len_max * i) + use_offset,
+				   use_len)) {
 				pr_err("error: verify failed at %#llx\n",
 						(long long)addr);
 				errcnt += 1;
@@ -566,8 +569,8 @@  static int __init mtd_oobtest_init(void)
 		if (bbt[i] || bbt[i + 1])
 			continue;
 		addr = (i + 1) * mtd->erasesize - mtd->writesize;
+		prandom_bytes_state(&rnd_state, writebuf, sz * cnt);
 		for (pg = 0; pg < cnt; ++pg) {
-			prandom_bytes_state(&rnd_state, writebuf, sz);
 			ops.mode      = MTD_OPS_AUTO_OOB;
 			ops.len       = 0;
 			ops.retlen    = 0;
@@ -575,7 +578,7 @@  static int __init mtd_oobtest_init(void)
 			ops.oobretlen = 0;
 			ops.ooboffs   = 0;
 			ops.datbuf    = NULL;
-			ops.oobbuf    = writebuf;
+			ops.oobbuf    = writebuf + pg * sz;
 			err = mtd_write_oob(mtd, addr, &ops);
 			if (err)
 				goto out;