All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mtd: mtd_oobtest: generate consitent data for verification
@ 2014-03-06 12:37 Akinobu Mita
  2014-03-07  3:42 ` Lee Jones
  0 siblings, 1 reply; 2+ messages in thread
From: Akinobu Mita @ 2014-03-06 12:37 UTC (permalink / raw
  To: linux-mtd
  Cc: George Cherian, Brian Norris, David Woodhouse, Akinobu Mita,
	Lothar Waßmann

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: linux-mtd@lists.infradead.org
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
v2: remove a dependency on unsubmitted patch

 drivers/mtd/tests/oobtest.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/tests/oobtest.c b/drivers/mtd/tests/oobtest.c
index 2e9e2d1..c5ea177 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;
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] mtd: mtd_oobtest: generate consitent data for verification
  2014-03-06 12:37 [PATCH v2] mtd: mtd_oobtest: generate consitent data for verification Akinobu Mita
@ 2014-03-07  3:42 ` Lee Jones
  0 siblings, 0 replies; 2+ messages in thread
From: Lee Jones @ 2014-03-07  3:42 UTC (permalink / raw
  To: Akinobu Mita
  Cc: David Woodhouse, George Cherian, Brian Norris, linux-mtd,
	Lothar Waßmann

> 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: linux-mtd@lists.infradead.org
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> v2: remove a dependency on unsubmitted patch
> 
>  drivers/mtd/tests/oobtest.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/tests/oobtest.c b/drivers/mtd/tests/oobtest.c
> index 2e9e2d1..c5ea177 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;

Can you bracket-up the new complicated math for clarity please?

>  		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,

Likewise, and beyond.

> +			   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;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-03-07  3:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-06 12:37 [PATCH v2] mtd: mtd_oobtest: generate consitent data for verification Akinobu Mita
2014-03-07  3:42 ` Lee Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.