* Re: [l2-mtd:master 149/149] ERROR: "__umoddi3" [drivers/mtd/devices/mtdram.ko] undefined! [not found] ` <560B293B.2060001@cn.fujitsu.com> @ 2015-09-30 0:29 ` Brian Norris 2015-09-30 1:01 ` [PATCH v2 0/3] mtdram: check offs and len in mtdram->erase Dongsheng Yang 0 siblings, 1 reply; 9+ messages in thread From: Brian Norris @ 2015-09-30 0:29 UTC (permalink / raw) To: Dongsheng Yang; +Cc: linux-mtd + linux-mtd On Wed, Sep 30, 2015 at 08:13:47AM +0800, Dongsheng Yang wrote: > On 09/30/2015 07:36 AM, kbuild test robot wrote: > >tree: git://git.infradead.org/users/dedekind/l2-mtd.git master > >head: 7827e3acad2df1c6537e5fe7211d216dabc60399 > >commit: 7827e3acad2df1c6537e5fe7211d216dabc60399 [149/149] mtd: mtdram: check offs and len in mtdram->erase > >config: i386-randconfig-i0-201539 (attached as .config) > >reproduce: > > git checkout 7827e3acad2df1c6537e5fe7211d216dabc60399 > > # save the attached .config to linux build tree > > make ARCH=i386 > > > >All error/warnings (new ones prefixed by >>): > > > >>>ERROR: "__umoddi3" [drivers/mtd/devices/mtdram.ko] undefined! > >>>ERROR: "__moddi3" [drivers/mtd/devices/mtdram.ko] undefined! > > Hi Brian, > It seems i386 does not support mod operation. Please help > to squash the patch attached. Thanx a lot. It's not just i386. > > > >--- > >0-DAY kernel test infrastructure Open Source Technology Center > >https://lists.01.org/pipermail/kbuild-all Intel Corporation > > > > From 4034d05f22e46430ac90fa67a2eff059f7fdc7a3 Mon Sep 17 00:00:00 2001 > From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> > Date: Wed, 30 Sep 2015 05:02:39 -0400 > Subject: [PATCH] mtdram: use mtd->erasesize_mask rather than mod function. > > kbuild test robot complain that on i386: > ERROR: "__umoddi3" [drivers/mtd/devices/mtdram.ko] undefined! > > So, fix it by replace '% mtd->erasesize' with '& mtd->erasesize_mask'; > > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> > --- > drivers/mtd/devices/mtdram.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c > index 73fa297..8124fc4 100644 > --- a/drivers/mtd/devices/mtdram.c > +++ b/drivers/mtd/devices/mtdram.c > @@ -37,13 +37,13 @@ static int check_offs_len(struct mtd_info *mtd, loff_t ofs, uint64_t len) > int ret = 0; > > /* Start address must align on block boundary */ > - if (ofs % mtd->erasesize) { > + if (ofs & mtd->erasesize_mask) { Hmm, but that only works if it's a power-of-two erasesize. Right now, mtdram technically allows odd sizes. You'll have to add more checks/restrictions to this driver before I can take this patch. I've just reverted the patch for now. You can send a better one that addresses all problems. Brian > pr_debug("%s: unaligned address\n", __func__); > ret = -EINVAL; > } > > /* Length must align on block boundary */ > - if (len % mtd->erasesize) { > + if (len & mtd->erasesize_mask) { > pr_debug("%s: length not block aligned\n", __func__); > ret = -EINVAL; > } > -- > 1.8.4.2 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 0/3] mtdram: check offs and len in mtdram->erase 2015-09-30 0:29 ` [l2-mtd:master 149/149] ERROR: "__umoddi3" [drivers/mtd/devices/mtdram.ko] undefined! Brian Norris @ 2015-09-30 1:01 ` Dongsheng Yang 2015-09-30 1:01 ` [PATCH v2 1/3] mtd: " Dongsheng Yang ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Dongsheng Yang @ 2015-09-30 1:01 UTC (permalink / raw) To: computersforpeace; +Cc: linux-mtd, Dongsheng Yang Hi Brian, This is V2 for mtdram: check offs and len in mtdram->erase. I replace '%' with mtd_mod_by_eb(). In addition, I update the new erasetest at the same time. Changelog: V1: - replace '%' with mtd_mod_by_eb(); Dongsheng Yang (3): mtd: mtdram: check offs and len in mtdram->erase mtd: test: refactor mtdtest_erase_eraseblock to introduce a new mtdtest_erase function mtd: tests: introduce a erase test drivers/mtd/devices/mtdram.c | 21 ++++++ drivers/mtd/tests/Makefile | 2 + drivers/mtd/tests/erasetest.c | 158 ++++++++++++++++++++++++++++++++++++++++++ drivers/mtd/tests/mtd_test.c | 44 ++++++++---- drivers/mtd/tests/mtd_test.h | 1 + 5 files changed, 214 insertions(+), 12 deletions(-) create mode 100644 drivers/mtd/tests/erasetest.c -- 1.8.4.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] mtd: mtdram: check offs and len in mtdram->erase 2015-09-30 1:01 ` [PATCH v2 0/3] mtdram: check offs and len in mtdram->erase Dongsheng Yang @ 2015-09-30 1:01 ` Dongsheng Yang 2015-10-20 1:09 ` Brian Norris 2015-09-30 1:01 ` [PATCH v2 2/3] mtd: test: refactor mtdtest_erase_eraseblock to introduce a new mtdtest_erase function Dongsheng Yang 2015-09-30 1:01 ` [PATCH v2 3/3] mtd: tests: introduce a erase test Dongsheng Yang 2 siblings, 1 reply; 9+ messages in thread From: Dongsheng Yang @ 2015-09-30 1:01 UTC (permalink / raw) To: computersforpeace; +Cc: linux-mtd, Dongsheng Yang We should prevent user to erasing mtd device with an unaligned offset or length. Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- drivers/mtd/devices/mtdram.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c index 8e28508..627a9bc 100644 --- a/drivers/mtd/devices/mtdram.c +++ b/drivers/mtd/devices/mtdram.c @@ -32,8 +32,29 @@ MODULE_PARM_DESC(erase_size, "Device erase block size in KiB"); // We could store these in the mtd structure, but we only support 1 device.. static struct mtd_info *mtd_info; +static int check_offs_len(struct mtd_info *mtd, loff_t ofs, uint64_t len) +{ + int ret = 0; + + /* Start address must align on block boundary */ + if (mtd_mod_by_eb(ofs, mtd)) { + pr_debug("%s: unaligned address\n", __func__); + ret = -EINVAL; + } + + /* Length must align on block boundary */ + if (mtd_mod_by_eb(len, mtd)) { + pr_debug("%s: length not block aligned\n", __func__); + ret = -EINVAL; + } + + return ret; +} + static int ram_erase(struct mtd_info *mtd, struct erase_info *instr) { + if (check_offs_len(mtd, instr->addr, instr->len)) + return -EINVAL; memset((char *)mtd->priv + instr->addr, 0xff, instr->len); instr->state = MTD_ERASE_DONE; mtd_erase_callback(instr); -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] mtd: mtdram: check offs and len in mtdram->erase 2015-09-30 1:01 ` [PATCH v2 1/3] mtd: " Dongsheng Yang @ 2015-10-20 1:09 ` Brian Norris 0 siblings, 0 replies; 9+ messages in thread From: Brian Norris @ 2015-10-20 1:09 UTC (permalink / raw) To: Dongsheng Yang; +Cc: linux-mtd On Wed, Sep 30, 2015 at 09:01:19AM +0800, Dongsheng Yang wrote: > We should prevent user to erasing mtd device with > an unaligned offset or length. > > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> Pushed patch 1 to l2-mtd.git. Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] mtd: test: refactor mtdtest_erase_eraseblock to introduce a new mtdtest_erase function 2015-09-30 1:01 ` [PATCH v2 0/3] mtdram: check offs and len in mtdram->erase Dongsheng Yang 2015-09-30 1:01 ` [PATCH v2 1/3] mtd: " Dongsheng Yang @ 2015-09-30 1:01 ` Dongsheng Yang 2015-09-30 1:01 ` [PATCH v2 3/3] mtd: tests: introduce a erase test Dongsheng Yang 2 siblings, 0 replies; 9+ messages in thread From: Dongsheng Yang @ 2015-09-30 1:01 UTC (permalink / raw) To: computersforpeace; +Cc: linux-mtd, Dongsheng Yang In original mtd_test, there is only one function to erase a block, but we need a function to accept [addr, size] to erase mtd device for mtd_erasetest. So refactor the original mtdtest_erase_eraseblock to introduce a new mtdtest_erase. Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- drivers/mtd/tests/mtd_test.c | 44 ++++++++++++++++++++++++++++++++------------ drivers/mtd/tests/mtd_test.h | 1 + 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/drivers/mtd/tests/mtd_test.c b/drivers/mtd/tests/mtd_test.c index 34736bb..4c4320c 100644 --- a/drivers/mtd/tests/mtd_test.c +++ b/drivers/mtd/tests/mtd_test.c @@ -9,24 +9,14 @@ int mtdtest_erase_eraseblock(struct mtd_info *mtd, unsigned int ebnum) { int err; - struct erase_info ei; loff_t addr = (loff_t)ebnum * mtd->erasesize; - memset(&ei, 0, sizeof(struct erase_info)); - ei.mtd = mtd; - ei.addr = addr; - ei.len = mtd->erasesize; - - err = mtd_erase(mtd, &ei); + err = mtdtest_erase(mtd, addr, mtd->erasesize, 0); if (err) { - pr_info("error %d while erasing EB %d\n", err, ebnum); + pr_info("failed to erase EB %d\n", ebnum); return err; } - if (ei.state == MTD_ERASE_FAILED) { - pr_info("some erase error occurred at EB %d\n", ebnum); - return -EIO; - } return 0; } @@ -111,3 +101,33 @@ int mtdtest_write(struct mtd_info *mtd, loff_t addr, size_t size, return err; } + +/* + * There is a parameter named as quiet, it's useful to clean the output + * when we are testing some negative cases. + */ +int mtdtest_erase(struct mtd_info *mtd, loff_t addr, size_t size, int quiet) +{ + int err; + struct erase_info ei; + + memset(&ei, 0, sizeof(struct erase_info)); + ei.mtd = mtd; + ei.addr = addr; + ei.len = size; + + err = mtd_erase(mtd, &ei); + if (err) { + if (!quiet) + pr_err("error %d while erasing (addr: %#llx, size: %lu)\n", err, addr, size); + return err; + } + + if (ei.state == MTD_ERASE_FAILED) { + if (!quiet) + pr_err("some erase error occurred in erasing (addr: %#llx, size: %lu)\n", addr, size); + return -EIO; + } + + return err; +} diff --git a/drivers/mtd/tests/mtd_test.h b/drivers/mtd/tests/mtd_test.h index 4b7bee1..c53558c 100644 --- a/drivers/mtd/tests/mtd_test.h +++ b/drivers/mtd/tests/mtd_test.h @@ -21,3 +21,4 @@ int mtdtest_erase_good_eraseblocks(struct mtd_info *mtd, unsigned char *bbt, int mtdtest_read(struct mtd_info *mtd, loff_t addr, size_t size, void *buf); int mtdtest_write(struct mtd_info *mtd, loff_t addr, size_t size, const void *buf); +int mtdtest_erase(struct mtd_info *mtd, loff_t addr, size_t size, int quiet); -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] mtd: tests: introduce a erase test 2015-09-30 1:01 ` [PATCH v2 0/3] mtdram: check offs and len in mtdram->erase Dongsheng Yang 2015-09-30 1:01 ` [PATCH v2 1/3] mtd: " Dongsheng Yang 2015-09-30 1:01 ` [PATCH v2 2/3] mtd: test: refactor mtdtest_erase_eraseblock to introduce a new mtdtest_erase function Dongsheng Yang @ 2015-09-30 1:01 ` Dongsheng Yang 2015-10-20 1:18 ` Brian Norris 2 siblings, 1 reply; 9+ messages in thread From: Dongsheng Yang @ 2015-09-30 1:01 UTC (permalink / raw) To: computersforpeace; +Cc: linux-mtd, Dongsheng Yang 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@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 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@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. + * + * Check MTD device erase. + * + * Author: Dongsheng Yang <yangds.fnst@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); + if (err != -EINVAL) { + pr_err("error: erasing (addr = mtd->size = %llu): it return %d, But expected %d.", + 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"); -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] mtd: tests: introduce a erase test 2015-09-30 1:01 ` [PATCH v2 3/3] mtd: tests: introduce a erase test Dongsheng Yang @ 2015-10-20 1:18 ` Brian Norris 2015-10-20 1:20 ` Brian Norris 0 siblings, 1 reply; 9+ messages in thread From: Brian Norris @ 2015-10-20 1:18 UTC (permalink / raw) To: Dongsheng Yang; +Cc: linux-mtd + 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@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@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@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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] mtd: tests: introduce a erase test 2015-10-20 1:18 ` Brian Norris @ 2015-10-20 1:20 ` Brian Norris 2015-10-20 1:42 ` Dongsheng Yang 0 siblings, 1 reply; 9+ messages in thread From: Brian Norris @ 2015-10-20 1:20 UTC (permalink / raw) To: Dongsheng Yang; +Cc: linux-mtd, Richard Weinberger, Boris Brezillon On Mon, Oct 19, 2015 at 06:18:13PM -0700, Brian Norris wrote: > + others really +others this time! > 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@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@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@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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] mtd: tests: introduce a erase test 2015-10-20 1:20 ` Brian Norris @ 2015-10-20 1:42 ` Dongsheng Yang 0 siblings, 0 replies; 9+ messages in thread From: Dongsheng Yang @ 2015-10-20 1:42 UTC (permalink / raw) To: Brian Norris; +Cc: linux-mtd, Richard Weinberger, Boris Brezillon On 10/20/2015 09:20 AM, Brian Norris wrote: > On Mon, Oct 19, 2015 at 06:18:13PM -0700, Brian Norris wrote: >> + others > > really +others this time! > >> On Wed, Sep 30, 2015 at 09:01:21AM +0800, Dongsheng Yang wrote: >>> This test will test the below cases. >>> [...] >>> + >>> +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/. Yes, I found the erase problem about mtdram in my debugging. Then I was wondering why we did not find it by running mtd/tests. Finally I found there is no testing about it in mtd/tests. So write one and send it out here. As you said, it's much more proper to put them in mtd-utils. I am totally agree with that. would port it to userspace. Thanx Yang >> >> Brian > . > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-10-20 1:49 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <201509300759.Ipr62suj%fengguang.wu@intel.com>
[not found] ` <560B293B.2060001@cn.fujitsu.com>
2015-09-30 0:29 ` [l2-mtd:master 149/149] ERROR: "__umoddi3" [drivers/mtd/devices/mtdram.ko] undefined! Brian Norris
2015-09-30 1:01 ` [PATCH v2 0/3] mtdram: check offs and len in mtdram->erase Dongsheng Yang
2015-09-30 1:01 ` [PATCH v2 1/3] mtd: " Dongsheng Yang
2015-10-20 1:09 ` Brian Norris
2015-09-30 1:01 ` [PATCH v2 2/3] mtd: test: refactor mtdtest_erase_eraseblock to introduce a new mtdtest_erase function Dongsheng Yang
2015-09-30 1:01 ` [PATCH v2 3/3] mtd: tests: introduce a erase test Dongsheng Yang
2015-10-20 1:18 ` Brian Norris
2015-10-20 1:20 ` Brian Norris
2015-10-20 1:42 ` Dongsheng Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).