From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pb0-f45.google.com ([209.85.160.45]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WLlgP-0005d7-9w for linux-mtd@lists.infradead.org; Fri, 07 Mar 2014 03:42:50 +0000 Received: by mail-pb0-f45.google.com with SMTP id uo5so3567925pbc.32 for ; Thu, 06 Mar 2014 19:42:27 -0800 (PST) Date: Fri, 7 Mar 2014 11:42:21 +0800 From: Lee Jones To: Akinobu Mita Subject: Re: [PATCH v2] mtd: mtd_oobtest: generate consitent data for verification Message-ID: <20140307034221.GB17063@lee--X1> References: <1394109426-8251-1-git-send-email-akinobu.mita@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1394109426-8251-1-git-send-email-akinobu.mita@gmail.com> Cc: David Woodhouse , George Cherian , Brian Norris , linux-mtd@lists.infradead.org, Lothar =?iso-8859-1?Q?Wa=DFmann?= List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , > 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 > Reported-by: Lothar Waßmann > Cc: George Cherian > Cc: Lothar Waßmann > Cc: David Woodhouse > Cc: Brian Norris > Cc: linux-mtd@lists.infradead.org > Signed-off-by: Akinobu Mita > --- > 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