| Message ID | 1374978118-11418-1-git-send-email-akinobu.mita@gmail.com |
|---|---|
| State | Accepted |
| Commit | 08efe91a1befa67bcc3799e0d29b7194803e1538 |
| Headers | show |
On Sun, 2013-07-28 at 11:21 +0900, Akinobu Mita wrote: > nandsim.pages_written[] is the array of unsigned char which is indexed > by the page number and used for identifying which pages have been written > when cache_file is used. Each entry holds 0 (not written) or 1 (written), > so it can be converted to bitmap. This reduces the allocation size of > pages_written[] by 1/8. Did you test this patch-set, if yes, how?
On Fri, 2013-08-02 at 17:38 +0300, Artem Bityutskiy wrote: > On Sun, 2013-07-28 at 11:21 +0900, Akinobu Mita wrote: > > nandsim.pages_written[] is the array of unsigned char which is indexed > > by the page number and used for identifying which pages have been written > > when cache_file is used. Each entry holds 0 (not written) or 1 (written), > > so it can be converted to bitmap. This reduces the allocation size of > > pages_written[] by 1/8. > > Did you test this patch-set, if yes, how? Note, I am asking because this is an ancient and probably poor piece of code, but quite important, so I am just trying to understand whether I should test your patches before applying, and if yes, how.
2013/8/3 Artem Bityutskiy <dedekind1@gmail.com>: > On Fri, 2013-08-02 at 17:38 +0300, Artem Bityutskiy wrote: >> On Sun, 2013-07-28 at 11:21 +0900, Akinobu Mita wrote: >> > nandsim.pages_written[] is the array of unsigned char which is indexed >> > by the page number and used for identifying which pages have been written >> > when cache_file is used. Each entry holds 0 (not written) or 1 (written), >> > so it can be converted to bitmap. This reduces the allocation size of >> > pages_written[] by 1/8. >> >> Did you test this patch-set, if yes, how? > > Note, I am asking because this is an ancient and probably poor piece of > code, but quite important, so I am just trying to understand whether I > should test your patches before applying, and if yes, how. I tested this with mtd/tests modules and mounting a filesystem through mtdblock. I also tried cache_file module parameter to test patch 1/6 because nandsim.pages_written[] is only used when cache_file is used. Also I tried second_id_byte module parameter to check both 8-bit and 16-bit bus width are working.
On Sun, 2013-07-28 at 11:21 +0900, Akinobu Mita wrote: > nandsim.pages_written[] is the array of unsigned char which is indexed > by the page number and used for identifying which pages have been written > when cache_file is used. Each entry holds 0 (not written) or 1 (written), > so it can be converted to bitmap. This reduces the allocation size of > pages_written[] by 1/8. Pushed all to l2-mtd.git, thanks. I wonder, what is your motivation for doing this? Did you just notice the imperfection and decided to fix it, or you really suffer from too much memory allocated? Thanks!
2013/8/5 Artem Bityutskiy <dedekind1@gmail.com>: > On Sun, 2013-07-28 at 11:21 +0900, Akinobu Mita wrote: >> nandsim.pages_written[] is the array of unsigned char which is indexed >> by the page number and used for identifying which pages have been written >> when cache_file is used. Each entry holds 0 (not written) or 1 (written), >> so it can be converted to bitmap. This reduces the allocation size of >> pages_written[] by 1/8. > > Pushed all to l2-mtd.git, thanks. Thanks. > I wonder, what is your motivation for doing this? Did you just notice > the imperfection and decided to fix it, or you really suffer from too > much memory allocated? I found all in this patch set when I was reading nandsim.c. So I did not suffer any real problems.
diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c index cb38f3d..a73dce0 100644 --- a/drivers/mtd/nand/nandsim.c +++ b/drivers/mtd/nand/nandsim.c @@ -363,7 +363,7 @@ struct nandsim { /* Fields needed when using a cache file */ struct file *cfile; /* Open file */ - unsigned char *pages_written; /* Which pages have been written */ + unsigned long *pages_written; /* Which pages have been written */ void *file_buf; struct page *held_pages[NS_MAX_HELD_PAGES]; int held_cnt; @@ -586,7 +586,8 @@ static int alloc_device(struct nandsim *ns) err = -EINVAL; goto err_close; } - ns->pages_written = vzalloc(ns->geom.pgnum); + ns->pages_written = vzalloc(BITS_TO_LONGS(ns->geom.pgnum) * + sizeof(unsigned long)); if (!ns->pages_written) { NS_ERR("alloc_device: unable to allocate pages written array\n"); err = -ENOMEM; @@ -1479,7 +1480,7 @@ static void read_page(struct nandsim *ns, int num) union ns_mem *mypage; if (ns->cfile) { - if (!ns->pages_written[ns->regs.row]) { + if (!test_bit(ns->regs.row, ns->pages_written)) { NS_DBG("read_page: page %d not written\n", ns->regs.row); memset(ns->buf.byte, 0xFF, num); } else { @@ -1525,9 +1526,9 @@ static void erase_sector(struct nandsim *ns) if (ns->cfile) { for (i = 0; i < ns->geom.pgsec; i++) - if (ns->pages_written[ns->regs.row + 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); - ns->pages_written[ns->regs.row + i] = 0; } return; } @@ -1560,7 +1561,7 @@ static int prog_page(struct nandsim *ns, int num) NS_DBG("prog_page: writing page %d\n", ns->regs.row); pg_off = ns->file_buf + ns->regs.column + ns->regs.off; off = (loff_t)ns->regs.row * ns->geom.pgszoob + ns->regs.column + ns->regs.off; - if (!ns->pages_written[ns->regs.row]) { + if (!test_bit(ns->regs.row, ns->pages_written)) { all = 1; memset(ns->file_buf, 0xff, ns->geom.pgszoob); } else { @@ -1580,7 +1581,7 @@ static int prog_page(struct nandsim *ns, int num) NS_ERR("prog_page: write error for page %d ret %ld\n", ns->regs.row, (long)tx); return -1; } - ns->pages_written[ns->regs.row] = 1; + __set_bit(ns->regs.row, ns->pages_written); } else { tx = write_file(ns, ns->cfile, pg_off, num, off); if (tx != num) {
nandsim.pages_written[] is the array of unsigned char which is indexed by the page number and used for identifying which pages have been written when cache_file is used. Each entry holds 0 (not written) or 1 (written), so it can be converted to bitmap. This reduces the allocation size of pages_written[] by 1/8. Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> Cc: Brian Norris <computersforpeace@gmail.com> Cc: Artem Bityutskiy <dedekind1@gmail.com> Cc: David Woodhouse <dwmw2@infradead.org> Cc: linux-mtd@lists.infradead.org --- drivers/mtd/nand/nandsim.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)