* [PATCH] mtd: nandsim: Allow nandsim to re-use persisted data
@ 2013-09-25 19:25 Matthew Gabeler-Lee
2013-11-07 15:12 ` Richard Weinberger
0 siblings, 1 reply; 3+ messages in thread
From: Matthew Gabeler-Lee @ 2013-09-25 19:25 UTC (permalink / raw)
To: linux-mtd
mtd: nandsim: Allow nandsim to re-use persisted data
Add optional (default disabled) module parameter to nandsim to instruct it
to assume that the cache file already has valid data in it. This allows
data written to the cache file (or device) to be re-used across reloads of
the nandsim module, and thus reboots of the host, in much the same way that
block2mtd allows.
Signed-off-by: Matthew Gabeler-Lee <matthew@beechwoods.com>
---
drivers/mtd/nand/nandsim.c | 42 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index bdc1d15..65ac60f 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -109,6 +109,7 @@ static unsigned int bitflips = 0;
static char *gravepages = NULL;
static unsigned int overridesize = 0;
static char *cache_file = NULL;
+static uint cache_file_written = 0;
static unsigned int bbt;
static unsigned int bch;
@@ -133,6 +134,7 @@ module_param(bitflips, uint, 0400);
module_param(gravepages, charp, 0400);
module_param(overridesize, uint, 0400);
module_param(cache_file, charp, 0400);
+module_param(cache_file_written, uint, 0400);
module_param(bbt, uint, 0400);
module_param(bch, uint, 0400);
@@ -166,6 +168,7 @@ MODULE_PARM_DESC(overridesize, "Specifies the NAND Flash size overriding the I
"The size is specified in erase blocks and as the exponent of a power of two"
" e.g. 5 means a size of 32 erase blocks");
MODULE_PARM_DESC(cache_file, "File to use to cache nand pages instead of memory");
+MODULE_PARM_DESC(cache_file_written, "Assume any pages in cache file have already been written, use their existing contents");
MODULE_PARM_DESC(bbt, "0 OOB, 1 BBT with marker in OOB, 2 BBT with marker in data area");
MODULE_PARM_DESC(bch, "Enable BCH ecc and set how many bits should "
"be correctable in 512-byte blocks");
@@ -592,6 +595,9 @@ static int alloc_device(struct nandsim *ns)
err = -ENOMEM;
goto err_close;
}
+ if (cache_file_written) {
+ memset(ns->pages_written, 0xFF, ns->geom.pgnum);
+ }
ns->file_buf = kmalloc(ns->geom.pgszoob, GFP_KERNEL);
if (!ns->file_buf) {
NS_ERR("alloc_device: unable to allocate file buf\n");
@@ -1484,7 +1490,17 @@ static void read_page(struct nandsim *ns, int num)
pos = (loff_t)NS_RAW_OFFSET(ns) + ns->regs.off;
tx = read_file(ns, ns->cfile, ns->buf.byte, num, pos);
if (tx != num) {
- NS_ERR("read_page: read error for page %d ret %ld\n", ns->regs.row, (long)tx);
+ if (ns->pages_written[ns->regs.row] == 0xFF) {
+ NS_WARN("read_page: read error assuming unwritten for page %d ret %ld\n", ns->regs.row, (long)tx);
+ ns->pages_written[ns->regs.row] = 0;
+ memset(ns->buf.byte, 0xFF, num);
+ tx = write_file(ns, ns->cfile, ns->buf.byte, num, pos);
+ if (tx != num) {
+ NS_ERR("read_page: write error for page %d ret %ld\n", ns->regs.row, (long)tx);
+ }
+ } else {
+ NS_ERR("read_page: read error for page %d ret %ld\n", ns->regs.row, (long)tx);
+ }
return;
}
do_bit_flips(ns, num);
@@ -1515,11 +1531,22 @@ static void erase_sector(struct nandsim *ns)
int i;
if (ns->cfile) {
- for (i = 0; i < ns->geom.pgsec; i++)
+ loff_t pos;
+ ssize_t tx;
+
+ memset(ns->file_buf, 0xff, ns->geom.pgszoob);
+ for (i = 0; i < ns->geom.pgsec; i++) {
if (__test_and_clear_bit(ns->regs.row + i,
ns->pages_written)) {
NS_DBG("erase_sector: freeing page %d\n", ns->regs.row + i);
+ pos = (loff_t)(ns->regs.row + i) * ns->geom.pgszoob;
+ tx = write_file(ns, ns->cfile, ns->file_buf, ns->geom.pgszoob, pos);
+ if (tx != ns->geom.pgszoob) {
+ NS_ERR("erase_sector: write error for page %d ret %ld\n", ns->regs.row, (long)tx);
+ }
}
+ ns->pages_written[ns->regs.row + i] = 0;
+ }
return;
}
@@ -1558,8 +1585,15 @@ static int prog_page(struct nandsim *ns, int num)
all = 0;
tx = read_file(ns, ns->cfile, pg_off, num, off);
if (tx != num) {
- NS_ERR("prog_page: read error for page %d ret %ld\n", ns->regs.row, (long)tx);
- return -1;
+ if (ns->pages_written[ns->regs.row] == 0xFF) {
+ NS_WARN("prog_page: read error assuming unwritten for page %d ret %ld\n", ns->regs.row, (long)tx);
+ ns->pages_written[ns->regs.row] = 0;
+ all = 1;
+ memset(ns->file_buf, 0xff, ns->geom.pgszoob);
+ } else {
+ NS_ERR("prog_page: read error for page %d ret %ld\n", ns->regs.row, (long)tx);
+ return -1;
+ }
}
}
for (i = 0; i < num; i++)
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mtd: nandsim: Allow nandsim to re-use persisted data
2013-09-25 19:25 [PATCH] mtd: nandsim: Allow nandsim to re-use persisted data Matthew Gabeler-Lee
@ 2013-11-07 15:12 ` Richard Weinberger
2013-11-07 17:36 ` Matthew Gabeler-Lee
0 siblings, 1 reply; 3+ messages in thread
From: Richard Weinberger @ 2013-11-07 15:12 UTC (permalink / raw)
To: Matthew Gabeler-Lee
Cc: pratibha, linux-mtd@lists.infradead.org, Artem Bityutskiy,
Ezequiel Garcia, Tanya Brokhman
Hi!
On Wed, Sep 25, 2013 at 9:25 PM, Matthew Gabeler-Lee <fastcat@gmail.com> wrote:
> mtd: nandsim: Allow nandsim to re-use persisted data
>
> Add optional (default disabled) module parameter to nandsim to instruct it
> to assume that the cache file already has valid data in it. This allows
> data written to the cache file (or device) to be re-used across reloads of
> the nandsim module, and thus reboots of the host, in much the same way that
> block2mtd allows.
>
> Signed-off-by: Matthew Gabeler-Lee <matthew@beechwoods.com>
> ---
> drivers/mtd/nand/nandsim.c | 42 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
> index bdc1d15..65ac60f 100644
> --- a/drivers/mtd/nand/nandsim.c
> +++ b/drivers/mtd/nand/nandsim.c
> @@ -109,6 +109,7 @@ static unsigned int bitflips = 0;
> static char *gravepages = NULL;
> static unsigned int overridesize = 0;
> static char *cache_file = NULL;
> +static uint cache_file_written = 0;
> static unsigned int bbt;
> static unsigned int bch;
>
> @@ -133,6 +134,7 @@ module_param(bitflips, uint, 0400);
> module_param(gravepages, charp, 0400);
> module_param(overridesize, uint, 0400);
> module_param(cache_file, charp, 0400);
> +module_param(cache_file_written, uint, 0400);
> module_param(bbt, uint, 0400);
> module_param(bch, uint, 0400);
>
> @@ -166,6 +168,7 @@ MODULE_PARM_DESC(overridesize, "Specifies the NAND
> Flash size overriding the I
> "The size is specified in erase blocks and
> as the exponent of a power of two"
> " e.g. 5 means a size of 32 erase blocks");
> MODULE_PARM_DESC(cache_file, "File to use to cache nand pages instead
> of memory");
> +MODULE_PARM_DESC(cache_file_written, "Assume any pages in cache file have
> already been written, use their existing contents");
> MODULE_PARM_DESC(bbt, "0 OOB, 1 BBT with marker in OOB, 2 BBT
> with marker in data area");
> MODULE_PARM_DESC(bch, "Enable BCH ecc and set how many bits
> should "
> "be correctable in 512-byte blocks");
> @@ -592,6 +595,9 @@ static int alloc_device(struct nandsim *ns)
> err = -ENOMEM;
> goto err_close;
> }
> + if (cache_file_written) {
> + memset(ns->pages_written, 0xFF, ns->geom.pgnum);
> + }
Hmm, that's a very ugly hack.
It will slow down nandsim because now it will ask the file cache every time.
If you want persistent storage we'd need a sane file format with header
which contains the page_written bitmask, the nand geometry, etc....
> ns->file_buf = kmalloc(ns->geom.pgszoob, GFP_KERNEL);
> if (!ns->file_buf) {
> NS_ERR("alloc_device: unable to allocate file
> buf\n");
> @@ -1484,7 +1490,17 @@ static void read_page(struct nandsim *ns, int num)
> pos = (loff_t)NS_RAW_OFFSET(ns) + ns->regs.off;
> tx = read_file(ns, ns->cfile, ns->buf.byte, num,
> pos);
> if (tx != num) {
> - NS_ERR("read_page: read error for page %d
> ret %ld\n", ns->regs.row, (long)tx);
> + if (ns->pages_written[ns->regs.row] == 0xFF)
> {
> + NS_WARN("read_page: read error
> assuming unwritten for page %d ret %ld\n", ns->regs.row, (long)tx);
> + ns->pages_written[ns->regs.row] = 0;
> + memset(ns->buf.byte, 0xFF, num);
> + tx = write_file(ns, ns->cfile,
> ns->buf.byte, num, pos);
> + if (tx != num) {
> + NS_ERR("read_page: write
> error for page %d ret %ld\n", ns->regs.row, (long)tx);
> + }
> + } else {
> + NS_ERR("read_page: read error for
> page %d ret %ld\n", ns->regs.row, (long)tx);
> + }
> return;
> }
> do_bit_flips(ns, num);
> @@ -1515,11 +1531,22 @@ static void erase_sector(struct nandsim *ns)
> int i;
>
> if (ns->cfile) {
> - for (i = 0; i < ns->geom.pgsec; i++)
> + loff_t pos;
> + ssize_t tx;
> + + memset(ns->file_buf, 0xff, ns->geom.pgszoob);
> + for (i = 0; i < ns->geom.pgsec; i++) {
> if (__test_and_clear_bit(ns->regs.row + i,
> ns->pages_written)) {
> NS_DBG("erase_sector: freeing page %d\n",
> ns->regs.row + i);
> + pos = (loff_t)(ns->regs.row + i) *
> ns->geom.pgszoob;
> + tx = write_file(ns, ns->cfile, ns->file_buf,
> ns->geom.pgszoob, pos);
> + if (tx != ns->geom.pgszoob) {
> + NS_ERR("erase_sector: write error
> for page %d ret %ld\n", ns->regs.row, (long)tx);
> + }
> }
> + ns->pages_written[ns->regs.row + i] = 0;
> + }
> return;
> }
>
> @@ -1558,8 +1585,15 @@ static int prog_page(struct nandsim *ns, int num)
> all = 0;
> tx = read_file(ns, ns->cfile, pg_off, num, off);
> if (tx != num) {
> - NS_ERR("prog_page: read error for page %d
> ret %ld\n", ns->regs.row, (long)tx);
> - return -1;
> + if (ns->pages_written[ns->regs.row] == 0xFF)
> {
> + NS_WARN("prog_page: read error
> assuming unwritten for page %d ret %ld\n", ns->regs.row, (long)tx);
> + ns->pages_written[ns->regs.row] = 0;
> + all = 1;
> + memset(ns->file_buf, 0xff,
> ns->geom.pgszoob);
> + } else {
> + NS_ERR("prog_page: read error for
> page %d ret %ld\n", ns->regs.row, (long)tx);
> + return -1;
> + }
> }
> }
> for (i = 0; i < num; i++)
> --
> 1.8.4.rc3
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
--
Thanks,
//richard
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mtd: nandsim: Allow nandsim to re-use persisted data
2013-11-07 15:12 ` Richard Weinberger
@ 2013-11-07 17:36 ` Matthew Gabeler-Lee
0 siblings, 0 replies; 3+ messages in thread
From: Matthew Gabeler-Lee @ 2013-11-07 17:36 UTC (permalink / raw)
To: Richard Weinberger
Cc: pratibha, linux-mtd@lists.infradead.org, Artem Bityutskiy,
Ezequiel Garcia, Tanya Brokhman
On 11/07/2013 10:12 AM, Richard Weinberger wrote:
> Hi!
Hi! Thanks for taking a look at this. PS: First kernel patch I've ever
done...
> On Wed, Sep 25, 2013 at 9:25 PM, Matthew Gabeler-Lee <fastcat@gmail.com> wrote:
>> @@ -592,6 +595,9 @@ static int alloc_device(struct nandsim *ns)
>> err = -ENOMEM;
>> goto err_close;
>> }
>> + if (cache_file_written) {
>> + memset(ns->pages_written, 0xFF, ns->geom.pgnum);
>> + }
>
> Hmm, that's a very ugly hack.
> It will slow down nandsim because now it will ask the file cache every time.
>
> If you want persistent storage we'd need a sane file format with header
> which contains the page_written bitmask, the nand geometry, etc....
Agreed that it's a hack. My thought process was to a) minimize the
changes and b) have the resulting persisted image be usable to write to
a real flash device, if that was desired. I'm not sure if the OOB data
would make part b not actually work, though. I agree that a real file
format (or split files) that tracks metadata would be better overall.
It would beg for some userspace tools to work with it.
As far as performance is concerned, if considering a real file format,
in my testing it appeared that the non-aligned writes resulting from the
OOB bytes may also have been a problem. AFAICT my application (UBI/FS)
never actually accesses those OOB bytes, so getting creative with where
they are stored in a file format could have some benefits.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-11-07 17:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-25 19:25 [PATCH] mtd: nandsim: Allow nandsim to re-use persisted data Matthew Gabeler-Lee
2013-11-07 15:12 ` Richard Weinberger
2013-11-07 17:36 ` Matthew Gabeler-Lee
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).