[PATCH v2 3/3] mtd: tests: introduce a erase test

[Brian Norris]

+ others

On Wed, Sep 30, 2015 at 09:01:21AM +0800, Dongsheng Yang wrote:
> This test will test the below cases.
> 
> Positive case:
> 	* erase all good block. (positive case)
> Negative case:
> 	* addr >= mtd size
> 	* addr + size > mtd size
> 	* unaligned addr
> 	* unaligned size
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst at cn.fujitsu.com>
> ---
>  drivers/mtd/tests/Makefile    |   2 +
>  drivers/mtd/tests/erasetest.c | 158 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 160 insertions(+)
>  create mode 100644 drivers/mtd/tests/erasetest.c

I don't think we're planning on adding any new tests here, unless
they provide real, significant benefit over equivalent tests in user
space. And I think this test can be done pretty easily in user space.

Try porting this to mtd-utils instead. We will probably be doing
improvements there and (eventually?) porting the in-kernel tests there
too. We'll probably want at least a new subdirectory under tests/. Stay
tuned.

> diff --git a/drivers/mtd/tests/Makefile b/drivers/mtd/tests/Makefile
> index 937a829..1308b62 100644
> --- a/drivers/mtd/tests/Makefile
> +++ b/drivers/mtd/tests/Makefile
> @@ -1,6 +1,7 @@
>  obj-$(CONFIG_MTD_TESTS) += mtd_oobtest.o
>  obj-$(CONFIG_MTD_TESTS) += mtd_pagetest.o
>  obj-$(CONFIG_MTD_TESTS) += mtd_readtest.o
> +obj-$(CONFIG_MTD_TESTS) += mtd_erasetest.o
>  obj-$(CONFIG_MTD_TESTS) += mtd_speedtest.o
>  obj-$(CONFIG_MTD_TESTS) += mtd_stresstest.o
>  obj-$(CONFIG_MTD_TESTS) += mtd_subpagetest.o
> @@ -11,6 +12,7 @@ obj-$(CONFIG_MTD_TESTS) += mtd_nandbiterrs.o
>  mtd_oobtest-objs := oobtest.o mtd_test.o
>  mtd_pagetest-objs := pagetest.o mtd_test.o
>  mtd_readtest-objs := readtest.o mtd_test.o
> +mtd_erasetest-objs := erasetest.o mtd_test.o
>  mtd_speedtest-objs := speedtest.o mtd_test.o
>  mtd_stresstest-objs := stresstest.o mtd_test.o
>  mtd_subpagetest-objs := subpagetest.o mtd_test.o
> diff --git a/drivers/mtd/tests/erasetest.c b/drivers/mtd/tests/erasetest.c
> new file mode 100644
> index 0000000..5c5827a
> --- /dev/null
> +++ b/drivers/mtd/tests/erasetest.c
> @@ -0,0 +1,158 @@
> +/*
> + * Copyright (C) 2015 Dongsheng Yang <yangds.fnst at cn.fujitsu.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; see the file COPYING. If not, write to the Free Software
> + * Foundation, 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.

FYI, checkpatch.pl complains about including the FSF mailing address
these days. It's not necessary, and it's potentially misleading if they
move.

> + *
> + * Check MTD device erase.
> + *
> + * Author: Dongsheng Yang <yangds.fnst at cn.fujitsu.com>
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/err.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/slab.h>
> +#include <linux/sched.h>
> +
> +#include "mtd_test.h"
> +
> +static int dev = -EINVAL;
> +module_param(dev, int, S_IRUGO);
> +MODULE_PARM_DESC(dev, "MTD device number to use");
> +
> +static struct mtd_info *mtd;
> +static unsigned char *bbt;
> +
> +static int pgsize;
> +static int ebcnt;
> +static int pgcnt;
> +
> +static int __init mtd_erasetest_init(void)
> +{
> +	uint64_t tmp;
> +	int err;
> +
> +	printk(KERN_INFO "\n");
> +	printk(KERN_INFO "=================================================\n");
> +
> +	if (dev < 0) {
> +		pr_info("Please specify a valid mtd-device via module parameter\n");
> +		return -EINVAL;
> +	}
> +
> +	pr_info("MTD device: %d\n", dev);
> +
> +	mtd = get_mtd_device(NULL, dev);
> +	if (IS_ERR(mtd)) {
> +		err = PTR_ERR(mtd);
> +		pr_err("error: Cannot get MTD device\n");
> +		return err;
> +	}
> +
> +	if (mtd->writesize == 1) {
> +		pr_info("not NAND flash, assume page size is 512 "
> +		       "bytes.\n");
> +		pgsize = 512;
> +	} else
> +		pgsize = mtd->writesize;
> +
> +	tmp = mtd->size;
> +	do_div(tmp, mtd->erasesize);
> +	ebcnt = tmp;
> +	pgcnt = mtd->erasesize / pgsize;
> +
> +	pr_info("MTD device size %llu, eraseblock size %u, "
> +	       "page size %u, count of eraseblocks %u, pages per "
> +	       "eraseblock %u, OOB size %u\n",
> +	       (unsigned long long)mtd->size, mtd->erasesize,
> +	       pgsize, ebcnt, pgcnt, mtd->oobsize);
> +
> +	err = -ENOMEM;
> +	bbt = kzalloc(ebcnt, GFP_KERNEL);
> +	if (!bbt)
> +		goto out;
> +	err = mtdtest_scan_for_bad_eraseblocks(mtd, bbt, 0, ebcnt);
> +	if (err)
> +		goto out;
> +
> +	/* Case.1 Erase all good eraseblocks */
> +	err = mtdtest_erase_good_eraseblocks(mtd, bbt, 0, ebcnt);
> +	if (err)
> +		goto out;
> +
> +	/* Case.2 addr >= mtd->size */
> +	err = mtdtest_erase(mtd, mtd->size, mtd->erasesize, 1);

Just FYI: some of these checks are hard-coded into the MTD API now, so
it's guaranteed an MTD will pass these. But you're still free to test
it.

> +	if (err != -EINVAL) {
> +		pr_err("error: erasing (addr = mtd->size = %llu): it return %d, But expected %d.",

The capitalization and grammar could be improved (though we still don't
need complete sentences). e.g.:

  "returned %d, but expected %d\n"

(you're also missing the trailing newline '\n' on all of these messages)

> +			mtd->size, err, -EINVAL);
> +		err = -EIO;
> +		goto out;
> +	}
> +
> +	/* Case.3 addr + size > mtd->size */
> +	err = mtdtest_erase(mtd, mtd->size, mtd->erasesize * 2, 1);
> +	if (err != -EINVAL) {
> +		pr_err("error: erasing (addr + size = %llu > mtd->size = %llu): it return %d, But expected %d.",
> +			mtd->size + mtd->erasesize * 2, mtd->size , err, -EINVAL);
> +		err = -EIO;
> +		goto out;
> +	}
> +
> +	/* Case.4 unaligned addr */
> +	err = mtdtest_erase(mtd, mtd->erasesize / 2, mtd->erasesize, 1);
> +	if (err != -EINVAL) {
> +		pr_err("error: erasing unaligned addr: %llu (erasesize: %u): it return %d, But expected %d.",
> +			((loff_t)mtd->erasesize) / 2, mtd->erasesize, err, -EINVAL);
> +		err = -EIO;
> +		goto out;
> +	}
> +	
> +	/* Case.5 unaligned size */
> +	err = mtdtest_erase(mtd, mtd->erasesize, mtd->erasesize / 2, 1);
> +	if (err != -EINVAL) {
> +		pr_err("error: erasing unaligned size: %u (erasesize: %u): it return %d, But expected %d.",
> +			mtd->erasesize / 2, mtd->erasesize, err, -EINVAL);
> +		err = -EIO;
> +		goto out;
> +	}
> +
> +	err = 0;
> +	if (err)
> +		pr_info("finished with errors\n");
> +	else
> +		pr_info("finished\n");
> +
> +out:
> +	kfree(bbt);
> +	put_mtd_device(mtd);
> +	if (err)
> +		pr_info("error %d occurred\n", err);
> +	printk(KERN_INFO "=================================================\n");
> +	return err;
> +}
> +module_init(mtd_erasetest_init);
> +
> +static void __exit mtd_erasetest_exit(void)
> +{
> +	return;
> +}
> +module_exit(mtd_erasetest_exit);
> +
> +MODULE_DESCRIPTION("Erase test module");
> +MODULE_AUTHOR("Dongsheng Yang");
> +MODULE_LICENSE("GPL");

Overall, I'm not sure if this test is really that useful. It does a few
sanity checks on the API, but it doesn't really test the erase function
itself. If anyone else thinks this test is interesting though, I suppose
there's not much harm in putting it into mtd-utils tests/.

Brian