Re: [PATCH v4] mtd: torturetest: add the support for random data pattern

[Ezequiel Garcia <ezequiel.garcia@free-electrons.com>]

Hi Huang,

On Tue, Feb 05, 2013 at 05:06:39PM +0800, Huang Shijie wrote:
> Add a new module parameter 'pattern'. The meaning of the pattern is:
>   (1) pattern == 0 for 55/AA pattern.
>   (2) pattern < 0 for random pattern.
>   (3) pattern > 0 for random pattern too, the value of the pattern is used
>       as the seed for the random generator.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
> v1 --> v2:
>    add seed support.
> 
> v2 --> v3:
>    fix a type in pr_info
> 
> v3 --> v4:
>    fix a confusing description.
> ---
>  drivers/mtd/tests/mtd_torturetest.c |   82 +++++++++++++++++++++++++---------
>  1 files changed, 60 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mtd/tests/mtd_torturetest.c b/drivers/mtd/tests/mtd_torturetest.c
> index 516cf66..671e8dd 100644
> --- a/drivers/mtd/tests/mtd_torturetest.c
> +++ b/drivers/mtd/tests/mtd_torturetest.c
> @@ -32,6 +32,7 @@
>  #include <linux/mtd/mtd.h>
>  #include <linux/slab.h>
>  #include <linux/sched.h>
> +#include <linux/random.h>
>  
>  #define RETRIES 3
>  
> @@ -64,6 +65,13 @@ module_param(cycles_count, uint, S_IRUGO);
>  MODULE_PARM_DESC(cycles_count, "how many erase cycles to do "
>  			       "(infinite by default)");
>  
> +static int pattern;
> +module_param(pattern, int, S_IRUGO);
> +MODULE_PARM_DESC(pattern, "0 for 55/AA pattern(default);"
> +			"less then 0 for random pattern;"
> +			"greater then 0 for random pattern too, "
> +			"the value of the parameter is the seed.");
> +
>  static struct mtd_info *mtd;
>  
>  /* This buffer contains 0x555555...0xAAAAAA... pattern */
> @@ -72,6 +80,8 @@ static unsigned char *patt_5A5;
>  static unsigned char *patt_A5A;
>  /* This buffer contains all 0xFF bytes */
>  static unsigned char *patt_FF;
> +/* This buffer contains the random data. */
> +static unsigned char *patt_rd;
>  /* This a temporary buffer is use when checking data */
>  static unsigned char *check_buf;
>  /* How many erase cycles were done */
> @@ -228,6 +238,12 @@ static int __init tort_init(void)
>  		pr_info("torturing just %d pages per eraseblock\n",
>  			pgcnt);
>  	pr_info("write verify %s\n", check ? "enabled" : "disabled");
> +	if (pattern == 0)
> +		pr_info("Test pattern is 55/AA.\n");
> +	else if (pattern < 0)
> +		pr_info("Test pattern is random.\n");
> +	else
> +		pr_info("Test pattern is random, seed is 0x%x.\n", pattern);
>  
>  	mtd = get_mtd_device(NULL, dev);
>  	if (IS_ERR(mtd)) {
> @@ -249,16 +265,24 @@ static int __init tort_init(void)
>  	}
>  
>  	err = -ENOMEM;
> -	patt_5A5 = kmalloc(mtd->erasesize, GFP_KERNEL);
> -	if (!patt_5A5) {
> -		pr_err("error: cannot allocate memory\n");

Once again: you don't want to print this message!
In case the kmalloc actually fails, your "cannot allocate memory"
message won't be of any use to anyone.

When the system gets out of memory it will print a lot
of more useful information, including a stack dump
(see warn_alloc_failed in mm/page_alloc.c).

Moreover, printing an unneeded message adds a string increasing kernel
size.

Joe Perches is massively removing this sort of printing,
and I'm sure he will appreciate if we stop adding them :-)

http://lkml.org/lkml/2013/2/11/364

> -		goto out_mtd;
> -	}
> +	if (!pattern) {
> +		patt_5A5 = kmalloc(mtd->erasesize, GFP_KERNEL);
> +		if (!patt_5A5) {
> +			pr_err("error: cannot allocate memory\n");

ditto.

> +			goto out_mtd;
> +		}
>  
> -	patt_A5A = kmalloc(mtd->erasesize, GFP_KERNEL);
> -	if (!patt_A5A) {
> -		pr_err("error: cannot allocate memory\n");

ditto.

> -		goto out_patt_5A5;
> +		patt_A5A = kmalloc(mtd->erasesize, GFP_KERNEL);
> +		if (!patt_A5A) {
> +			pr_err("error: cannot allocate memory\n");

ditto.

> +			goto out_patt_5A5;
> +		}
> +	} else {
> +		patt_rd = kmalloc(mtd->erasesize, GFP_KERNEL);
> +		if (!patt_rd) {
> +			pr_err("error: cannot allocate memory\n");

ditto.

> +			goto out_patt_rd;
> +		}
>  	}
>  
>  	patt_FF = kmalloc(mtd->erasesize, GFP_KERNEL);
> @@ -277,14 +301,20 @@ static int __init tort_init(void)
>  
>  	/* Initialize patterns */
>  	memset(patt_FF, 0xFF, mtd->erasesize);
> -	for (i = 0; i < mtd->erasesize / pgsize; i++) {
> -		if (!(i & 1)) {
> -			memset(patt_5A5 + i * pgsize, 0x55, pgsize);
> -			memset(patt_A5A + i * pgsize, 0xAA, pgsize);
> -		} else {
> -			memset(patt_5A5 + i * pgsize, 0xAA, pgsize);
> -			memset(patt_A5A + i * pgsize, 0x55, pgsize);
> +	if (!pattern) {
> +		for (i = 0; i < mtd->erasesize / pgsize; i++) {
> +			if (!(i & 1)) {
> +				memset(patt_5A5 + i * pgsize, 0x55, pgsize);
> +				memset(patt_A5A + i * pgsize, 0xAA, pgsize);
> +			} else {
> +				memset(patt_5A5 + i * pgsize, 0xAA, pgsize);
> +				memset(patt_A5A + i * pgsize, 0x55, pgsize);
> +			}
>  		}
> +	} else {
> +		/* set the seed. */
> +		if (pattern > 0)
> +			prandom_seed(pattern);
>  	}
>  
>  	/*
> @@ -335,10 +365,15 @@ static int __init tort_init(void)
>  			}
>  
>  			/* Write the pattern */
> -			if ((eb + erase_cycles) & 1)
> -				patt = patt_5A5;
> -			else
> -				patt = patt_A5A;
> +			if (!pattern) {
> +				if ((eb + erase_cycles) & 1)
> +					patt = patt_5A5;
> +				else
> +					patt = patt_A5A;
> +			} else {
> +				patt = patt_rd;
> +				prandom_bytes(patt, mtd->erasesize);
> +			}
>  			err = write_pattern(i, patt);
>  			if (err)
>  				goto out;
> @@ -350,8 +385,9 @@ static int __init tort_init(void)
>  				if (err) {
>  					pr_info("verify failed for %s"
>  					       " pattern\n",
> -					       ((eb + erase_cycles) & 1) ?
> -					       "0x55AA55..." : "0xAA55AA...");
> +					       pattern ? "random pattern" :
> +					       (((eb + erase_cycles) & 1) ?
> +					       "0x55AA55..." : "0xAA55AA..."));
>  					goto out;
>  				}
>  				cond_resched();
> @@ -386,6 +422,8 @@ out_patt_A5A:
>  	kfree(patt_A5A);
>  out_patt_5A5:
>  	kfree(patt_5A5);
> +out_patt_rd:
> +	kfree(patt_rd);
>  out_mtd:
>  	put_mtd_device(mtd);
>  	if (err)
> -- 
> 1.7.0.4
> 
> 

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com