Patchwork mtdtest: Check bbt at index of (eb + 1) when we access 2 EBs.

login
register
mail settings
Submitter Dongsheng Yang
Date Aug. 18, 2014, 2:27 a.m.
Message ID <1408328879-22771-1-git-send-email-yangds.fnst@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/380670/
State Rejected
Headers show

Comments

Dongsheng Yang - Aug. 18, 2014, 2:27 a.m.
When we want to access 2 eraseblocks, we need to make sure the
two eraseblocks both are not bad.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 drivers/mtd/tests/stresstest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Brian Norris - Sept. 28, 2014, 12:40 a.m.
On Mon, Aug 18, 2014 at 10:27:59AM +0800, Dongsheng Yang wrote:
> When we want to access 2 eraseblocks, we need to make sure the
> two eraseblocks both are not bad.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  drivers/mtd/tests/stresstest.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/tests/stresstest.c b/drivers/mtd/tests/stresstest.c
> index c9d42cc..076d409 100644
> --- a/drivers/mtd/tests/stresstest.c
> +++ b/drivers/mtd/tests/stresstest.c
> @@ -60,7 +60,7 @@ again:
>  	eb = prandom_u32();
>  	/* Read or write up 2 eraseblocks at a time - hence 'ebcnt - 1' */
>  	eb %= (ebcnt - 1);
> -	if (bbt[eb])
> +	if (bbt[eb] || bbt[eb + 1])

Was this change actually needed? Did you see some kind of test failure
without it?

From what I can see, stresstest.c already handles the second block just
fine; it skips the block with code like this:

static int do_read(void)
{
...
        if (bbt[eb + 1]) {
                if (offs >= mtd->erasesize)
                        offs -= mtd->erasesize;
                if (offs + len > mtd->erasesize)
                        len = mtd->erasesize - offs;
        }
...
}

and similar in do_write().

Plus, your patch actually skips *any* block where the subsequent block
is bad. That is not ideal, as it skews the randomness.

So unless I'm missing something, I will not take this patch.

>  		goto again;
>  	return eb;
>  }

Regards,
Brian

Patch

diff --git a/drivers/mtd/tests/stresstest.c b/drivers/mtd/tests/stresstest.c
index c9d42cc..076d409 100644
--- a/drivers/mtd/tests/stresstest.c
+++ b/drivers/mtd/tests/stresstest.c
@@ -60,7 +60,7 @@  again:
 	eb = prandom_u32();
 	/* Read or write up 2 eraseblocks at a time - hence 'ebcnt - 1' */
 	eb %= (ebcnt - 1);
-	if (bbt[eb])
+	if (bbt[eb] || bbt[eb + 1])
 		goto again;
 	return eb;
 }